Fix weak symbols on arm and aarch64

[• rafael]

Summary

Given

.weak target
.global _start
_start:
b target

The intention is that the branch goes to the instruction after the branch, effectively turning it on a nop. The branch adds the runtime PC, but we were adding it statically too.

I noticed the oddity by inspection, but llvm-objdump seems to agree, since it now prints things like:

b #-4 <_start+0x4>

[peter.smith]

Looks good to me, thanks for spotting the mistake and apologies for making it in the first place. I've made a minor suggestion about making the default case unreachable but I don't think that this is that important.

ELF/InputSection.cpp

409

Might be worth making the default case unreachable.

The ABI (ARM ELF 4.5.1.1) says:
During linking, the value of an undefined weak reference is:

• Zero if the relocation type is absolute
• The address of the place if the relocation type is pc-relative
• The nominal base address if the relocation type is base-relative.

The intention for just returning A was that default: would cover the absolute case. However as long as getARMUndefinedRelativeWeakVA is only called from pc-relative locations this wouldn't happen so I don't expect the default case to ever get hit.

[• espindola]

305212

ELF/InputSection.cpp

void InputSection::copyRelocations(uint8_t *Buf, ArrayRef<RelTy> Rels) {
 
   }
 }
 
 static uint32_t getARMUndefinedRelativeWeakVA(uint32_t Type, uint32_t A,
                                               uint32_t P) {
   switch (Type) {
   case R_ARM_THM_JUMP11:
-    return P + 2;
+    return P + 2 + A;
   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;
+    return P + 4 + A;
   case R_ARM_THM_CALL:
     // We don't want an interworking BLX to ARM
-    return P + 5;
+    return P + 5 + A;
   default:
-    return A;
+    return P + 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;
+    return P + 4 + A;
   default:
-    return A;
+    return P + A;
   }
 }
 
 // ARM SBREL relocations are of the form S + A - B where B is the static base
 // The ARM ABI defines base to be "addressing origin of the output segment
 // defining the symbol S". We defined the "addressing origin"/static base to be
 // the base of the PT_LOAD segment containing the Body.
 // The procedure call standard only defines a Read Write Position Independent
return InX::MipsGot->getVA() + InX::MipsGot->getBodyEntryOffset(Body, A) -
            InX::MipsGot->getGp();
   case R_MIPS_TLSGD:
     return InX::MipsGot->getVA() + InX::MipsGot->getTlsOffset() +
            InX::MipsGot->getGlobalDynOffset(Body) - InX::MipsGot->getGp();
   case R_MIPS_TLSLD:
     return InX::MipsGot->getVA() + InX::MipsGot->getTlsOffset() +
            InX::MipsGot->getTlsIndexOff() - InX::MipsGot->getGp();
   case R_PAGE_PC:
-  case R_PLT_PAGE_PC:
+  case R_PLT_PAGE_PC: {
+    uint64_t Dest;
     if (Body.isUndefined() && !Body.isLocal() && Body.symbol()->isWeak())
-      return getAArch64Page(A);
-    return getAArch64Page(Body.getVA(A)) - getAArch64Page(P);
-  case R_PC:
+      Dest = getAArch64Page(A);
+    else
+      Dest = getAArch64Page(Body.getVA(A));
+    return Dest - getAArch64Page(P);
+  }
+  case R_PC: {
+    uint64_t Dest;
     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);
-    }
-    return Body.getVA(A) - P;
+        Dest = getARMUndefinedRelativeWeakVA(Type, A, P);
+      else if (Config->EMachine == EM_AARCH64)
+        Dest = getAArch64UndefinedRelativeWeakVA(Type, A, P);
+      else
+        Dest = Body.getVA(A);
+    } else {
+      Dest = Body.getVA(A);
+    }
+    return Dest - P;
   case R_PLT:
     return Body.getPltVA() + A;
   case R_PLT_PC:
   case R_PPC_PLT_OPD:
     return Body.getPltVA() + A - P;
   case R_PPC_OPD: {
     uint64_t SymVA = Body.getVA(A);
     // If we have an undefined weak symbol, we might get here with a symbol

test/ELF/aarch64-undefined-weak.s

  .word target - .
 // R_AARCH64_PREL64
  .xword target - .
 // R_AARCH64_PREL16
  .hword target - .
 
 // CHECK: Disassembly of section .text:
 // 131076 = 0x20004
-// CHECK:         20000: {{.*}} b       #131076
-// CHECK-NEXT:    20004: {{.*}} bl      #131080
-// CHECK-NEXT:    20008: {{.*}} b.eq    #131084
-// CHECK-NEXT:    2000c: {{.*}} cbz     x1, #131088
+// CHECK:         20000: {{.*}} b       #4
+// CHECK-NEXT:    20004: {{.*}} bl      #4
+// CHECK-NEXT:    20008: {{.*}} b.eq    #4
+// CHECK-NEXT:    2000c: {{.*}} cbz     x1, #4
 // CHECK-NEXT:    20010: {{.*}} adr     x0, #0
-// CHECK-NEXT:    20014: {{.*}} adrp    x0, #0
+// CHECK-NEXT:    20014: {{.*}} adrp    x0, #-131072
 // CHECK:         20018: {{.*}} .word   0x00000000
 // CHECK-NEXT:    2001c: {{.*}} .word   0x00000000
 // CHECK-NEXT:    20020: {{.*}} .word   0x00000000
 // CHECK-NEXT:    20024: {{.*}} .short  0x0000

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

 _start:
  bl target
  b target
  b.w target
 
 // CHECK: Disassembly of section .text:
 // CHECK-NEXT: _start:
 // 69636 = 0x11004 = next instruction
-// CHECK:         11000: {{.*}} bl      #69636
-// CHECK-NEXT:    11004: {{.*}} b.w     #69640
-// CHECK-NEXT:    11008: {{.*}} b.w     #69644
+// CHECK:         11000: {{.*}} bl      #0
+// CHECK-NEXT:    11004: {{.*}} b.w     #0 <_start+0x8>
+// CHECK-NEXT:    11008: {{.*}} b.w     #0 <_start+0xC>

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

  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: {{.*}} beq.w   #69636
-// CHECK-NEXT:    11004: {{.*}} b.w     #69640
-// CHECK-NEXT:    11008: {{.*}} bl      #69644
+// CHECK:         11000: {{.*}} beq.w   #0 <_start+0x4>
+// CHECK-NEXT:    11004: {{.*}} b.w     #0 <_start+0x8>
+// CHECK-NEXT:    11008: {{.*}} bl      #0
 // blx is transformed into bl so we don't change state
-// CHECK-NEXT:    1100c: {{.*}} bl      #69648
+// CHECK-NEXT:    1100c: {{.*}} bl      #0
 // CHECK-NEXT:    11010: {{.*}} movt    r0, #0
 // CHECK-NEXT:    11014: {{.*}} movw    r0, #0

test/ELF/arm-undefined-weak.s

  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: {{.*}} b       #69636
-// CHECK-NEXT:    11004: {{.*}} bl      #69640
+// CHECK:         11000: {{.*}} b       #-4 <_start+0x4>
+// CHECK-NEXT:    11004: {{.*}} bl      #-4 <_start+0x8>
 // blx is transformed into bl so we don't change state
-// CHECK-NEXT:    11008: {{.*}} bl      #69644
+// CHECK-NEXT:    11008: {{.*}} bl      #-4 <_start+0xC>
 // CHECK-NEXT:    1100c: {{.*}} movt    r0, #0
 // CHECK-NEXT:    11010: {{.*}} movw    r0, #0
 // CHECK:         11014: {{.*}} .word   0x00000000