Skip to content

Commit

Permalink
[BOLT] Fix ValidateMemRefs pass (llvm#94406)
Browse files Browse the repository at this point in the history
In ValidateMemRefs pass, when we validate references in the form of
`Symbol + Addend`, we should check `Symbol` not `Symbol + Addend`
against aliasing a jump table.

Recommitting with a modified test case:
llvm#88838

Co-authored-by: sinan <[email protected]>
  • Loading branch information
maksfb and linsinan1995 authored Jun 4, 2024
1 parent 330e8a7 commit c923d39
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 4 deletions.
8 changes: 4 additions & 4 deletions bolt/lib/Passes/ValidateMemRefs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,7 @@ bool ValidateMemRefs::checkAndFixJTReference(BinaryFunction &BF, MCInst &Inst,
if (!BD)
return false;

const uint64_t TargetAddress = BD->getAddress() + Offset;
JumpTable *JT = BC.getJumpTableContainingAddress(TargetAddress);
JumpTable *JT = BC.getJumpTableContainingAddress(BD->getAddress());
if (!JT)
return false;

Expand All @@ -43,8 +42,9 @@ bool ValidateMemRefs::checkAndFixJTReference(BinaryFunction &BF, MCInst &Inst,
// the jump table label with a regular rodata reference. Get a
// non-JT reference by fetching the symbol 1 byte before the JT
// label.
MCSymbol *NewSym = BC.getOrCreateGlobalSymbol(TargetAddress - 1, "DATAat");
BC.MIB->setOperandToSymbolRef(Inst, OperandNum, NewSym, 1, &*BC.Ctx, 0);
MCSymbol *NewSym = BC.getOrCreateGlobalSymbol(BD->getAddress() - 1, "DATAat");
BC.MIB->setOperandToSymbolRef(Inst, OperandNum, NewSym, Offset + 1, &*BC.Ctx,
0);
LLVM_DEBUG(dbgs() << "BOLT-DEBUG: replaced reference @" << BF.getPrintName()
<< " from " << BD->getName() << " to " << NewSym->getName()
<< " + 1\n");
Expand Down
63 changes: 63 additions & 0 deletions bolt/test/X86/jt-symbol-disambiguation-4.s
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
## If the operand references a symbol that differs from the jump table label,
## no reference updating is required even if its target address resides within
## the jump table's range.
## In this test case, consider the second instruction within the main function,
## where the address resulting from 'c + 17' corresponds to one byte beyond the
## address of the .LJTI2_0 jump table label. However, this operand represents
## an offset calculation related to the global variable 'c' and should remain
## unaffected by the jump table.

# REQUIRES: system-linux

# RUN: llvm-mc -filetype=obj -triple x86_64-unknown-unknown %s -o %t.o
# RUN: %clang -no-pie %t.o -o %t.exe -Wl,-q
# RUN: llvm-bolt --funcs=main,foo/1 %t.exe -o %t.exe.bolt --print-normalized \
# RUN: 2>&1 | FileCheck %s

.text
.globl main
.type main,@function
main:
# CHECK: Binary Function "main"
pushq %rbp
movq %rsp, %rbp
movq $-16, %rax
movl c+17(%rax), %edx
# CHECK: movl c+17(%rax), %edx
cmpl $255, %edx
je .LCorrect
movl $1, %eax
popq %rbp
ret
.LCorrect:
movl $0, %eax
popq %rbp
ret

.p2align 4, 0x90
.type foo,@function
foo:
# CHECK: Binary Function "foo
movq $0, %rax
jmpq *.LJTI2_0(,%rax,8)
# CHECK: jmpq *{{.*}} # JUMPTABLE
addl $-36, %eax
.LBB2_2:
addl $-16, %eax
retq
.section .rodata,"a",@progbits
.type c,@object
.data
.globl c
.p2align 4, 0x0
c:
.byte 1
.byte 0xff
.zero 14
.size c, 16
.LJTI2_0:
.quad .LBB2_2
.quad .LBB2_2
.quad .LBB2_2
.quad .LBB2_2

0 comments on commit c923d39

Please sign in to comment.