From 65025cbf4f5bfbb34f649a0c6d2cb71f75c948ea Mon Sep 17 00:00:00 2001 From: ReaperOfVeriod Date: Wed, 14 Oct 2026 04:19:01 +0200 Subject: [PATCH] The vtbl terminator: three test counts, all defensible, one of them wrong Reconciles the discrepancy flagged when the dispatch tables went in. Three committed tests counted lvl.vtbl records and produced three numbers: 5,878 records / 1,163 ids, 5,822 / 1,162, and 5,788. 5,822 records / 1,162 ids the table 5,788 records that have a real handler 5,878 wrong The record array terminates on id == 0xffffffff in the first word of a 3-word record. Two of the tests reached that word with `if c != MAX { ... }` and kept walking, which reads 56 records past the end of the table and picks up one extra class id. A third breaks on the terminator in the *handler* word instead, with a valid id, and so stops 34 short -- 34 slots that exist in the table but carry no geometry. Both stopping points are correct for the question each test asks. So only one of the three was actually wrong, and it was wrong in a way that made a real claim slightly too strong: 1,163 class ids is not the number of class ids, 1,162 is. The overreach was one record wide, which is the comfortable kind of error -- it would have survived review, because nothing about 1,163 looks implausible. Worth being plain about why this sat unresolved for so long. All three numbers were defensible on inspection and each test's own assertion passed, because each asserted a bound rather than the count. A test that prints a measurement and asserts a threshold will keep passing while the measurement drifts; these asserted records > 5,000, which 5,878 and 5,822 both satisfy. Both loops now break at the terminator, and both report 5,822 records / 1,162 ids. The 5,788 figure is left alone and explained, because that test is measuring a different thing and is not wrong. --- crates/gc-extract/tests/disc.rs | 14 ++++++++++---- docs/formats/core-data-regions.md | 25 +++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/crates/gc-extract/tests/disc.rs b/crates/gc-extract/tests/disc.rs index 4cb2faa..0ee17e3 100644 --- a/crates/gc-extract/tests/disc.rs +++ b/crates/gc-extract/tests/disc.rs @@ -5774,10 +5774,14 @@ fn moby_class_ids_come_from_lvl_vtbl_not_from_the_class_count() { }; assert!(secs.len() >= 4, "level {lv}: {} sections", secs.len()); let s = &secs[3]; + // The record table ENDS at the terminator. Walking past it with + // `continue` reads whatever follows and inflates the count by 56 records + // and one class id -- which is how this test and the dispatch test came to + // disagree (5,878/1,163 against 5,822/1,162). Break. for r in 0..s.data.len() / 12 { let c = u32::from_le_bytes(s.data[r * 12..r * 12 + 4].try_into().unwrap()); if c == u32::MAX { - continue; + break; } classes.insert(c); records += 1; @@ -5875,12 +5879,14 @@ fn vtbl_class_ids_are_global_and_do_not_index_the_class_table() { min_count = min_count.min(count); let secs = parse_overlay_sections(&ov).expect("sections"); let before = ids.len(); + // Terminator ends the table; see the note in lvl_vtbl_class_ids_are_global. for r in 0..secs[3].data.len() / 12 { let c = u32::from_le_bytes(secs[3].data[r * 12..r * 12 + 4].try_into().unwrap()); - if c != u32::MAX { - ids.insert(c); - records_seen += 1; + if c == u32::MAX { + break; } + ids.insert(c); + records_seen += 1; } let added = ids.len() - before; if lv == 0 { diff --git a/docs/formats/core-data-regions.md b/docs/formats/core-data-regions.md index 9dc6235..c61b427 100644 --- a/docs/formats/core-data-regions.md +++ b/docs/formats/core-data-regions.md @@ -3535,3 +3535,28 @@ as a significant finding, and the correction is not visible in the numbers alone The gate's second half is therefore met, with the caveat that "agrees" means "never contradicts", and our result is the finer of the two partitions. + + +## The vtbl terminator, and why three counts were all defensible + +Three committed tests counted `lvl.vtbl` records and disagreed: 5,878 records / +1,163 ids, 5,822 / 1,162, and 5,788. All three were measuring something, and none +of the tests was simply wrong. + +**5,822 records / 1,162 ids is the table.** The record array terminates on +`id == 0xffffffff` in the *first* word of a 3-word record. Two tests were skipping +that word with `continue` and walking on into whatever follows the table, which +picked up 56 records past the end and one extra class id. Breaking at the +terminator is the correct reading of a dispatched table, and both tests now do. + +**5,788 is the records with a real handler.** Of the 5,822 records, 34 carry the +terminator in the *handler* word with a valid id — slots that exist in the table +but have no geometry. A third test breaks on the handler word rather than the id +word, and so stops 34 short. Both stopping points are correct for what that test +is measuring. + +**5,878 was the bug.** It was the first number above, reading past the table. + +The disagreement was not a decomp mismatch and not a hardware question. It was two +sentences in a test saying `if c != MAX { ... }` and `if c == MAX { break }`, which +are the same condition with opposite answers to whether the table ends there.