[LLD][ARM][AArch64] ARM and AArch64 undefined weak reference values for relative relocations

[peter.smith]

Summary

The ARM 32 and 64-bit ABI does not use 0 for undefined weak references that are used in PC relative relocations. In particular:

• A branch relocation to an undefined weak resolves to the next instruction.
• In all other cases the symbol resolves to the place so that S + A - P resolves to A.

The branch relocation resolving to the next instruction is the most important as it is a common idiom in statically linked ARM and AArch64 code to use branches to weak references to initialization functions of optional parts of the program. More importantly it can also cause spurious relocation out of range errors as the (S + A) - P of the branch relocation evaluates to (0 + A) - P which is often a large negative number.

References:

• ARM: http://infocenter.arm.com/help/topic/com.arm.doc.ihi0044f/IHI0044F_aaelf.pdf
  • Section 4.5.1.1 Weak references and 4.6.1.4 Static ARM Relocations, search for Call and Jump Relocations
• AArch64: http://infocenter.arm.com/help/topic/com.arm.doc.ihi0056c/IHI0056C_beta_aaelf64.pdf
  • Section 4.5.1.1 Weak references and Section 4.6.7 Call and Jump Relocations

[ruiu]

ELF/InputSection.cpp

297–298

You can remove template <class ELFT> and use uint32_t instad of ELFT::uint or uint64_t, because we know it is ARM32.

320–323

Ditto. ELF::uint -> uint64_t.

414

Doesn't isWeak() imply !isLocal()? I assume that all weak symbols are not weak.

419

nit: Since the last one ends with return, remove else.

426

Ditto.

[peter.smith]

Thank you for the comments. I've update the diff with all but the comment about Weak and Local. I've put an inline comment about the assertion in Symbols.h SymbolBody::symbol() that prevents !IsLocal() from being removed.

[ruiu]

I think you forgot to include Symbols.h into this patch.

ELF/InputSection.cpp

261

uint64_t -> uint32_t?

[peter.smith]

I've made the suggested change to elf32_t.

Apologies I'm a bit confused about Symbols.h as I haven't made any changes to it, I was just pointing out that the existing implementation of symbol has an assert for !isLocal(). It is possible to put a test for isWeak() into SymbolBody, that returns false if the SymbolBody is a local and only forwards on to Symbol if it isn't. Would you like me to add that?

Existing implementation:
inline Symbol *SymbolBody::symbol() {

assert(!isLocal());
return reinterpret_cast<Symbol *>(reinterpret_cast<char *>(this) -
                                  offsetof(Symbol, Body));

}

[ruiu]

LGTM

[grimar]

lld/trunk/ELF/InputSection.cpp

266 ↗

