[BOLT] AArch64: Read all static relocations

[yota9]

Summary

Read static relocs on the same address, as dynamic in order to update
constant island data address properly.

[rafauler]

Can you give more details on what is broken when only the dynamic relocation is consumed? I'm also eager for the aarch64 tests to be enabled on x86 as well, so I can build these on my machine and inspect to understand what's being fixed. Let me know if this test is going to be enabled for x86 as well.

[yota9]

Can you give more details on what is broken when only the dynamic relocation is consumed?

From the runtime standpoint nothing is broken, since we update dynamic relocations and at runtime the memory value would be right. Without static relocation the value won't be updated in place in the binary. I just don't see any reason for aarch64 to skip the static relocations currently. Also the D122106 needs this patch so both static and dynamic relocs would be available.

I'm also eager for the aarch64 tests to be enabled on x86 as well,

I'm trying to write cross-platform tests when possible. As for X86 - the condition to skip static relocs was added on purpose, according to @maksfb commit message it was done because "BOLT fails
to validate the extracted value at the address.". We validate values only for x86 currently, so this problem does not affect aarch64 platform. I think it is out of scope of this review to fix x86 platform too :)

[rafauler]

LGTM

bolt/lib/Rewrite/RewriteInstance.cpp

if (Relocation::isTLS(RType)) {
       if (BC->isX86())
         continue;
 
       // The non-got related TLS relocations on AArch64 also could be skipped.
       if (!Relocation::isGOT(RType))
         continue;
     }
 
-    if (BC->getDynamicRelocationAt(Rel.getOffset())) {
+    if (!IsAArch64 && BC->getDynamicRelocationAt(Rel.getOffset())) {
       LLVM_DEBUG(
           dbgs() << "BOLT-DEBUG: address 0x"
                  << Twine::utohexstr(Rel.getOffset())
                  << " has a dynamic relocation against it. Ignoring static "
                     "relocation.\n");
       continue;
     }
 

bolt/test/AArch64/constant-island-alignment.s

 // This test checks that the constant island is aligned after BOLT tool.
 // In case the nop before .Lci will be removed the pointer to exit function
 // won't be alinged and the test will fail.
 
 # RUN: llvm-mc -filetype=obj -triple aarch64-unknown-unknown \
 # RUN:   %s -o %t.o
 # RUN: %clang %cflags -fPIC -pie %t.o -o %t.exe -Wl,-q \
-# RUN:    -nostartfiles -nodefaultlibs -lc
+# RUN:    -nostartfiles -nodefaultlibs -Wl,-z,notext
 # RUN: llvm-bolt %t.exe -o %t.bolt -use-old-text=0 -lite=0 -trap-old-code
 # RUN: llvm-objdump -d --disassemble-symbols='$d' %t.bolt | FileCheck %s
 
 .text
 .align 4
 .global
 .type dummy, %function
 dummy:
   add x0, x0, #1
   ret
 
 .global
 .type exitOk, %function
 exitOk:
   mov x0, #0
-  bl exit
+  ret
 
 .global _start
 .type _start, %function
 _start:
   adrp x0, .Lci
   ldr x0, [x0, #:lo12:.Lci]
   blr x0
-  mov x1, #1
-  bl exit
+  mov x0, #1
+  ret
   nop
 # CHECK: {{0|8}} <$d>:
 .Lci:
   .xword exitOk
   .xword 0

bolt/test/AArch64/constant_island_pie_update.s

+// This test checks that the constant island value is updated if it
+// has dynamic relocation.
+
+# RUN: llvm-mc -filetype=obj -triple aarch64-unknown-unknown \
+# RUN:   %s -o %t.o
+# RUN: %clang %cflags -fPIC -pie %t.o -o %t.exe -Wl,-q -nostdlib -Wl,-z,notext
+# RUN: llvm-bolt %t.exe -o %t.bolt -use-old-text=0 -lite=0
+# RUN: llvm-objdump -j .text -dR %t.bolt | FileCheck %s
+
+# CHECK: R_AARCH64_RELATIVE *ABS*+0x[[#%x,ADDR:]]
+# CHECK: [[#ADDR]] <exitLocal>:
+# CHECK: {{.*}} <$d>:
+# CHECK-NEXT: {{.*}} .word 0x{{[0]+}}[[#ADDR]]
+# CHECK-NEXT: {{.*}} .word 0x00000000
+
+  .text
+  .align 4
+  .local exitLocal
+  .type exitLocal, %function
+exitLocal:
+  add x1, x1, #1
+  add x1, x1, #1
+  ret
+  .size exitLocal, .-exitLocal
+
+  .global _start
+  .type _start, %function
+_start:
+  mov x0, #0
+  adr x1, .Lci
+  ldr x1, [x1]
+  blr x1
+  mov x0, #1
+  bl exitLocal
+  nop
+.Lci:
+  .xword exitLocal
+  .size _start, .-_start