Repository navigation
fix(ghidra): keep the cross-format branch callee multi-block - #1382
Conversation
The cross-format oracle requires a callee whose recovered control flow has more than one basic block, and rea_cross_branch provided it through a symmetric if/else of tail calls. Clang 21 folds that shape into a conditional negate plus one tail branch, a single basic block, so the lane rejected a correct Ghidra analysis of the fixture. Give the branch a loop whose trip count depends on a volatile global: no compiler can eliminate the backedge, and the callee stays multi-block at every optimization level on every toolchain.
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
The failed shard is the JavaScriptSemanticGraphBuilder short-circuit mutation test, which also failed on main run 38003459598 at the same base commit (08ec2bf) before this branch started CI, and passed on main run 38003459600 minutes later at the same commit.
|
The two failed checks are the same pre-existing main-branch flake, not this change: |
|
Follow-up on the failed shard: it is not a flake after all. The same test fails deterministically on main since 001851f (bisected locally: 39dabd0 passes, 001851f fails, reproduced 15/15 on Windows and 8/8 on Linux; main run 38003459598 caught it at 23:15). That commit's capture-time ordering proves the short-circuit mutation cannot reach the earlier alias target, so the graph now correctly reports |
|
TL;DR: Clang 22 does the same fold. This fix works there too. I tested this on Fedora 44, not Kali. The tools were newer than yours. What ran:
I also built The head then moved to Not run: the full test suite. No test files changed, so there was no focused run. I only tried a Linux x64 host. Test setup
🤖 Generated with Claude Code (Opus 5.5) 🦊 |
b8a8190 restored the guarded-mutation uncertainty the failing test encodes, so the shard should pass against the new merge base.
|
TL;DR: The fix in this PR holds on main: with a one-line local change to a schema check, all seven fixtures pass. The commit before this merge still fails with the old multi-block error. As shipped, the lane on main stops before that, at the schema check, which #1433 broke. I ran
The two as-shipped runs fail on this line: The first fold below explains the as-shipped failure. My earlier run on The merged head Not run
Thanks, BaaRai, for the fix and the clear clang 21 repro. Why the as-shipped runs failLine 43 of My local change to the checkI changed - validators.set(tool.name, ajv.compile(tool.outputSchema));
+ validators.set(tool.name, (tool.outputSchema?.$id && ajv.getSchema(tool.outputSchema.$id)) || ajv.compile(tool.outputSchema));Tools that share an output schema id on mainThe same eight groups show up on the parent.
Test setup
🤖 Generated with Claude Code (Opus 5.5) 🦊 |
|
Thanks Cassie, this report was complete: reproduced on main byte for byte, and your reuse approach is exactly right. I applied it in #1543 together with the third site with the same pattern, verify/electron/tool-contracts.mjs, which compiles both input and output schemas through one shared Ajv instance. Validated on the real lanes there: cross-format completes all seven fixtures green on main plus the fix (which also confirms the merged fixture end to end), the Hopper boundary phase now runs past the compile and stops at the known #341 demo string-typing boundary, and the Electron tool-contract probes complete with schema-validated results. #1527 covers the two boundary scripts but its validation notes the lanes were not run, so #1543 supersedes it if you prefer one PR carrying the evidence. |
…ck-callee fix(ghidra): keep the cross-format branch callee multi-block
|
Thanks for adjusting the cross-format fixture, @BaaRai. The volatile, runtime-dependent loop gives |
Summary
The cross-format oracle walks the entry's callees for one function whose recovered control flow has more than one basic block, and
rea_cross_branchprovided it through a symmetric if/else of tail calls. Clang 21 folds that shape intocmp; cneg; b— a single basic block — sofindCrossBranchAddressfailed withcross-arm64-elf did not recover the multi-block calleeagainst a correct Ghidra analysis. The branch now runs a loop whose trip count depends on the fixture's volatile global, which no compiler can eliminate, keeping the callee multi-block at every optimization level.Problem and expected behavior
Smallest trigger:
npm run verify:ghidra:cross-formatwith clang 21 (reproduced on Kali rolling: clang 21.1.8, ld.lld 21.1.8, gcc 16.2 host compiler, Ghidra 12.1.4, JDK 21, node 24.19). The lane fails on the first cross target:Violated invariant: the oracle needs a fixture function that is multi-block after compilation, but symmetric tail-call if/else codegen is a toolchain decision. Disassembly of the clang 21 AArch64 build of the previous source:
One basic block, so Ghidra correctly reports
basicblock_countof one and the verifier rejects it. This is the same class as the-O0dense-switch lowering fixed for the host lane in #1374, and the dedicated switch lane already avoids it by construction.Change and scope
tests/conformance/ghidra/cross-format.c:rea_cross_branchnow computes an adjusted value and decrements it byrea_cross_global(volatile) inside awhileloop before the tail call. The loop is a real backedge at runtime-unknown trip count, so it survives every optimization level; on clang 21 AArch64 the function now compiles to a compare, a conditional select, and ab.lt-guarded loop with a backward branch and the volatile load. No symbols, call structure, entry chain, string evidence, or dense-switch shape change; PE and Mach-O builds of the same source are unaffected beyond the same guaranteed block structure.Not included: no verifier, bridge, provider, or contract changes — the oracle itself is correct; only the fixture's compiler-independence was missing.
Contract and boundary impact
Evidence and regression coverage
GHIDRA_INSTALL_DIRat Ghidra 12.1.4 and JDK 21:verify:ghidra:cross-formatfails atcross-arm64-elf did not recover the multi-block callee; the clang 21 disassembly above shows the single-block fold.verify:ghidra:cross-formatcompletes with exit 0 andok: trueacross all seven fixtures (debug, stripped, type-layout, jump-table, cross-arm64-elf, cross-x86_64-pe, cross-x86_64-mach-o) withcleanup: complete, andverify:ghidra:aarch64-jump-table(which compiles the same source for itsaarch64-jump-tabletarget) still completes with exit 0 across all seven of its fixtures, confirming no regression from the fixture change.