(On Diff #77327)

I just noticed that we have now the next code here:

case R_ARM_THM_JUMP11:
  return P + 2 + A;

And it seems it is uncovered by any test case we have now, in 2018.
Reporting just in case. (I do not know ARM enough to add a test, unfortunately)

[peter.smith]

lld/trunk/ELF/InputSection.cpp

266 ↗

(On Diff #77327)

I should be able to come up with something. In theory it would need something like:

.thumb
b.n undefined_weak_symbol

There is the possibility that llvm-mc relaxation may decide to intervene and widen the branch though. I'll see what I can come up with.

[peter.smith]

I've proposed a test case in https://reviews.llvm.org/D50234 it does need a binary file as llvm-mc does indeed relax the b.n into a b.w.

ELF/InputSection.cpp

 
 // Page(Expr) is the page address of the expression Expr, defined
 // as (Expr & ~0xFFF). (This applies even if the machine page size
 // supported by the platform has a different value.)
 static uint64_t getAArch64Page(uint64_t Expr) {
   return Expr & (~static_cast<uint64_t>(0xFFF));
 }
 
+static uint32_t getARMUndefinedRelativeWeakVA(uint32_t Type,
+                                              uint32_t A,
+                                              uint32_t P) {
+  switch (Type) {
+  case R_ARM_THM_JUMP11:
+    return P + 2;
+  case R_ARM_CALL:
+  case R_ARM_JUMP24:
+  case R_ARM_PC24:
+  case R_ARM_PLT32:
+  case R_ARM_PREL31:
+  case R_ARM_THM_JUMP19:
+  case R_ARM_THM_JUMP24:
+    return P + 4;
+  case R_ARM_THM_CALL:
+    // We don't want an interworking BLX to ARM
+    return P + 5;
+  default:
+    return A;
+  }
+}
+
+static uint64_t getAArch64UndefinedRelativeWeakVA(uint64_t Type,
+                                                  uint64_t A,
+                                                  uint64_t P) {
+  switch (Type) {
+  case R_AARCH64_CALL26:
+  case R_AARCH64_CONDBR19:
+  case R_AARCH64_JUMP26:
+  case R_AARCH64_TSTBR14:
+    return P + 4;
+  default:
+    return A;
+  }
+}
+
 template <class ELFT>
 static typename ELFT::uint getSymVA(uint32_t Type, typename ELFT::uint A,
                                     typename ELFT::uint P,
                                     const SymbolBody &Body, RelExpr Expr) {
   switch (Expr) {
   case R_HINT:
   case R_TLSDESC_CALL:
     llvm_unreachable("cannot relocate hint relocs");
   case R_TLSLD:
     return Out<ELFT>::Got->getTlsIndexOff() + A - Out<ELFT>::Got->getSize();
   case R_TLSLD_PC:
     return Out<ELFT>::Got->getTlsIndexVA() + A - P;
   case R_THUNK_ABS:
     return Body.getThunkVA<ELFT>() + A;
   case R_THUNK_PC:
   case R_THUNK_PLT_PC:
     return Body.getThunkVA<ELFT>() + A - P;
   case R_PPC_TOC:
     return getPPC64TocBase() + A;
   case R_TLSGD:
     return Out<ELFT>::Got->getGlobalDynOffset(Body) + A -
            Out<ELFT>::Got->getSize();
   case R_TLSGD_PC:
     return Out<ELFT>::Got->getGlobalDynAddr(Body) + A - P;
   case R_TLSDESC:
     return Out<ELFT>::Got->getGlobalDynAddr(Body) + A;
   case R_TLSDESC_PAGE:
     return getAArch64Page(Out<ELFT>::Got->getGlobalDynAddr(Body) + A) -
            getAArch64Page(P);
   case R_PLT:
     return Body.getPltVA<ELFT>() + A;
   case R_PLT_PC:
   case R_PPC_PLT_OPD:
     return Body.getPltVA<ELFT>() + A - P;
   case R_SIZE:
if (Out<ELF64BE>::Opd) {
       uint64_t OpdEnd = OpdStart + Out<ELF64BE>::Opd->getSize();
       bool InOpd = OpdStart <= SymVA && SymVA < OpdEnd;
       if (InOpd)
         SymVA = read64be(&Out<ELF64BE>::OpdBuf[SymVA - OpdStart]);
     }
     return SymVA - P;
   }
   case R_PC:
+    if (Body.isUndefined() && !Body.isLocal() && Body.symbol()->isWeak()) {
+      // On ARM and AArch64 a branch to an undefined weak resolves to the
+      // next instruction, otherwise the place.
+      if (Config->EMachine == EM_ARM)
+        return getARMUndefinedRelativeWeakVA(Type, A, P);
+      if (Config->EMachine == EM_AARCH64)
+        return getAArch64UndefinedRelativeWeakVA(Type, A, P);
+    }
   case R_RELAX_GOT_PC:
     return Body.getVA<ELFT>(A) - P;
   case R_PLT_PAGE_PC:
   case R_PAGE_PC:
+    if (Body.isUndefined() && !Body.isLocal() && Body.symbol()->isWeak())
+      return getAArch64Page(A);
     return getAArch64Page(Body.getVA<ELFT>(A)) - getAArch64Page(P);
   }
   llvm_unreachable("Invalid expression");
 }
 
 // This function applies relocations to sections without SHF_ALLOC bit.
 // Such sections are never mapped to memory at runtime. Debug sections are
 // an example. Relocations in non-alloc sections are much easier to

test/ELF/aarch64-undefined-weak.s

+// RUN: llvm-mc -filetype=obj -triple=aarch64-none-linux %s -o %t
+// RUN: ld.lld %t -o %t2 2>&1
+// RUN: llvm-objdump -triple=aarch64-none-linux -d %t2 | FileCheck %s
+// REQUIRES: aarch64
+
+// Check that the ARM 64-bit ABI rules for undefined weak symbols are applied.
+// Branch instructions are resolved to the next instruction. Undefined
+// Symbols in relative are resolved to the place so S - P + A = A.
+
+ .weak target
+
+ .text
+ .global _start
+_start:
+// R_AARCH64_JUMP26
+ b target
+// R_AARCH64_CALL26
+ bl target
+// R_AARCH64_CONDBR19
+ b.eq target
+// R_AARCH64_TSTBR14
+ cbz x1, target
+// R_AARCH64_ADR_PREL_LO21
+ adr x0, target
+// R_AARCH64_ADR_PREL_PG_HI21
+ adrp x0, target
+// R_AARCH64_PREL32
+ .word target - .
+// R_AARCH64_PREL64
+ .xword target - .
+// R_AARCH64_PREL16
+ .hword target - .
+
+// CHECK: Disassembly of section .text:
+// 131076 = 0x20004
+// CHECK:         20000:       01 80 00 14     b       #131076
+// CHECK-NEXT:    20004:       02 80 00 94     bl      #131080
+// CHECK-NEXT:    20008:       60 00 10 54     b.eq    #131084
+// CHECK-NEXT:    2000c:       81 00 10 b4     cbz     x1, #131088
+// CHECK-NEXT:    20010:       00 00 00 10     adr     x0, #0
+// CHECK-NEXT:    20014:       00 00 00 90     adrp    x0, #0
+// CHECK:         20018:       00 00 00 00     .word   0x00000000
+// CHECK-NEXT:    2001c:       00 00 00 00     .word   0x00000000
+// CHECK-NEXT:    20020:       00 00 00 00     .word   0x00000000
+// CHECK-NEXT:    20024:       00 00           .short  0x0000

test/ELF/arm-thumb-no-undefined-thunk.s

  .global
 _start:
  bl target
  b target
  b.w target
 
 // CHECK: Disassembly of section .text:
 // CHECK-NEXT: _start:
-// CHECK-NEXT:    11000:        ee f7 fe ef     blx     #-69636
-// CHECK-NEXT:    11004:        ee f7 fc bf     b.w     #-69640
-// CHECK-NEXT:    11008:        ee f7 fa bf     b.w     #-69644
+// 69636 = 0x11004 = next instruction
+// CHECK:         11000:        11 f0 02 f8     bl      #69636
+// CHECK-NEXT:    11004:        11 f0 04 b8     b.w     #69640
+// CHECK-NEXT:    11008:        11 f0 06 b8     b.w     #69644

test/ELF/arm-thumb-undefined-weak.s

+// RUN: llvm-mc -filetype=obj -triple=thumbv7a-none-linux-gnueabi %s -o %t
+// RUN: ld.lld %t -o %t2 2>&1
+// RUN: llvm-objdump -triple=thumbv7a-none-linux-gnueabi -d %t2 | FileCheck %s
+// REQUIRES: arm
+
+// Check that the ARM ABI rules for undefined weak symbols are applied.
+// Branch instructions are resolved to the next instruction. Relative
+// relocations are resolved to the place.
+
+ .syntax unified
+
+ .weak target
+
+ .text
+ .global _start
+_start:
+// R_ARM_THM_JUMP19
+ beq.w target
+// R_ARM_THM_JUMP24
+ b.w target
+// R_ARM_THM_CALL
+ bl target
+// R_ARM_THM_CALL with exchange
+ blx target
+// R_ARM_THM_MOVT_PREL
+ movt r0, :upper16:target - .
+// R_ARM_THM_MOVW_PREL_NC
+ movw r0, :lower16:target - .
+
+// CHECK: Disassembly of section .text:
+// 69636 = 0x11004
+// CHECK:         11000:       11 f0 02 80     beq.w   #69636
+// CHECK-NEXT:    11004:       11 f0 04 b8     b.w     #69640
+// CHECK-NEXT:    11008:       11 f0 06 f8     bl      #69644
+// blx is transformed into bl so we don't change state
+// CHECK-NEXT:    1100c:       11 f0 08 f8     bl      #69648
+// CHECK-NEXT:    11010:       c0 f2 00 00     movt    r0, #0
+// CHECK-NEXT:    11014:       40 f2 00 00     movw    r0, #0

test/ELF/arm-undefined-weak.s

+// RUN: llvm-mc -filetype=obj -triple=armv7a-none-linux-gnueabi %s -o %t
+// RUN: ld.lld %t -o %t2 2>&1
+// RUN: llvm-objdump -triple=armv7a-none-linux-gnueabi -d %t2 | FileCheck %s
+// REQUIRES: arm
+
+// Check that the ARM ABI rules for undefined weak symbols are applied.
+// Branch instructions are resolved to the next instruction. Undefined
+// Symbols in relative are resolved to the place so S - P + A = A.
+
+ .syntax unified
+
+ .weak target
+
+ .text
+ .global _start
+_start:
+// R_ARM_JUMP24
+ b target
+// R_ARM_CALL
+ bl target
+// R_ARM_CALL with exchange
+ blx target
+// R_ARM_MOVT_PREL
+ movt r0, :upper16:target - .
+// R_ARM_MOVW_PREL_NC
+ movw r0, :lower16:target - .
+// R_ARM_REL32
+ .word target - .
+
+// CHECK: Disassembly of section .text:
+// 69636 = 0x11004
+// CHECK:         11000:       01 44 00 ea     b       #69636
+// CHECK-NEXT:    11004:       02 44 00 eb     bl      #69640
+// blx is transformed into bl so we don't change state
+// CHECK-NEXT:    11008:       03 44 00 eb     bl      #69644
+// CHECK-NEXT:    1100c:       00 00 40 e3     movt    r0, #0
+// CHECK-NEXT:    11010:       00 00 00 e3     movw    r0, #0
+// CHECK:         11014:       00 00 00 00     .word   0x00000000
+