Skip to content
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

[BOLT] Support computed goto and allow map addrs inside functions #120267

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 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
9 changes: 8 additions & 1 deletion bolt/lib/Rewrite/RewriteInstance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2439,6 +2439,13 @@ void RewriteInstance::readDynamicRelocations(const SectionRef &Section,
if (Symbol)
SymbolIndex[Symbol] = getRelocationSymbol(InputFile, Rel);

const uint64_t SymAddress = SymbolAddress + Addend;
BinaryFunction *Func = BC->getBinaryFunctionContainingAddress(SymAddress);
if (Func && !Func->isInConstantIsland(SymAddress)) {
if (const uint64_t SymOffset = SymAddress - Func->getAddress())
Func->addEntryPointAtOffset(SymOffset);
}
Comment on lines +2444 to +2447
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add more checks? E.g., what happens if Symbol points to a function and SymAddress falls into another function? What if the reference is in the constant island? It's better to fail at build time.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll look into adding more checks.


BC->addDynamicRelocation(Rel.getOffset(), Symbol, RType, Addend);
}
}
Expand Down Expand Up @@ -5599,7 +5606,7 @@ uint64_t RewriteInstance::getNewFunctionOrDataAddress(uint64_t OldAddress) {
for (const BinaryBasicBlock &BB : *BF)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not directly related to your change, but we should use BinaryFunction::forEachEntryPoint() here.

if (BB.isEntryPoint() &&
(BF->getAddress() + BB.getOffset()) == OldAddress)
return BF->getOutputAddress() + BB.getOffset();
return BB.getOutputStartAddress();
}
BC->errs() << "BOLT-ERROR: unable to get new address corresponding to "
"input address 0x"
Expand Down
67 changes: 67 additions & 0 deletions bolt/test/AArch64/computed-goto.s
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
// This test checks that BOLT creates entry points for addresses
// referenced by dynamic relocations.
// The test also checks that BOLT can map addresses inside functions.

// Checks for error and entry points.
# RUN: llvm-mc -filetype=obj -triple aarch64-unknown-unknown %s -o %t.o
# RUN: %clang %cflags %t.o -o %t.exe -Wl,-q
# RUN: llvm-bolt %t.exe -o %t.bolt 2>&1 | FileCheck %s
# RUN: llvm-bolt %t.exe -o %t.bolt --print-cfg | FileCheck --check-prefix=CHECK-ENTRYS %s
Rin18 marked this conversation as resolved.
Show resolved Hide resolved

// Checks for dynamic relocations.
# RUN: llvm-readelf -dr %t.bolt > %t.out.txt
# RUN: llvm-objdump -j .rela.dyn -d %t.bolt >> %t.out.txt
# RUN: FileCheck --check-prefix=CHECK-RELOCS %s --input-file=%t.out.txt

// Before bolt could handle mapping addresses within moved functions, it
// would bail out with an error of the form:
// BOLT-ERROR: unable to get new address corresponding to input address 0x10390 in function main. Consider adding this function to --skip-funcs=...
// These addresses arise if computed GOTO is in use.
// Check that bolt does not emit any error.
# CHECK-NOT: BOLT-ERROR
Rin18 marked this conversation as resolved.
Show resolved Hide resolved

// Check that there are dynamic relocations.
# CHECK-RELOCS: Dynamic section at offset {{.*}} contains {{.*}} entries:
# CHECK-RELOCS: Relocation section '.rela.dyn' at offset {{.*}} contains {{.*}} entries

// Check that dynamic relocations were updated
# CHECK-RELOCS: [[#%x,OFF:]] [[#%x,INFO_DYN:]] R_AARCH64_RELATIVE [[#%x,ADDR:]]
# CHECK-RELOCS-NEXT: [[#OFF + 8]] {{0*}}[[#INFO_DYN]] R_AARCH64_RELATIVE [[#ADDR + 8]]
# CHECK-RELOCS: [[#ADDR]] <unknown>
# CHECK-RELOCS: [[#ADDR + 8]] <unknown>

// Check that BOLT registers extra entry points for dynamic relocations.
# CHECK-ENTRYS: Binary Function "main" after building cfg {
# CHECK-ENTRYS: IsMultiEntry: 1
# CHECK-ENTRYS: .Ltmp0 {{.*}}
# CHECK-ENTRYS-NEXT: Secondary Entry Point: {{.*}}
# CHECK-ENTRYS: .Ltmp1 {{.*}}
# CHECK-ENTRYS-NEXT: Secondary Entry Point: {{.*}}

.globl main
.p2align 2
.type main,@function
main:
.cfi_startproc
adrp x8, .L__const.main.ptrs+8
add x8, x8, :lo12:.L__const.main.ptrs+8
ldr x9, [x8], #8
br x9

.Label0: // Block address taken
ldr x9, [x8], #8
br x9

.Label1: // Block address taken
mov w0, #42
ret

.Lfunc_end0:
.size main, .Lfunc_end0-main
.cfi_endproc
.type .L__const.main.ptrs,@object
.section .data.rel.ro,"aw",@progbits
.p2align 3, 0x0
.L__const.main.ptrs:
.xword .Label0
.xword .Label1
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
int main(int argc, char *argv[]) {
static const void *T1[] = { &&L1, &&L2 };
static const void *T2[] = { &&L2, &&L3 };
static const void *T1[] = {&&L1, &&L2};
static const void *T2[] = {&&L2, &&L3};

const void **T = (argc > 1) ? T1 : T2;

Expand Down
16 changes: 0 additions & 16 deletions bolt/test/X86/indirect-goto-pie.test

This file was deleted.

2 changes: 1 addition & 1 deletion bolt/test/X86/indirect-goto.test
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
## Check llvm-bolt processes binaries compiled from sources that use indirect goto.
RUN: %clang %cflags -no-pie %S/Inputs/indirect_goto.c -Wl,-q -o %t
RUN: %clang %cflags -no-pie %S/../Inputs/indirect_goto.c -Wl,-q -o %t
RUN: llvm-bolt %t -o %t.null --relocs=1 --print-cfg --print-only=main \
RUN: --strict \
RUN: 2>&1 | FileCheck %s
Expand Down
13 changes: 13 additions & 0 deletions bolt/test/dynamic-relocs.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// This test checks that BOLT creates entry points for addresses
// referenced by dynamic relocations.
Rin18 marked this conversation as resolved.
Show resolved Hide resolved

RUN: %clang %cflags %S/Inputs/indirect_goto.c -o %t.exe -Wl,-q
Rin18 marked this conversation as resolved.
Show resolved Hide resolved
RUN: llvm-bolt %t.exe -o %t.bolt --print-cfg | FileCheck %s

// Check that bolt does not emit any error.
CHECK-NOT: BOLT-ERROR
Rin18 marked this conversation as resolved.
Show resolved Hide resolved

// Check that BOLT registers extra entry points for dynamic relocations.
CHECK: Binary Function "main" after building cfg {
CHECK: IsMultiEntry: 1
CHECK: Secondary Entry Points : {{.*}}
Loading