Skip to content

Commit b87bf74

Browse files
committed
[BOLT] Fix creation of invalid CFG in presence of dead code
When there is a direct jump right after an indirect one, in the absence of code jumpting to this direct jump, this is obviously dead code. However, BOLT was failing to recognize that by mistakenly placing both jmp instructions in the same basic block, and creating wrong successor edges. Fix that, so we can safely run UCE on that. This bug also causes validateCFG to fail and BOLT to crash if it is running ICP on that function. Reviewed By: #bolt, Amir Differential Revision: https://reviews.llvm.org/D148055
1 parent 88f4091 commit b87bf74

File tree

2 files changed

+67
-0
lines changed

2 files changed

+67
-0
lines changed

bolt/lib/Core/BinaryFunction.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2038,6 +2038,7 @@ bool BinaryFunction::buildCFG(MCPlusBuilder::AllocatorIdTy AllocatorId) {
20382038
MCInst *PrevInstr = PrevBB->getLastNonPseudoInstr();
20392039
assert(PrevInstr && "no previous instruction for a fall through");
20402040
if (MIB->isUnconditionalBranch(Instr) &&
2041+
!MIB->isIndirectBranch(*PrevInstr) &&
20412042
!MIB->isUnconditionalBranch(*PrevInstr) &&
20422043
!MIB->getConditionalTailCall(*PrevInstr) &&
20432044
!MIB->isReturn(*PrevInstr)) {

bolt/test/X86/unreachable-jmp.s

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
# This checks that we don't create an invalid CFG when there is an
2+
# unreachable direct jump right after an indirect one.
3+
4+
# REQUIRES: system-linux
5+
6+
# RUN: llvm-mc -filetype=obj -triple x86_64-unknown-unknown \
7+
# RUN: %s -o %t.o
8+
# RUN: link_fdata %s %t.o %t.fdata
9+
# RUN: llvm-strip --strip-unneeded %t.o
10+
# RUN: %clang %cflags -no-pie %t.o -o %t.exe -Wl,-q -nostdlib
11+
# RUN: llvm-bolt %t.exe -o %t.out --data %t.fdata \
12+
# RUN: --eliminate-unreachable --print-cfg | FileCheck %s
13+
14+
.globl _start
15+
.type _start, %function
16+
_start:
17+
.cfi_startproc
18+
# FDATA: 0 [unknown] 0 1 _start 0 0 1
19+
push %rbp
20+
mov %rsp, %rbp
21+
push %rbx
22+
push %r14
23+
subq $0x20, %rsp
24+
movq %rdi, %rcx
25+
b:
26+
jmpq *JUMP_TABLE(,%rcx,8)
27+
# FDATA: 1 _start #b# 1 _start #hotpath# 0 20
28+
# Unreachable direct jump here. Our CFG should still make sense and properly
29+
# place this instruction in a new basic block.
30+
jmp .lbb2
31+
.lbb1: je .lexit
32+
.lbb2:
33+
xorq %rax, %rax
34+
addq $0x20, %rsp
35+
pop %r14
36+
pop %rbx
37+
pop %rbp
38+
ret
39+
hotpath:
40+
movq $2, %rax
41+
addq $0x20, %rsp
42+
pop %r14
43+
pop %rbx
44+
pop %rbp
45+
ret
46+
.lexit:
47+
movq $1, %rax
48+
addq $0x20, %rsp
49+
pop %r14
50+
pop %rbx
51+
pop %rbp
52+
ret
53+
.cfi_endproc
54+
.size _start, .-_start
55+
56+
.rodata
57+
.globl JUMP_TABLE
58+
JUMP_TABLE:
59+
.quad .lbb1
60+
.quad .lbb2
61+
.quad hotpath
62+
63+
# No basic blocks above should have 4 successors! That is a bug.
64+
# CHECK-NOT: Successors: {{.*}} (mispreds: 0, count: 20), {{.*}} (mispreds: 0, count: 0), {{.*}} (mispreds: 0, count: 0), {{.*}} (mispreds: 0, count: 0)
65+
# Check successful removal of stray direct jmp
66+
# CHECK: UCE removed 1 block

0 commit comments

Comments
 (0)