Skip to content

Conversation

@maksfb
Copy link
Contributor

@maksfb maksfb commented Feb 27, 2025

In analyzeInstructionForFuncReference(), use MCPlusBuilder interface while scanning symbolic operands of MCInst. Should be NFC on x86, but will make the function work on other architectures. Note that it's currently unused on non-x86 as its functionality is exclusive to safe ICF that runs on x86 only.

In analyzeInstructionForFuncReference(), use MCPlusBuilder interface
while scanning symbolic operands of MCInst. Should be NFC on x86, but
will make the function work on other architectures. Note that it's
currently unused on non-x86 as its functionality is exclusive to safe
ICF that runs on x86 only.
@llvmbot
Copy link
Member

llvmbot commented Feb 27, 2025

@llvm/pr-subscribers-bolt

Author: Maksim Panchenko (maksfb)

Changes

In analyzeInstructionForFuncReference(), use MCPlusBuilder interface while scanning symbolic operands of MCInst. Should be NFC on x86, but will make the function work on other architectures. Note that it's currently unused on non-x86 as its functionality is exclusive to safe ICF that runs on x86 only.


Full diff: https://github.com/llvm/llvm-project/pull/129131.diff

1 Files Affected:

  • (modified) bolt/lib/Core/BinaryFunction.cpp (+4-8)
diff --git a/bolt/lib/Core/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp
index ff5eb5cf6e1eb..5a1ff58b72f75 100644
--- a/bolt/lib/Core/BinaryFunction.cpp
+++ b/bolt/lib/Core/BinaryFunction.cpp
@@ -1544,15 +1544,11 @@ MCSymbol *BinaryFunction::registerBranch(uint64_t Src, uint64_t Dst) {
 }
 
 void BinaryFunction::analyzeInstructionForFuncReference(const MCInst &Inst) {
-  for (const MCOperand &Op : MCPlus::primeOperands(Inst)) {
-    if (!Op.isExpr())
+  for (unsigned OpNum = 0; OpNum < MCPlus::getNumPrimeOperands(Inst); ++OpNum) {
+    const MCSymbol *Symbol = BC.MIB->getTargetSymbol(Inst, OpNum);
+    if (!Symbol)
       continue;
-    const MCExpr &Expr = *Op.getExpr();
-    if (Expr.getKind() != MCExpr::SymbolRef)
-      continue;
-    const MCSymbol &Symbol = cast<MCSymbolRefExpr>(Expr).getSymbol();
-    // Set HasAddressTaken for a function regardless of the ICF level.
-    if (BinaryFunction *BF = BC.getFunctionForSymbol(&Symbol))
+    if (BinaryFunction *BF = BC.getFunctionForSymbol(Symbol))
       BF->setHasAddressTaken(true);
   }
 }

@maksfb maksfb merged commit 074c2c6 into llvm:main Mar 1, 2025
12 checks passed
@maksfb maksfb deleted the gh-fix-instr-scan branch March 6, 2025 02:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants