Declare dScMgBase_c slot 32 (Virtual80) across the minigame family - #2107
Conversation
Slot 32 of dScMgBase_c's 36-entry vtable, base body ov004:0x020b27f4.
One override in the whole family -- dScMgSlot3_c at ov006:0x0210aa60 --
which makes this the cheapest slot in the campaign after 22's zero.
Shipped as Virtual80, NOT AfterClsn, and this is the same defect settled
at slot 31 one fork further out. AfterClsn IS a real ROM name:
include/PathLift.h:58 declares it and _ZN16dPathLiftActor_c9AfterClsnEi
is a genuine mangled symbol. But dPathLiftActor_c derives from
dBgActor_c, which derives from dActor_c, and dScMgBase_c is a dScene_c:
fBase_c -> dBase_c -> dActor_c -> dBgActor_c -> dPathLiftActor_c
fBase_c -> dBase_c -> dScene_c -> dScMgBase_c
The chains share only dBase_c, which adds no virtual of its own, so the
two slot 32s have fBase_c's first eighteen entries in common and nothing
else. That AfterClsn also takes an int; this slot takes nothing. A
whole-image scan for the dispatch pair at +0x80 (ldr rN,[rM,#0x80] with
Rn != pc, then blx rN within three instructions) finds exactly three
sites: ov004:0x020b0900 on this branch, ov002:0x020effa4 and
ov064:0x02116e58 on that one.
Slots 31 and 32 are one function twice, against the two display engines.
31 is the SUB screen, called last by BeforeInitResources; 32 is the MAIN
screen, called first by AfterInitResources(u32). Same three
read-modify-writes leaving BG1CNT at exactly the class's base bits, same
scroll reset, same &= ~2 on the engine's BG-enable shadow (data_0209d454
against data_0209d45c -- the two words slot 30 restores the DISPCNTs
from), same language-indexed character file plus a shared screen map,
file 0x5b against 0x67. It touches no collision.
Arity measured at that one call site: ldr r2,[r0]; mov r5,r0;
ldr r2,[r2,#0x80]; mov r4,r1; blx r2. r0 is `this`, and r1 is
AfterInitResources's own vfSuccess, parked in r4 BECAUSE the call
clobbers it. Return type stays a hint for the fourth consecutive slot:
the caller overwrites r0 with a literal 2 on the next instruction.
Verification
rombuild -j 16 --no-rom: 11,088/11,088 reproducing, 106/106 exact,
100.000000% of compared bytes, ROM-build analysis PASS
romdata_check vs the slot-31 baseline: verifiedBytes 35416 -> 35420
(+4), partialBytes 11748 -> 11872 (+124). 128 bytes = 32 tables x
one word, which is what declaring exactly one slot must cost.
symbols 1246, verified 465, partial 253, differs 6, totalRecords
7560 all unchanged; the differing set is unchanged in both
directions.
check_dead_references clean; port_refcheck 405/405;
check_header_offsets --changed origin/main 0 mismatched;
check_src_tu_compiles 97/97; check_duplicate_sources 11038 stems,
none doubled; langmode ratchet PASS.
Three slots left: 33-35.
andrewboudreau
added
the
attribution-override
✅ PR validation — PassedCommitted merge passes; 1 contributor credit reassignment(s) noted, not a blocker. Full merge validation
Contributor credit moved (1)
Byte-verified means the range carries Warnings: contributor attribution changed, not a blocker (1 changed, 0 lost -- src/_ZN11dScMgBase_c9Virtual80Ev.c: tangosdev -> andrewboudreau). Per-file link-check detailAll 192 changed file(s) compile to the ROM byte-for-byte with correct relocation targets.
The private worker commits a test merge, builds the stock ROM profile, compares every executable module, measures matched and source-built code, checks contributor lineage, and verifies affected relocations. The mod profile is opt-in and is not part of this merge gate. |
|
Self-review. I wrote this, and this repo has one shared account so no PR here can be APPROVED by a second identity — signing off in a comment instead, with what I checked and what I could not. Verified on the merge tree, not just the branch. Byte evidence. Also clean: Why the name is
So this ships as Arity, measured. Dispatch-pair scan across arm9 and all 103 overlays for What it actually does — 31 and 32 are one function twice, once per display engine. Slot 31 is the SUB screen and is called last by What this PR does NOT establish, on the record:
Three slots left: 33, 34, 35. Slot 35 closes Landing stays with you. |
andrewboudreau
left a comment
andrewboudreau
left a comment
There was a problem hiding this comment.
Reviewed at 567649493a6b. Three asks. One is the systemic return-type split that already holds the four PRs below this; one is an evidence-strength claim I checked hard and could not sustain; one is a word. The config half is clean, and I want to say that first, because on the two PRs either side of this stack the config half is exactly where the defect was.
The config half is clean, and I looked for a specific thing
Both renames are address- and size-preserving:
ov004 -func_ov004_020b27f4 kind:function(arm,size=0x8c) addr:0x020b27f4
+_ZN11dScMgBase_c9Virtual80Ev kind:function(arm,size=0x8c) addr:0x020b27f4
ov006 -func_ov006_0210aa60 kind:function(arm,size=0xa8) addr:0x0210aa60
+_ZN12dScMgSlot3_c9Virtual80Ev kind:function(arm,size=0xa8) addr:0x0210aa60
No coined data symbols anywhere in the diff. I checked for that specifically — I have open changes-requested on #2096 and #2104 over names invented for data_ovNNN_* rows sitting inside a _ZTV extent, and this PR does not do it.
Neither renamed file changed a line of code. I diffed both across the rename with git diff -M; the only hunks are the // @symbol line and comments.
Virtual80 is the right spelling — slot 32 is vtable+0x80, and Virtual7C and Virtual50 are already tracked, so this invents no convention.
Static gates on the real merge tree, both bases:
vs origin/cpp/minigame-slot31 8/8 pass -> pass
vs origin/main 8/8 pass -> pass
1. The return-type split, now the seventh and eighth
include/dScMgBase_c.h:703 virtual int Virtual80(); /* slot 32 */
include/dScMgSlot3_c.h:61 virtual int Virtual80(); /* slot 32 */
src/_ZN11dScMgBase_c9Virtual80Ev.c:28 void _ZN11dScMgBase_c9Virtual80Ev(void)
src/_ZN12dScMgSlot3_c9Virtual80Ev.cpp:29 extern "C" void _ZN12dScMgSlot3_c9Virtual80Ev(void)
The dScMgBase_c census across the stack:
| ref | header/definition mismatches |
|---|---|
origin/main |
2 |
origin/cpp/minigame-slot30 |
5 |
origin/cpp/minigame-slot31 |
6 |
origin/cpp/minigame-slot32 |
7 |
Plus dScMgSlot3_c::Virtual80, an eighth distinct symbol in a different class. Every one is header int / definition void. Nothing in the tree compares those two surfaces, so this compiles and links clean — Virtual80 is absent from include/decl_common.h, so the checker I opened in #2105 reports nothing here either. That gap is mine and I will close it.
On the other four I asked only for consistency and left the choice to you. Here I want to recommend void, and I think this PR supplies the argument for it.
Two reasons, both from your own work.
Your notes file already says void. notes/dScMgBase_c-slots-18-35.md on this branch:
| 29 | 0x74 | ... | OnAimedAtWithEgg | void(Obj*) |
| 30 | 0x78 | ... | OnAimedAtWithEggReturnVec | void(char*) |
| 31 | 0x7C | ... | (none) -> Virtual7C | void(void) |
| 32 | 0x80 | ... | (none) -> Virtual80 | void(void) |
Four rows, four void. The headers say int for all four. The notes read as the considered artifact; the headers read as the accident.
The "keep int, return the free r0 value" escape does not travel to the override. For the base it does — the body's last statement is LoadCompressedFileAt(...), so the value is already in r0. For dScMgSlot3_c::Virtual80 the last statement is a volatile store:
*(volatile unsigned short*)0x400000a = (*(volatile unsigned short*)0x400000a & 0x43) | 0x1118;There is no free return value there. Taking the int route on this PR means finding something to return in a body that ends by writing hardware — which is where a return 0; gets manufactured, and that is a mov r0,#0 the ROM does not have. Declaring void in both headers costs one word each and matches what both bodies actually do.
Either resolution is acceptable to me. Split is not, and I would rather it were settled once at the base of the stack than patched five times.
2. MEASURED overstates the arity evidence
The header says:
arity: no explicit parameters, MEASURED at that one call site.
I tried to confirm this and found that the load-bearing step does not hold. Everything below is measured. The site count and the branch identification are yours, and both check out.
Your three-site count is right, and so is the other-branch call. I confirmed +0x80 dispatch pairs at exactly ov004:0x020b0900, ov002:0x020effa4 and ov064:0x02116e58 across arm9 and 103 overlays. And I can now show the dPathLiftActor_c half positively rather than take it on trust — three vtables put AfterClsn at slot 32:
ov002/relocs.txt from:0x0210aff0 to:0x020eff18 _ZTV16dPathLiftActor_c @0x0210af70 +0x80
ov036/relocs.txt from:0x0211401c to:0x020eff18 _ZTV15daObjRcCarpet_c @0x02113f9c +0x80
ov100/relocs.txt from:0x021485fc to:0x020eff18 _ZTV15daObjPathLift_c @0x0214857c +0x80
Good call. That is the right identification and I would not have found it by inspection.
But the inference from mov r4,r1 is backwards. The argument is that r1 is parked into r4 because the call clobbers it, so "a callee that consumed r1 would not need it parked first". Here is the whole caller:
0x020b08f8 ldr r2,[r0]
0x020b08fc mov r5,r0
0x020b0900 ldr r2,[r2,#0x80]
0x020b0904 mov r4,r1 <- parked
0x020b0908 blx r2
0x020b090c mov r0,#2
0x020b0910 bl 0x0201fcd4
0x020b0914 bl 0x020ae330
0x020b0918 mov r0,r5
0x020b091c mov r1,r4 <- the caller's own later use
0x020b0920 bl 0x0202e62c
vfSuccess is handed to 0x0202e62c three calls after the dispatch. The caller must preserve it across all three regardless of what any of them read. A callee that did consume r1 would need it parked for exactly the same reason. So the park is evidence about the caller's liveness and says nothing about the callee's signature — and r1 holds a meaningful value at the moment of blx, which is also the shape a real argument would have.
The contrast with slot 31 is what a conclusive measurement looks like. On #2106 I checked your +0x7C scan: 39 sites, and all 39 load the pointer into r1. That is structurally decisive, because r1 cannot simultaneously carry a second argument. At +0x80 the three sites use r1, r2 and r3. Register choice there is tracking local liveness, not the callee's signature, so the same style of argument does not transfer.
One correction, and it cuts your way on the substance. The header groups ov002 and ov064 together as the AfterClsn sites. ov064 cannot be one:
0x02116e50 mov r0,r5
0x02116e54 ldr r1,[r0]
0x02116e58 ldr r1,[r1,#0x80]
0x02116e5c blx r1
The pointer is in r1, so no second argument can be passed — and _ZN16dPathLiftActor_c9AfterClsnEi mangles Ei, one int parameter. ov064 also carries zero dPathLiftActor_c symbols, and 0x02116e58 sits inside func_ov064_02116d1c, in the daOts_c neighbourhood. The ov002 site, meanwhile, is the two-argument one and establishes it explicitly:
0x020eff98 mov r0,r1
0x020eff9c ldr r3,[r0]
0x020effa0 mov r1,r2 <- second argument, established
0x020effa4 ldr r3,[r3,#0x80]
So the three sites are three hierarchies with at least two different arities at the same offset. None of that makes your conclusion wrong — the ov002 site shows what establishing an argument looks like, and ov004 does not do it, which is a real if indirect point in favour of nullary. It makes it a hint of the same strength as the return type, from a sample of one.
The ask is one word: drop MEASURED, or qualify it the way the return type is qualified (A HINT). Keep the rest of the block, including the sentence about r1 being vfSuccess — that is true and useful. It is only the conclusion drawn from the park that does not follow. The ov064 attribution should come out, since the register makes it impossible.
3. Off-by-one: this is the fifth, not the fourth
:366 slot 28 -- "the first in this campaign"
:425 slot 29 -- "the second consecutive slot that no body pins"
:510 slot 30 -- "the THIRD consecutive slot no body pins"
:606 slot 31 -- "the third consecutive slot no body pins" <- inherited from #2106
:693 slot 32 -- "and the FOURTH consecutive slot no body pins" <- should be FIFTH
Slot 31 already claims third on #2106 and I flagged it there; slot 32 then counts from the wrong number. Both live in include/dScMgBase_c.h, so please fix them in whichever of the two PRs lands second rather than in both — otherwise the second one conflicts on the same lines.
Ordering
Stack is #2099 → #2100 → #2102 → #2106 → #2107, and all five carry changes-requested from me on the same return-type split. That is deliberate: I would rather it be decided once at the base than five times. Nothing above is a byte-match concern, and the promotion mechanics in this one I checked and they hold.
1 participant
Slot 32 of
dScMgBase_c's 36-entry vtable. Base bodyov004:0x020b27f4,one descendant override in the whole family (
dScMgSlot3_c,ov006:0x0210aa60) — the cheapest slot in the campaign after 22's zero.Stacked on #2106 (slot 31). Base is
cpp/minigame-slot31.The name:
Virtual80, notAfterClsnThis is the defect settled at slot 31, one fork further out — and it is the more
misleading of the two, because
AfterClsnis a real ROM name.include/PathLift.h:58declares it and_ZN16dPathLiftActor_c9AfterClsnEiis agenuine mangled symbol. But:
The chains share only
dBase_c, which adds no virtual of its own, so the twoslot 32s have
fBase_c's first eighteen entries in common and nothing else.dPathLiftActor_c::AfterClsnalso takes anint; this slot takes nothing.Independent image-side check: scanning arm9 + all 103 overlays for the dispatch
pair at
+0x80—ldr rN,[rM,#0x80]withRn != pc, followed within threeinstructions by
blx rN— finds exactly three sites. One isov004:0x020b0900(this branch); the other two areov002:0x020effa4andov064:0x02116e58, which aredPathLiftActor_c::AfterClsnon that branch.Virtual80is the offset spellingfBase_calready uses (Virtual34,Virtual38) and the one slots 33-35 are recorded under innotes/dScMgBase_c-slots-18-35.md. It costs no new convention.What the slot actually is
Slots 31 and 32 are one function twice, against the two display engines.
Virtual7C)Virtual80)BG1CNT0x0400100a→0x100x0400000a→0x1000SetSubBg1Offset(0,0)SetBg1Offset(0,0)data_0209d454 &= ~2data_0209d45c &= ~2G2S::GetBG1CharPtrfunc_02054ea8G2S::GetBG1ScrPtr, file0x5bG2::GetBG1ScrPtr, file0x67BeforeInitResources, lastAfterInitResources(u32), firstThe two shadow words are exactly the pair slot 30 restores the two DISPCNTs
from, so 30/31/32 are one mechanism. It builds this minigame's top-screen
background. It touches no collision.
dScMgSlot3_c's override is the base body verbatim plus one more write, leavingBG1CNTat0x1118rather than0x1000— the same layer, pointed at thisminigame's own character and screen base blocks.
Arity and return type, measured
At
ov004:0x020b0900:r0isthis.r1is the caller's own parameter, parked inr4because thecall clobbers it and handed to
0x0203188cafterwards — a callee that consumedr1would not need it saved first. Return type stays a hint for the fourthconsecutive slot.
Verification
rombuild -j 16 --no-rom: 11,088/11,088 reproducing, 106/106 exact,100.000000% of compared bytes, ROM-build analysis PASS.
romdata_checkvs the slot-31 baseline:verifiedBytes 35416 -> 35420(+4),partialBytes 11748 -> 11872(+124). 128 bytes = 32 tables × one word,which is what declaring exactly one slot must cost.
symbols1246,verified465,partial253,differs6,totalRecords7560 — all unchanged. The
differingset is unchanged in both directions.check_dead_referencesclean ·port_refcheck405/405 ·check_header_offsets --changed origin/main0 mismatched ·check_src_tu_compiles97/97 ·check_duplicate_sources11038 stems, nonedoubled · langmode ratchet PASS.
What this PR does NOT establish
Virtual80is a placeholder, not a ROM name. The cartridge names nothingat this slot on this branch.
intis a hint, not a measurement — nothing reads the result.func_02054ea8is almost certainlyG2::GetBG1CharPtr: the G2S pair is0x02054e88/0x02055148and the G2 pair is0x02054ea8/0x02055168, thesame
+0x20apart, and slot 31/32 use them in the same two positions. That isan arm9 rename outside this campaign's scope and is deliberately not taken
here.
dActor_c's labels by index, staysthe owner's call.
Three slots left: 33, 34, 35.