Skip to content

[BOLT] Handle internal calls in ValidateInternalCalls #105736

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
Aug 27, 2024
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
2 changes: 2 additions & 0 deletions bolt/include/bolt/Core/BinaryFunction.h
Original file line number Diff line number Diff line change
Expand Up @@ -1692,6 +1692,8 @@ class BinaryFunction {

void setPseudo(bool Pseudo) { IsPseudo = Pseudo; }

void setPreserveNops(bool Value) { PreserveNops = Value; }

BinaryFunction &setUsesGnuArgsSize(bool Uses = true) {
UsesGnuArgsSize = Uses;
return *this;
Expand Down
20 changes: 4 additions & 16 deletions bolt/lib/Core/BinaryFunction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1339,22 +1339,10 @@ Error BinaryFunction::disassemble() {
BC.getBinaryFunctionContainingAddress(TargetAddress))
TargetFunc->setIgnored();

if (IsCall && containsAddress(TargetAddress)) {
if (TargetAddress == getAddress()) {
// Recursive call.
TargetSymbol = getSymbol();
} else {
if (BC.isX86()) {
// Dangerous old-style x86 PIC code. We may need to freeze this
// function, so preserve the function as is for now.
PreserveNops = true;
} else {
BC.errs() << "BOLT-WARNING: internal call detected at 0x"
<< Twine::utohexstr(AbsoluteInstrAddr)
<< " in function " << *this << ". Skipping.\n";
IsSimple = false;
}
}
if (IsCall && TargetAddress == getAddress()) {
// A recursive call. Calls to internal blocks are handled by
// ValidateInternalCalls pass.
TargetSymbol = getSymbol();
}

if (!TargetSymbol) {
Expand Down
9 changes: 6 additions & 3 deletions bolt/lib/Passes/ValidateInternalCalls.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -302,24 +302,27 @@ bool ValidateInternalCalls::analyzeFunction(BinaryFunction &Function) const {
}

Error ValidateInternalCalls::runOnFunctions(BinaryContext &BC) {
if (!BC.isX86())
return Error::success();

// Look for functions that need validation. This should be pretty rare.
std::set<BinaryFunction *> NeedsValidation;
for (auto &BFI : BC.getBinaryFunctions()) {
BinaryFunction &Function = BFI.second;
for (BinaryBasicBlock &BB : Function) {
for (MCInst &Inst : BB) {
if (getInternalCallTarget(Function, Inst)) {
BC.errs() << "BOLT-WARNING: internal call detected in function "
<< Function << '\n';
NeedsValidation.insert(&Function);
Function.setSimple(false);
Function.setPreserveNops(true);
break;
}
}
}
}

if (!BC.isX86())
return Error::success();

// Skip validation for non-relocation mode
if (!BC.HasRelocations)
return Error::success();
Expand Down
65 changes: 65 additions & 0 deletions bolt/test/AArch64/internal-call.s
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
## Test that llvm-bolt detects internal calls and marks the containing function
## as non-simple.

# RUN: llvm-mc -filetype=obj -triple aarch64-unknown-unknown %s -o %t.o
# RUN: %clang %cflags %t.o -o %t.exe -Wl,-q -static
# RUN: llvm-bolt %t.exe -o %t.null --print-all 2>&1 | FileCheck %s

# CHECK: Binary Function "_start" after building cfg
# CHECK: internal call detected in function _start
# CHECK-NOT: Binary Function "_start" after validate-internal-calls

.text
.globl _start
.type _start, %function
_start:
.cfi_startproc
.LBB00:
mov x11, #0x1fff
cmp x1, x11
b.hi .Ltmp1

.entry1:
movi v4.16b, #0x0
movi v5.16b, #0x0
subs x1, x1, #0x8
b.lo .Ltmp2

.entry2:
ld1 { v2.2d, v3.2d }, [x0], #32
ld1 { v0.2d, v1.2d }, [x0], #32

.Ltmp2:
uaddlp v4.4s, v4.8h
uaddlp v4.2d, v4.4s
mov x0, v4.d[0]
mov x1, v4.d[1]
add x0, x0, x1
ret x30

.Ltmp1:
mov x8, x30

.Lloop:
add x5, x0, x9
mov x1, #0xface
movi v4.16b, #0x0
movi v5.16b, #0x0
bl .entry2
add x4, x4, x0
mov x0, x5
sub x7, x7, x10
cmp x7, x11
b.hi .Lloop

mov x1, x7
bl .entry1
add x0, x4, x0
mov x30, x8
ret x30

.cfi_endproc
.size _start, .-_start

## Force relocation mode.
.reloc 0, R_AARCH64_NONE
Loading