[PATCH] D147150: [BOLT] Remove original relocs when moving JTs in relocation mode

[Amir Ayupov via Phabricator via llvm-commits]

Amir created this revision.
Amir added a reviewer: bolt.
Herald added a reviewer: rafauler.
Herald added subscribers: treapster, ayermolo.
Herald added a reviewer: maksfb.
Herald added a project: All.
Amir requested review of this revision.
Herald added subscribers: llvm-commits, yota9.
Herald added a project: LLVM.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147150

Files:
  bolt/include/bolt/Core/JumpTable.h
  bolt/lib/Core/BinaryEmitter.cpp
  bolt/lib/Core/JumpTable.cpp
  bolt/test/X86/normalize-jump-table-target.s


Index: bolt/test/X86/normalize-jump-table-target.s
===================================================================
--- /dev/null
+++ bolt/test/X86/normalize-jump-table-target.s
@@ -0,0 +1,32 @@
+# 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
+
+  .globl main
+main:
+  .cfi_startproc
+  jmp main
+  jmpq  *JT(,%rax,8)
+a:
+  shlq  %rax
+.Ltmp:
+  nop
+c:
+  ret
+  .cfi_endproc
+
+.rodata
+JT:
+  .quad .Ltmp
+  .quad a
Index: bolt/lib/Core/JumpTable.cpp
===================================================================
--- bolt/lib/Core/JumpTable.cpp
+++ bolt/lib/Core/JumpTable.cpp
@@ -79,7 +79,7 @@
   return Patched;
 }
 
-void bolt::JumpTable::updateOriginal() {
+void bolt::JumpTable::updateOriginal(bool Erase) {
   BinaryContext &BC = getSection().getBinaryContext();
   const uint64_t BaseOffset = getAddress() - getSection().getAddress();
   uint64_t EntryOffset = BaseOffset;
@@ -92,7 +92,8 @@
     // to the original jump table.
     if (BC.HasRelocations)
       getOutputSection().removeRelocationAt(EntryOffset);
-    getOutputSection().addRelocation(EntryOffset, Entry, RelType, RelAddend);
+    if (!Erase)
+      getOutputSection().addRelocation(EntryOffset, Entry, RelType, RelAddend);
     EntryOffset += EntrySize;
   }
 }
Index: bolt/lib/Core/BinaryEmitter.cpp
===================================================================
--- bolt/lib/Core/BinaryEmitter.cpp
+++ bolt/lib/Core/BinaryEmitter.cpp
@@ -745,7 +745,7 @@
     if (opts::PrintJumpTables)
       JT.print(outs());
     if (opts::JumpTables == JTS_BASIC && BC.HasRelocations) {
-      JT.updateOriginal();
+      JT.updateOriginal(/*Erase=*/false);
     } else {
       MCSection *HotSection, *ColdSection;
       if (opts::JumpTables == JTS_BASIC) {
@@ -768,6 +768,7 @@
           HotSection = BF.hasProfile() ? ReadOnlySection : ReadOnlyColdSection;
           ColdSection = HotSection;
         }
+        JT.updateOriginal(/*Erase=*/true);
       }
       emitJumpTable(JT, HotSection, ColdSection);
     }
Index: bolt/include/bolt/Core/JumpTable.h
===================================================================
--- bolt/include/bolt/Core/JumpTable.h
+++ bolt/include/bolt/Core/JumpTable.h
@@ -120,7 +120,7 @@
                           MCSymbol *NewDest);
 
   /// Update jump table at its original location.
-  void updateOriginal();
+  void updateOriginal(bool Erase);
 
   /// Print for debugging purposes.
   void print(raw_ostream &OS) const override;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D147150.509340.patch
Type: text/x-patch
Size: 3104 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230329/f39fcbbc/attachment.bin>