Skip to content

[AUTOMATED] Fix SPARC branch conditions clobbered by delay slots - #1287

Merged
mahaloz merged 9 commits into
Noelo-Lab:mainfrom
thotypous:fix/sparc-delay-slot-flags
Oct 29, 2026
Merged

mahaloz merged 9 commits into
Noelo-Lab:mainfrom
thotypous:fix/sparc-delay-slot-flags

Conversation

@thotypous

Copy link
Copy Markdown
Contributor

Problem

A flag-writing SPARC delay slot can replace the condition used by the branch itself. This independent example should clear a byte only when both arguments are below the limit:

python3 -c "from pathlib import Path; Path('/tmp/sparc-delay.bin').write_bytes(bytes.fromhex('80a2000a0a80000580a2400a81c3e00801000000010000002abffffdc02ac00081c3e00801000000'))"
kuna decompile /tmp/sparc-delay.bin 0x1000 --raw-image \
  --target sparc:BE:32:default --base 0x1000 --assert-strict \
  --assert 'prototype 0x1000 void sub_1000(unsigned int previous, unsigned int current, unsigned int limit, unsigned char *out)'

Before the fix, the body loses the first comparison:

  if (current < limit)
    *out = 0;

Compiling that emitted C clears the byte for previous=1, current=0, limit=1; it should leave the byte unchanged.

Fix

  • Snapshot the ICC/XCC cs, neg, and vs condition operands before delay-slot expansion, matching the neighboring condition operands.
  • Preserve the slot's flag updates for subsequent instructions and the existing annul behavior. No engine heuristic or option is added.
  • Retain behavioral execution/parser checks instead of obsolete temporary-layout, corpus-count, and declaration-spelling expectations.

Tests

  • Emulator: 120 incoming/slot flag cases, failing with old specs. Unmodified emitted C passes 1,000 boundary combinations at both -O0 and -O2.
  • Debian SPARC32/64: 3,302 functions across 23 executables, no errors or missing functions in either arm. All 14 changed functions were reviewed against their flag-writing delay slots; this is not a whole-function equivalence proof.
  • CI command set: parity 675/675 and 3225/3225; tooling 114/114; CLI probes 306/306; lint, spec/catalog/counters, Ghidra simulation, and Node/WASI checks pass.
  • Full public decbench corpus: 770 binaries / 96,103 requested functions across 40 projects. Both arms recover 95,197 functions, with the same 906 recovery gaps; recovered C/payloads, coverage, GED scores, and type-match scores are unchanged. This corpus has no SPARC binaries.
  • Byte-match also covers all 770 binaries, using native GCC plus real ARM/MinGW cross-compilers: 95,197 evaluated, 78,639 recompilable and 16,558 unrecompilable in each arm; no metric errors or toolchain abstentions. All nine default numeric-score differences have byte-identical scoring inputs and equal scores in a timeout-disabled diagnostic; the original default scores are retained.
  • Workspace remains non-green: locked_scalar_declarations::dwarf_function_pointer_local_keeps_its_declaration fails its native execution check with both base and corrected specs. Its behavioral check remains enabled.
  • Optional external-toolchain cases follow the repository's skip gates when their compiler is unavailable.

Engine-only timing covers 701 unchanged functions, with two warmup rounds discarded and seven samples per arm. The timer surrounds decompile_one; process startup, loading, C printing, and serialization are excluded.

Median thread CPU time Base PR Delta
Decompilation step 24.708 s 24.762 s +0.22%

@thotypous thotypous changed the title Fix SPARC branch conditions clobbered by delay slots [AUTOMATED] Fix SPARC branch conditions clobbered by delay slots Oct 26, 2026
mahaloz and others added 4 commits October 29, 2026 00:24
Keep this change to the SPARC fix. The stage-corpus count pin in
kuna-base/src/xml.rs is a repo convention (docs/agents.md); restore it,
bumped to 590 for kuna-sparc-delay-condition.xml, and restore the
counters.py site and decbench-loop.md text that followed its removal.
Restore the function-pointer declaration assertion in
locked_scalar_declarations.rs, which is x86 and unrelated to SPARC.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013BBaWTixHX2E5cBwneY5aW
The non-annulled BPr branches (brz, brlez, brlz, brnz, brgz, brgez) read
rs1 after expanding the delay slot, so `brz,pt %o0,L; mov 5,%o0` never
took the branch and decompiled to `return 5;`. Snapshot the register test
before delayslot(1), as the annulled forms already do. The emulator test
now covers all six conditions with a slot that rewrites rs1 (32/64-bit,
annulled and not), and the stage test gains a `pick` function whose zero
path was lost.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013BBaWTixHX2E5cBwneY5aW
…ad of dropping them

The SPARC branch-condition snapshots move the compiled specs' temporary
layout. Regenerate the two SPARC compiler.sha256 entries with the pinned
C++ sleigh_opt (it reproduces the previous digests on the previous
sources, and slacomp produces the new ones byte-for-byte) rather than
removing them, and keep the gh6990-returnpair golden lift replay: its
refreshed capture differs only in unique-space offsets (+0xc00 on 150
lines), with the same instructions, operations and temporary aliasing.
Re-record the stages baseline for the two new register-branch assertions.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013BBaWTixHX2E5cBwneY5aW
…ports Noelo-Lab#1291)

