[AUTOMATED] feat(p6): framelayout — report the recovered stack frame, not just the surviving declarations (DIV-97) - #355
Merged
Conversation
… not just the surviving declarations (DIV-97)
`ActionRestructureVarnode` rebuilds the stack-frame layout from scratch on every
`mainloop` pass and clears the previous pass's unlocked symbols first, so a slot
is in the FINAL layout only if a stack Varnode for it was still live at the last
pass. At -O0 every local is a spill slot: `RuleLoadVarnode`/`RuleStoreVarnode`
fold the store/load pair into a sized stack COPY, copy-propagation folds that,
dead-code removal takes the Varnode, and the slot survives in an early layout and
in no later one.
Dropping it from the emitted C is right -- there is no expression left to declare
(zlib `zlibCompileFlags` is correctly one `return 0xa9;`) -- but the FRAME still
has the slot, and `extract_variables` is documented as a description of the
recovered frame, the way IDA's stack view and Binary Ninja's variable list are.
It was silently reporting a subset of it: over 82,049 scored decbench functions
kuna reported 154,716 of the 184,522 ground-truth stack variables (84%), against
angr's 203,329 and IDA's 178,337, while being the most PRECISE column of the five.
`restructure_varnode` now folds each pass's NO_CATEGORY stack symbols into a
per-`Funcdata` union (first writer wins -- the earliest pass saw the most dataflow
standing) and `extract_variables` appends every recorded offset no parameter or
surviving local already covers. Two spellings are normalized on the way out,
both defects in their own right: a slot the type system never committed to is
carried as `xunknown1[N]` and rendered `char[N]`, asserting an element type the
recovery never established, so it is reported as the width-only `undefined<N>`;
and Ghidra's internal `$$undefNNNNNNNN` placeholder for an unnamed symbol is
renamed to its stack-view form `local_<hex>` rather than leaking onto a public
interface.
JSON surface only -- no p-code, no emitted C -- which is why the datatest corpus
cannot observe it and GED/byte_match cannot move.
Measured with `scripts/decbench/typesweep.py`, the type analog of `optsweep` this
commit also adds (both arms one kuna binary over the same 803 slices, invoked
exactly as decbench's `kuna_raw` backend does, scored through
`TypeMatchMetric.compute_for_binary`; baseline arm agrees with the tree's
published per-function verdict on 77,837/78,887 = 98.67%):
type_match perfect 4,916 -> 5,943 (+1,027)
moved ONTO perfect 1,028 moved OFF perfect 1
improved 8,942 worsened 55
aggregate 18,650.16 -> 21,240.73
That is 4th place to 2nd on the one metric kuna ranked worst on, past IDA (5,062)
and Binary Ninja (5,771), 424 behind angr (6,367).
What the number is NOT: a control that ignores the binary entirely -- spraying a
CONSTANT slot list at offsets -64..-4 -- reproduces 93-98% of the same gain. The
metric rewards coverage, so this measures REPORTING COVERAGE, not improved type
recovery, and must not be cited as evidence that kuna's inference got better.
The reason to ship is the surface contract, not the score.
Gates: `make test` 675/675 PARITY OK (no re-pin), `make test-stages` 568/568
PARITY OK, `make rust-test` green, `make check-spec` green.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FvpHGrvHyKDNmLSPVnZw2G
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.
What
kuna decompile-all --json'svariablesarray is documented as a description of the recovered stack frame — the analogue of IDA's stack view and Binary Ninja's variable list. It was silently reporting a subset of it.ActionRestructureVarnoderebuilds the frame layout from scratch on everymainlooppass and clears the previous pass's unlocked symbols first, so a slot is in the FINAL layout only if a stack Varnode for it was still live at the last pass. At-O0every local is a spill slot:RuleLoadVarnode/RuleStoreVarnodefold the store/load pair into a sized stack COPY, copy-propagation folds that, dead-code removal takes the Varnode — and the slot survives in an early layout and in no later one.Dropping it from the emitted C is right (zlib
zlibCompileFlagsis correctly onereturn 0xa9;). Dropping it from the frame report is not.How
restructure_varnodefolds each pass'sNO_CATEGORYstack symbols into a per-Funcdataunion (Funcdata::record_frame_slots; first writer wins, because the earliest pass saw the most dataflow standing). Recorded unconditionally — one map insert per stack symbol per pass.extract_variablesappends every recorded offset no parameter or surviving local already covers, underoption framelayout(default ON).xunknown1[N]and renderedchar[N]— asserting an element type the recovery never established — so it is reported as the width-onlyundefined<N>;$$undefNNNNNNNNplaceholder is renamed to its stack-view formlocal_<hex>instead of leaking onto a public interface.JSON surface only — no p-code, no emitted C. The datatest corpus cannot observe it and GED/byte_match cannot move.
Measurement
New
scripts/decbench/typesweep.py— the type analog ofoptsweep. Both arms are one kuna binary over the same 803 slices, invoked exactly as decbench'skuna_rawbackend does, rebuilt into aDecompilationResultover the tree's own checkpoint function set and scored throughTypeMatchMetric.compute_for_binary(the same callreeval_typematch.pymakes, so the per-binary offset calibration is identical to the published one).Harness control: the baseline arm agrees with the tree's published per-function
type_matchon 77,837 of 78,887 shared functions (98.67%); the residual is kuna-build drift over the 65 commits since the tree was scored.type_matchperfecttype_matchThat is 4th place to 2nd on the one metric kuna ranked worst on (it is 1st on GED and 1st on byte_match) — past IDA (5,062) and Binary Ninja (5,771), 424 behind angr (6,367).
The single off-perfect regression is structural and priced in the DIV row: adding a slot at a ground-truth offset lets the metric's offset pass claim a variable the name pass would otherwise have claimed with a better type.
What the number is NOT
A control that ignores the binary entirely — spraying a constant slot list at offsets −64..−4 into every function — reproduces 93–98% of the same gain. The metric's denominator is the ground-truth variable set, so an entry at a plausible frame offset converts a miss into either a hit or a same-scoring wrong answer: coverage alone buys most of the score.
The slots this option reports are real (each was minted by a
restructure_varnodepass from live stack Varnodes) and the control is not what shipped — but +1,027 measures reporting coverage, not improved type recovery, and must not be cited as evidence that kuna's inference got better. The reason to ship is the surface contract. The cost to price against it: ~255k additional entries on a public JSON surface (the WASM front-end reads it; the Ghidra front-end does not), most of them width-onlyundefined<N>.Gates
make test— 675/675 PARITY OK (no re-pin)make test-stages— 568/568 PARITY OKmake rust-test— green (330 result blocks, 0 failures)make check-spec— greenkuna catalog --check— greenCatalog counts bumped: 124 → 125 settables, tier totals 26→27 core, live-field 36→37, static-form rows 123→124,
phase_catalog.jsonfixture regenerated.New two-pass gate
decompiler/crates/kuna-console/tests/verify_framelayout.rs: OFF is the pre-feature surface, ON is a strict superset that only ever appends stack slots at offsets OFF did not report, never leaks$$undef, and never reportschar[N]for an uncommitted type. Asserted non-vacuous.Docs: DIV-97 in
docs/history.md,docs/spec/06-variables-and-merge.md§6.2,docs/options.mdregenerated.🤖 Generated with Claude Code
https://claude.ai/code/session_01FvpHGrvHyKDNmLSPVnZw2G