[llvm] [BOLT] Support computed goto and allow map addrs inside functions. (PR #120267)

Rin Dobrescu via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 2 05:43:54 PST 2025


https://github.com/Rin18 updated https://github.com/llvm/llvm-project/pull/120267

>From f6435e5499dd0384820381bf85995b823789c279 Mon Sep 17 00:00:00 2001
From: Rin Dobrescu <rin.dobrescu at arm.com>
Date: Tue, 17 Dec 2024 16:43:33 +0000
Subject: [PATCH 1/3] [BOLT][AArch64] Create entry points for addresses
 referenced by dynamic relocations and allow getNewFunctionOrDataAddress to
 map addrs inside functions.

By adding addresses referenced by dynamic relocations as entry points,
this patch fixes an issue where bolt fails on code using computing
goto's. This also fixes a mapping issue with the bugfix from this
PR: https://github.com/llvm/llvm-project/pull/117766.
---
 bolt/lib/Rewrite/RewriteInstance.cpp | 10 ++++++-
 bolt/test/AArch64/computed-goto.s    | 39 ++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+), 1 deletion(-)
 create mode 100644 bolt/test/AArch64/computed-goto.s

diff --git a/bolt/lib/Rewrite/RewriteInstance.cpp b/bolt/lib/Rewrite/RewriteInstance.cpp
index 4329235d470497..55fcd6b6e782c4 100644
--- a/bolt/lib/Rewrite/RewriteInstance.cpp
+++ b/bolt/lib/Rewrite/RewriteInstance.cpp
@@ -2439,6 +2439,14 @@ void RewriteInstance::readDynamicRelocations(const SectionRef &Section,
     if (Symbol)
       SymbolIndex[Symbol] = getRelocationSymbol(InputFile, Rel);
 
+    const uint64_t SymAddress = SymbolAddress + Addend;
+    BinaryFunction *Func = BC->getBinaryFunctionContainingAddress(SymAddress);
+    if(Func){
+      const uint64_t FunctionOffset = SymAddress - Func->getAddress();
+      if(FunctionOffset)
+        Func->addEntryPointAtOffset(FunctionOffset);
+    }
+
     BC->addDynamicRelocation(Rel.getOffset(), Symbol, RType, Addend);
   }
 }
@@ -5599,7 +5607,7 @@ uint64_t RewriteInstance::getNewFunctionOrDataAddress(uint64_t OldAddress) {
         for (const BinaryBasicBlock &BB : *BF)
           if (BB.isEntryPoint() &&
               (BF->getAddress() + BB.getOffset()) == OldAddress)
-            return BF->getOutputAddress() + BB.getOffset();
+            return BB.getOutputStartAddress();
       }
       BC->errs() << "BOLT-ERROR: unable to get new address corresponding to "
                     "input address 0x"
diff --git a/bolt/test/AArch64/computed-goto.s b/bolt/test/AArch64/computed-goto.s
new file mode 100644
index 00000000000000..043f9a8e37e6b0
--- /dev/null
+++ b/bolt/test/AArch64/computed-goto.s
@@ -0,0 +1,39 @@
+# RUN: llvm-mc -filetype=obj -triple aarch64-unknown-unknown %s -o %t.o
+# RUN: %clang %cflags %t.o -o %t.exe -Wl,-q
+# RUN: llvm-bolt %t.exe -o %t.bolt 2>&1 | FileCheck %s
+
+## Before bolt could handle mapping addresses within moved functions, it
+## would bail out with an error of the form:
+## BOLT-ERROR: unable to get new address corresponding to input address 0x10390 in function main. Consider adding this function to --skip-funcs=...
+## These addresses arise if computed GOTO is in use.
+## Check that bolt does not emit any error.
+
+# CHECK-NOT: BOLT-ERROR
+
+.globl  main
+.p2align        2
+.type   main, at function
+main:
+.cfi_startproc
+        adrp    x8, .L__const.main.ptrs+8
+        add     x8, x8, :lo12:.L__const.main.ptrs+8
+        ldr     x9, [x8], #8
+        br      x9
+
+.Label0: // Block address taken
+        ldr     x9, [x8], #8
+        br      x9
+
+.Label1: // Block address taken
+        mov     w0, #42
+        ret
+
+.Lfunc_end0:
+.size   main, .Lfunc_end0-main
+.cfi_endproc
+        .type   .L__const.main.ptrs, at object
+        .section        .data.rel.ro,"aw", at progbits
+        .p2align        3, 0x0
+.L__const.main.ptrs:
+        .xword  .Label0
+        .xword  .Label1

>From dd67da54eba30b68350cc95f6e3eeb31d9dbb9ce Mon Sep 17 00:00:00 2001
From: Rin Dobrescu <rin.dobrescu at arm.com>
Date: Wed, 18 Dec 2024 12:11:10 +0000
Subject: [PATCH 2/3] Fix clang-format and address PR comments.

---
 bolt/lib/Rewrite/RewriteInstance.cpp | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/bolt/lib/Rewrite/RewriteInstance.cpp b/bolt/lib/Rewrite/RewriteInstance.cpp
