Skip to content

Commit 3698994

Browse files
rafaelaulerrafaelauler
committed
[BOLT] Always move JTs in jump-table=move
We should always move jump tables when requested. Previously, we were not moving jump tables of non-simple functions in relocation mode. That caused a bug detailed in the attached test case: in PIC jump tables, we force jump tables to be moved, but if they are not moved because the function is not simple, we could incorrectly update original entries in .rodata, corrupting it under special circumstances (see testcase). Reviewed By: #bolt, maksfb Differential Revision: https://reviews.llvm.org/D137357
1 parent 687ce3d commit 3698994

File tree

3 files changed

+146
-2
lines changed

3 files changed

+146
-2
lines changed

‎bolt/lib/Core/BinaryEmitter.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -740,10 +740,12 @@ void BinaryEmitter::emitJumpTables(const BinaryFunction &BF) {
740740

741741
for (auto &JTI : BF.jumpTables()) {
742742
JumpTable &JT = *JTI.second;
743+
// Only emit shared jump tables once, when processing the first parent
744+
if (JT.Parents.size() > 1 && JT.Parents[0] != &BF)
745+
continue;
743746
if (opts::PrintJumpTables)
744747
JT.print(outs());
745-
if ((opts::JumpTables == JTS_BASIC || !BF.isSimple()) &&
746-
BC.HasRelocations) {
748+
if (opts::JumpTables == JTS_BASIC && BC.HasRelocations) {
747749
JT.updateOriginal();
748750
} else {
749751
MCSection *HotSection, *ColdSection;

‎bolt/test/X86/Inputs/jt-pic-linkerscript.ld

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
# Linker script used by jump-table-pic-conflict.s test.
2+
# .rodata needs to appear before .text
3+
4+
SECTIONS
5+
{
6+
. = 0x201120;
7+
.rodata : { *(.rodata) }
8+
.eh_frame : { *(.eh_frame) }
9+
.text : { *(.text) }
10+
}

‎bolt/test/X86/jump-table-pic-conflict.s

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
# Check cases when the first PIC jump table entries of one function can be
2+
# interpreted as valid last entries of the previous function.
3+
4+
# Conditions to trigger the bug: Function A and B have jump tables that
5+
# are adjacent in memory. We run in lite relocation mode. Function B
6+
# is not disassembled because it does not have profile. Function A
7+
# triggers a special conditional that forced BOLT to rewrite its jump
8+
# table in-place (instead of moving it) because it is marked as
9+
# non-simple (in this case, containing unknown control flow). The
10+
# first entry of B's jump table (a PIC offset) happens to be a valid
11+
# address inside A when added to A's jump table base address. In this
12+
# case, BOLT could overwrite B's jump table, corrupting it, thinking
13+
# the first entry of it is actually part of A's jump table.
14+
15+
# REQUIRES: system-linux
16+
17+
# RUN: llvm-mc -filetype=obj -triple x86_64-unknown-unknown \
18+
# RUN: %s -o %t.o
19+
# RUN: link_fdata %s %t.o %t.fdata
20+
# RUN: llvm-strip --strip-unneeded %t.o
21+
# RUN: ld.lld %t.o -o %t.exe -q -T %S/Inputs/jt-pic-linkerscript.ld
22+
# RUN: llvm-bolt %t.exe -relocs -o %t.out -data %t.fdata \
23+
# RUN: -lite=1
24+
# RUN: llvm-readelf -S %t.out | FileCheck --check-prefix=CHECK %s
25+
# The output binary is runnable, but we check for test success with
26+
# readelf. This is another way to check this bug:
27+
# COM: %t.out
28+
29+
# BOLT needs to create a new rodata section, indicating that it
30+
# successfully moved the jump table in _start.
31+
# CHECK: [{{.*}}] .bolt.org.rodata
32+
33+
.globl _start
34+
.type _start, %function
35+
_start:
36+
.cfi_startproc
37+
# FDATA: 0 [unknown] 0 1 _start 0 0 1
38+
push %rbp
39+
mov %rsp, %rbp
40+
mov 0x8(%rbp), %rdi
41+
cmpq $3, %rdi
42+
ja .L5
43+
jmp .L6
44+
# Unreachable code, here to mark this function as non-simple
45+
# (containing unknown control flow) with a stray indirect jmp
46+
jmp *%rax
47+
.L6:
48+
decq %rdi
49+
leaq .LJT1(%rip), %rcx
50+
movslq (%rcx, %rdi, 4), %rax
51+
addq %rcx, %rax
52+
jmp *%rax
53+
.L1:
54+
leaq str1(%rip), %rsi
55+
jmp .L4
56+
.L2:
57+
leaq str2(%rip), %rsi
58+
jmp .L4
59+
.L3:
60+
leaq str3(%rip), %rsi
61+
jmp .L4
62+
.L5:
63+
leaq str4(%rip), %rsi
64+
.L4:
65+
movq $1, %rdi
66+
movq $10, %rdx
67+
movq $1, %rax
68+
syscall
69+
mov 0x8(%rbp), %rdi
70+
decq %rdi
71+
callq func_b
72+
movq %rax, %rdi
73+
movq $231, %rax
74+
syscall
75+
pop %rbp
76+
ret
77+
.cfi_endproc
78+
.size _start, .-_start
79+
80+
.globl func_b
81+
.type func_b, %function
82+
func_b:
83+
.cfi_startproc
84+
push %rbp
85+
mov %rsp, %rbp
86+
cmpq $3, %rdi
87+
ja .L2_6
88+
# FT
89+
leaq .LJT2(%rip), %rcx
90+
movslq (%rcx, %rdi, 4), %rax
91+
addq %rcx, %rax
92+
jmp *%rax
93+
.L2_1:
94+
movq $0, %rax
95+
jmp .L2_5
96+
.L2_2:
97+
movq $1, %rax
98+
jmp .L2_5
99+
.L2_3:
100+
movq $2, %rax
101+
jmp .L2_5
102+
.L2_4:
103+
movq $3, %rax
104+
jmp .L2_5
105+
.L2_6:
106+
movq $-1, %rax
107+
.L2_5:
108+
popq %rbp
109+
ret
110+
.cfi_endproc
111+
.size func_b, .-func_b
112+
113+
.rodata
114+
str1: .asciz "Message 1\n"
115+
str2: .asciz "Message 2\n"
116+
str3: .asciz "Message 3\n"
117+
str4: .asciz "Highrange\n"
118+
# Special case where the first .LJT2 entry is a valid offset of
119+
# _start when interpreted with .LJT1 as a base address.
120+
.LJT1:
121+
.long .L1-.LJT1
122+
.long .L2-.LJT1
123+
.long .L3-.LJT1
124+
.long .L3-.LJT1
125+
.long .L3-.LJT1
126+
.long .L3-.LJT1
127+
.long .L3-.LJT1
128+
.LJT2:
129+
.long .L2_1-.LJT2
130+
.long .L2_2-.LJT2
131+
.long .L2_3-.LJT2
132+
.long .L2_4-.LJT2

0 commit comments

Comments
 (0)