[AUTOMATED] feat(ghidra): Phase 4 — full response encode (rename/retype, switch recovery, real signatures in the GUI)

[mahaloz]

Phase 4 of the Ghidra integration (docs/ghidra-integration.md §12; Phases 1–3 = #135/#317/#318): the decompileAt response now carries the FULL first-<function> document — <localdb>, <highlist>, <jumptablelist>, <prototype>, <parammeasures> — plus the markup type-token fix, which is what lights up the native GUI features on a kuna core: real signature display, variable rename/retype that persists, switch recovery through the stock analyzer, and the param-ID analyzer.

What lights up (live-verified on stock Ghidra 12.1.2, fmt::main, pyghidra rig)

Every line below is a [PASS] from integrations/ghidra/live-smoke/phase4_features.py (new, committed) — the full report is reproduced at the end.

feature	before (Phase 3)	after (this PR)
Signature	prototype absent from the response; GUI showed the DB stub state	int main(int argc,char **argv) — model __stdcall, extrapop 8, ret int, 2 params, from <prototype> + localdb cat-0 symbols
HighSymbols	<localdb> absent — no rename/retype targets at all	2 params + 19 locals decode as HighSymbols with nonzero ids + storage
Rename (the keystone)	dead (no symbols)	HighFunctionDBUtil.updateDBVariable renames v11 @ RSI:8 → the DB variable at the same storage changes, zero collateral renames, and the new name survives the event-driven re-decompile (kuna_ren_probe in the next getC())
Retype	dead	retype to ulong persists across re-decompile (typelocked localdb seed)
Param rename	dead	argc → kuna_param_probe lands on the DB Parameter (cat-0 + slot index + exact storage — no forced full-signature commit)
Switch analyzer	<jumptablelist> absent — analyzer no-op	HighFunction.getJumpTables() = 10 cases + 10 labels; after deleting all references from the BRANCHIND, DecompilerSwitchAnalysisCmd rebuilds all 10 case references from kuna's tables
Param-ID analyzer	<parammeasures> stubbed	action paramid + parammeasures → getHighParamID() decodes 2 inputs + 1 output (ranked)
getC() type spelling	unsigned_long__a1 (the IllegalCharCppTransformer mangle)	unsigned long *a1 — zero mangled tokens

The encode (kuna-decomp)

Upstream child order addr → localdb → ast → highlist → jumptablelist → prototype (Funcdata::encode, funcdata.cc:734):

• Datatype::encodeRef/encode (substrate/dtype.rs) — the cross-cutting type marshal-out: <typeref name id> for id-carrying types (variable-length emits the size-independent hash_size id + instance size; get_unsized_id/has_same_variable_base completed), full <type> per C++ subclass override otherwise (pointer/array one-level descent, struct field/bitfield interleave, enum <val>s, <def> typedefs, PTRREL's full-ptrto + <off>). Differential-tested against kuna's own shipped decode: what encode_ref writes, decode_type resolves to the identical interned Rc.
• FuncProto::encode (p4_calls/fspec.rs) + EffectRecord::encode + model-diff effect/likelytrash lists. Params deliberately travel as localdb cat-0 symbols (upstream's symbol-backed store shape).
• <localdb> — ScopeLocal::encode (p6_variables/varmap.rs) → Database::encode_scope/Symbol::encode{_header,_body}/SymbolEntry::encode (p0_knowledge/database.rs), nametree order, equate symbols included.
• <highlist> — Funcdata::encode_high + the encode-time symbol-link pass kuna_link_high_symbols (the C++ ActionNameVars::linkSymbols stand-in: kuna names highs with strings, so the Symbols the wire needs are attached/materialized just before encoding; runs only from Funcdata::encode, so the standalone path is untouched). Five-way class rule verbatim (variable.cc:839).
• <jumptablelist> — the already-ported JumpTable::encode wired in, emitted independently of savetree; the session jumpload toggle now reaches FlowInfo::record_jumploads per decompile (loadtables collected).
• <parammeasures> — ParamIDAnalysis unstubbed (the justproto arm now reads the real recovered FuncProto), <rank> always on; under action paramid it is the ONLY doc child (ghidra_process.cc:318).
• Markup type tokens — EmitMarkup::tag_type splits a rendered declarator into word <type> tokens + <syntax> separators (template payloads stay intact). Markup-emitter-only, so standalone output is byte-identical by construction.

Symbol-id discipline (r5 §5)

• Invented locals/params carry kuna's internal SYMBOL_ID_BASE-range ids (legal on the wire native→Java; Java's decode already zeroes received internal-range ids — remote_provider.rs:522).
• Globals echo the REAL host DB id: GlobalEntry/GlobalContainer gained symbol_id, filled from the getMappedSymbols record; <high class="global" symref> is emitted only when a real id is known — never fabricated (a wrong non-internal id silently renames the wrong DB symbol).

The rename/retype persistence loop

A GUI edit is a DB write + event-driven re-decompile; Java's next getMappedSymbols answer carries the edited local in the function <localdb> (LocalSymbolMap.grabFromFunction). kuna now decodes those non-param locals (RemoteLocalVar) and seeds the fresh Funcdata:

• typelocked (a retype; Java sends typelock=false only for Undefined types) → real mapped/usepoint symbol seeds (survive restructure's typelock-keep rule);
• namelocked-only (a plain rename) → name recommendations — the C++ ScopeLocal::nameRecommend mechanism (such symbols never survive clearUnlockedCategory(-1) upstream either), applied by the ActionNameVars port (recommended_name_for, three-arm use-address match per varmap.cc:1050).

Root-caused live: the first implementation seeded renames as symbols and they vanished at restructure — exactly why upstream keeps them as recommendations. Verified against a live DecompileDebug capture (<symbol typelock="false" namelock="true" cat="-1"><typeref name="undefined8">).

Harness (ghidra-sim)

parse_decompile_doc now decode-validates the whole response the way Java would, and assert_phase4_traps asserts every r5 §3 hard-throw on every decompiled function:
nonzero symbol ids; ≥1 SymbolEntry (+ uselimit rangelist) per mapsym; positional <parent>+<rangelist> before <symbollist>; localdb-before-highlist and ast-before-highlist order; legal high class vocabulary; local/param symref resolves in the just-decoded localdb; repref resolves in the just-decoded ast; prototype model+extrapop+returnsym(addr+type); <rank> per measure; cat-0 symbols carry the slot index; unknown <function>/<doc> children are a test failure (Java discards the whole result).

New configuration tests: the switch-analyzer shape (noc+notree+jumpload → function-only doc, no ast/highlist/markup, jumptablelist with ≥2 dests and ≥1 loadtable), the paramid shape (parammeasures-ONLY doc), and the rename-persistence echo-back (serve a kuna-encoded local back renamed with typelock=false → the new name renders and re-encodes).

Pin table (ghidra_sim_e2e.rs, faillog)

pin	Phase 3	now
PIN_FAILLOG_MANGLED_TOKENS	[57, 10, 24]	[0, 0, 0]
PIN_FAILLOG_DIFF_FLOOR (measured)	[0.10, 0.15, 0.19] (0.170/0.184/0.264)	[0.02, 0.04, 0.06] (0.050/0.079/0.099)
PIN_FAILLOG_DIFF_CEILING	[0.25, 0.29, 0.34]	[0.09, 0.12, 0.15]
PIN_FAILLOG_C_LINES	[283, 39, 92]	unchanged (the split moves tokens, not text)
register/unique/placeholder/query pins	—	unchanged

The ghidra-vs-CLI gap collapsed to pure per-function analysis skew — the getC() mangling half of the residue is gone.

Gates

make test 675/675 PARITY OK · make test-stages PARITY OK · make rust-test green (full workspace) · make check-spec OK · make test-ghidra green (incl. the release breadth test). No baselines re-pinned; no options added; no ghidra-mode default changed (no new DIV rows). Spec prose updated in the same PR (00-overview, 04, 05, 06) + docs/ghidra-integration.md Phase-4 checkboxes and §9 seam table.

Deliberately deferred (documented in ghidra-integration.md)

• structureGraph (FunctionGraph nested layout only), the four signature commands (BSim), overlay spaces, getStringData charset fidelity — all with clean §10 degradation.
• <hash> dynamic-storage symbols for unique-space locals in the link pass (renames of unique-storage temps fall back to Java's address-keyed DynamicEntry.build path meanwhile); <override>/child-<scope> statics (Java skips both); the standalone collectNameRecs harvest + dynamic-hash recommendation list.
• Live observation (not a defect): fmt/main's jumptable reports 0 loadtables (model-dependent collection); the sim gate pins loadtables ≥1 on faillog where the emulated table walk collects them.

Live rig

integrations/ghidra/live-smoke/phase4_features.py (committed; same env as kuna_vs_stock.py). Full run on fmt::main:

[PASS] decompileCompleted
signature: int main(int argc,char **argv);
prototype: model=__stdcall extrapop=8 ret=int nparams=2
[PASS] signature renders with parameters
[PASS] getC() carries no IllegalChar-mangled type tokens
[PASS] locals decoded as HighSymbols (2 params, 19 locals)
[PASS] rename round-trip writes the RIGHT DB variable — v11 @ RSI:8 (pc 00101c77) -> kuna_ren_probe; db-hit=['RSI:8']; collateral-renames=[]
[PASS] re-decompile shows the new name
[PASS] retype round-trip — wanted ulong, got ['ulong']
[PASS] param rename lands on the DB parameter — argc -> DB params now ['kuna_param_probe', 'argv']
[PASS] HighFunction.getJumpTables decodes <jumptablelist> — @001019fb cases=10 labels=10
[PASS] DecompilerSwitchAnalysisCmd rebuilds the case references — cleared to 0, rebuilt 10 refs
[PASS] getHighParamID decodes <parammeasures> — inputs=2 outputs=1

---

Review revisions (adversarial review round — 6 confirmed findings, all fixed)

Commits 49e1b84f (code+tests) and e6b85c1b (docs).

#	finding	fix
C1≡C5 (MED)	<vardecl symref> still emitted the Phase-2 varnode-create-index placeholder — unresolvable now that every real id is SYMBOL_ID_BASE-range, so Java logged "Invalid symbol reference" per declaration per decompile and rename/retype was dead on declaration-line tokens	The link pass now runs before the markup, and the declaration carries the real LocalSymbolMap id + the representative's varref; create-index survives only as the fallback for a high the analysis left symbol-less. Making that reorder safe required the pass to stop mutating the analysis scope — see the design note below.
C2 (HIGH)	host locals whose only entry was a <hash> DynamicEntry were dropped on decode — the storage class Java writes for every requiresDynamicStorage variable (unique-space representatives, splitOutMergeGroup products), so renaming one silently reverted	Hash entries kept (RemoteEntry::hash, RemoteLocalVar::hash) and applied through the full upstream mechanism: ScopeLocal::dynRecommend + recoverNameRecommendationsForSymbols's hash loop (kuna_apply_dynamic_recommendations over DynamicHash::find_varnode, at the top of the naming pass); typelocked ones seed as dynamic Symbols. Hash budget pinned to upstream's 8 — Java hardcodes the same (DynamicHash.java:440) and a disagreeing hash cannot round-trip.
C3 (MED)	recommendations matched only the high's name representative's def address; upstream's findVarnodeWritten matches any varnode of (size,addr) written at the usepoint, so renames recorded at another instance reverted	Matched across every instance, plus upstream's !sym->isNameUndefined() guard and a cat-0 guard (a parameter's name comes from the prototype).
C4 (MED)	the encode-time link pass bound any unattached high to the smallest containing entry with none of the naming pass's conflict logic — a high deliberately routed to vN inherited the parameter's id, so renaming it renamed the parameter	The naming pass now records its bind decisions (HighVariable::kuna_link_symbol); the encode materializes symbols only for highs left unbound, and a covered-but-unbound (conflict) high gets a data-flow-hashed wire symbol — upstream's buildDynamicSymbol — never the conflicting entry's id.
C6 (MED)	decoded cat-0 parameter storage and the <prototype model> were parsed then discarded; kuna echoed model-rederived storage, so Java's checkFullCommit force-committed a kuna-rederived signature over the user's on any param rename	Both carried through (RemoteParam::storage, RemoteFunctionFacts::{model,param_storage} → apply_locked_prototype_with_model + apply_mapped_params); add_param_symbol categorizes an existing overlapping entry as the parameter slot instead of skipping it.
LOWs	(a) defensive encode_scope skips could orphan a <high symref> — the Java hard-throw the skip guards against; (b) two vacuous live-rig PASS criteria; (c) missing r5 §3 0-sized-datatype trap	(a) skipped ids are withheld from symref too (Database::encodable_symbol_ids); (b) collateral detection now commits the decoded locals first (21 DB variables on fmt/main) and the param check asserts decoded-vs-DB storage/ordinal equality before renaming, so a forced full commit can no longer masquerade as an in-place edit; (c) trap added.

Design note: wire-only symbols (why the C text is provably unchanged)

The reviewers' C1 fix (link pass before the markup) was re-derived as text-neutral. It was not — the harness caught it: mutating the local scope before printing moved faillog sub_2620 from 283 → 284 lines. The pass therefore no longer touches the analysis scope at all: it emits database::WireSymbol records — encoded into <localdb> and referenced by <high symref>/<vardecl symref>, never entering the scope — so it cannot perturb decompilation wherever it runs. c_lines is back to 283/39/92, i.e. byte-identical C, and that pin is the standing proof.

Pins (this round)

pin	before	after
PIN_FAILLOG_C_LINES	[283, 39, 92]	unchanged (the invariant)
PIN_FAILLOG_MANGLED_TOKENS	[0, 0, 0]	unchanged
diff floors / ceilings	[0.02…] / [0.09…]	unchanged (0.050/0.079/0.099)
PIN_FAILLOG_VARDECL_UNRESOLVED	(new)	[0, 1, 0] — was effectively "every declaration"; one group-member residue documented as a follow-up
query traffic (getPcode/getMappedSymbols)	1314 / 1448	unchanged

New tests

• unit dynamic_name_recommendation_renames_the_hashed_variable — hashes a real varnode, seeds the recommendation, applies it, asserts the name and a Symbol land on that variable (the C2 mechanism, non-vacuously).
• sim ghidra_sim_faillog_high_symrefs_are_not_shared_with_params — no non-param <high> may reference a cat-0 parameter symbol (the C4 regression).
• sim ghidra_sim_faillog_dynamic_rename_persistence — hash-storage rename round-trip through the oracle's new dynamic-entry serving.
• harness <vardecl symref> resolution + the 0-sized-datatype trap; the oracle serves <hash> locals and typelock=false/true shapes.

Live re-verification (stock Ghidra 12.1.2, fmt::main)

13/13 PASS, including the strengthened criteria:

[PASS] DB carries committed locals to detect collateral renames — 21 DB variables
[PASS] rename round-trip writes the RIGHT DB variable — v9 @ RAX:8 -> kuna_ren_probe; db-hit=['RAX:8']; collateral-renames=[]
[PASS] re-decompile shows the new name
[PASS] retype round-trip — wanted ulong, got ['ulong']
[PASS] encoded param storage/ordinals match the DB (no forced full commit) — decoded=[('argc','EDI:4',0),('argv','RSI:8',1)] db=[('argc','EDI:4'),('argv','RSI:8')]
[PASS] param rename lands on the DB parameter
[PASS] DecompilerSwitchAnalysisCmd rebuilds the case references — rebuilt 10 refs
[PASS] getHighParamID decodes <parammeasures> — inputs=2 outputs=1

Gates (post-fix)

make test 675/675 PARITY OK · make test-stages PARITY OK · make rust-test exit 0 (316 suites) · make check-spec OK · make test-ghidra green. No baseline re-pins, no options, no DIV rows.

---

Review round 2 (delta review of e6b85c1b + the red CI gate) — 10 findings

The branch was red on its own gate: ghidra_sim_sort_grep_breadth (the #[ignore]d sort/grep breadth test that make test-ghidra and the CI gates job run with --include-ignored) failed on tests/bug-repro/sort / sub_6370 with "NO <vardecl symref> resolves against <localdb>". Fixed, plus the nine other findings from the delta review.

The CI failure — root cause

sub_6370 declares two stack aggregates, char v1 [16] and char v2 [24], that are only ever used as memcmp(v1,v2,0x10) — i.e. reached exclusively through a &sym reference. The whole HighVariable of such a variable is the constant PTRSUB offset operand (dumped: one instance, const space, is_constant=true, is_addr_tied=false), and kuna_link_high_symbols skips constants by design (upstream's if (!vn->isPersist()) create guard never fires for one), while kuna_high_symbol_wire_id's re-derivation loop only scans addr-tied instances. Both declarations therefore fell back to the varnode create index — 2 of 2, hence "wholesale". The Symbols themselves were in <localdb> all along (v1/v2, cat −1, real ids).

The missing link was an actual port gap, not a heuristic: Funcdata::link_symbol_reference copies the referenced Symbol's name, offset and type onto that high but dropped its identity, where upstream Varnode::setSymbolReference (varnode.cc:465) → HighVariable::setSymbolReference(entry->getSymbol(), off) (variable.cc:283) carries the Symbol itself. The identity is now recorded (HighVariable::kuna_ref_symbol) and read by the declaration's symref.

It is deliberately a separate field from kuna_link_symbol: the latter feeds <high symref>, and such a high encodes class="constant", where Java's HighConstant.decode does nothing at all with a mapped local symref — so nothing is gained there, and keeping it out preserves the one-owner invariant ghidra_sim_faillog_high_symrefs_are_not_shared_with_params asserts.

This also closed the pinned residual: PIN_FAILLOG_VARDECL_UNRESOLVED moves [0,1,0] → [0,0,0] — the sub_3320 survivor was the same class. Every declaration in the corpus now resolves, and the pin doc says ZERO is the contract, not a high-water mark.

Test hygiene — why a green local run shipped a red CI

The previous round's recorded verification run (scratchpad/ghidra_sim_run.log) reads:

test ghidra_sim_sort_grep_breadth ... ignored, heavier fixtures (…)
test result: ok. 8 passed; 0 failed; 1 ignored

— i.e. cargo test -p kuna-ghidra --release without --include-ignored. The breadth test was never executed locally, and nothing said so.

That is a harness bug, and make test-ghidra is now two canaries instead of a bare cargo test:

1. the CI skip canary (skipping (… .sla …)) — with specs missing or unusable every ghidra-sim test prints a skip notice and returns early by design, so a green run proves nothing. CI greps for it; the Makefile did not. Worktrees hit this constantly (KUNA_SPECS/SLEIGHHOME do not reach the cargo suites).
2. a breadth canary — the target now demands the literal test ghidra_sim_sort_grep_breadth ... ok line, so any invocation that loses --include-ignored fails loudly instead of exiting 0.

The breadth test itself was also strengthened per the reviewer's note that the generic resolved > 0 gate only catches wholesale breakage: it now asserts 0 unresolved per target (and prints vardecl_unresolved/vardecls), so a 20/20 → 1/20 regression can no longer pass.

The other nine

#	sev	finding	disposition
2	HIGH	add_param_symbol promoted any storage-overlapping symbol into parameter slot i; the cited ProtoStoreSymbol::setInput does the opposite (looks up by slot, and removes+recreates on an addr/size mismatch). Not ghidra-gated — it runs in ActionRestructureVarnode, and clear_unlocked_category_negative only clears cat < 0, so a wrong promotion is permanent	Fixed: categorize only on an exact (addr, size) match; comment rewritten to the real upstream mechanism and to both failure modes (checkFullCommit storage compare; getDatabaseParameter keying the DB slot off getCategoryIndex)
3	MED	WireSymbol::encode bypassed the 0-size-type filter added for scope symbols → MappedEntry.decode hard-throw discards the whole result	Fixed two ways: kuna_link_high_symbols routes a 0-sized dtype to the hashed shape (DynamicEntry.decode has no size check), and WireSymbol::is_encodable is the backstop in both the encode and the id set
4	MED	<vardecl symref> was not filtered through the encodable predicate → a defensively-skipped symbol yields an unresolvable declaration ref	Fixed: every branch of kuna_high_symbol_wire_id now passes ScopeLocal::symbol_is_encodable (new O(1) single-symbol form — the set build would be per-declaration on every decompile) and falls back to the create index
5	MED	kuna_apply_dynamic_recommendations runs at the top of the naming pass, where the ported isNameUndefined guard is vacuous, and it creates a Symbol where upstream renames one	Guarded, not moved (kuna fuses linkSymbols + vN assignment into one walk, so there is no "after linking, before defaults" point): the equivalent guard is applied against the scope — a hash landing on storage the walk will bind to a function_parameter, or to any Symbol with a defined name, is skipped. Divergence documented in docs/spec/06 and the ghidra-integration deferred list. Also: find_varnode's vnlist2[pos] is now a checked .get() (the hash is host-supplied), and the hardcoded 8 carries the "not dynamichashmax" note
6	MED	the wire arm of encode_high skipped the encodable_symbols guard; encodable is empty for a has_no_code() function → class="local" + symref against a <localdb> never written → HighLocal.decode hard-throw	Fixed: the wire arm takes the same gate (falls to class="other")
7	LOW	Funcdata::clear() left kuna_wire_symbols / kuna_wire_symbol_for_high behind — HighVariableId-keyed maps outliving the arena that issued the ids	Fixed: both cleared with high_bank
8	LOW	declaration tokens carried varref; upstream emitVarDecl pushes an explicitly null Varnode, and the omission is load-bearing (ClangVariableToken.getHighVariable returns inst.getHigh() from inside its inst != null block, never reaching the parent-decl fallback)	Dropped the varref — upstream shape restored; the create index remains the symref fallback
9	LOW	slot skew: to_pieces compacts out params with no dtype while param_storage carried absolute rp.index → count/index disagreement → checkFullCommit fires	Fixed: param_storage slots are counted in the same compacted basis
10	LOW	live rig: len(before) >= 2 hard-fails on a function with one DB local; p0 = decoded[0] if decoded else p0 swapped the rename target mid-section	Fixed: gate on >= 1 (0 committed = the rig's premise failed) with a NOTE at 1; the rename target and its recorded old name now both come from the post-commit result

Doc correction (reviewer-verified): checkFullCommit inspects the parameter count, each categoryIndex and each storage — never the model name. The C6 prose in docs/spec/00-overview.md said otherwise; corrected to "the storage echo is the load-bearing half; the model rides along because the storage kuna would otherwise derive comes from it".

Pins (this round)

pin	before	after
PIN_FAILLOG_VARDECL_UNRESOLVED	[0, 1, 0]	[0, 0, 0] — the residual was the same &sym-reference class; ZERO is now the contract
breadth vardecl_unresolved	(unasserted)	0 per target, 6/6 targets (sort + grep)
PIN_FAILLOG_C_LINES	[283, 39, 92]	unchanged — the standing byte-identical-C proof
PIN_FAILLOG_MANGLED_TOKENS / placeholders / registers / uniques	—	unchanged
diff floors / ceilings (measured)	0.050 / 0.079 / 0.099	unchanged
query traffic (getPcode 1314, distinct 801, getMappedSymbols 1448)	—	unchanged

Gates (round 2)

make test 675/675 PARITY OK · make test-stages 484/484 PARITY OK · make rust-test green · make check-spec OK · make test-ghidra green with the breadth test proven to have run (ghidra_sim_sort_grep_breadth ... ok, 11 sim tests, both canaries armed). No baseline re-pins, no options added, no DIV rows.

🤖 Generated with Claude Code

https://claude.ai/code/session_01AfGYKwUWvPYYj1Aw7tcLhC