Repository navigation
fix: keep shrink-wrapped prologues, early-return bodies and switch tables in their function - #386
Conversation
…unction MSVC arm64 puts an early-exit check ahead of the register saves and starts the .pdata record at the saves, so the record and the prologue scan both named the body as a function and the check became a separate one. A prologue or record start that the code before it branches or falls into now seeds that code instead, unless it is called or has a symbol. Saves of FP registers or a stack allocation ahead of the recognized store are read as part of the prologue the same way. On x86 a jcc into a candidate backed by nothing but its prologue bytes is followed as an ordinary block instead of being left for its own analysis. Fixes danielplohmann#384
tbnz targets, stack allocation as frame setup, prologue-run seeding with and without an exception record, and hoisted records admitted when the candidate timeout trips after the record pass.
…t return MSVC places an x86 switch table right after the function's last block. When that block ends in a noreturn call, decoding ran on into the table and from it into the next function. Blocks now end at a table the function reads, and a function that reached a table before its dispatch is analyzed once more with the table known. The byte table of a two-level switch is booked as data as well, so the gap scan no longer starts functions inside it. Refs danielplohmann#384
|
Pushed a followup commit addressing two minor edge cases:
|
A block cut at a table now ends the way every other cut does, so a function whose only block runs into its datum is still counted and kept. A gap function the discarded first pass reverted, and the repeated pass no longer reaches, is analyzed again instead of being lost. The image base an x64 two-level switch indexes from is no longer booked as a table, and a byte table range over the jump table itself takes the biased path whatever its bytes are. Refs danielplohmann#384
danielplohmann
left a comment
There was a problem hiding this comment.
Thanks, this is a big step for #384. I re-measured the fixtures from #385 independently (de-xored, disassembleFile with and without PDB, a function counts if it starts at the map address and nothing else starts before the next one):
| build | master + #385 | this PR + #385 |
|---|---|---|
| x86 | 9/11 (PDB and no PDB) | 11/11 |
| x64 | 10/11 | 11/11 |
| arm64 | 11/11 PDB, 9/11 no PDB | 11/11 |
That matches your numbers exactly, and the full suite passes with #385 merged in. The aarch64 walk-back reads well: it stops at padding, bl/blr, a record start or 16 words, needs a conditional branch or pure frame setup, rejects data islands through _decodesAsCode, leaves PAC/BTI entries alone, and stays in the aarch64 manager. No concerns there.
Two things I'd like changed before this goes into 4.10.0, both reproduced on raw buffers (scripts below):
1. The decode stop in RecursiveDisassembler fires on any data byte, not only on switch tables (RecursiveDisassembler.py:305-310, and the re-analysis trigger at :406-409).
state.data_bytes also holds every absolute memory operand booked by X86Backend._recordAbsoluteDataRefs. In a buffer or dump without code_areas nothing filters those, so a function that reads its own code (a breakpoint check, a self-patching stub, a packer) gets cut at the bytes it reads. Dumps are the main input, so I don't want this one in:
# push ebp; mov ebp,esp; cmp byte ptr [F+0x10], 0xcc; je L; nop*4; F+0x10: mov eax,1; L: pop ebp; ret
import struct
from smda.Disassembler import Disassembler
base = 0x401000
code = bytearray(b"\xcc" * 0x100)
body = b"\x55\x8b\xec\x80\x3d" + struct.pack("<I", base + 0x10) + b"\xcc" + b"\x74\x09" + b"\x90" * 4
body += b"\xb8\x01\x00\x00\x00\x5d\xc3"
code[: len(body)] = body
for fn in Disassembler().disassembleBuffer(bytes(code), base, 32).getFunctions():
print(hex(fn.offset), fn.num_instructions)master: 11 instructions, this PR: 10 (mov eax, 1 at F+0x10 is gone). Could the stop (and the second pass) use a separate set that only the jump-table analyzers fill (intel JumpTableAnalyzer, aarch64 analyzers.py)? That keeps the case_b4_aggregate fix. It would also stop the stop from touching ARM64 output by accident: case_b3_control on the arm64 fixture loses an eon behind the noreturn bl, which is fine in itself, but the CHANGELOG bullet says x86/x64 only and carries the (intel) prefix although the change lives in common/.
2. The x86 "jcc into a prologue-only candidate is not a tailcall" rule absorbs real functions (X86Backend.py:410-412, :423-432).
If A is analyzed first and does jne B, and B is a separate function that is only reached through a pointer (callback, vtable), B now becomes part of A:
import struct
from smda.Disassembler import Disassembler
base = 0x401000
code = bytearray(b"\xcc" * 0x200)
A, B, C = base + 0x80, base + 0xa0, base + 0xc0
def put(addr, bs): code[addr - base: addr - base + len(bs)] = bs
def rel(src, size, dst): return struct.pack("<i", dst - (src + size))
caller = bytearray()
for _ in range(8):
caller += b"\xe8" + rel(base + len(caller), 5, A)
caller += b"\xe8" + rel(base + len(caller), 5, C) + b"\xc3"
put(base, bytes(caller))
put(A, b"\x85\xc9\x0f\x85" + rel(A + 2, 6, B) + b"\x31\xc0\xc3") # test ecx,ecx; jne B; xor eax,eax; ret
put(B, b"\x55\x8b\xec\x8b\x45\x08\x5d\xc3") # 16-aligned, behind CC padding
put(C, b"\x55\x8b\xec\x68" + struct.pack("<I", B) + b"\xff\x10\x83\xc4\x04\x5d\xc3") # push offset B
for fn in Disassembler().disassembleBuffer(bytes(code), base, 32).getFunctions():
print(hex(fn.offset), fn.num_instructions)master: A has 4 instructions and B is its own function. This PR: A has 9 and B is gone. The rule isn't needed for the #385 fixtures (they stay 11/11 with _isEvidencedEntry forced to True); only X86BodyBehindAnEarlyReturnTest and the clang computeJD gain depend on it. Counting "16-aligned and preceded by CC/90 padding" as entry evidence, or limiting the rule to a target directly behind the jcc function's own code, would both keep the MSVC early-return case, since that body follows the ret directly.
Smaller, not blocking:
:406-409: during the tailcall re-drain, the throwaway first pass can run past the table into a gap function and revert it via_revertGapFunction. The second pass stops at the table and doesn't take those instructions back, so that code ends up in no function. And if the first pass bails out (suspicious-instruction abort on x64 RVA tables, timeout), the second pass never happens. Worth a look while you are in there.__gsfailureis lost on every /GS ARM64 MSVC image without a PDB, not once (it's in the #385 arm64 fixture too), and it changes__security_check_cookie's hashes. Accepting it is fine. Please pin it in a test so the trade-off stays visible.- #385 goes in first, so please add the regression test over those fixtures here; then CI enforces the 11/11 rather than a review.
- CHANGELOG: add "reproduced on the bundled #385 fixtures"; say that the 44 x86 / 1 x64 sqlite starts that go away were false positives inside switch tables (as written it reads like a loss); fix the prefix and the "x86/x64 only" in Compatibility (see 1); both bullets are past the six-line guide.
RecursiveDisassembler.py:307checks only the first and last byte of an instruction against the data bytes, so a short range inside an instruction is missed. That's fine for tables, but if the set stays broad it matters.
This is in the v4.10.0 milestone. With 1 and 2 addressed and the fixture test in, I'm happy to merge.
…ding Decoding now stops, and a function is analyzed a second time, only at bytes a jump-table analyzer booked as a table. Other data references, such as a function reading its own code in a buffer without code areas, no longer cut the function. The check covers every byte of an instruction. A jcc into a prologue-only candidate no longer absorbs it when the candidate sits on a 16-aligned address behind CC/90 padding, so a function reached only through a pointer stays a function of its own. An early-return body follows its ret directly and stays where it was. Refs danielplohmann#384
|
Thanks for the careful re-measurement. Both blockers are addressed in the new commit, and both of your scripts are now regression tests. 1. Decode stop. Decoding now stops on a separate 2. jcc rule. Smaller items:
Measured:
|
…ays intact Each function of cases.obj in the x86, x64 and ARM64 /O2 builds must be one function, with and without the PDB, and nothing may start inside it. Also pins the known trade-off on ARM64: without the PDB, __gsfailure is part of __security_check_cookie. Refs danielplohmann#384
|
Thanks, both blockers are fixed. I re-ran
Leaving the first-pass abort for a follow-up with its own measurement is fine with me. #385 is on master now (along with #381 and #383), so please merge master and push |
|
already merged with main. |
What this changes
AArch64: MSVC puts an early-exit check ahead of the register saves, and the .pdata record (and the prologue scan) starts at the saves. When the code just before a frame setup branches or falls into it, the function is now seeded at that code instead of at the saves. The walk back stops at padding, a call, another record start, or 16 words, and the run has to decode as code and contain a conditional branch (or be all frame setup), so constants and tail calls into the next function are left alone. A record start that is called or named by a symbol keeps its own entry. Prologues that save FP registers or allocate the stack before the recognized store are seeded where they start.
x86: a jcc into a candidate that has nothing behind it but a prologue (no symbol, record, call reference, ...) is no longer taken as a tailcall, so the block after an early
retstays in its function.x86/x64 switch tables: decoding stops at a table the function reads, so a block ending in a call that does not return no longer runs into the jump table and on into the next function (
case_b4_aggregate). A function that reaches a table before the dispatch reading it is analyzed once more with the table known. The byte table of a two-level switch is booked as data too; that is whatcase_b2_whitespacehits: its "+0xd5 / +0x10d" start is a gap-scan function inside the byte table behind the jump table, not a block afterret.How it was verified
On the #385 fixtures (linker map as truth), 11/11 for every build, with and without PDB (master: x86 9/11, x64 10/11, arm64 9/11 without PDB). A test over them is ready to add once #385 is in.
Against
ddffed4, PDB functions as truth (samples not bundled):python.org 3.13.7
python313.dllARM64 without PDB: +24 entries, -1 (__gsfailure, reached byb.nefrom__security_check_cookieexactly likesqlite3_errcodereaches its body, so it can't be told apart without symbols). Starts inside functions 55 to 23. With the PDB: no change.python313.dllx86: +2 functions, none lost, starts inside functions 116 to 43 (with and without PDB).official sqlite 3.54 DLLs: ARM64
sqlite3_errcodeis one function again (491 split starts merged back); x86 drops 44 starts and x64 one, all inside switch tables.clang-built sqlite x86/ARM64: unchanged apart from the computeJD split.
candidate scan on python313 ARM64 +0.16s, total runtime unchanged.
make lintandmake formatmake test-all(2259 passed, 2 skipped),tycleanCHANGELOG.mdentry under## [Unreleased]Golden fixtures unchanged — FROSTYFERRET (Mach-O ARM64) drops 0x10000573c, a cbz shrink-wrap body of 0x100005718 that LC_FUNCTION_STARTS does not list; counts updated in testMachoFunctionStartCandidates.
Closes #384