Guard the write side, and bound a subscript by the extent the DWARF states - #13
Merged
Conversation
…tates
The scripting surface had no write API at all, so a script reaching for one
dropped to the raw bus. That bus rounds a misaligned store down and discards a
store to ROM, both silently. Alignment matters more here than on the read side: a
misaligned store does not merely write the wrong place, it overwrites the value
next door, changing the state being observed.
- write8/write16/write32/writeBytes are new, carrying the same guards as the
reads. The alignment message names the write alternative, not the read one.
- Variable paths take subscripts, bounds-checked against the DWARF extent:
readVariable('gLayers[2].width'), writeVariable('gGrid[1][3]', 0). Element 4 of
a 4-element array is a real address — whatever the linker placed next — so
without the bound it reads as plausible data and writes as corruption of
something never named.
- symbolExtent(name) reports an object's size and whether it came from st_size or
from the DWARF type, and a write starting inside a known extent that runs past
its end is refused, naming what it would have hit.
- addressToSymbol resolves linker-defined globals. An ldscript `gFoo = 0x...;` is
SHN_ABS/NOTYPE; excluding those left it unable to name a single data global in
a decomp ELF — it returned null for all 489 of them in the ELF used here.
Two things measurement changed:
- The subscript bound first used raw arrayDims, which reads GCC 2.95's unsized
`extern T x[][8]` as extent 1 and rejected every index above 0. It now uses the
variable-normalized dims, so an unstated dimension is unbounded rather than
bounded at 1. A guard that refuses legitimate work is its own defect.
- The overrun guard first fired on ordinary writes: a size-0 symbol's range is
inferred out to the next symbol, so any address in the gap after a small object
looked like an overrun of it. It now requires the address to be genuinely
inside the object.
Scope note: the bound only applies where the index is expressed. An address
computed past an array's end lands wholly inside its neighbour, and from an
address alone that is indistinguishable from a deliberate write there — the
guard cannot recover the intent, and the tests say so rather than implying
otherwise.
Both new guards were ablated. Disabling the bounds check fails the array-path
tests; disabling the overrun check fails the extent test — which it did NOT do on
the first attempt, because that guard had no coverage at all until a test was
written for it.
67 tests in gba-emulator (was 60), 232 in debug-info (was 223). check-types,
lint, format:check, build and test all exit 0.
… claim
- Both changesets cut to what changed.
- docs/scripting.md trimmed: the API sections say what the calls do and what they
refuse, without arguing the case for each guard.
- The extent claim was wrong in its reasoning. "In a decomp the answer is almost
always dwarf" named the wrong discriminator: what decides is how the symbol was
DECLARED, not what kind of project it belongs to. A global defined in C is
STT_OBJECT and the assembler sizes it; one placed by the linker is
SHN_ABS/NOTYPE with no size, so its extent can only come from the type of a C
extern declaration. Measured across the four test projects and one real decomp:
project data syms st_size dwarf none
agbcc-min 20 19 0 1
devkitarm-min 19 18 0 1
mips-min 13 12 0 1
ppc-min 13 12 0 1
klonoa-eod 489 0 453 36
Decomps do hit the second case constantly, because a fixed RAM address cannot be
a C definition — but that is a consequence of the declaration, not of the genre.
- The subscript tests move from a standalone spec onto the real project ELFs, and
run against all four toolchains and both byte orders rather than one. They use
fixtures every project already compiles: `g_rom_table` (3 x 2 bytes) for an array
global and `Probe.name` (6 x 1) for an array member. The malformed-path cases go
through resolveVariable on a real ELF instead of testing the parser directly.
The synthetic linker-globals unit test is replaced by one over `gAbsGlobal`, which
the two ARM projects really do place from their ldscript. That fixture also pins
the third extent case honestly: it is declared in no C, so symbolExtent reports
null rather than a guess.
Ablations re-run against the moved tests: disabling the bounds check now fails 8
tests (2 x 4 projects, up from 2), and excluding NOTYPE symbols from the address
index fails the linker-placed-global tests on both ARM projects.
253 tests in debug-info (was 232); check-types, lint, format:check, build and test
all exit 0.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Item 3 of the analysis-surface audit: the setup that silently isn't what you asked for.
The write side had no API
The scripting engine exposed reads but no writes, so a script needing one dropped to
rt.gba.bus.write*— which rounds a misaligned store down and discards a store to ROM, both silently. A misaligned store does not merely write the wrong place; it overwrites the value next door, changing the state being observed.write8/write16/write32/writeBytes(address, size, value)are new and carry the read guards: misaligned and undecoded addresses throw, and so does a write to read-only memory.Subscripts, bounded by the DWARF
readVariable/writeVariablepaths take subscripts, and every index is checked against the extent the DWARF states:Element 4 of a 4-element array is a real address — whatever the linker placed next — so without the bound it reads as plausible data and writes as corruption of something never named. On a real decomp ELF, a 4×28-byte layer array's element 4 begins at exactly the address of the pointer that array's owner dereferences.
Scope, stated plainly: the bound applies only where the index is expressed. An address computed past the end lands wholly inside its neighbour, which from an address alone is indistinguishable from a deliberate write there. The guard cannot recover the intent, and the tests say so rather than implying otherwise.
Extents, and where they come from
symbolExtent(name)returns{ size, source }—'st_size'or'dwarf'— ornullwhen nothing states it. Which one answers depends on how the symbol was declared: a global defined in C isSTT_OBJECTand the assembler sizes it; one placed by the linker (gFoo = 0x03000000;) isSHN_ABS/NOTYPEwith no size, so its extent can only come from the type of a Cexterndeclaration. Measured:Decomps hit the second case constantly, because a fixed RAM address cannot be a C definition — but that follows from the declaration, not from the genre.
A write starting inside a known extent and running past its end is refused, naming what it would have hit:
addressToSymbolnow resolves linker-placed globals too. ExcludingNOTYPE/SHN_ABSleft it unable to name a single data global in a decomp ELF —nullfor all 489.Verification
Everything new is tested against the real project ELFs in
test-projects, across all four toolchains and both byte orders, using fixtures each project already compiles (g_rom_table,Probe.name, and the ldscript-placedgAbsGlobalon the two ARM projects). Malformed paths go throughresolveVariableon a real ELF rather than testing the parser directly.Both new guards were ablated: disabling the bounds check fails 8 tests (2 × 4 projects); excluding
NOTYPEsymbols from the address index fails the linker-placed-global tests on both ARM projects. The overrun guard's ablation failed to fail on the first attempt — it had no coverage at all until a test was written for it.253 tests in
debug-info(was 223), 67 ingba-emulator(was 60).check-types,lint,format:check,buildandtestall exit 0.Not included
The reported
press()/pressSequence()disagreement does not reproduce. Measured per button on a real game, both register identically in the game's own edge-detected input global, in both measurement styles — 0 disagreements across all 10 buttons. Nothing was changed for it.