Resolve the stage-corpus count (591 = 83 datatests + 508 stages) and
keep both lift-and-flow spec paragraphs. Re-record the stages baseline
on the merged tree (3229/3229).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013BBaWTixHX2E5cBwneY5aW
@mahaloz

mahaloz commented Oct 29, 2026

Copy link
Copy Markdown
Contributor

[AUTOMATED] Pushed four commits on top of e501856:

  • Register branches had the same bug. Non-annulled brz/brlez/brlz/brnz/brgz/brgez read rs1 after the delay slot, so brz,pt %o0,L; mov 5,%o0 decompiled to return 5;. GCC -O2 for sparc64 emits this often (92 sites in sqlite3.o; it lost checks in zlib deflate/deflateInit2_). These branches now test the register before delayslot(1). The emulator test covers all six conditions, and the stage test gains a pick function.
  • Restored the guards this PR removed: the stage-corpus count in kuna-base/src/xml.rs (now 591), counters.py, docs/decbench-loop.md, and the x86 function-pointer assertion in locked_scalar_declarations.rs.
  • Refreshed the goldens instead of deleting them. The SPARC compiler.sha256 digests were regenerated with the pinned C++ sleigh_opt, which reproduces the old digests on the old sources; slacomp produces the same new ones. The gh6990-returnpair lift replay is back, and its capture differs only in unique offsets.
  • Merged origin/main ([AUTOMATED] fix(sleigh): set the reservation in PowerPC lwarx #1259, [AUTOMATED] fix(analysis): recognize PPC32 dynamic import contracts #1291) to resolve the count, spec and baseline conflicts.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013BBaWTixHX2E5cBwneY5aW
@mahaloz mahaloz closed this Oct 29, 2026
@mahaloz mahaloz reopened this Oct 29, 2026
mahaloz and others added 3 commits October 29, 2026 01:03
Resolve the stage-corpus count (592) and re-record the stages baseline
on the merged tree (3233/3233).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013BBaWTixHX2E5cBwneY5aW
… one entry (Noelo-Lab#1260)

A switch whose index a call writes through a pointer keeps only one case. Its other indexes fall through to the code after the switch. The example below is PPC64: a stack slot is set to 0, a helper writes the switch index through its address, and a five-entry table dispatches on it. Each case returns 40–43, and entries 0 and 4 share a body.

```sh
cat > select.s <<'EOF'
	.abiversion 2
	.globl select
	.type select,@function
select:
	.long 0x7c0802a6,0xf8010010,0xf821ff71,0x39200000,0x38a10070,0x91210070,0x48000091
	.long 0x2f83000b,0x419e0074,0x81210070,0x2b890004,0x419d0060,0x7d2a4b78,0x429f0005
	.long 0x7d0802a6,0x3d080000,0x39080020,0x794a1764,0x7d2852aa,0x7d294214,0x7d2903a6
	.long 0x4e800420,0x00000014,0x0000001c,0x00000024,0x0000002c,0x00000014,0x38600028
	.long 0x48000028,0x38600029,0x48000020,0x3860002a,0x48000018,0x3860002b,0x48000010
	.long 0x38600063,0x48000008,0x38600001,0x38210090,0xe8010010,0x7c0803a6,0x4e800020
	.long 0x90850000,0x38600000,0x4e800020
EOF
powerpc-linux-gnu-as -a64 select.s -o select.o
kuna decompile select.o select
```

```c
  if ((unsigned long)v1[0] <= 4) {
    switch(*(int *)((unsigned long)v1[0] * 4 + 0x400058) + 0x400058L) {
      case 0x40006c:
        return 0x28;

    }
  }
  return 99;
```

Ghidra prints `case 0: case 4: … case 1: … case 2: … case 3:` for the same code.

## Fix

- During flow, the table is recovered on a partial copy of the function. That copy doesn't yet know the call takes the slot's address, so it reads back the stored 0 and builds a one-entry table. The full analysis later finds five entries, but the switch has already been built with one out-edge.
- Port upstream's multistage restart. When the flow-time table has one address and the late model has more, `matchModel` records a multistage hint for the jump and restarts the function. The next flow re-queues the table through `checkForMultistage`/`recoverMultistage`.
- The second stage runs on the same kind of partial copy, so it can fold the index again. The hint therefore also carries the late model's targets, and a second stage that again finds one address uses them.
- A jump restarts at most once, and only when every value in the late model's range passes the guards on its path and every target is mapped and within the `0xffff` reach `sanityCheck` allows. A model wider than its guards would otherwise turn rows a lower bound excludes into cases.
- When the restart budget runs out, the last pass (which still labels its table) is printed with an `Exceeded maximum restarts with more pending` warning.

## Tests

- `kuna-cli` `ppc64_delayed_constant_switch`, each big- and little-endian:
- the function above keeps `case 0: case 4:`, `case 1:` to `case 3:` and `default:`;
- a `5 <= i && i <= 9` guard over a table whose rows 0–4 also hold mapped targets keeps the lower bound, and prints none of those rows as cases;
- a chain of nine such switches keeps a structured body under the restartlimit. The first fails without the fix. The other two fail if the restart skips the guard check or re-follows flow at the limit.
- No datatest or stage expectation moved, and a sweep of 478 ELF fixtures changes no function.


Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013BBaWTixHX2E5cBwneY5aW
@mahaloz
mahaloz merged commit f7e7458 into Noelo-Lab:main Oct 29, 2026
2 checks passed
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.

3 participants