Merge pull request #651 from RetroPortingToolKit/fix/aot-dispatch-entry-coverage

AOT: preserve dispatch-only entries without splitting their hosts

docs/AOT_SHARDING.md

@@ -102,6 +102,12 @@ generic detector misses it. `entry_word` or `entries` can supply independently
 verified loader entry points. These facts do not turn every plausible code word
 into a proven function. There is no recipe option to ignore a failed compiler.
 
+Use `dispatch_entries` for independently verified dispatch PCs that may be inside
+an existing function, such as jump-table case labels. They are required by the
+coverage audit but do not force new function boundaries. The existing decoder and
+reachability checks decide whether each PC is a callable root or a hosted interior;
+an interior declaration cannot truncate the function that owns it.
+
 Each recipe compiles in an isolated cache. `tools/audit_aot_cache.py` checks every
 native pair's ABI/exports, every manifest guard against known original bytes,
 and nonzero native coverage for every recipe. Loader entry points supplied by

tools/aot_overlay_pipeline.py

@@ -427,6 +427,9 @@ def spec_sources(disc, spec):
                     grouped[key]['aliases'].append(name)
                     require(grouped[key]['spec']['entries'] == placement.get('entries', []),
                             'Duplicate bank has conflicting declared entries')
+                    require(grouped[key]['spec'].get('dispatch_entries', []) ==
+                            placement.get('dispatch_entries', spec.get('dispatch_entries', [])),
+                            'Duplicate bank has conflicting dispatch entries')
                     continue
                 source = dict(name=name, base=base, body=body,
                     spec={**spec, **placement, 'entries': placement.get('entries', []),
@@ -579,6 +582,19 @@ def declared_entries(source, disc):
     return entries
 
 
+def declared_dispatch_entries(source):
+    """Required PCs whose observation does not establish a function boundary."""
+    entries = {number(x) for x in source['spec'].get('dispatch_entries', [])}
+    base, size = source['base'], len(source['body'])
+    require(all(base <= entry < base + size and entry % 4 == 0 for entry in entries),
+            f"Dispatch entry outside image: {source['name']}")
+    return entries
+
+
+def required_entries(source, disc):
+    return declared_entries(source, disc) | declared_dispatch_entries(source)
+
+
 def foreign_call_roots(source, sources):
     """Entries of `source` that other images call directly (`jal`).
 
@@ -612,7 +628,7 @@ def make_fixed_record(source, disc, sources=(), cross_image=False):
     seeds |= entries
     require(seeds, f"No static entries: {source['name']}")
     page, data = extractor.page_aligned_region(base, body)
-    return extractor.rec(page, data, sorted(seeds), dispatch_extra=sorted(entries),
+    return extractor.rec(page, data, sorted(seeds), dispatch_extra=sorted(required_entries(source, disc)),
                          producer_ranges=[(base, base + len(body))], static_discovery=direct | entries)
 
 
@@ -693,11 +709,13 @@ def prepare(profile, disc, records, output, views=None):
     # found the image without finding that particular callable entry.
     for source in sources:
         entries = declared_entries(source, disc)
-        if entries:
+        dispatch = required_entries(source, disc)
+        if dispatch:
             require(source['name'] in singles, 'Declared entries need a complete source recipe')
             record = singles[source['name']]
             for key in ('function_entry_pcs', 'dispatch_entry_pcs', 'static_dispatch_entry_pcs', 'seeds'):
-                record[key] = [hex(pc) for pc in sorted(entries | {number(pc) for pc in record[key]})]
+                extra = dispatch if key in ('dispatch_entry_pcs', 'static_dispatch_entry_pcs') else entries
+                record[key] = [hex(pc) for pc in sorted(extra | {number(pc) for pc in record[key]})]
             record['static_discovery_entry_pcs'] = [hex(pc) for pc in sorted(entries |
                 {number(pc) for pc in record.get('static_discovery_entry_pcs', [])})]
     by_name = {source['name']: source for source in sources}
@@ -730,7 +748,7 @@ def prepare(profile, disc, records, output, views=None):
                 if any(lo <= number(alias['start']) <= number(alias['entry']) < number(alias['end']) <= hi
                        for lo, hi in bounds)]
             require(all(eligible(entry) for source, _, _ in matches
-                        for entry in declared_entries(source, disc)), 'Excluded required loader entry')
+                        for entry in required_entries(source, disc)), 'Excluded required loader entry')
         if record.get('producer') == 'bios_resident_manifest':
             names = ['BIOS resident manifest']
             bounds = [(number(r['start']), number(r['end'])) for r in record['producer_ranges']]
@@ -746,7 +764,7 @@ def prepare(profile, disc, records, output, views=None):
         write_json(path, [record])
         jobs.append(dict(name=f'{index:03d}-' + '+'.join(names), sources=names,
                          required_entries=sorted({entry for source, _, _ in matches
-                                                  for entry in declared_entries(source, disc)}),
+                                                  for entry in required_entries(source, disc)}),
                          known_ranges=bounds, input=str(path.resolve()), load_addr=hex(load),
                          size=len(data), sha256=hashlib.sha256(data).hexdigest(),
                          recipe_sha256=hashlib.sha256(json.dumps(record, sort_keys=True).encode()).hexdigest(),

tools/tests/test_aot_overlay_pipeline.py

@@ -1148,6 +1148,59 @@ class CrossImageRootsTests(unittest.TestCase):
         self.assertEqual(pipeline.foreign_call_roots(data, [data, self.caller]), [])
 
 
+class DispatchEntryTests(unittest.TestCase):
+    """An observed switch case is not a new function boundary."""
+
+    def setUp(self):
+        self.base = base = 0x80100000
+        self.body = bytearray(0x100)
+        words = {0x00: 0x27BDFFF0, 0x04: 0x3C088010, 0x08: 0x250800C0,
+                 0x0C: 0x2C810002, 0x10: 0x10200013, 0x18: 0x00044880,
+                 0x1C: 0x01094021, 0x20: 0x8D0A0000, 0x28: 0x01400008,
+                 0x40: 0x24020001, 0x44: 0x08000000 | ((base + 0x60) >> 2 & 0x3FFFFFF),
+                 0x50: 0x24020002, 0x54: 0x08000000 | ((base + 0x60) >> 2 & 0x3FFFFFF),
+                 0x60: 0x03E00008, 0x64: 0x27BD0010,
+                 0xC0: base + 0x40, 0xC4: base + 0x50}
+        for offset, word in words.items():
+            struct.pack_into('<I', self.body, offset, word)
+        self.body = bytes(self.body)
+        self.spec = dict(method='fixed_address_files', files=['CODE'], load_addr=hex(base),
+                         entries=[hex(base)], dispatch_entries=[hex(base + 0x50)])
+
+    def test_required_case_keeps_host_and_other_cases_reachable(self):
+        import compile_overlays as compiler
+        with tempfile.TemporaryDirectory() as directory:
+            disc = SectorDisc(directory, {'CODE': self.body})
+            record = pipeline.extractor.rec(self.base, self.body, [self.base])
+            result = pipeline.prepare(dict(game_id='TEST', images=[self.spec], expected_records=1),
+                                      disc, [record], Path(directory))
+            job = result['jobs'][0]
+            self.assertEqual(job['required_entries'], [self.base, self.base + 0x50])
+            capture = json.loads(Path(job['input']).read_text())[0]
+            target = self.base + 0x50
+            for key in ('function_entry_pcs', 'static_discovery_entry_pcs', 'seeds'):
+                self.assertNotIn(target, {int(x, 16) for x in capture.get(key, [])})
+            seeds, audit = compiler.classify_overlay_seeds(
+                capture, self.body, self.base, len(self.body), 0, {})
+            self.assertIn(f'interior 0x{target:08X}', seeds)
+            self.assertEqual(audit['included_reasons'][target], 'DISPATCH_INTERIOR')
+
+    def test_invalid_dispatch_declaration_is_rejected(self):
+        source = dict(name='CODE', base=self.base, body=self.body, spec=self.spec)
+        for target in (self.base - 4, self.base + 1, self.base + len(self.body)):
+            with self.subTest(target=target), self.assertRaisesRegex(ValueError, 'Dispatch entry outside'):
+                pipeline.required_entries({**source, 'spec': {**self.spec,
+                                          'dispatch_entries': [hex(target)]}}, None)
+
+    def test_exclusion_cannot_hide_required_dispatch(self):
+        spec = {**self.spec, 'excluded_ranges': [dict(start=self.base + 0x50, end=self.base + 0x54,
+                reason='invented fallback', sha256=sha(self.body[0x50:0x54]))]}
+        record = pipeline.extractor.rec(self.base, self.body, [self.base])
+        with tempfile.TemporaryDirectory() as directory, self.assertRaisesRegex(ValueError, 'required loader entry'):
+            pipeline.prepare(dict(images=[spec], expected_records=1, strict_bounds=True),
+                             FakeDisc({'CODE': self.body}), [record], Path(directory))
+
+
 class StaticGenerationTest(unittest.TestCase):
     """The `static` action as `psxrecomp_cli.py generate` drives it."""