Skip to content

[AUTOMATED] fix(p9): arraysubfield - a partial access into an array symbol reports the size it actually touches - #348

Merged
mahaloz merged 2 commits into
mainfrom
feat/arraysubfield
Aug 27, 2026
Merged

[AUTOMATED] fix(p9): arraysubfield - a partial access into an array symbol reports the size it actually touches#348
mahaloz merged 2 commits into
mainfrom
feat/arraysubfield

Conversation

@mahaloz

@mahaloz mahaloz commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Supersedes #288 ([PROPOSAL] wideintrender), rescoped to its Option A only. B and C are
recorded as blocked/refiled below; C is now issue #343.

A partial access into a plain array-typed symbol never entered the partial-symbol walk,
so it rendered a subscript chosen without ever reading the access size. An eight-byte store
came out as v1[0] = a0 — a statement that names one char, and therefore claims a
one-byte store
. That is wrong output, not a style preference.

The reproducers, verbatim

1. The reported oneinterp-bee eval_result_noun @ 0xa3e0
(/home/mahaloz/dc_finals_26/b_chall/jail/interp-bee), mov %rdi,%rdx; xor %eax,%eax; ret:

/* before */                          /* after */
undefined16 sub_a3e0(uint8 a0)        undefined16 sub_a3e0(uint8 a0)
{                                     {
  char v1 [16];                         char v1 [16];

  v1[8] = 0;                            v1._8_8_ = 0;
  v1[0] = a0;                           v1._0_8_ = a0;
  return v1 << 0x40;                    return v1 << 0x40;
}                                     }

2. Locally built __int128 shiftunsigned __int128 shift64(unsigned long x) { return (unsigned __int128)x << 64; }, gcc -O2 -c. Byte-identical asm to #1, byte-identical output.

/* before */  v1[8] = 0;  v1[0] = a0;      /* after */  v1._8_8_ = 0;  v1._0_8_ = a0;

3. A plain 16-byte struct returntypedef struct { unsigned long tag, payload; } val_t; val_t mkval(unsigned long p) { val_t v; v.tag = 0; v.payload = p; return v; }. Same output
again, which is the point: this is not __int128-specific, it is the 16-byte
register-pair return generally.

Two more shapes the original proposal did not mention, both also repaired on the store side:

/* add128, before */          /* after */
v1[0] = a2 + a0;              v1._0_8_ = a2 + a0;
v1[8] = a3 + a1 + ...;        v1._8_8_ = a3 + a1 + ...;
return v1;                    return v1;                 <- still a bare array return

/* mul_wide, before */        /* after */
v1[8] = 0; v1[0] = a0;        v1._8_8_ = 0; v1._0_8_ = a0;
v2[8] = 0; v2[0] = a1;        v2._8_8_ = 0; v2._0_8_ = a1;
return v1 * v2;               return v1 * v2;            <- still array * array

Root cause

decompiler/crates/kuna-decomp/src/p9_emit/printc.rs, ~10 lines.

Upstream PrintC::pushSymbolDetail (printlanguage.cc:256-258) routes every partial
cover of a mapped Symbol through pushPartialSymbol and lets that walk's per-type arms pick
the token. kuna's port routed TYPE_STRUCT and TYPE_UNION in, but split TYPE_ARRAY off
into a dedicated branch at printc.rs:7241 that computes

let index = sym_off / elsize;   // then emits name[index]

and never reads v.get_size(). For undefined1[16] (elsize == 1) an eight-byte access at
offset 0 became v1[0] and at offset 8 became v1[8].

The correct machinery was already ported and already working — it was simply unreachable for
a plain array:

  • push_partial_symbol_ir's TYPE_ARRAY arm (printc.rs:6620) carries upstream's
    TypeArray::getSubEntry guard, noff + sz <= elsize;
  • on failure it falls to PartialEntry::Unnamed(off, sz) (printc.rs:6720) →
    printlanguage::unnamed_field (printc.rs:6812) → ._0_8_.

kuna emits that token constantly — 828 renders across the 12 binaries measured below — but
had emitted it zero times on an array-typed symbol, across all 14,080 functions.

