perf(intel): cut on padding that falls through, not only on padding after a call

[r0ny123]

A function reached by falling through GCC's inter-function padding was decoded as a continuation of its predecessor, and the two came back as one function.

Root cause

The alignment cut had exactly one way in:

elif previous_address is not None and i_address != start_addr and previous_mnemonic == "call":

The theory behind that is that padding follows a function that ended in a noreturn call. GCC pads between functions whatever the previous one ended with — a ret, a jmp, a tail call — so everything reached by falling through such padding was invisible to the cut.

Fix

The other way in is the padding itself. _paddingFillsToAlignment reads the run whole: every byte from the current address to the next 16-byte boundary has to be alignment filler, and the run has to stop at that boundary.

Reading the run rather than just its first encoding is what carries the change. Of the addresses it refuses that a bare "not on a 16-byte boundary" test would have cut, 1,598 of 1,778 are not functions:

corpus	addresses the run test refuses	of which real
C/C++	331	169
Go	1,365	1
Rust	82	10

That is still not enough on its own. GCC aligns loop heads with the same encodings it pads between functions with, and both runs end on the boundary, so the run test cannot separate them. So padding reached by falling through also needs its seed to decode as a function entry, on every format — the entry-shape gate that was PE-only becomes format-independent for this path. The call path keeps the old exemption, because the call is the prior that the previous function ended.

Without that second condition the bundled ELF fixtures gained five "functions" inside one unrolled string routine (add rax, 4; mov cl, byte ptr [rax]; …), all 16-byte aligned, all with one inbound edge — loop heads, caught by testPaddedLandingPad's control case and by the pinned fixture counts.

Measured

296,225 ground-truth functions over 191 samples — gcc/clang/mingw C/C++, Go and Rust, 8 optimisation and link configurations each — exact function-start match:

corpus	truth	TPR	PPV
C/C++ (120 cells)	96,052	93.18% → 96.06%	90.52% → 93.20%
Go (47 cells)	166,337	unchanged	unchanged
Rust (24 cells)	33,836	97.74% → 98.47%	85.18% → 85.34%
pooled	296,225	97.29% → 98.31%	92.83% → 93.70%

+3,009 true starts, −2,676 false ones. Two true starts lost in total, both on C/C++; Rust loses none. All 47 Go cells are bit-identical, which is what an x86 padding rule ought to do to images whose pclntab already names every function.

An earlier variant without the entry-shape gate was better on Rust recall (+346 vs +246) but worse on Rust precision (84.82% vs 85.34%) and lost 12 true starts there. The gated version improves both metrics on the two corpora it moves and costs nothing on C/C++ or Go — those are bit-identical between the two variants, all 167 cells.

Where the baseline comes from. These figures are a paired base-vs-branch run where the base is the branch's own parent, so the deltas are this change's alone. That parent carried the work now open as #299 and #300 plus the AArch64 changes stacked after them, none of which touch this path — but the absolute PPV/TPR columns are that tree's, not plain master's, and the honest way to read the table is as the delta rather than as two absolute numbers.

Malware corpus

Against the 158-sample corpus the Malpedia Benchmark uses, classified with the repo's own evaluate_runtime.py labeller:

files differing: 2 of 158
  likely_recovery              1
  likely_new_false_positive    1

Two files change by one address each, nothing dropped. That corpus is not reachable from a fork PR — Evaluate & Report and Malpedia Benchmark both report skipped here — so the run above is a local one, recorded because the comparison cannot happen in CI on this PR.

Cost

Analysis time +3.4%, from a paired run (base/PR/PR/base, min-of-2, one process at a time) over ten samples of that corpus: 10.02s → 10.36s. The gate warns at 20% and fails at 30%.

Two things keep it there: the predicate returns before touching the buffer when the address is already on a boundary, and it rejects on the first byte via the existing GAP_SEQUENCE_FIRST_BYTES set before scanning the run at all. Without that first-byte rejection the same measurement is +7.2%. MAX_GAP_SEQUENCE_LENGTH is derived once at import instead of per scan step.

Also in here

GAP_SEQUENCES gains the two CS-prefixed nops binutils emits for widths 5 and 8. It already had the 0x90-prefixed forms of both. Unlike the REX-prefixed sequences the suite explicitly bans from the shared table, 0x2e means the same thing in both modes — and the new test asserts that rather than assuming it, decoding each in 32- and 64-bit capstone and checking it comes back as one lea of the stated length.

Tests

Eight new cases, six of which fail on the parent commit:

• _paddingFillsToAlignment: a run that fills the line, a run assembled from several encodings, a lone nop inside real code, a run that carries on past the boundary, an address already on a boundary, and a window truncated at one.
• The cut is taken after a non-call when the run fills the line, and not taken when it stops short.
• The non-call path needs an entry-shaped seed on ELF too.
• The CS-prefixed nops decode as one lea in both modes.

Validation

ruff check / ruff format --check pass, ty check exit 0, full suite green on this branch rebased onto current master (1824 passed, 1 skipped, 2584 subtests), diff coverage 100% on all 23 changed lines against upstream/master, pinned fixture counts unchanged (bashlite 177, mirai 150).

Independent of #299 and #300 — neither touches X86Backend.analyzeFunction or the gap-sequence table — and merges cleanly against both.

The ty commit at the tip

The last commit on this branch is #302's fix, cherry-picked unchanged.

Master fails Code Quality on its own right now: 572acd7 bumped ty to 0.0.74, and its new unsound-return-statement / unsound-assignment rules become errors under [tool.ty.rules] all = "error". That is 41 errors across 15 files, none of which this PR touches — so every PR opened against master inherits a red check that says nothing about its own diff.

Carrying #302's commit here means this PR's CI reflects only this PR's work. It is the same commit, unmodified, so the moment #302 lands this one is empty and falls out on rebase — there is nothing to untangle later. If you would rather look at this branch without it, merge #302 first and I will drop it.

Checked against ty 0.0.74 itself rather than whatever an older local pin resolves to: on master it exits 1 with 41 errors, on this branch it exits 0 with none.