index 55fcd6b6e782c4..1d8bce5c8a8881 100644
--- a/bolt/lib/Rewrite/RewriteInstance.cpp
+++ b/bolt/lib/Rewrite/RewriteInstance.cpp
@@ -2441,10 +2441,9 @@ void RewriteInstance::readDynamicRelocations(const SectionRef &Section,
 
     const uint64_t SymAddress = SymbolAddress + Addend;
     BinaryFunction *Func = BC->getBinaryFunctionContainingAddress(SymAddress);
-    if(Func){
-      const uint64_t FunctionOffset = SymAddress - Func->getAddress();
-      if(FunctionOffset)
-        Func->addEntryPointAtOffset(FunctionOffset);
+    if (Func && !Func->isInConstantIsland(SymAddress)) {
+      if (const uint64_t SymOffset = SymAddress - Func->getAddress())
+        Func->addEntryPointAtOffset(SymOffset);
     }
 
     BC->addDynamicRelocation(Rel.getOffset(), Symbol, RType, Addend);

>From 1dd6d5a0d6b572036a004bb87a1a204fa0c5dd84 Mon Sep 17 00:00:00 2001
From: Rin Dobrescu <rin.dobrescu at arm.com>
Date: Tue, 31 Dec 2024 12:45:05 +0000
Subject: [PATCH 3/3] Remove redundant test and expand different test.

---
 bolt/test/AArch64/computed-goto.s    | 38 ++++++++++++++++++++++++----
 bolt/test/X86/indirect-goto-pie.test | 16 ------------
 2 files changed, 33 insertions(+), 21 deletions(-)
 delete mode 100644 bolt/test/X86/indirect-goto-pie.test

diff --git a/bolt/test/AArch64/computed-goto.s b/bolt/test/AArch64/computed-goto.s
index 043f9a8e37e6b0..654bdfb7a5bf80 100644
--- a/bolt/test/AArch64/computed-goto.s
+++ b/bolt/test/AArch64/computed-goto.s
@@ -1,15 +1,43 @@
+// This test checks that BOLT creates entry points for addresses
+// referenced by dynamic relocations.
+// The test also checks that BOLT can map addresses inside functions.
+
+// Checks for error and entry points.
 # RUN: llvm-mc -filetype=obj -triple aarch64-unknown-unknown %s -o %t.o
 # RUN: %clang %cflags %t.o -o %t.exe -Wl,-q
 # RUN: llvm-bolt %t.exe -o %t.bolt 2>&1 | FileCheck %s
+# RUN: llvm-bolt %t.exe -o %t.bolt --print-cfg | FileCheck --check-prefix=CHECK-ENTRYS %s
 
-## Before bolt could handle mapping addresses within moved functions, it
-## would bail out with an error of the form:
-## BOLT-ERROR: unable to get new address corresponding to input address 0x10390 in function main. Consider adding this function to --skip-funcs=...
-## These addresses arise if computed GOTO is in use.
-## Check that bolt does not emit any error.
+// Checks for dynamic relocations.
+# RUN: llvm-readelf -dr %t.bolt > %t.out.txt
+# RUN: llvm-objdump -j .rela.dyn -d %t.bolt >> %t.out.txt
+# RUN: FileCheck --check-prefix=CHECK-RELOCS %s --input-file=%t.out.txt
 
+// Before bolt could handle mapping addresses within moved functions, it
+// would bail out with an error of the form:
+// BOLT-ERROR: unable to get new address corresponding to input address 0x10390 in function main. Consider adding this function to --skip-funcs=...
+// These addresses arise if computed GOTO is in use.
+// Check that bolt does not emit any error.
 # CHECK-NOT: BOLT-ERROR
 
+// Check that there are dynamic relocations.
+# CHECK-RELOCS:     Dynamic section at offset {{.*}} contains {{.*}} entries:
+# CHECK-RELOCS:     Relocation section '.rela.dyn' at offset {{.*}} contains {{.*}} entries
+
+// Check that dynamic relocations were updated
+# CHECK-RELOCS: [[#%x,OFF:]] [[#%x,INFO_DYN:]] R_AARCH64_RELATIVE [[#%x,ADDR:]]
+# CHECK-RELOCS-NEXT: [[#OFF + 8]] {{0*}}[[#INFO_DYN]] R_AARCH64_RELATIVE [[#ADDR + 8]]
+# CHECK-RELOCS: [[#ADDR]] <unknown>
+# CHECK-RELOCS: [[#ADDR + 8]] <unknown>
+
+// Check that BOLT registers extra entry points for dynamic relocations.
+# CHECK-ENTRYS: Binary Function "main" after building cfg {
+# CHECK-ENTRYS:  IsMultiEntry: 1
+# CHECK-ENTRYS: .Ltmp0 {{.*}}
+# CHECK-ENTRYS-NEXT: Secondary Entry Point: {{.*}}
+# CHECK-ENTRYS: .Ltmp1 {{.*}}
+# CHECK-ENTRYS-NEXT: Secondary Entry Point: {{.*}}
+
 .globl  main
 .p2align        2
 .type   main, at function
diff --git a/bolt/test/X86/indirect-goto-pie.test b/bolt/test/X86/indirect-goto-pie.test
deleted file mode 100644
index 3311c1aec061c5..00000000000000
--- a/bolt/test/X86/indirect-goto-pie.test
+++ /dev/null
@@ -1,16 +0,0 @@
-## Check that llvm-bolt fails to process PIC binaries with computed goto, as the
-## support is not there yet for correctly updating dynamic relocations
-## referencing code inside functions.
-
-REQUIRES: x86_64-linux
-
-RUN: %clang %S/Inputs/indirect_goto.c -o %t -fpic -pie -Wl,-q
-RUN: not llvm-bolt %t -o %t.bolt --relocs=1 --print-cfg --print-only=main \
-RUN:   2>&1 | FileCheck %s
-
-## Check that processing works if main() is skipped.
-RUN: llvm-bolt %t -o %t.bolt --relocs=1 --skip-funcs=main
-
-CHECK:  jmpq    *%rax # UNKNOWN CONTROL FLOW
-
-CHECK: BOLT-ERROR: unable to get new address



More information about the llvm-commits mailing list