The fix admits plain TYPE_ARRAY to the routing gate. Re-routing rather than a size guard
on the standalone branch
: re-routing preserves arr[3] for a genuine in-element access via
the walk's own arm and keeps descending afterwards (an array of unions still resolves to
arr[3].ffield); a bare guard would have dropped in-element accesses to a bare name. The
comment block at printc.rs:7186 already argued for exactly this change and asserted it was
byte-inert for plain arrays — the measurement below falsifies that, and the comment now says
what is true.

Gating: unflagged, decided after measuring

CLAUDE.md: a strict bug fix that only ever corrects wrong output needs no flag; when in
doubt, gate it. So the blast radius was measured first.

kuna decompile-all --json | jq -r '.functions[].code', before vs after, over 12 binaries
(tests/bug-repro/*, four /usr/bin, both b-chall binaries, plus the local reproducer):

binary functions changed functions changed lines
betaflight_STM32F405.elf 5797 70 944
interp-bee 1363 75 1564
jit-bee 1279 73 1812
ssh 983 39 262
tar 885 24 284
grep 449 10 39
sort 490 6 14
openssl 1995 5 17
gzip 145 1 5
libselinux.so.1 621 1 2
faillog 69 0 0
repro.o 4 4 10
total 14,080 308 4,953

Every one of the 4,953 lines is accounted for by a checker that demands each removed
subscript reappear as a sized member on the same name:

kind lines
a subscript that spanned elements becoming the sized member 4,937
a within-element truncation gaining the explicit cast the walk's finalcast arm emits 13
a bare array name used as a scalar operand becoming the piece it actually is 3
lost subscript with no replacement (would be a regression) 0
unexplained 0

._<off>_<size>_ renders on a declared array local: 0 → 4,509 (in 220 functions).

Sampled hunks:

 grep     sub_6a70   - *(unsigned long *)v17 = s_21060[0];
                     + *(unsigned long *)v17 = s_21060._0_8_;
 tar      sub_23890  - dat_80b40 = s_67680[0x40];
                     + dat_80b40 = s_67680._64_8_;
 beta     sub_800fcf8- ... && (sub_800fcc8(0) == s_800fd37[1]))
                     + ... && (sub_800fcc8(0) == s_800fd37._1_4_))
 openssl  sub_63f10  - v10[8] = *(uint4 *)&v6[1];
                     + v10._8_4_ = *(uint4 *)&v6[1];
 ssh      sub_cce0   - v65[0] = 0;                  /* uint4 v65[260]; a 1-byte store */
                     + v65[0]._0_1_ = 0;
 gzip     sub_4570   - v17[0] = 0;                  /* char v17[8]; a 16-byte store */
                     + v17._0_16_ = 0;
 beta     sub_8051f38- v7[0] = CONCAT22(v7,...);     /* a bare array as a scalar operand */
                     + v7[0] = CONCAT22(v7[0]._2_2_,...);
 beta     sub_804ab58- v5 = (char *)sub_804c98c(v4,v7[0]);
                     + v5 = (char *)sub_804c98c(v4,(char)v7[0]);

The ssh/gzip pair is worth reading twice: v65[0] = 0 was a one-byte store rendered
as a whole uint4 element, and v17[0] = 0 was a sixteen-byte store rendered as one
char. The defect ran in both directions.

Zero genuine arr[N] in-element renders move. There is no judgment call in the changed
set, so it ships unflagged, and because it restores upstream's own rendering it earns a
Convergences row in docs/history.md, not a DIV row (the registry's own rule: "a port
defect whose fix moves kuna back onto upstream's default earns no DIV row"). No DIV number
is claimed
, so there is nothing to renumber at merge.

The clearest single witness is tests/stages/kuna-rustabi.xml, where one function contains
both kinds at once:

  v2._1_7_ = 0;            /* was v2[1]   -- a 7-byte write  */
  v2[0] = a0 < 0xb;        /* unchanged   -- a real 1-byte element write */
  v2._8_4_ = v1;           /* was v2[8]   -- a 4-byte write  */
  v2._12_4_ = 0;           /* was v2[0xc] -- a 4-byte write  */

What this does NOT fix

return v1 << 0x40; survives. This PR does not produce valid C, and the title does not
claim it does. Shifting the whole container is upstream-faithful (Ghidra emits it too) and
still not compilable.

The proof is quantitative: the scout's invalid-C detector — an array used as a bare operand
of << >> * / % ^ & |, or bare-returned — counts exactly the same functions before and
after
. Reproducing the scout's 201 over its seven binaries: interp-bee 99, jit-bee 90,
sort 5, grep 4, openssl 2, libselinux 1, gzip 0 = 201 before, 201 after (236 over the
twelve measured here, also unchanged). This fixes the store size, not the container.

Options B and C: why they are not here

B — a scalar 16-byte integer type (P5). Blocked on there being no C spelling for one.
substrate/dtype.rs:6088 is the right site and the change there is small, but
p9_emit/kuna_ctypes.rs:109 (integer_spelling) knows only int_size / long_size /
long_long_size — there is no 16-byte entry, so a scalar 16-byte TYPE_INT prints
uint16, a kuna type name and still not compilable C. B needs an __int128 spelling first.
Its payoff is also narrow: 147 of the 201 invalid-C functions are 16/32-byte SIMD blobs
(AVX2 pmovmskb chains) that are TYPE_UNKNOWN and must stay arrays — B's own carve-out
excludes them. Confirmed: --option ctypes on changes nothing here.

C — a mid-end fold. Its target subset is a coverage gap in the already-ported
RuleDivOpt
(p3_dataflow/ruleaction_6.rs:2094), which demonstrably fires: in sort
sub_d040 @ 0xd040 the same function recovers a0 / 3, a0 / v3 and a0 % v3 while
leaving SUB168(v1 * ZEXT816(0xaaaaaaaaaaaaaaab),8) & 0xfffffffffffffffe — the (a0/3)*2
half of an a0 % 3 — raw. Filed as #343. It is P3 work with nothing to do with
rendering.

Interaction with --option returnpair single (unmentioned in the proposal)

returnpair single already produces the valid-but-lossy IDA rendering for the return
subcase, and did so before this PR:

$ kuna decompile ./repro.o shift64  --option returnpair single
unsigned long shift64(void) { return 0; }
$ kuna decompile ./repro.o mul_wide --option returnpair single
long mul_wide(long a0,long a1) { return a0 * a1; }

The two are orthogonal and do not interact: returnpair single removes the 16-byte
container entirely, so no array symbol survives for the partial walk to render. Verified
identical output before and after this change on all three reproducers.

Test

tests/stages/kuna-arraysubfield.xml (new). One image, four functions, gcc -O2 -no-pie:
lo8/hi8 read 8 bytes at offsets 0 and 8 of a char g[16] global (the repair), elem
reads one byte at offset 3 (the preservation), shift64 is the reported reproducer.

Because the change is unflagged there is no option to switch off for a two-pass arm; the
before/after is pinned instead by assertion #3 (min=0 — no size-blind subscript survives on
either wide read) against #1/#2, and reproduced by hand against a pre-fix binary:

             before                     after
lo8      return g[0];               return g._0_8_;
hi8      return g[8];               return g._8_8_;
elem     return g[3];               return g[3];        (unchanged)

Assertion #7 pins return v1 << 0x40; — the part that is deliberately not fixed.

Four existing stage assertions and two kuna-console integration-test pins moved, every one
of them a pin on the defective spelling: ghdec-returncopysplit #1/#5/#6,
kuna-retinputhalf #2, kuna-rustabi #5 (plus a new kuna-rustabi #11 pinning the
preserved one-byte subscript), verify_return_uncomputed, verify_rustabi_pair.

