[BOLT] Split unmarked tail code after CFI-bounded functions into separate functions (#176173)
Detect trailing unmarked code after CFI-bounded predecessors (__BOLT_FDE_FUNC* or symbols whose FDE address range equals their size) and promote it into separate BinaryFunction objects named __BOLT_UNMARKED_TAILat<addr>. A predecessor is only considered when: - the target is AArch64 - it is not a pseudo or fragment function - it has slack beyond its recorded size (MaxSize > Size) - it is CFI-bounded - no function is already registered at the tail start address The trailing region is measured conservatively and we bail out (leaving the slack attached to the predecessor) when: - the tail range is not fully contained in the predecessor's origin section - a data marker ($d) or a constant island overlaps the region (data-in-code) - disassembly fails or yields a zero-length instruction - after trimming trailing padding, the tail is empty (i.e. it contained only padding) or does not end in a return - any bytes past the decoded code are non-zero, i.e. not pure zero padding. The bug was introduced in commit [cd27741](cd27741c11)
This commit is contained in:
Elvina Yakubova
GitHub
parent
b8de72900b
commit
f901499b11
3 changed files with 282 additions and 0 deletions
|
|
@ -280,6 +280,12 @@ private:
|
|||
/// symbol table has been processed.
|
||||
void adjustFunctionBoundaries(DenseMap<uint64_t, MarkerSymType> &MarkerSyms);
|
||||
|
||||
/// Promote unmarked executable regions after CFI-bounded functions into
|
||||
/// separate BinaryFunction objects (__BOLT_FDE_FUNC* or named symbols
|
||||
/// whose FDE range matches their size).
|
||||
void
|
||||
splitUnmarkedTailFunctions(DenseMap<uint64_t, MarkerSymType> &MarkerSyms);
|
||||
|
||||
/// Make .eh_frame section relocatable.
|
||||
void relocateEHFrameSection();
|
||||
|
||||
|
|
|
|||
|
|
@ -1383,6 +1383,7 @@ void RewriteInstance::discoverFileObjects() {
|
|||
|
||||
// Now that all the functions were created - adjust their boundaries.
|
||||
adjustFunctionBoundaries(MarkerSymbols);
|
||||
splitUnmarkedTailFunctions(MarkerSymbols);
|
||||
|
||||
// Annotate functions with code/data markers in AArch64
|
||||
for (auto &[Address, Type] : MarkerSymbols) {
|
||||
|
|
@ -2083,6 +2084,163 @@ void RewriteInstance::disassemblePLT() {
|
|||
}
|
||||
}
|
||||
|
||||
namespace {
|
||||
|
||||
/// True when the predecessor body ends at its FDE boundary, so trailing bytes
|
||||
/// are outside both symtab size and unwind info:
|
||||
/// (1) __BOLT_FDE_FUNC* (FDE with no symtab entry), or
|
||||
/// (2) a normal symbol whose size equals its FDE address range.
|
||||
bool isCFIBoundedTailPredecessor(const BinaryFunction &BF,
|
||||
const CFIReaderWriter &CFI) {
|
||||
if (BF.getOneName().starts_with("__BOLT_FDE_FUNC"))
|
||||
return true;
|
||||
auto FDEI = CFI.getFDEs().find(BF.getAddress());
|
||||
if (FDEI == CFI.getFDEs().end())
|
||||
return false;
|
||||
return FDEI->second->getAddressRange() == BF.getSize();
|
||||
}
|
||||
|
||||
/// AArch64 padding / filler instructions that may appear in slack but are not
|
||||
/// standalone callable functions.
|
||||
bool isAArch64TailPaddingInst(const BinaryContext &BC, const MCInst &Inst) {
|
||||
if (BC.MIB->isNoop(Inst) || BC.MIB->isTrap(Inst))
|
||||
return true;
|
||||
return false;
|
||||
}
|
||||
|
||||
/// True if the decoded tail looks like real callable code, not padding.
|
||||
/// Expects trailing filler to have already been trimmed, so the last
|
||||
/// instruction must be the tail's terminator.
|
||||
bool isValidAArch64UnmarkedTail(const BinaryContext &BC,
|
||||
ArrayRef<MCInst> Insts) {
|
||||
if (Insts.empty())
|
||||
return false;
|
||||
|
||||
// Expect a callable snippet; inlined tails end with ret.
|
||||
return BC.MIB->isReturn(Insts.back());
|
||||
}
|
||||
|
||||
/// Disassemble a prefix of [TailStart, TailStart + TrailingExtent) and return
|
||||
/// the length of the callable tail. Trailing filler instructions (nop/trap)
|
||||
/// after the terminator are trimmed and treated as slack, and any remaining
|
||||
/// bytes in the range must be zero padding.
|
||||
/// Returns 0 if the region is not valid unmarked code.
|
||||
uint64_t
|
||||
measureAArch64UnmarkedTail(BinaryContext &BC, const BinaryFunction &Pred,
|
||||
DenseMap<uint64_t, MarkerSymType> &MarkerSyms,
|
||||
uint64_t TailStart, uint64_t TrailingExtent) {
|
||||
auto hasDataMarkerAt = [&MarkerSyms](uint64_t Address) {
|
||||
auto It = MarkerSyms.find(Address);
|
||||
return It != MarkerSyms.end() && It->second == MarkerSymType::DATA;
|
||||
};
|
||||
|
||||
// Pred was registered from an executable section during symbol/FDE discovery.
|
||||
BinarySection &Section = *Pred.getOriginSection();
|
||||
|
||||
// adjustFunctionBoundaries() set Pred->MaxSize so [Pred, Pred+MaxSize) fits
|
||||
// in the section and stops at the next symbol/function.
|
||||
if (!Section.containsRange(TailStart, TrailingExtent))
|
||||
return 0;
|
||||
|
||||
StringRef Contents = Section.getContents();
|
||||
const uint64_t SectionOffset = TailStart - Section.getAddress();
|
||||
const uint8_t *Bytes =
|
||||
reinterpret_cast<const uint8_t *>(Contents.data()) + SectionOffset;
|
||||
|
||||
SmallVector<MCInst, 4> Insts;
|
||||
SmallVector<uint64_t, 4> InstSizes;
|
||||
uint64_t CodeLen = 0;
|
||||
while (CodeLen < TrailingExtent) {
|
||||
if (hasDataMarkerAt(TailStart + CodeLen))
|
||||
return 0;
|
||||
if (Pred.isInConstantIsland(TailStart + CodeLen))
|
||||
return 0;
|
||||
|
||||
MCInst Inst;
|
||||
uint64_t Size = 0;
|
||||
ArrayRef<uint8_t> Slice(Bytes + CodeLen, TrailingExtent - CodeLen);
|
||||
if (!BC.SymbolicDisAsm->getInstruction(Inst, Size, Slice,
|
||||
TailStart + CodeLen, nulls()) ||
|
||||
!Size)
|
||||
break;
|
||||
Insts.push_back(Inst);
|
||||
InstSizes.push_back(Size);
|
||||
CodeLen += Size;
|
||||
}
|
||||
|
||||
// Ignore trailing filler (nop/trap) after the terminator. The filler is
|
||||
// treated as slack, just like the zero padding checked below, so real
|
||||
// binaries with post-ret padding are still recognized.
|
||||
uint64_t TailLen = CodeLen;
|
||||
size_t CallableInsts = Insts.size();
|
||||
while (CallableInsts > 0 &&
|
||||
isAArch64TailPaddingInst(BC, Insts[CallableInsts - 1])) {
|
||||
--CallableInsts;
|
||||
TailLen -= InstSizes[CallableInsts];
|
||||
}
|
||||
|
||||
if (!TailLen || !isValidAArch64UnmarkedTail(
|
||||
BC, ArrayRef(Insts).take_front(CallableInsts)))
|
||||
return 0;
|
||||
|
||||
// Everything past the decoded region must be zero padding. The decoded
|
||||
// filler in [TailLen, CodeLen) is already validated as nop/trap above.
|
||||
for (uint64_t I = CodeLen; I < TrailingExtent; ++I) {
|
||||
if (Bytes[I] != 0)
|
||||
return 0;
|
||||
}
|
||||
|
||||
return TailLen;
|
||||
}
|
||||
|
||||
} // namespace
|
||||
|
||||
void RewriteInstance::splitUnmarkedTailFunctions(
|
||||
DenseMap<uint64_t, MarkerSymType> &MarkerSyms) {
|
||||
if (!BC->isAArch64())
|
||||
return;
|
||||
|
||||
std::vector<BinaryFunction *> Worklist;
|
||||
for (BinaryFunction &BF : llvm::make_second_range(BC->getBinaryFunctions())) {
|
||||
if (BF.isPseudo() || BF.isFragment())
|
||||
continue;
|
||||
if (BF.getMaxSize() == BF.getSize())
|
||||
continue;
|
||||
if (!isCFIBoundedTailPredecessor(BF, *CFIRdWrt))
|
||||
continue;
|
||||
Worklist.push_back(&BF);
|
||||
}
|
||||
|
||||
for (BinaryFunction *Pred : Worklist) {
|
||||
const uint64_t TailStart = Pred->getAddress() + Pred->getSize();
|
||||
const uint64_t TrailingExtent = Pred->getMaxSize() - Pred->getSize();
|
||||
|
||||
if (BC->getBinaryFunctionAtAddress(TailStart))
|
||||
continue;
|
||||
|
||||
const uint64_t CodeLen = measureAArch64UnmarkedTail(
|
||||
*BC, *Pred, MarkerSyms, TailStart, TrailingExtent);
|
||||
if (!CodeLen)
|
||||
continue;
|
||||
|
||||
BinarySection &Section = *Pred->getOriginSection();
|
||||
|
||||
Pred->setMaxSize(Pred->getSize());
|
||||
|
||||
std::string TailName =
|
||||
"__BOLT_UNMARKED_TAILat" + Twine::utohexstr(TailStart).str();
|
||||
BC->errs() << "BOLT-WARNING: detected executable code after CFI-bounded "
|
||||
<< "function " << *Pred << "; creating synthetic function "
|
||||
<< TailName
|
||||
<< ". Consider adding a symbol or FDE for this code.\n";
|
||||
|
||||
// Inherit [TailStart, TailStart + TrailingExtent) from Pred's old extent.
|
||||
BinaryFunction *TailBF = BC->createBinaryFunction(
|
||||
TailName, Section, TailStart, CodeLen, CodeLen);
|
||||
TailBF->setMaxSize(TrailingExtent);
|
||||
}
|
||||
}
|
||||
|
||||
void RewriteInstance::adjustFunctionBoundaries(
|
||||
DenseMap<uint64_t, MarkerSymType> &MarkerSyms) {
|
||||
for (auto BFI = BC->getBinaryFunctions().begin(),
|
||||
|
|
@ -2126,6 +2284,13 @@ void RewriteInstance::adjustFunctionBoundaries(
|
|||
if (It == MarkerSyms.end() || It->second != MarkerSymType::DATA) {
|
||||
// This is potentially another entry point into the function.
|
||||
uint64_t EntryOffset = NextSymRefI->first - Function.getAddress();
|
||||
// At the FDE/symbol end, unmarked tail code may follow; split will
|
||||
// promote it to __BOLT_UNMARKED_TAIL instead of a secondary entry.
|
||||
if (BC->isAArch64() && EntryOffset == Function.getSize() &&
|
||||
isCFIBoundedTailPredecessor(Function, *CFIRdWrt)) {
|
||||
++NextSymRefI;
|
||||
continue;
|
||||
}
|
||||
LLVM_DEBUG(dbgs() << "BOLT-DEBUG: adding entry point to function "
|
||||
<< Function << " at offset 0x"
|
||||
<< Twine::utohexstr(EntryOffset) << '\n');
|
||||
|
|
|
|||
111
bolt/test/AArch64/entry-point-at-function-end.s
Normal file
111
bolt/test/AArch64/entry-point-at-function-end.s
Normal file
|
|
@ -0,0 +1,111 @@
|
|||
## Promote unmarked code after a CFI-bounded predecessor into __BOLT_UNMARKED_TAIL.
|
||||
## Covers:
|
||||
## 1) FDE-only region (no symtab entry; BOLT names it __BOLT_FDE_FUNC* or .text/N)
|
||||
## 2) Named function (FDE range == symbol size) -> named_sub + tail
|
||||
## Caller is in a separate section; -Wl,-q keeps PC-relative relocs on the bl.
|
||||
|
||||
# RUN: llvm-mc -filetype=obj -triple aarch64-unknown-unknown %s -o %t.o
|
||||
# RUN: %clang %cflags %t.o -pie -Wl,-q -o %t.exe
|
||||
# RUN: llvm-bolt %t.exe -o %t.bolt --print-disasm -v=1 2>&1 | FileCheck %s
|
||||
|
||||
# CHECK-COUNT-3: BOLT-WARNING: detected executable code after CFI-bounded function
|
||||
# CHECK-SAME: creating synthetic function __BOLT_UNMARKED_TAILat
|
||||
# CHECK-SAME: Consider adding a symbol or FDE for this code.
|
||||
|
||||
## Scenario 1a: FDE-only predecessor at the section start (named after .text).
|
||||
## The tail has trailing filler nops after its ret: they are trimmed from the
|
||||
## callable Size but remain part of MaxSize slack.
|
||||
# CHECK: Binary Function ".text/{{[0-9]+}}" after disassembly
|
||||
# CHECK: Size : 0x14
|
||||
# CHECK: MaxSize : 0x14
|
||||
# CHECK: Binary Function "{{.*}}__BOLT_UNMARKED_TAIL{{.*}}" after disassembly
|
||||
# CHECK: Size : 0x8
|
||||
# CHECK: MaxSize : 0x10
|
||||
|
||||
## Scenario 1b: FDE-only predecessor discovered from the FDE alone.
|
||||
# CHECK: Binary Function "__BOLT_FDE_FUNC{{.*}}" after disassembly
|
||||
# CHECK: Size : 0x14
|
||||
# CHECK: MaxSize : 0x14
|
||||
# CHECK: Binary Function "{{.*}}__BOLT_UNMARKED_TAIL{{.*}}" after disassembly
|
||||
# CHECK: Size : 0x8
|
||||
# CHECK: MaxSize : 0x8
|
||||
|
||||
## Scenario 2: named predecessor (FDE address range matches .size).
|
||||
# CHECK: Binary Function "named_sub" after disassembly
|
||||
# CHECK: Size : 0x14
|
||||
# CHECK: MaxSize : 0x14
|
||||
# CHECK: Binary Function "{{.*}}__BOLT_UNMARKED_TAIL{{.*}}" after disassembly
|
||||
# CHECK: Size : 0x10
|
||||
# CHECK: MaxSize : 0x10
|
||||
|
||||
# CHECK-COUNT-3: bl __BOLT_UNMARKED_TAILat{{.*}}
|
||||
|
||||
.section .text.fdeonly,"ax",@progbits
|
||||
|
||||
## Scenario 1a: FDE-only predecessor at the section start (no symbol table entry).
|
||||
.Lfde_sub:
|
||||
.cfi_startproc
|
||||
stp x29, x30, [sp, #-16]!
|
||||
mov x29, sp
|
||||
mov w0, #42
|
||||
ldp x29, x30, [sp], #16
|
||||
ret
|
||||
.cfi_endproc
|
||||
|
||||
.Ltail_after_fde:
|
||||
nop
|
||||
ret
|
||||
nop
|
||||
nop
|
||||
|
||||
## Scenario 1b: identical FDE-only region, not at the section start.
|
||||
.Lfde_sub2:
|
||||
.cfi_startproc
|
||||
stp x29, x30, [sp, #-16]!
|
||||
mov x29, sp
|
||||
mov w0, #42
|
||||
ldp x29, x30, [sp], #16
|
||||
ret
|
||||
.cfi_endproc
|
||||
|
||||
.Ltail_after_fde2:
|
||||
nop
|
||||
ret
|
||||
|
||||
.section .text.named,"ax",@progbits
|
||||
|
||||
## Scenario 2: named predecessor (FDE address range matches .size).
|
||||
.globl named_sub
|
||||
.type named_sub, %function
|
||||
named_sub:
|
||||
.cfi_startproc
|
||||
stp x29, x30, [sp, #-16]!
|
||||
mov x29, sp
|
||||
mov w0, #7
|
||||
ldp x29, x30, [sp], #16
|
||||
ret
|
||||
.cfi_endproc
|
||||
.size named_sub, .-named_sub
|
||||
|
||||
.Ltail_after_named:
|
||||
mov x0, x0
|
||||
mov w1, #7
|
||||
add w0, w0, w1
|
||||
ret
|
||||
|
||||
.section .text.entry,"ax",@progbits
|
||||
|
||||
.globl _start
|
||||
.type _start, %function
|
||||
_start:
|
||||
.cfi_startproc
|
||||
stp x29, x30, [sp, #-16]!
|
||||
mov x29, sp
|
||||
bl .Ltail_after_fde
|
||||
bl .Ltail_after_fde2
|
||||
bl .Ltail_after_named
|
||||
ldp x29, x30, [sp], #16
|
||||
mov w0, #0
|
||||
ret
|
||||
.cfi_endproc
|
||||
.size _start, .-_start
|
||||
Loading…
Reference in a new issue