Skip to content

[BOLT] Fix references in ignored functions in CFG state #140678

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 2, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions bolt/include/bolt/Core/BinaryFunction.h
Original file line number Diff line number Diff line change
Expand Up @@ -2335,6 +2335,7 @@ class BinaryFunction {
releaseCFG();
CurrentState = State::Emitted;
}
clearList(Relocations);
}

/// Process LSDA information for the function.
Expand Down
6 changes: 2 additions & 4 deletions bolt/lib/Core/BinaryContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1032,10 +1032,8 @@ void BinaryContext::adjustCodePadding() {

if (!hasValidCodePadding(BF)) {
if (HasRelocations) {
if (opts::Verbosity >= 1) {
this->outs() << "BOLT-INFO: function " << BF
<< " has invalid padding. Ignoring the function.\n";
}
this->errs() << "BOLT-WARNING: function " << BF
<< " has invalid padding. Ignoring the function\n";
BF.setIgnored();
} else {
BF.setMaxSize(BF.getSize());
Expand Down
31 changes: 21 additions & 10 deletions bolt/lib/Core/BinaryFunction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1531,8 +1531,6 @@ Error BinaryFunction::disassemble() {
if (uint64_t Offset = getFirstInstructionOffset())
Labels[Offset] = BC.Ctx->createNamedTempSymbol();

clearList(Relocations);

if (!IsSimple) {
clearList(Instructions);
return createNonFatalBOLTError("");
Expand Down Expand Up @@ -3237,16 +3235,26 @@ void BinaryFunction::setTrapOnEntry() {
}

void BinaryFunction::setIgnored() {
IsIgnored = true;

if (opts::processAllFunctions()) {
// We can accept ignored functions before they've been disassembled.
// In that case, they would still get disassembled and emited, but not
// In that case, they would still get disassembled and emitted, but not
// optimized.
assert(CurrentState == State::Empty &&
"cannot ignore non-empty functions in current mode");
IsIgnored = true;
if (CurrentState != State::Empty) {
BC.errs() << "BOLT-ERROR: cannot ignore non-empty function " << *this
<< " in current mode\n";
exit(1);
}
return;
}

IsSimple = false;
LLVM_DEBUG(dbgs() << "Ignoring " << getPrintName() << '\n');

if (CurrentState == State::Empty)
return;

clearDisasmState();

// Clear CFG state too.
Expand All @@ -3266,9 +3274,11 @@ void BinaryFunction::setIgnored() {

CurrentState = State::Empty;

IsIgnored = true;
IsSimple = false;
LLVM_DEBUG(dbgs() << "Ignoring " << getPrintName() << '\n');
// Fix external references in the original function body.
if (BC.HasRelocations) {
LLVM_DEBUG(dbgs() << "Scanning refs in " << *this << '\n');
scanExternalRefs();
}
}

void BinaryFunction::duplicateConstantIslands() {
Expand Down Expand Up @@ -3755,7 +3765,6 @@ void BinaryFunction::postProcessBranches() {

MCSymbol *BinaryFunction::addEntryPointAtOffset(uint64_t Offset) {
assert(Offset && "cannot add primary entry point");
assert(CurrentState == State::Empty || CurrentState == State::Disassembled);

const uint64_t EntryPointAddress = getAddress() + Offset;
MCSymbol *LocalSymbol = getOrCreateLocalLabel(EntryPointAddress);
Expand All @@ -3764,6 +3773,8 @@ MCSymbol *BinaryFunction::addEntryPointAtOffset(uint64_t Offset) {
if (EntrySymbol)
return EntrySymbol;

assert(CurrentState == State::Empty || CurrentState == State::Disassembled);

if (BinaryData *EntryBD = BC.getBinaryDataAtAddress(EntryPointAddress)) {
EntrySymbol = EntryBD->getSymbol();
} else {
Expand Down
1 change: 1 addition & 0 deletions bolt/lib/Rewrite/RewriteInstance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3433,6 +3433,7 @@ void RewriteInstance::disassembleFunctions() {
BC->outs() << "BOLT-INFO: could not disassemble function " << Function
<< ". Will ignore.\n";
// Forcefully ignore the function.
Function.scanExternalRefs();
Function.setIgnored();
});

Expand Down
33 changes: 33 additions & 0 deletions bolt/test/AArch64/patch-ignored.s
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
## Check that llvm-bolt patches functions that are getting ignored after their
## CFG was constructed.

# RUN: %clang %cflags %s -o %t.exe -Wl,-q
# RUN: llvm-bolt %t.exe -o %t.bolt --force-patch 2>&1 | FileCheck %s
# RUN: llvm-objdump -d %t.bolt | FileCheck %s --check-prefix=CHECK-OBJDUMP

.text

## The function is too small to be patched and BOLT is forced to ignore it under
## --force-patch. Check that the reference to _start is updated.
# CHECK: BOLT-WARNING: failed to patch entries in unpatchable
.globl unpatchable
.type unpatchable, %function
unpatchable:
.cfi_startproc
# CHECK-OBJDUMP: <unpatchable>:
# CHECK-OBJDUMP-NEXT: bl {{.*}} <_start>
bl _start
ret
.cfi_endproc
.size unpatchable, .-unpatchable

.globl _start
.type _start, %function
_start:
.cfi_startproc
cmp x0, 1
b.eq .L0
.L0:
ret x30
.cfi_endproc
.size _start, .-_start
44 changes: 44 additions & 0 deletions bolt/test/X86/patch-ignored.s
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
## Check that llvm-bolt patches functions that are getting ignored after their
## CFG was constructed.

# RUN: %clang %cflags %s -o %t.exe -Wl,-q
# RUN: llvm-bolt %t.exe -o %t.bolt 2>&1 | FileCheck %s
# RUN: llvm-objdump -d %t.bolt | FileCheck %s --check-prefix=CHECK-OBJDUMP

.text

## After the CFG is built, the following function will be marked as ignored
## due to the presence of the internal call.
# CHECK: BOLT-WARNING: will skip the following function
# CHECK-NEXT: internal_call
.globl internal_call
.type internal_call, %function
internal_call:
.cfi_startproc
# CHECK-OBJDUMP: <internal_call>:
call .L1
jmp .L2
# CHECK-OBJDUMP: jmp
.L1:
jmp _start
# CHECK-OBJDUMP: jmp
# CHECK-OBJDUMP-SAME: <_start>
ret
.L2:
jmp _start
# CHECK-OBJDUMP: jmp
# CHECK-OBJDUMP-SAME: <_start>
.cfi_endproc
.size internal_call, .-internal_call

.globl _start
.type _start, %function
_start:
.cfi_startproc
cmpq %rdi, 1
jne .L0
movq %rdi, %rax
.L0:
ret
.cfi_endproc
.size _start, .-_start
Loading