verify_return_uncomputed deserves a note: it also carries a min=0 arm — "nothing should
write the phantom high half", spelled !code.contains("[8] =") — which this repair would
have made vacuously true rather than failing. It was widened to reject both spellings
(!contains("[8] =") && !contains("._8_8_ =")) so it keeps testing what it was written to
test.

Speed

Interleaved, alternating arms, min-of-4, decompile-all --json. The box runs several
agents concurrently
, which is why the arms are interleaved and why the min is reported:

binary before (min) after (min) delta
grep 14.58 s 13.97 s −4.2 %
sort 8.18 s 7.99 s −2.3 %
gzip 14.64 s 14.81 s +1.2 %

All noise, all inside the 5 % budget. The change adds no work — it selects a different arm of
an existing walk.

Gates

$ make test
datatests: 675/675 assertions passed
exit: 0

=== baseline parity ===
PARITY OK                       <- docs/baseline.json UNTOUCHED

$ make test-stages
datatests: 562/562 assertions passed
exit: 0

=== baseline parity ===
PARITY OK

$ make check-spec
check-spec OK (lenient mode)
$ python3 tools/check_spec.py --strict
check-spec OK (strict mode)

$ kuna catalog --check
catalog OK: documents exactly the registered kuna options
$ cargo test --workspace --no-fail-fast
exit 0   -- 325 test binaries, 0 failed, 0 errors

