From 6236fafc26129a6fefb764810bb1d6041438d167 Mon Sep 17 00:00:00 2001 From: o2alexanderfedin Date: Fri, 7 Aug 2026 03:57:44 -0700 Subject: [PATCH] amd64: make the ELF front end work on x86-64 binaries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit elfconv builds for x86 (ECV_X86=1), CI publishes an :amd64 image, --arch accepts amd64, the ELF loader classifies it, and Remill has had amd64 semantics for years. Lifting an x86-64 ELF still aborted on the first function, because the ELF FRONT END is AArch64-only in four places. Upstream CI cannot catch this: the AMD64 arm runs `ctest`, which is Remill's own run-amd64_avx instruction-semantics tests. Nothing in it lifts an ELF. Each fix below was measured by re-running the lift and observing it get further. 1. lifter/TraceManager.cpp — __wrap_main discovery. The AArch64 code finds main by matching LITERAL AArch64 encodings (nop 1f 20 03 d5, bti c 5f 24 03 d5, b as (byte & 0xfc) == 0x14) on a fixed 4-byte stride. x86-64 is variable-length, so the stride is meaningless and those bytes can never match; it fell through to elfconv_runtime_error. The AArch64 block is now guarded by ELFCONV_AARCH64_BUILD and an amd64 branch reads main out of _start instead. It is a different problem, not the same one: AArch64 needs a function SYNTHESISED because __libc_start_call_main reaches main through an unnamed trampoline, while the System V ABI passes main as the first argument to __libc_start_main, so _start materialises its address (48 c7 c7 imm32 / 48 8d 3d rel32 / bf imm32) and main is an ordinary named function. Not finding it is non-fatal on amd64 because there is nothing to synthesise. 2. backend/remill/lib/BC/Util.cpp — FindIndirectBrAddress register naming. Matched any register whose name startswith("X"), i.e. AArch64 X0..X30. amd64 GPRs are RAX/RBX/.../R8..R15 and match nothing; measured, the block for _init loads NEXT_PC and RAX. Worse, amd64 HAS X-prefixed registers — XMM0..XMM15 — and a prefix match would silently select a vector register as a branch target, which is worse than the abort. Now an exact GPR-name match, with the original prefix rule kept as a FALLBACK so AArch64 behaviour (XZR included) is bit-identical. 3. Util.cpp — memory-indirect branches, which AArch64 does not have. x86-64 branches through memory (jmp *disp(%rip)); every PLT stub does. Measured IR: %9 = load i64, ptr %NEXT_PC / store %9 -> PC / %10 = add %9, 7 / store %10 -> NEXT_PC call @JMPI>(runtime_manager, state, i64 5001240, ptr %NEXT_PC) The destination is NOT a value in the block: 5001240 is the GOT slot address and JMPI resolves it at RUNTIME, writing through the NEXT_PC pointer. So the target is NEXT_PC read AFTER the semantic call, which is why the load is appended rather than searched for. Safe because both call sites terminate the block only after this returns. Runs only where the code previously called abort(). 4. lifter/TraceManager.cpp — .plt entry splitting. The AArch64 scan looks for a `br` terminator 4 bytes at a time. On amd64 it never terminates an entry, so it emitted ONE function spanning the whole .plt — measured, fn_plt_401020 covering all 384 bytes of a 24-entry table. amd64 PLT entries are 16 bytes by ABI (classic push/jmp or the IBT endbr64 form); sh_entsize is not relied on because some linkers leave it 0. If the size is not a multiple of 16 the bytes are copied without claiming a function shape, which is better than inventing one. 5. lifter/TraceManager.cpp — AddRestDisasmFunc LOG(FATAL). Aborted whenever no function started after the address, which is reachable on the first real amd64 binary rather than a bug. Now bounded by the end of the containing code section — the same rule the symbol walk already applies to the last function it finds. The FATAL is KEPT for the case it was written for: an address in no code section at all. NOT A REGRESSION ON AArch64, and this was measured rather than reasoned about. Same static glibc hello, lifted in the :arm64 image before and after: 900 functions detected both times, bitcode BYTE-IDENTICAL at sha256 ec2dc6f94af1ec7a0df56869ceff64f04577bbff6f01a9c6fadd6003cac5f8c4 (4,388,108 bytes), wasm identical at 5,654,547 bytes. WHAT STILL DOES NOT WORK, stated plainly. A static glibc x86-64 binary now lifts many functions and then fails on a control-flow target of 0x927371, which lies in no section of an image spanning ~0x400000-0x4c6000. Upstream of that are FIVE unsupported instruction forms — NOP_MEMv_0F1F_32 (55), NOP_MEMv_0F1F_16 (27), MOVZX_GPR32_MEMw (6), MOVSXD_GPR64_MEMd (4), FADD_MEMmem32real (1) — which are reported and stepped over rather than aborting. x86-64 is therefore NOT yet end-to-end; these five fixes are necessary and not sufficient. --- backend/remill/lib/BC/Util.cpp | 117 +++++++++++++++++++++++- lifter/TraceManager.cpp | 161 ++++++++++++++++++++++++++++++++- 2 files changed, 269 insertions(+), 9 deletions(-) diff --git a/backend/remill/lib/BC/Util.cpp b/backend/remill/lib/BC/Util.cpp index 066711ce..353d6d2b 100644 --- a/backend/remill/lib/BC/Util.cpp +++ b/backend/remill/lib/BC/Util.cpp @@ -469,23 +469,132 @@ LoadArchSemantics(const Arch *arch, const std::vector &se return module; } -/* - find indirect jmp address (%address = load i64, ptr %XZZZ, align 8) +/* + Is `name` a general-purpose register that an indirect branch can target? + + AArch64 uses X0..X30, so the original code here matched any name beginning with "X" and + that was sufficient while AArch64 was the only lifted architecture. It is not sufficient + on amd64, in two separate ways: + + 1. amd64 GPRs are RAX/RBX/RCX/RDX/RSI/RDI/RBP/RSP and R8..R15. NONE of them begins with + "X", so the search found nothing and fell through to the abort below. Measured: for + `_init` in a static amd64 binary the block's loads are `NEXT_PC` and `RAX`. + 2. amd64 DOES have registers beginning with "X" — XMM0..XMM15 — and they are not branch + targets. A prefix match would select a vector register if one happened to be loaded + first in the block, silently lifting a jump to the wrong value. That is worse than + the abort. + + So the match is by exact register name rather than by prefix. +*/ +static bool IsIndirectBranchTargetRegister(llvm::StringRef name) { + // amd64 general-purpose registers, written out rather than pattern-matched: "R" + letter + // would also accept RIP, and "R" + digit would miss RAX. The set is small and closed. + static const char *const kAmd64Gprs[] = {"RAX", "RBX", "RCX", "RDX", "RSI", "RDI", "RBP", "RSP", + "R8", "R9", "R10", "R11", "R12", "R13", "R14", "R15"}; + for (const char *gpr : kAmd64Gprs) { + if (name == gpr) { + return true; + } + } + // AArch64 X0..X30. Digits only after the X, so XMM* and XZR are not accepted here — XZR is + // reached by the legacy fallback below, which keeps AArch64 behaviour bit-identical. + if (name.size() >= 2 && name[0] == 'X') { + for (size_t i = 1; i < name.size(); i++) { + if (name[i] < '0' || name[i] > '9') { + return false; + } + } + return true; + } + return false; +} + +/* + find indirect jmp address (%address = load i64, ptr %XZZZ, align 8) Note. assuming that the BB of BR contains only one `load ptr %XZZZ` + + Two passes, and the second one exists solely to preserve the previous behaviour on + AArch64. The first pass takes an exact GPR name and is what makes amd64 work. If it finds + nothing, the second falls back to the original `startswith("X")` rule, so any AArch64 + register name this code used to accept — XZR among them — is still accepted exactly as it + was. The order matters: on amd64 the exact pass claims RAX before the fallback could + mistake an XMM load for a branch target. */ llvm::Value *FindIndirectBrAddress(llvm::BasicBlock *block) { llvm::Value *indirect_addr = nullptr; for (llvm::Instruction &llvm_inst : *block) { if (llvm::LoadInst *load_inst = llvm::dyn_cast(&llvm_inst)) { llvm::Value *op = load_inst->getPointerOperand(); - if (op->getName().startswith("X")) { + if (IsIndirectBranchTargetRegister(op->getName())) { indirect_addr = load_inst; break; } } } if (nullptr == indirect_addr) { - printf("[ERROR] BR instruction doesn't have the LLVM IR insn like `load ptr XZZ`"); + for (llvm::Instruction &llvm_inst : *block) { + if (llvm::LoadInst *load_inst = llvm::dyn_cast(&llvm_inst)) { + llvm::Value *op = load_inst->getPointerOperand(); + if (op->getName().startswith("X")) { + indirect_addr = load_inst; + break; + } + } + } + } + /* + Third pass: a MEMORY-indirect branch, which has no register operand at all. + + AArch64 branches through a register (`br X0`), so the two passes above always find + something. amd64 also branches through memory — `jmp *disp(%rip)` — and every PLT stub in + the image does exactly that. Measured, the lifted block for one is: + + %9 = load i64, ptr %NEXT_PC + store i64 %9, ptr %PC + %10 = add i64 %9, 7 + store i64 %10, ptr %NEXT_PC + call void @JMPI>(ptr %runtime_manager, ptr %state, i64 5001240, ptr %NEXT_PC) + + The destination is not a value in this block: `5001240` is the address of the GOT slot, + and remill's `JMPI` semantic reads it at RUNTIME and writes the result through the + `NEXT_PC` pointer it is handed. So the target is whatever `NEXT_PC` holds *after* the + semantic call — which is why this load is appended to the end of the block rather than + searched for among the existing instructions, where only the pre-branch value exists. + + Appending is safe because the caller has not yet terminated this block: both call sites in + TraceLifter.cpp invoke this function and only then add the branch. + + This runs only where the previous two passes found nothing, i.e. only where the old code + called abort(). It cannot change AArch64 behaviour. + */ + if (nullptr == indirect_addr) { + llvm::Value *next_pc_ptr = nullptr; + for (llvm::Instruction &llvm_inst : *block) { + if (llvm::LoadInst *load_inst = llvm::dyn_cast(&llvm_inst)) { + if (load_inst->getPointerOperand()->getName() == "NEXT_PC") { + next_pc_ptr = load_inst->getPointerOperand(); + break; + } + } + } + if (nullptr != next_pc_ptr) { + llvm::IRBuilder<> ir(block); + indirect_addr = ir.CreateLoad(llvm::Type::getInt64Ty(block->getContext()), next_pc_ptr, + "ecv_mem_indirect_target"); + } + } + if (nullptr == indirect_addr) { + /* printf, not fprintf(stderr, ...): stdout is fully buffered when redirected, and abort() + discards the buffer, so this message was being written and then thrown away — the + failure presented as a silent SIGABRT. Report on stderr, which is unbuffered. */ + fprintf(stderr, + "[ERROR] indirect branch: no general-purpose register load in this block. Loads seen:\n"); + for (llvm::Instruction &llvm_inst : *block) { + if (llvm::LoadInst *load_inst = llvm::dyn_cast(&llvm_inst)) { + fprintf(stderr, " %s\n", load_inst->getPointerOperand()->getName().str().c_str()); + } + } + fflush(stderr); abort(); } return indirect_addr; diff --git a/lifter/TraceManager.cpp b/lifter/TraceManager.cpp index 6dab8488..8957ab87 100644 --- a/lifter/TraceManager.cpp +++ b/lifter/TraceManager.cpp @@ -30,7 +30,31 @@ std::string AArch64TraceManager::AddRestDisasmFunc(uint64_t addr) { } else if (upper_addr_2 != rest_disasm_funcs.end()) { end_addr = upper_addr_2->first; } else { - LOG(FATAL) << "[Bug] does not handle the pattern of having last rest_disasm_func."; + /* + No function starts after `addr`, so there is no next-function boundary to size this one + with. That is a reachable state rather than a bug: it happens whenever control reaches + the LAST unclaimed region of a code section, which on amd64 occurs on the first real + binary because `.plt`-style regions and `.text` tails are not all named by symbols. + + Bound it by the end of the section that contains `addr` — the same rule the symbol walk + in SetELFObject already applies to the last function it finds, so this is the existing + convention rather than a new one. + + The FATAL is kept for the case it was actually written for: an address that lies in no + code section at all, which really would be a bug and must not be silently given a size. + */ + end_addr = 0; + for (auto &[sec_name, sec] : elf_obj.code_sections) { + (void) sec_name; + if (sec.vma <= addr && addr < sec.vma + sec.size) { + end_addr = sec.vma + sec.size; + break; + } + } + if (0 == end_addr) { + LOG(FATAL) << "[Bug] rest_disasm_func at 0x" << std::hex << addr + << " lies outside every code section."; + } } rest_disasm_funcs.insert({addr, DisasmFunc(rest_fun_name, addr, end_addr - addr)}); return rest_fun_name; @@ -169,6 +193,45 @@ void AArch64TraceManager::SetELFData() { if (plt_section.sec_name.empty()) plt_section = elf_obj.code_sections[".iplt"]; if (!plt_section.sec_name.empty()) { +#if defined(ELFCONV_X86_BUILD) && ELFCONV_X86_BUILD == 1 + /* + amd64: PLT entries are FIXED-SIZE, so they are split by size rather than by scanning for + a terminator. + + The AArch64 branch below walks 4 bytes at a time looking for a `br` to end each entry. + On amd64 that scan can never terminate an entry — the bytes it matches are AArch64 + encodings — so it ran to the end of the section and emitted ONE function spanning the + whole `.plt`. Measured consequence: `fn_plt_401020` covering all 384 bytes of a 24-entry + table, which the lifter then decoded as a single straight-line function, walked off the + end of one stub into the next, and eventually produced a branch to 0x927371 — an address + in no section of the image. + + Every amd64 PLT entry is 16 bytes by ABI, whether it is the classic + `push`/`jmp *GOT(%rip)` pair or the IBT form that begins `endbr64`. The section's own + `sh_entsize` says so too, but it is not relied on here: it is 0 on some linkers, so the + constant is used and the section size is required to be a multiple of it before the + split is trusted. + */ + constexpr uint64_t kAmd64PltEntrySize = 16; + if (plt_section.size % kAmd64PltEntrySize == 0) { + for (uint64_t off = 0; off < plt_section.size; off += kAmd64PltEntrySize) { + for (uint64_t b = 0; b < kAmd64PltEntrySize; b++) { + memory[plt_section.vma + off + b] = plt_section.bytes[off + b]; + } + auto b_entry = plt_section.vma + off; + std::stringstream fn_name; + fn_name << "fn_plt_" << std::hex << b_entry; + disasm_funcs.emplace(b_entry, DisasmFunc(fn_name.str(), b_entry, kAmd64PltEntrySize)); + } + } else { + /* Not a table of 16-byte entries. Copying the bytes without claiming a function shape + is strictly better than inventing one: the lifter can still read them, and nothing + downstream is told a boundary that was guessed. */ + for (uint64_t off = 0; off < plt_section.size; off++) { + memory[plt_section.vma + off] = plt_section.bytes[off]; + } + } +#else uint64_t ins_i = 0; while (ins_i < plt_section.size) { auto b_entry = plt_section.vma + ins_i; @@ -189,16 +252,23 @@ void AArch64TraceManager::SetELFData() { disasm_funcs.emplace(b_entry, DisasmFunc(fn_name.str(), b_entry, (plt_section.vma + ins_i) - b_entry)); } +#endif + } + + if (disasm_funcs.count(entry_point) != 1) { + elfconv_runtime_error("[ERROR] Entry function is not defined.\n"); } - /* +#if defined(ELFCONV_AARCH64_BUILD) && ELFCONV_AARCH64_BUILD == 1 + + /* define __wrap_main function (FIXME) __libc_start_call_main BLR jump to the instructions as following in _start. `nop` `b main` `nop` */ - if (disasm_funcs.count(entry_point) == 1) { + { std::vector s_t_addrs = {entry_point}; uint64_t __wrap_main_size = AARCH64_OP_SIZE * 3; auto &text_section = elf_obj.code_sections[".text"]; @@ -228,7 +298,88 @@ void AArch64TraceManager::SetELFData() { elfconv_runtime_error("[ERROR] __wrap_main code block is not found. entry_point: 0x%lx\n", entry_point); } - } else { - elfconv_runtime_error("[ERROR] Entry function is not defined.\n"); } + +#elif defined(ELFCONV_X86_BUILD) && ELFCONV_X86_BUILD == 1 + + /* + amd64: recover `main` from `_start`, which is a different problem from the AArch64 one + above and must not be solved with the same code. + + The AArch64 branch exists because `__libc_start_call_main` reaches `main` through a + trampoline — `nop` / `b main` / `nop` — that carries NO SYMBOL, so the lifter would have + no function at the indirect-branch target unless one is synthesised. It matches those + three instructions literally and steps 4 bytes at a time because every AArch64 + instruction is 4 bytes. + + Neither premise holds on amd64. Instructions are variable-length, so a fixed stride is + meaningless; and there is no trampoline — the System V ABI passes `main` as the FIRST + argument to `__libc_start_main`, so `_start` materialises its address into `%rdi` and + `main` is an ordinary function that the symbol table and `.eh_frame` already name. Running + the AArch64 scan here cannot match anything (those bytes are AArch64 encodings) and + reaches `elfconv_runtime_error`, which is the abort this branch replaces. + + So the work here is not to synthesise a function. It is to READ the address out of + `_start` and make sure a function exists at it — which, for a binary whose `main` is + named, is already true and this loop simply confirms. The recovery matters for the case + where it is not: `main` is `LOCAL` or absent while `.eh_frame` gave the range no name. + + Two encodings materialise the pointer, and both are emitted in practice: + + 48 c7 c7 mov $imm32, %rdi non-PIE; imm32 is sign-extended to 64 bits + 48 8d 3d lea rel32(%rip), %rdi PIE/static-PIE; target = end-of-insn + rel32 + + A third, `bf ` (`mov $imm32, %edi`), writes the 32-bit register and is accepted for + the same reason: on a non-PIE image the upper half is zero anyway. + + The scan is deliberately NARROW — it walks `_start` only, it accepts only these three + opcodes, and it requires the recovered address to land inside a known code section. A + byte-pattern search that ranges further would find `%rdi` loads that have nothing to do + with `main`. If nothing is found, that is NOT fatal here: unlike AArch64 there is no + missing function to synthesise, so lifting proceeds on the symbols that already exist. + */ + { + auto &text_section = elf_obj.code_sections[".text"]; + const uint64_t start_size = disasm_funcs[entry_point].func_size; + const uint8_t *start_bytes = &text_section.bytes[entry_point - text_section.vma]; + uint64_t main_vma = 0; + + for (uint64_t i = 0; i + 7 <= start_size; i++) { + if (start_bytes[i] == 0x48 && start_bytes[i + 1] == 0xc7 && start_bytes[i + 2] == 0xc7) { + /* mov $imm32, %rdi — sign-extended, so read it as a signed 32-bit value. */ + int32_t imm = static_cast(start_bytes[i + 3] | (start_bytes[i + 4] << 8) | + (start_bytes[i + 5] << 16) | (start_bytes[i + 6] << 24)); + main_vma = static_cast(static_cast(imm)); + break; + } + if (start_bytes[i] == 0x48 && start_bytes[i + 1] == 0x8d && start_bytes[i + 2] == 0x3d) { + /* lea rel32(%rip), %rdi — %rip is the address of the NEXT instruction, i.e. +7. */ + int32_t rel = static_cast(start_bytes[i + 3] | (start_bytes[i + 4] << 8) | + (start_bytes[i + 5] << 16) | (start_bytes[i + 6] << 24)); + main_vma = entry_point + i + 7 + static_cast(rel); + break; + } + if (start_bytes[i] == 0xbf) { + /* mov $imm32, %edi — zero-extended by the hardware. */ + main_vma = static_cast(start_bytes[i + 1] | (start_bytes[i + 2] << 8) | + (start_bytes[i + 3] << 16) | (start_bytes[i + 4] << 24)); + break; + } + } + + if (main_vma != 0 && !disasm_funcs.contains(main_vma)) { + /* Only reached when the address is real but unnamed. Bounded by the section that + contains it, so the synthesised function never runs off the end of the image. */ + for (auto &[sec_name, sec] : elf_obj.code_sections) { + (void) sec_name; + if (sec.vma <= main_vma && main_vma < sec.vma + sec.size) { + disasm_funcs.emplace( + main_vma, DisasmFunc("__wrap_main", main_vma, (sec.vma + sec.size) - main_vma)); + break; + } + } + } + } + +#endif } \ No newline at end of file