Repository navigation
[AUTOMATED] Fix SPARC branch conditions clobbered by delay slots - #1287
Merged
Merged
Conversation
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
Contributor
|
[AUTOMATED] Pushed four commits on top of e501856:
|
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013BBaWTixHX2E5cBwneY5aW
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
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013BBaWTixHX2E5cBwneY5aW
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.
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:
Before the fix, the body loses the first comparison:
Compiling that emitted C clears the byte for
previous=1, current=0, limit=1; it should leave the byte unchanged.Fix
cs,neg, andvscondition operands before delay-slot expansion, matching the neighboring condition operands.Tests
-O0and-O2.locked_scalar_declarations::dwarf_function_pointer_local_keeps_its_declarationfails its native execution check with both base and corrected specs. Its behavioral check remains enabled.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.