CI skips the workspace suite on an internal PR, so this is the author-side gate. The first
run (pre-fix) was 321 binaries green and 2 failedverify_return_uncomputed and
verify_rustabi_pair, both pins on the defective spelling, both updated as described above;
both pass now.

Rebase note. The parity/spec/catalog numbers above are re-measured on the current head.
The exit 0 / 325 binaries workspace-suite result was measured one rebase earlier
(0be6d478, base a1683bb0); the branch has since been rebased again onto a newer main
that merged two further stage tests, taking the corpus count 217 → 219 and the stages
baseline to 562 keys. The change itself is byte-identical across that rebase — the
reproducer still emits v1._0_8_ = a0; on the current head — and the workspace suite is
re-running on it.

Docs

  • docs/features/wideintrender/docs/features/arraysubfield/, proposal.md rewritten as
    the record of what shipped, what did not, and the blocking facts for B and C.
  • docs/spec/09-emission.md §9.2 — new prose on which symbols enter the partial walk and why
    an array subscript is emitted only where the subscript is the whole truth about the access.
  • docs/history.md — a Convergences row (no DIV number).

Merge notes

  • Corpus file count 219 (kuna-base/src/xml.rs) and docs/baseline-stages.json
    re-recorded (562 keys). This branch has already been rebased twice mid-flight as sibling
    stage tests landed (215 → 216 → 217 → 219). Both files collide on every such rebase: resolve
    the count to base + all merged (it must equal ls tests/stages/*.xml tests/datatests/*.xml | wc -l) and re-record the baseline rather than hand-merging it.
  • No option added, so no catalog counts change.
  • No DIV number is claimed — this is a Convergences row, not a divergence. DIV-90, DIV-91
    and DIV-92 all landed on main while this was in flight, which is precisely why not claiming
    one was the right call: there is nothing here to renumber at merge.
  • PR [AUTOMATED] feat(p5): rustadt — synthesize the rustc tagged two-variant return type and emit its constructors #333 (rustadt, draft) touches the same undefined16 return seam; its recorded
    before/after column (v2[8] = v1; v2[0] = 1; v2[0xc] = 0;) is the pre-fix spelling and
    would need re-recording if it lands after this.

🤖 Generated with Claude Code

https://claude.ai/code/session_011KpG7qK6BDFZyPnyo4r1c1

mahaloz and others added 2 commits August 27, 2026 20:50
…8-bit register-pair returns

eval_result_noun @ 0xa3e0 and ~20 Bee value constructors render a 16-byte RAX:RDX
return as `char v1[16]; ... return v1 << 0x40;` (invalid C; v1[0]=a0 truncates an
8-byte store). Entangled across variable-merge (P6), the type factory's
undefined1[16] default (P5, Ghidra-faithful), and the printer's array-index vs
sized-subfield notation (P9). Three candidate gated fixes (sized sub-field writes;
scalar wide-int typing; CONCAT-shift folding). Root-caused from the interp-bee IDA
study; proposal only.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017irbkwk5TigYwPSP7DrfPj
…ymbol reports the size it actually touches

A top-level array-typed symbol never entered the partial-symbol walk.  Upstream
`PrintC::pushSymbolDetail` (printlanguage.cc:256-258) routes every partial cover
of a mapped Symbol through `pushPartialSymbol` and lets that walk's per-type arms
choose the token; the port routed STRUCT and UNION in but split ARRAY off into a
dedicated branch (printc.rs:7241) that computes `index = symboloff /
elementAlignSize` and emits `name[index]` WITHOUT reading the access size.

So an 8-byte write at offset 0 of an `undefined1[16]` rendered `v1[0] = a0` -- a
statement that names one `char` and therefore claims a one-byte store -- and at
offset 8, `v1[8] = 0`, the ninth byte.  That is wrong output, not a style
preference.

The correct machinery was already ported and already worked; it was simply
unreachable for a plain array.  `push_partial_symbol_ir`'s TYPE_ARRAY arm
(printc.rs:6620) carries upstream's `TypeArray::getSubEntry` guard
(`noff + sz <= elsize`) and on failure falls to `PartialEntry::Unnamed(off, sz)`
-> `printlanguage::unnamed_field` -> `._0_8_`.  The fix admits plain TYPE_ARRAY
to the routing gate.  Re-routing rather than a size guard on the standalone
branch: re-routing keeps `arr[3]` for a genuine in-element access through the
walk's own arm and keeps descending afterwards (an array of unions still
resolves to `arr[3].ffield`).

Unflagged.  Blast radius measured first, over 14,080 functions in 12 binaries:
308 functions / 4,953 lines change and every one is accounted for -- 4,937 are a
subscript that spanned elements becoming the sized member, 13 are a
within-element truncation gaining the explicit cast the walk's finalcast arm
emits, 3 are a bare array name becoming the piece it actually is.  ZERO genuine
in-element renders move.  No judgment call in the set, so no option; and because
it restores upstream's own rendering it is a Convergences row in docs/history.md,
not a DIV row.

NOT fixed: `return v1 << 0x40;`.  Shifting the whole container is
upstream-faithful and still not compilable C -- the scout's invalid-C detector
counts exactly the same functions before and after.  This fixes the store size,
not the container.  The two follow-ups are recorded in the rewritten
docs/features/arraysubfield/proposal.md: a scalar 16-byte integer type is blocked
on `kuna_ctypes.rs::integer_spelling` having no 16-byte entry, and the mid-end
half turned out to be a RuleDivOpt coverage gap, filed as #343.

Four existing stage assertions and two kuna-console integration-test pins moved,
all of them pins on the defective spelling (ghdec-returncopysplit #1/#5/#6,
kuna-retinputhalf #2, kuna-rustabi #5, verify_return_uncomputed,
verify_rustabi_pair).  verify_return_uncomputed also carries a min=0 arm that the
repair would have made VACUOUS rather than failing -- `!contains("[8] =")` no
longer describes the shape it was written to reject -- so it was widened to
reject both spellings.  kuna-rustabi is the clearest witness of what the fix
discriminates: one function whose 7- and 4-byte writes become `v2._1_7_` /
`v2._8_4_` while its one-byte discriminant write stays `v2[0]`.

Supersedes the wideintrender proposal (PR #288), rescoped to its Option A.
Corpus file count 216 -> 217 (a sibling PR took 215 -> 216 while this was in
flight; the rebase resolution is base + all merged); docs/baseline-stages.json
re-recorded (547 keys).  No DIV number is claimed -- DIV-90/91/92 all landed on
main during this work, which is exactly why a Convergences row was the right
place for it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011KpG7qK6BDFZyPnyo4r1c1
@mahaloz
mahaloz force-pushed the feat/arraysubfield branch from 0be6d47 to 3bd78ad Compare August 27, 2026 20:52
@mahaloz
mahaloz merged commit 72d286f into main Aug 27, 2026
9 checks passed
@mahaloz
mahaloz deleted the feat/arraysubfield branch August 27, 2026 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant