[BOLT] Update skipRelocation for aarch64

[yota9]

Summary

The ld might relax ADRP+ADD or ADRP+LDR sequences to the ADR+NOP, add
the new case to the skipRelocation for aarch64.

Vladislav Khmelevsky,
Advanced Software Technology Lab, Huawei

[yota9]

@rafaelauler Gentle ping

[rafauler]

bolt/lib/Core/Relocation.cpp

227

By ld, do you mean GNU ld? How is the test testing for that? (since it uses lld)

[yota9]

bolt/lib/Core/Relocation.cpp

227

No, I meant generic linker, not bfd. The lld has this optimization too (https://reviews.llvm.org/D117614)

[rafauler]

bolt/test/runtime/plt-lld.test

1–2

Is it possible to add a check by dumping the binary symbol table to verify that these are properly updated? having the test as is (checking if a binary runs) is quite opaque for reviewers and makes it a bit hard for us to be helpful and understand what is going on and what your patch is fixing. If we treat these relocations that were relaxed to ADR as real relocs (meaning, we do not have your patch), what does it happen? Does BOLT crash? Do PLT pointers get messed up?

[rafauler]

bolt/lib/Core/Relocation.cpp

227

Can you replace ld by "linker", then?

[yota9]

bolt/test/runtime/plt-lld.test

1–2

I will add extra test now.

what does it happen?

The pointers in code are messed up, so we will have runtime problems. E.g.
Let's suppose the GOT relocations were relaxed to the local symbol definition

1036c:       d503201f        nop
                     1036c: R_AARCH64_ADR_GOT_PAGE   foo2
10370:       10000080        adr     x0, 10380 <foo2>
                     10370: R_AARCH64_LD64_GOT_LO12_NC       foo2

After bolt without this patch we will have:

400024:       90ffe000        adrp    x0, 0 <_DYNAMIC-0x20388>
400028:       91000000        add     x0, x0, #0x0

Because the extracted value was messed up (we were expecting other instruction)

[yota9]

Add test

[yota9]

Nit

[rafauler]

Looks good, thanks!

bolt/lib/Core/Relocation.cpp

 
 bool skipRelocationProcessX86(uint64_t Type, uint64_t Contents) {
   return false;
 }
 
 bool skipRelocationProcessAArch64(uint64_t Type, uint64_t Contents) {
   auto IsMov = [](uint64_t Contents) -> bool {
     // The bits 28-23 are 0b100101
-    if ((Contents & 0x1f800000) == 0x12800000)
-      return true;
-    return false;
+    return (Contents & 0x1f800000) == 0x12800000;
   };
 
   auto IsB = [](uint64_t Contents) -> bool {
     // The bits 31-26 are 0b000101
-    if ((Contents & 0xfc000000) == 0x14000000)
-      return true;
-    return false;
+    return (Contents & 0xfc000000) == 0x14000000;
+  };
+
+  auto IsAdr = [](uint64_t Contents) -> bool {
+    // The bits 31-24 are 0b0xx10000
+    return (Contents & 0x9f000000) == 0x10000000;
   };
 
   auto IsNop = [](uint64_t Contents) -> bool { return Contents == 0xd503201f; };
 
   // The linker might eliminate the instruction and replace it with NOP, ignore
   if (IsNop(Contents))
     return true;
 
bool skipRelocationProcessAArch64(uint64_t Type, uint64_t Contents) {
   case ELF::R_AARCH64_TLSDESC_ADR_PAGE21:
   case ELF::R_AARCH64_TLSIE_LD64_GOTTPREL_LO12_NC:
   case ELF::R_AARCH64_TLSIE_ADR_GOTTPREL_PAGE21: {
     if (IsMov(Contents))
       return true;
   }
   }
 
-  // The ld might replace load/store instruction with jump and
+  // The linker might replace load/store instruction with jump and
   // veneer due to errata 843419
   // https://documentation-service.arm.com/static/5fa29fddb209f547eebd361d
   // Thus load/store relocations for these instructions must be ignored
   // NOTE: We only process GOT and TLS relocations this way since the
   // addend used in load/store instructions won't change after bolt
   // (it is important since the instruction in veneer won't have relocation)
   switch (Type) {
   default:
     break;
   case ELF::R_AARCH64_LD64_GOT_LO12_NC:
   case ELF::R_AARCH64_TLSIE_LD64_GOTTPREL_LO12_NC:
   case ELF::R_AARCH64_TLSDESC_LD64_LO12: {
     if (IsB(Contents))
       return true;
   }
   }
 
+  // The linker might relax ADRP+ADD or ADRP+LDR sequences to the ADR+NOP
+  switch (Type) {
+  default:
+    break;
+  case ELF::R_AARCH64_ADR_PREL_PG_HI21:
+  case ELF::R_AARCH64_ADD_ABS_LO12_NC:
+  case ELF::R_AARCH64_ADR_GOT_PAGE:
+  case ELF::R_AARCH64_LD64_GOT_LO12_NC:
+    if (IsAdr(Contents))
+      return true;
+  }
+
   return false;
 }
 
 uint64_t adjustValueX86(uint64_t Type, uint64_t Value, uint64_t PC) {
   switch (Type) {
   default:
     llvm_unreachable("not supported relocation");
   case ELF::R_X86_64_32:

bolt/test/AArch64/Inputs/skip-got-rel.yaml

+--- !ELF
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_DYN
+  Machine:         EM_AARCH64
+  Entry:           0x10364
+ProgramHeaders:
+  - Type:            PT_PHDR
+    Flags:           [ PF_R ]
+    VAddr:           0x40
+    Align:           0x8
+  - Type:            PT_INTERP
+    Flags:           [ PF_R ]
+    FirstSec:        .interp
+    LastSec:         .interp
+    VAddr:           0x238
+  - Type:            PT_LOAD
+    Flags:           [ PF_R ]
+    FirstSec:        .interp
+    LastSec:         .dynamic
+    Align:           0x10000
+  - Type:            PT_LOAD
+    Flags:           [ PF_X, PF_R ]
+    FirstSec:        .text
+    LastSec:         .text
+    VAddr:           0x10348
+    Align:           0x10000
+  - Type:            PT_LOAD
+    Flags:           [ PF_W, PF_R ]
+    FirstSec:        .dynamic
+    LastSec:         .got
+    VAddr:           0x20388
+    Align:           0x10000
+  - Type:            PT_DYNAMIC
+    Flags:           [ PF_W, PF_R ]
+    FirstSec:        .dynamic
+    LastSec:         .dynamic
+    VAddr:           0x20388
+    Align:           0x8
+  - Type:            PT_GNU_RELRO
+    Flags:           [ PF_R ]
+    FirstSec:        .dynamic
+    LastSec:         .got
+    VAddr:           0x20388
+  - Type:            PT_GNU_STACK
+    Flags:           [ PF_W, PF_R ]
+    Align:           0x0
+Sections:
+  - Name:            .interp
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC ]
+    Address:         0x238
+    AddressAlign:    0x1
+    Content:         2F6C69622F6C642D6C696E75782D616172636836342E736F2E3100
+  - Name:            .dynsym
+    Type:            SHT_DYNSYM
+    Flags:           [ SHF_ALLOC ]
+    Address:         0x258
+    Link:            .dynstr
+    AddressAlign:    0x8
+  - Name:            .dynstr
+    Type:            SHT_STRTAB
+    Flags:           [ SHF_ALLOC ]
+    Address:         0x28C
+    AddressAlign:    0x1
+  - Name:            .rela.dyn
+    Type:            SHT_RELA
+    Flags:           [ SHF_ALLOC ]
+    Address:         0x290
+    Link:            .dynsym
+    AddressAlign:    0x8
+    Relocations:
+      - Offset:          0x20448
+        Type:            R_AARCH64_RELATIVE
+        Addend:          66432
+  - Name:            .text
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    Address:         0x10348
+    AddressAlign:    0x4
+    Content:         FF4300D1E00700F9E80740F908014092E003082AFF430091C0035FD6FD7BBFA9FD0300911F2003D580000010F5FFFF97FD7BC1A8C0035FD6C0035FD6
+  - Name:            .dynamic
+    Type:            SHT_DYNAMIC
+    Flags:           [ SHF_WRITE, SHF_ALLOC ]
+    Address:         0x20388
+    Link:            .dynstr
+    AddressAlign:    0x8
+    Entries:
+      - Tag:             DT_FLAGS_1
+        Value:           0x8000000
+      - Tag:             DT_RELA
+        Value:           0x290
+      - Tag:             DT_RELASZ
+        Value:           0x18
+      - Tag:             DT_RELAENT
+        Value:           0x18
+      - Tag:             DT_RELACOUNT
+        Value:           0x1
+      - Tag:             DT_SYMTAB
+        Value:           0x258
+      - Tag:             DT_SYMENT
+        Value:           0x18
+      - Tag:             DT_STRTAB
+        Value:           0x28C
+      - Tag:             DT_STRSZ
+        Value:           0x1
+      - Tag:             DT_GNU_HASH
+        Value:           0x270
+      - Tag:             DT_NULL
+        Value:           0x0
+  - Name:            .got
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_WRITE, SHF_ALLOC ]
+    Address:         0x20448
+    AddressAlign:    0x8
+    Content:         '0000000000000000'
+  - Name:            .rela.text
+    Type:            SHT_RELA
+    Flags:           [ SHF_INFO_LINK ]
+    Link:            .symtab
+    AddressAlign:    0x8
+    Info:            .text
+    Relocations:
+      - Offset:          0x1036C
+        Symbol:          foo2
+        Type:            R_AARCH64_ADR_GOT_PAGE
+      - Offset:          0x10370
+        Symbol:          foo2
+        Type:            R_AARCH64_LD64_GOT_LO12_NC
+      - Offset:          0x10374
+        Symbol:          foo
+        Type:            R_AARCH64_CALL26
+Symbols:
+  - Name:            .text
+    Type:            STT_SECTION
+    Section:         .text
+    Value:           0x10348
+  - Name:            ex2.c
+    Type:            STT_FILE
+    Index:           SHN_ABS
+  - Name:            '$x.0 (1)'
+    Section:         .text
+    Value:           0x10380
+  - Name:            .interp
+    Type:            STT_SECTION
+    Section:         .interp
+    Value:           0x238
+  - Name:            _DYNAMIC
+    Section:         .dynamic
+    Value:           0x20388
+    Other:           [ STV_HIDDEN ]
+  - Name:            foo
+    Type:            STT_FUNC
+    Section:         .text
+    Binding:         STB_GLOBAL
+    Value:           0x10348
+    Size:            0x1C
+  - Name:            _start
+    Type:            STT_FUNC
+    Section:         .text
+    Binding:         STB_GLOBAL
+    Value:           0x10364
+    Size:            0x1C
+  - Name:            foo2
+    Type:            STT_FUNC
+    Section:         .text
+    Binding:         STB_GLOBAL
+    Value:           0x10380
+    Size:            0x4
+DynamicSymbols:  []
+...

bolt/test/AArch64/skip-got-rel.test

+// This test checks that the binary with relaxed ADRP+LDR instructions is
+// processed normally with BOLT and the ADR instruction andress is recognized
+// normally.
+
+RUN: yaml2obj %p/Inputs/skip-got-rel.yaml &> %t.exe
+RUN: llvm-bolt %t.exe -o /dev/null -print-cfg -print-only=_start | FileCheck %s
+
+CHECK: adr  x0, foo2

bolt/test/runtime/plt-lld.test

 // This test checks that the pointers to PLT are properly updated.
 // The test is using lld linker.
 
-// RUN: %clang %cflags %p/../Inputs/plt.c -fuse-ld=lld \
-// RUN:    -o %t.lld.exe -Wl,-q
-// RUN: llvm-bolt %t.lld.exe -o %t.lld.bolt.exe -use-old-text=0 -lite=0
-// RUN: %t.lld.bolt.exe | FileCheck %s
+// Non-PIE:
+RUN: %clang %cflags -no-pie %p/../Inputs/plt.c -fuse-ld=lld \
+RUN:    -o %t.lld.exe -Wl,-q
+RUN: llvm-bolt %t.lld.exe -o %t.lld.bolt.exe -use-old-text=0 -lite=0
+RUN: %t.lld.bolt.exe | FileCheck %s
 
-// CHECK: Test completed
+// PIE:
+RUN: %clang %cflags -fPIC -pie %p/../Inputs/plt.c -fuse-ld=lld \
+RUN:    -o %t.lld.pie.exe -Wl,-q
+RUN: llvm-bolt %t.lld.pie.exe -o %t.lld.bolt.pie.exe -use-old-text=0 -lite=0
+RUN: %t.lld.bolt.pie.exe | FileCheck %s
+
+CHECK: Test completed