Skip to content

Commit

Permalink
[BOLT][AArch64] Fixes assertion errors occurred when perf2bolt was ex…
Browse files Browse the repository at this point in the history
…ecuted

BOLT only checks for the most common indirect branch pattern during
the branch analyzation. Needs to extend the current logic with other cases
which slightly differs from the expected one.
This instruction sequences comes from libc,
it occurs when the binary is static.

As a workaround mark it as UNKNOWN branch and add support for them
in a follow up PR later.

Fixes: #83114
  • Loading branch information
kaadam committed Jun 11, 2024
1 parent c63a622 commit a2a220e
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 2 deletions.
29 changes: 27 additions & 2 deletions bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -706,8 +706,20 @@ class AArch64MCPlusBuilder : public MCPlusBuilder {
unsigned ShiftVal = AArch64_AM::getArithShiftValue(OperandExtension);
AArch64_AM::ShiftExtendType ExtendType =
AArch64_AM::getArithExtendType(OperandExtension);
if (ShiftVal != 2)
llvm_unreachable("Failed to match indirect branch! (fragment 2)");
if (ShiftVal != 2) {
// TODO: handle that case where ShiftVal != 2.
// The following code sequence below has no shift amount,
// the range could be 0 to 4.
// The pattern comes from libc, it occurs when the binary is static.
// adr x6, 0x219fb0 <sigall_set+0x88>
// add x6, x6, x14, lsl #2
// ldr w7, [x6]
// add x6, x6, w7, sxtw => no shift amount
// br x6
errs() << "BOLT-WARNING: "
"Failed to match indirect branch: ShiftVAL != 2 \n";
return false;
}

if (ExtendType == AArch64_AM::SXTB)
ScaleValue = 1LL;
Expand Down Expand Up @@ -752,6 +764,19 @@ class AArch64MCPlusBuilder : public MCPlusBuilder {
return true;
}

if (DefJTBaseAdd->getOpcode() == AArch64::ADR) {
// TODO: handle that case when we do not have adrp/add pair.
// It also occurs when the binary is static.
// adr x13, 0x215a18 <_nl_value_type_LC_COLLATE+0x50>
// ldrh w13, [x13, w12, uxtw #1]
// adr x12, 0x247b30 <__gettextparse+0x5b0>
// add x13, x12, w13, sxth #2
// br x13
errs() << "BOLT-WARNING: Failed to match indirect branch: "
"nop/adr instead of adrp/add \n";
return false;
}

assert(DefJTBaseAdd->getOpcode() == AArch64::ADDXri &&
"Failed to match jump table base address pattern! (1)");

Expand Down
31 changes: 31 additions & 0 deletions bolt/test/AArch64/test-indirect-branch.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// Test how BOLT handles indirect branch sequence of instructions in
// AArch64MCPlus builder.
// This test checks that case when we have no shift amount after add
// instruction. This pattern comes from libc, so needs to build '-static'
// binary to reproduce the issue easily.
//
// adr x6, 0x219fb0 <sigall_set+0x88>
// add x6, x6, x14, lsl #2
// ldr w7, [x6]
// add x6, x6, w7, sxtw => no shift amount
// br x6
// It also tests another case when we use '-fuse-ld=lld' along with '-static'
// which produces the following sequence of intsructions:
//
// nop => nop/adr instead of adrp/add
// adr x13, 0x215a18 <_nl_value_type_LC_COLLATE+0x50>
// ldrh w13, [x13, w12, uxtw #1]
// adr x12, 0x247b30 <__gettextparse+0x5b0>
// add x13, x12, w13, sxth #2
// br x13

// REQUIRES: system-linux
// RUN: %clang %s -o %t.exe -Wl,-q -static -fuse-ld=lld \
// RUN: --target=aarch64-unknown-linux-gnu
// RUN: llvm-bolt %t.exe -o %t.bolt --print-cfg \
// RUN: -v=1 2>&1 | FileCheck --match-full-lines %s

// CHECK: BOLT-WARNING: Failed to match indirect branch: nop/adr instead of adrp/add
// CHECK: BOLT-WARNING: Failed to match indirect branch: ShiftVAL != 2

int main() { return 42; }

0 comments on commit a2a220e

Please sign in to comment.