[llvm] [BOLT] Remove original relocs when moving JTs in relocation mode (PR #91656)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-bolt

Author: Amir Ayupov (aaupov)

<details>
<summary>Changes</summary>

Erase original jump table relocations in relocation mode with -jump-tables=move
(or higher). Otherwise we end up updating the original data relocations which
may no longer reference existing labels, as demonstrated by the test case.

Accepted in https://reviews.llvm.org/D147150

Test Plan: added normalize-jump-table-target.s


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


4 Files Affected:

- (modified) bolt/include/bolt/Core/JumpTable.h (+1-1) 
- (modified) bolt/lib/Core/BinaryEmitter.cpp (+3-1) 
- (modified) bolt/lib/Core/JumpTable.cpp (+3-1) 
- (added) bolt/test/X86/normalize-jump-table-target.s (+44) 


``````````diff
diff --git a/bolt/include/bolt/Core/JumpTable.h b/bolt/include/bolt/Core/JumpTable.h
index 52b9ccee1f7e1..dd55a93784e64 100644
--- a/bolt/include/bolt/Core/JumpTable.h
+++ b/bolt/include/bolt/Core/JumpTable.h
@@ -120,7 +120,7 @@ class JumpTable : public BinaryData {
                           MCSymbol *NewDest);
 
   /// Update jump table at its original location.
-  void updateOriginal();
+  void updateOriginal(bool Zero);
 
   /// Print for debugging purposes.
   void print(raw_ostream &OS) const override;
diff --git a/bolt/lib/Core/BinaryEmitter.cpp b/bolt/lib/Core/BinaryEmitter.cpp
index 6f86ddc774544..3fb56bf15aaa3 100644
--- a/bolt/lib/Core/BinaryEmitter.cpp
+++ b/bolt/lib/Core/BinaryEmitter.cpp
@@ -777,7 +777,7 @@ void BinaryEmitter::emitJumpTables(const BinaryFunction &BF) {
     if (opts::PrintJumpTables)
       JT.print(BC.outs());
     if (opts::JumpTables == JTS_BASIC && BC.HasRelocations) {
-      JT.updateOriginal();
+      JT.updateOriginal(/*Zero=*/false);
     } else {
       MCSection *HotSection, *ColdSection;
       if (opts::JumpTables == JTS_BASIC) {
@@ -802,6 +802,8 @@ void BinaryEmitter::emitJumpTables(const BinaryFunction &BF) {
         }
       }
       emitJumpTable(JT, HotSection, ColdSection);
+      if (opts::JumpTables > JTS_BASIC && BC.HasRelocations)
+        JT.updateOriginal(/*Zero=*/true);
     }
   }
 }
diff --git a/bolt/lib/Core/JumpTable.cpp b/bolt/lib/Core/JumpTable.cpp
index 65e1032c579b5..86a6d3d11f2ea 100644
--- a/bolt/lib/Core/JumpTable.cpp
+++ b/bolt/lib/Core/JumpTable.cpp
@@ -79,7 +79,7 @@ bool bolt::JumpTable::replaceDestination(uint64_t JTAddress,
   return Patched;
 }
 
-void bolt::JumpTable::updateOriginal() {
+void bolt::JumpTable::updateOriginal(bool Zero) {
   BinaryContext &BC = getSection().getBinaryContext();
   const uint64_t BaseOffset = getAddress() - getSection().getAddress();
   uint64_t EntryOffset = BaseOffset;
@@ -92,6 +92,8 @@ void bolt::JumpTable::updateOriginal() {
     // to the original jump table.
     if (BC.HasRelocations)
       getOutputSection().removeRelocationAt(EntryOffset);
+    if (Zero)
+      Entry = BC.registerNameAtAddress("Zero", 0, 0, 0);
     getOutputSection().addRelocation(EntryOffset, Entry, RelType, RelAddend);
     EntryOffset += EntrySize;
   }
diff --git a/bolt/test/X86/normalize-jump-table-target.s b/bolt/test/X86/normalize-jump-table-target.s
new file mode 100644
index 0000000000000..a22d3df0ec1a2
--- /dev/null
+++ b/bolt/test/X86/normalize-jump-table-target.s
@@ -0,0 +1,44 @@
+# This test reproduces an issue with removing a basic block which is referenced
+# from rodata section ("externally referenced offset"/jump table target).
+# If the block is removed (by remove-nops + NormalizeCFG), and BOLT updates the
+# original jump table (with jump-tables=move), this causes an emission error
+# (undefined temporary symbol).
+
+# REQUIRES: system-linux,bolt-runtime
+
+# 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 %t.exe -o %t.out --instrument --instrumentation-file=%t.tmp \
+# RUN:   2>&1 | FileCheck %s --check-prefix=CHECK-BOLT
+
+# CHECK-BOLT-NOT:  undefined temporary symbol
+
+# Grab the jump table address in original text section (.bolt.org.text)
+# RUN: llvm-objdump -dj.bolt.org.text %t.out > %t.log
+# Inspect jump table entries, should be two zero values
+# RUN: llvm-objdump -sj.rodata %t.out >> %t.log
+# RUN: FileCheck %s --input-file %t.log --check-prefix=CHECK-OBJDUMP
+
+# CHECK-OBJDUMP: jmpq *0x[[#%x,JTADDR:]](,%rax,8)
+# CHECK-OBJDUMP: [[#JTADDR]] 00000000 00000000 00000000 00000000
+
+  .globl main
+main:
+  .cfi_startproc
+  jmp main
+  jmpq  *JT(,%rax,8)
+a:
+  shlq  %rax
+.Ltmp:
+  nop
+c:
+  ret
+  .cfi_endproc
+
+.rodata
+# pad to ensure the jump table starts at 16-byte aligned address so it can be
+# matched in objdump output
+.p2align 4
+JT:
+  .quad .Ltmp
+  .quad a

``````````

</details>


https://github.com/llvm/llvm-project/pull/91656