Skip to content

Commit 1b4bd4e

Browse files
authored
[BOLT][AArch64] Remove assertions from jump table heuristic (#124372)
The code for jump table detection on AArch64 asserts liberally whenever the input instruction sequence does not match the expected pattern. As a result, BOLT fails to process binaries with such sequences instead of ignoring functions with unknown control flow. Remove asserts in analyzeIndirectBranchFragment() and mark indirect jumps as instructions with unknown control flow instead.
1 parent db1ee18 commit 1b4bd4e

File tree

3 files changed

+58
-25
lines changed

3 files changed

+58
-25
lines changed

bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp

Lines changed: 24 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -834,6 +834,8 @@ class AArch64MCPlusBuilder : public MCPlusBuilder {
834834
/// # of this BB)
835835
/// br x0 # Indirect jump instruction
836836
///
837+
/// Return true on successful jump table instruction sequence match, false
838+
/// otherwise.
837839
bool analyzeIndirectBranchFragment(
838840
const MCInst &Inst,
839841
DenseMap<const MCInst *, SmallVector<MCInst *, 4>> &UDChain,
@@ -842,6 +844,8 @@ class AArch64MCPlusBuilder : public MCPlusBuilder {
842844
// Expect AArch64 BR
843845
assert(Inst.getOpcode() == AArch64::BR && "Unexpected opcode");
844846

847+
JumpTable = nullptr;
848+
845849
// Match the indirect branch pattern for aarch64
846850
SmallVector<MCInst *, 4> &UsesRoot = UDChain[&Inst];
847851
if (UsesRoot.size() == 0 || UsesRoot[0] == nullptr)
@@ -879,8 +883,8 @@ class AArch64MCPlusBuilder : public MCPlusBuilder {
879883
// Parsed as ADDXrs reg:x8 reg:x8 reg:x12 imm:0
880884
return false;
881885
}
882-
assert(DefAdd->getOpcode() == AArch64::ADDXrx &&
883-
"Failed to match indirect branch!");
886+
if (DefAdd->getOpcode() != AArch64::ADDXrx)
887+
return false;
884888

885889
// Validate ADD operands
886890
int64_t OperandExtension = DefAdd->getOperand(3).getImm();
@@ -897,8 +901,8 @@ class AArch64MCPlusBuilder : public MCPlusBuilder {
897901
// ldr w7, [x6]
898902
// add x6, x6, w7, sxtw => no shift amount
899903
// br x6
900-
errs() << "BOLT-WARNING: "
901-
"Failed to match indirect branch: ShiftVAL != 2 \n";
904+
LLVM_DEBUG(dbgs() << "BOLT-DEBUG: "
905+
"failed to match indirect branch: ShiftVAL != 2\n");
902906
return false;
903907
}
904908

@@ -909,7 +913,7 @@ class AArch64MCPlusBuilder : public MCPlusBuilder {
909913
else if (ExtendType == AArch64_AM::SXTW)
910914
ScaleValue = 4LL;
911915
else
912-
llvm_unreachable("Failed to match indirect branch! (fragment 3)");
916+
return false;
913917

914918
// Match an ADR to load base address to be used when addressing JT targets
915919
SmallVector<MCInst *, 4> &UsesAdd = UDChain[DefAdd];
@@ -920,18 +924,15 @@ class AArch64MCPlusBuilder : public MCPlusBuilder {
920924
return false;
921925
}
922926
MCInst *DefBaseAddr = UsesAdd[1];
923-
assert(DefBaseAddr->getOpcode() == AArch64::ADR &&
924-
"Failed to match indirect branch pattern! (fragment 3)");
927+
if (DefBaseAddr->getOpcode() != AArch64::ADR)
928+
return false;
925929

926930
PCRelBase = DefBaseAddr;
927931
// Match LOAD to load the jump table (relative) target
928932
const MCInst *DefLoad = UsesAdd[2];
929-
assert(mayLoad(*DefLoad) &&
930-
"Failed to match indirect branch load pattern! (1)");
931-
assert((ScaleValue != 1LL || isLDRB(*DefLoad)) &&
932-
"Failed to match indirect branch load pattern! (2)");
933-
assert((ScaleValue != 2LL || isLDRH(*DefLoad)) &&
934-
"Failed to match indirect branch load pattern! (3)");
933+
if (!mayLoad(*DefLoad) || (ScaleValue == 1LL && !isLDRB(*DefLoad)) ||
934+
(ScaleValue == 2LL && !isLDRH(*DefLoad)))
935+
return false;
935936

936937
// Match ADD that calculates the JumpTable Base Address (not the offset)
937938
SmallVector<MCInst *, 4> &UsesLoad = UDChain[DefLoad];
@@ -941,7 +942,6 @@ class AArch64MCPlusBuilder : public MCPlusBuilder {
941942
isRegToRegMove(*DefJTBaseAdd, From, To)) {
942943
// Sometimes base address may have been defined in another basic block
943944
// (hoisted). Return with no jump table info.
944-
JumpTable = nullptr;
945945
return true;
946946
}
947947

@@ -953,24 +953,27 @@ class AArch64MCPlusBuilder : public MCPlusBuilder {
953953
// adr x12, 0x247b30 <__gettextparse+0x5b0>
954954
// add x13, x12, w13, sxth #2
955955
// br x13
956-
errs() << "BOLT-WARNING: Failed to match indirect branch: "
957-
"nop/adr instead of adrp/add \n";
956+
LLVM_DEBUG(dbgs() << "BOLT-DEBUG: failed to match indirect branch: "
957+
"nop/adr instead of adrp/add\n");
958958
return false;
959959
}
960960

961-
assert(DefJTBaseAdd->getOpcode() == AArch64::ADDXri &&
962-
"Failed to match jump table base address pattern! (1)");
961+
if (DefJTBaseAdd->getOpcode() != AArch64::ADDXri) {
962+
LLVM_DEBUG(dbgs() << "BOLT-DEBUG: failed to match jump table base "
963+
"address pattern! (1)\n");
964+
return false;
965+
}
963966

964967
if (DefJTBaseAdd->getOperand(2).isImm())
965968
Offset = DefJTBaseAdd->getOperand(2).getImm();
966969
SmallVector<MCInst *, 4> &UsesJTBaseAdd = UDChain[DefJTBaseAdd];
967970
const MCInst *DefJTBasePage = UsesJTBaseAdd[1];
968971
if (DefJTBasePage == nullptr || isLoadFromStack(*DefJTBasePage)) {
969-
JumpTable = nullptr;
970972
return true;
971973
}
972-
assert(DefJTBasePage->getOpcode() == AArch64::ADRP &&
973-
"Failed to match jump table base page pattern! (2)");
974+
if (DefJTBasePage->getOpcode() != AArch64::ADRP)
975+
return false;
976+
974977
if (DefJTBasePage->getOperand(1).isExpr())
975978
JumpTable = DefJTBasePage->getOperand(1).getExpr();
976979
return true;
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
## Verify that BOLT does not crash while encountering instruction sequence that
2+
## does not perfectly match jump table pattern.
3+
4+
# REQUIRES: system-linux
5+
6+
# RUN: llvm-mc -filetype=obj -triple aarch64-unknown-unknown %s -o %t.o
7+
# RUN: %clang %cflags --target=aarch64-unknown-linux %t.o -o %t.exe -Wl,-q
8+
# RUN: llvm-bolt %t.exe -o %t.bolt --print-cfg 2>&1 | FileCheck %s
9+
10+
.section .text
11+
.align 4
12+
.globl _start
13+
.type _start, %function
14+
_start:
15+
sub w0, w0, #0x4a
16+
## The address loaded into x22 is undefined. However, the instructions that
17+
## follow ldr, use the x22 address as a regular jump table.
18+
ldr x22, [x29, #0x98]
19+
ldrb w0, [x22, w0, uxtw]
20+
adr x1, #12
21+
add x0, x1, w0, sxtb #2
22+
br x0
23+
# CHECK: br x0 # UNKNOWN
24+
.L0:
25+
ret
26+
.size _start, .-_start
27+
28+
## Force relocation mode.
29+
.reloc 0, R_AARCH64_NONE

bolt/test/AArch64/test-indirect-branch.s

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,11 @@
33

44
// clang-format off
55

6-
// REQUIRES: system-linux
6+
// REQUIRES: system-linux, asserts
7+
78
// RUN: llvm-mc -filetype=obj -triple aarch64-unknown-unknown %s -o %t.o
89
// RUN: %clang %cflags --target=aarch64-unknown-linux %t.o -o %t.exe -Wl,-q
9-
// RUN: llvm-bolt %t.exe -o %t.bolt --print-cfg --strict\
10+
// RUN: llvm-bolt %t.exe -o %t.bolt --print-cfg --strict --debug-only=mcplus \
1011
// RUN: -v=1 2>&1 | FileCheck %s
1112

1213
// Pattern 1: there is no shift amount after the 'add' instruction.
@@ -39,7 +40,7 @@ _start:
3940
// svc #0
4041

4142
// Pattern 1
42-
// CHECK: BOLT-WARNING: Failed to match indirect branch: ShiftVAL != 2
43+
// CHECK: BOLT-DEBUG: failed to match indirect branch: ShiftVAL != 2
4344
.globl test1
4445
.type test1, %function
4546
test1:
@@ -57,7 +58,7 @@ test1_2:
5758
ret
5859

5960
// Pattern 2
60-
// CHECK: BOLT-WARNING: Failed to match indirect branch: nop/adr instead of adrp/add
61+
// CHECK: BOLT-DEBUG: failed to match indirect branch: nop/adr instead of adrp/add
6162
.globl test2
6263
.type test2, %function
6364
test2:

0 commit comments

Comments
 (0)