Skip to content

[BOLT][X86] Fix getTargetSymbol() #133834

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 1, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion bolt/include/bolt/Core/MCPlusBuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -1266,7 +1266,11 @@ class MCPlusBuilder {

/// Return MCSymbol extracted from the expression.
virtual const MCSymbol *getTargetSymbol(const MCExpr *Expr) const {
if (auto *SymbolRefExpr = dyn_cast<const MCSymbolRefExpr>(Expr))
if (auto *BinaryExpr = dyn_cast<const MCBinaryExpr>(Expr))
return getTargetSymbol(BinaryExpr->getLHS());

auto *SymbolRefExpr = dyn_cast<const MCSymbolRefExpr>(Expr);
if (SymbolRefExpr && SymbolRefExpr->getKind() == MCSymbolRefExpr::VK_None)
return &SymbolRefExpr->getSymbol();

return nullptr;
Expand Down
12 changes: 2 additions & 10 deletions bolt/lib/Target/AArch64/AArch64MCPlusBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -862,20 +862,12 @@ class AArch64MCPlusBuilder : public MCPlusBuilder {
if (AArchExpr && AArchExpr->getSubExpr())
return getTargetSymbol(AArchExpr->getSubExpr());

auto *BinExpr = dyn_cast<MCBinaryExpr>(Expr);
if (BinExpr)
return getTargetSymbol(BinExpr->getLHS());

auto *SymExpr = dyn_cast<MCSymbolRefExpr>(Expr);
if (SymExpr && SymExpr->getKind() == MCSymbolRefExpr::VK_None)
return &SymExpr->getSymbol();

return nullptr;
return MCPlusBuilder::getTargetSymbol(Expr);
}

const MCSymbol *getTargetSymbol(const MCInst &Inst,
unsigned OpNum = 0) const override {
if (!getSymbolRefOperandNum(Inst, OpNum))
if (!OpNum && !getSymbolRefOperandNum(Inst, OpNum))
return nullptr;

const MCOperand &Op = Inst.getOperand(OpNum);
Expand Down
10 changes: 1 addition & 9 deletions bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -338,15 +338,7 @@ class RISCVMCPlusBuilder : public MCPlusBuilder {
if (RISCVExpr && RISCVExpr->getSubExpr())
return getTargetSymbol(RISCVExpr->getSubExpr());

auto *BinExpr = dyn_cast<MCBinaryExpr>(Expr);
if (BinExpr)
return getTargetSymbol(BinExpr->getLHS());

auto *SymExpr = dyn_cast<MCSymbolRefExpr>(Expr);
if (SymExpr && SymExpr->getKind() == MCSymbolRefExpr::VK_None)
return &SymExpr->getSymbol();

return nullptr;
return MCPlusBuilder::getTargetSymbol(Expr);
}

const MCSymbol *getTargetSymbol(const MCInst &Inst,
Expand Down
6 changes: 1 addition & 5 deletions bolt/lib/Target/X86/X86MCPlusBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1796,11 +1796,7 @@ class X86MCPlusBuilder : public MCPlusBuilder {
if (!Op.isExpr())
return nullptr;

auto *SymExpr = dyn_cast<MCSymbolRefExpr>(Op.getExpr());
if (!SymExpr || SymExpr->getKind() != MCSymbolRefExpr::VK_None)
return nullptr;

return &SymExpr->getSymbol();
return MCPlusBuilder::getTargetSymbol(Op.getExpr());
}

bool analyzeBranch(InstructionIterator Begin, InstructionIterator End,
Expand Down
33 changes: 33 additions & 0 deletions bolt/test/X86/lite-mode-target-expr.s
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
## Check that llvm-bolt properly updates references in unoptimized code when
## such references are non-trivial expressions.

# RUN: %clang %cflags %s -o %t.exe -Wl,-q -no-pie
# RUN: llvm-bolt %t.exe -o %t.bolt --funcs=_start
# RUN: llvm-objdump -d --disassemble-symbols=_start %t.bolt > %t.out
# RUN: llvm-objdump -d --disassemble-symbols=cold %t.bolt >> %t.out
# RUN: FileCheck %s < %t.out

## _start() will be optimized and assigned a new address.
# CHECK: [[#%x,ADDR:]] <_start>:

## cold() is not optimized, but references to _start are updated.
# CHECK-LABEL: <cold>:
# CHECK-NEXT: movl $0x[[#ADDR - 1]], %ecx
# CHECK-NEXT: movl $0x[[#ADDR]], %ecx
# CHECK-NEXT: movl $0x[[#ADDR + 1]], %ecx

.text
.globl cold
.type cold, %function
cold:
movl $_start-1, %ecx
movl $_start, %ecx
movl $_start+1, %ecx
ret
.size cold, .-cold

.globl _start
.type _start, %function
_start:
ret
.size _start, .-_start