fix(aarch64): candidate-quality follow-ups to the BTI landing-pad series

[r0ny123]

Six more changes to AArch64 candidate discovery, all in the same two scans, continuing directly from #310.

This branch contains #310's three commits as well — its work is what the first change here builds on, and the two cannot be separated without rewriting both. Review the seven commits after fix(aarch64): stop the refused-pad walk at a trap, or take #310 first and this rebases to just those.

1. Both interior tests read a stale snapshot

getFunctionStartCandidates() is taken before analysis begins and gap analysis never adds to it, so a function the gap scan discovered is in code_map and absent from that set. Both interior tests in the AArch64 candidate manager read that difference as evidence of an interior:

• _gapRunFlowsIntoInterior treats a branch to such a function as a branch into somebody's body, so a run of branch veneers whose targets the gap scan found gets suppressed;
• _isLikelyInteriorBtiCandidate refuses a landing pad at an address the gap scan already recovered as a function.

Both now ask disassembly.functions too. On the bundled osx.frostyferret fixture this alone recovers 8 more functions, every one of them an address LC_FUNCTION_STARTS declares, with nothing lost.

2. The two gap-run walks stop at different words, and that is correct

_gapRunFlowsIntoInterior and _endOfRefusedLandingPadRun share _GAP_RUN_LIMIT, but since #310 the second also stops at brk/hlt/udf. That reads like an omission — AArch64Backend.analyzeInstruction ends a function on all three through END_INS — and it is not.

The two walks answer different questions. _endOfRefusedLandingPadRun decides how far to skip, and one instruction too far steps over a real entry and loses it outright, so it must stop at the earliest boundary it can defend. _gapRunFlowsIntoInterior decides whether to suppress; nothing is skipped either way and reading further only gathers more evidence about the same candidate. A trap is not a boundary the image declares — mid-body it is a bounds check or a __builtin_trap — so the words behind it still belong to the enclosing routine.

Counted rather than argued: adding is_trap to the suppression walk releases 12 candidates on the 11-cell ARM64 Mach-O corpus (two sites, each swept one word at a time), 0 of them true starts, and 0 candidates on the 72-cell built AArch64 ELF corpus. Both released sites sit 0x15c and 0x1d8 inside functions the ground truth declares.

No behaviour change — the comment on each walk now states why its terminator set is what it is and points at the other, plus a new test file that pins the asymmetry so a future reader does not "fix" it.

3. A function may open with a conditional branch

The gap scan refused any candidate whose first word was one:

if is_conditional_branch(word):  # a function never opens with a cond branch

The claim is false. At -O2 a compiler hoists an argument check above the frame setup routinely:

cbz   x0, .Lreturn_null      <- the entry
stp   x29, x30, [sp, #-32]!  <- what got booked instead
mov   x29, sp

On the 72-cell AArch64 ELF corpus this shape occurs at 230 routines whose entry is one word before the start that was booked, 203 of them opening on a conditional branch.

Removing a refusal reduces false positives here, which is worth explaining. The refusal did not merely lose the entry: the frame setup one word in is 4-aligned and nothing calls it, so the alignment floor keeps it out of the first pass, and the start booked in the entry's place came from the same gap scan four bytes past the entry it had just stepped over — with the guard's own branch target often becoming a second one. Recovering the entry deletes both wrong starts.

What the refusal was buying, on those 72 cells: 558 addresses refused, of which 462 are function starts the ground truth names. Four in five of what it held back was real.

corpus	cells	truth	TP	FP	ΔTP	ΔFP
Built C/C++ AArch64 ELF	72	60,326	59,167 → 59,516	3,023 → 2,484	+349	−539
ARM64 Mach-O (LC_FUNCTION_STARTS)	11	2,753	2,532 → 2,538	388 → 382	+6	−6
Built Go, seven platforms	47	166,337	165,627 → 165,627	9,816 → 9,812	0	−4

Not one cell in any of the three gains a false positive. One cell of 83 loses recall and is worth naming rather than averaging away: on lz4_gcc-arm64_O1, recovering 0xcf48 lets its analysis run on through the four truth functions after it — net −2 TP and −2 FP on that cell. That is the merge hazard of recovering an earlier entry, not a property of the guard.

The other call site of is_conditional_branch, in locateAddressRefCandidates, is untouched and correct: it clears register provenance at a control-flow edge, which a conditional branch is whatever may open a function. Its docstring carried the false claim and now states the encoding fact it is actually for.

4. Skip the address scan when metadata already named the functions

locateAddressRefCandidates exists to reach entries no call names — a pointer handed to a callback, a table walked at run time. An image that ships a full function map has none of those left over: the symbol pass consumes the map, every entry is already a candidate, and what the scan then produces is the addresses that are materialized but are not entries. On a Go binary that is thousands of them for no recovered function at all.

The question is asked of the image rather than of its container or its language, both of which have been the wrong variable before: of the addresses this image's own bl instructions call, how many did metadata name before this pass ran? A stripped C or C++ object answers near zero however it was linked; one carrying a complete map answers near one whoever compiled it.

bl targets are the denominator because they are the one population both kinds of image have in proportion to how many functions they contain, and because they are known at exactly this point — the scan that resolves them has just finished. The threshold sits at 0.95, in the upper half of a bimodal distribution with a wide empty interval between the modes, because the two errors are not symmetric: running the scan when it is useless costs precision, skipping it when it is not costs recall.

A rate needs a sample, so an image resolving fewer than 512 bl targets is left to the scan whatever share of those few metadata named — one corpus image resolves four in the whole binary, and metadata naming all four says nothing.

5. Turn the Mach-O address-materialization scan on

USE_MACHO_ADDRESS_REF_CANDIDATES shipped off because the scan was all cost on images with a function map. With the coverage gate above deciding that per image, it can default on: on the bundled osx.frostyferret fixture it reaches 2 more starts the linker declares and removes one address that is not in the table.

What the bundled fixtures do

Both are updated in a single trailing commit so the movement can be read in one place.

• osx.frostyferret (ARM64 Mach-O): the primary pass recovers 256 functions where it recovered 246, and 128 of the linker's own LC_FUNCTION_STARTS entries where it recovered 117. Eleven table entries gained, one non-table address (0x100004c48) dropped. Every gain is an address the linker itself declares; the single loss is not, so it is a false positive going.
• aarch64_static (ELF): 278 functions either way, one instruction more, and the routine at 0x40DF34 is reported at 0x40DF30. That word is a hoisted cbz x14. 0x40DF30 is 16-aligned and follows the previous function's ret and its padding nop; 0x40DF34 is 4-aligned and mid-line; neither carries an inbound direct branch. This is the one baseline movement that is a judgement rather than a measurement, and it is called out rather than folded into a count.

Tests

• tests/testAArch64GapRunTerminators.py — nine tests in two classes pinning the terminator asymmetry, behavioural and walk-level, mutation-checked by adding or is_trap(word) to the suppression walk and watching the behavioural assertion and one walk-level assertion turn red while every control stays green.
• tests/testAArch64HoistedGuardEntry.py — the hoisted guard recovered as the entry with the whole recovered set compared, not one address; a control that the same word class after ordinary code inside an already-claimed routine is still not booked (the case the guard existed for); and a control that a conditional branch opening an unclaimed gap now books the block's first word rather than the word behind it.
• tests/testAArch64MetadataCoverageGate.py — the gate over synthetic images at both sides of the threshold and below the sample floor, plus the reset behaviour that matters for a manager driven one pass at a time.

Validation

ruff check / ruff format --check pass, ty check exit 0, full suite green on this branch rebased onto current master, diff coverage 100% on the changed source lines.

Branch shape

Rebased onto current master (395c88d) as the flat ten commits, dropping the three pre-squash copies of #310 and the fix(ci) commit that #302 made redundant. This is the exact set and order you listed.

The rebase is content-neutral: the tree is 38b27ba, byte for byte what the merge-based version resolved to, so the measurements above stand unchanged and nothing needs re-running. ruff check, ruff format --check and ty 0.0.74 are clean on it; the full suite was 1918 passed / 1 skipped / 2593 subtests on this identical tree.

One thing to know if you take #309 first: #312 still carries copies of these ten, so it will hit the same squash artifact once this lands, in the same way and with the same fix. Its content reduces to a single commit either way.

Changelog note

Flagging this for whoever writes the release entry, since it is the one thing here that changes behaviour for every consumer rather than only for the images it measures on:

USE_MACHO_ADDRESS_REF_CANDIDATES now defaults on. The AArch64 adr/adrp address-materialization scan is gated per image by metadata call-target coverage rather than per container, so it no longer runs on images whose metadata already names their functions. Measured on the bundled 11-sample ARM64 Mach-O corpus: +31 true starts, −5 false ones, TPR 86.916% → 88.424%, with no cell losing recall.

I have not touched CHANGELOG.md here — its entries are per released version and there is no unreleased section, so writing one would mean picking a version number, which is yours to decide at release time.

[danielplohmann]

Reviewed and wanted — see #309 (comment) for the details, since the rebase pass for #309, this and #312 is being tracked in one place.

Short version: #310 landing as a squash means this branch's copies of its three commits (9233624, fa31fe1, e8626b7) no longer read as ancestors of master and collide with the squashed version. Dropping those three leaves your own ten, which cherry-pick onto current master without a conflict. Measured +31 true positives and −5 false ones on the bundled ARM64 Mach-O corpus with no cell losing recall, and the hoisted-guard change is confirmed on aarch64_static_xored at the instruction level. Nothing outstanding but the rebase.

[r0ny123]

Rebased to the flat ten and force-pushed — head is now 3cf5e9c, exactly the set and order you listed, with the three pre-squash copies of #310 and the redundant fix(ci) commit gone.

Confirmed content-neutral before pushing: the tree is 38b27ba6eaea1d85f65f11d19d9fff25b6f604f1, byte for byte what the merge-based version resolved to. So everything you measured still holds and there is nothing to re-verify — ruff check, ruff format --check and ty 0.0.74 are clean, and the full suite was 1918 passed / 1 skipped / 2593 subtests on this same tree.

#309 and #312 are untouched and still mergeable. #312 will hit the squash artifact once this lands, exactly as you predicted, and reduces to its single commit either way.