ARM: replace branches to undefined weak functions with NOP

[lenykholodov]

Summary

According to the code model (ARM, Thumb, Thumb2) this patch updates the b/bl/blx 0 instructions with NOP.

[denis-protivensky]

lib/ReaderWriter/ELF/ARM/ARMRelocationHandler.cpp

509 ↗

(On Diff #23174)

The NOP instruction is supported by only limited number of ARM architectures.
You may at least add a TODO at the beginning of the function to indicate this.

539 ↗

(On Diff #23174)

Maybe better to log only successive fixups? Don't think the information about unresolved weaks is really useful.

[lenykholodov]

• rabase with master;
• debug output only for successfull fixups;
• todo remark.

[denis-protivensky]

LGTM with nits.

lib/ReaderWriter/ELF/ARM/ARMRelocationHandler.cpp

500 ↗

(On Diff #23361)

The name of the function should reflect that you only fixup call-like instructions.
fixupUnresolvedWeakCall sounds more correct.

539 ↗

(On Diff #23361)

You mix *unresolved* with *undefined* in this whole fragment of added code.
Please, be consistent. I think *unresolved* sounds better here.

541 ↗

(On Diff #23361)

result is a bad name for variable. Consider something having word fixed in it.

[lenykholodov]

1. Rebase with master.
2. Update code according to the comments from reviewers.

lld/trunk/lib/ReaderWriter/ELF/ARM/ARMRelocationHandler.cpp

DEBUG(llvm::dbgs() << "\t\tHandle " << LLVM_FUNCTION_NAME << " -";
         llvm::dbgs() << " P: 0x" << Twine::utohexstr(P);
         llvm::dbgs() << " result: 0x" << Twine::utohexstr((uint32_t)result)
                      << "\n");
 
   const uint32_t mask = 0xFFF;
   applyArmReloc(location, (uint32_t)result & mask, mask);
 }
 
+/// \brief Fixup unresolved weak reference with NOP instruction
+static bool fixupUnresolvedWeakCall(uint8_t *location,
+                                    Reference::KindValue kindValue) {
+  //TODO: workaround for archs without NOP instruction
+  switch (kindValue) {
+  case R_ARM_THM_CALL:
+  case R_ARM_THM_JUMP24:
+    // Thumb32 NOP.W
+    write32le(location, 0x8000F3AF);
+    break;
+  case R_ARM_THM_JUMP11:
+    // Thumb16 NOP
+    write16le(location, 0xBF00);
+    break;
+  case R_ARM_CALL:
+  case R_ARM_JUMP24:
+    // A1 NOP<c>, save condition bits
+    applyArmReloc(location, 0x320F000, 0xFFFFFFF);
+    break;
+  default:
+    return false;
+  }
+
+  return true;
+}
+
 std::error_code ARMTargetRelocationHandler::applyRelocation(
     ELFWriter &writer, llvm::FileOutputBuffer &buf, const AtomLayout &atom,
     const Reference &ref) const {
   uint8_t *atomContent = buf.getBufferStart() + atom._fileOffset;
   uint8_t *loc = atomContent + ref.offsetInAtom();
   uint64_t target = writer.addressOfAtom(ref.target());
   uint64_t reloc = atom._virtualAddr + ref.offsetInAtom();
 
   if (ref.kindNamespace() != Reference::KindNamespace::ELF)
     return std::error_code();
   assert(ref.kindArch() == Reference::KindArch::ARM);
 
+  // Fixup unresolved weak references
+  if (!target) {
+    bool isCallFixed = fixupUnresolvedWeakCall(loc, ref.kindValue());
+
+    if (isCallFixed) {
+      DEBUG(llvm::dbgs() << "\t\tFixup unresolved weak reference '";
+            llvm::dbgs() << ref.target()->name() << "'";
+            llvm::dbgs() << " at address: 0x" << Twine::utohexstr(reloc);
+            llvm::dbgs() << (isCallFixed ? "\n" : " isn't possible\n"));
+      return std::error_code();
+    }
+  }
+
   // Calculate proper initial addend for the relocation
   const Reference::Addend addend =
       readAddend(loc, ref.kindValue()) + ref.addend();
 
   // Flags that the relocation addresses Thumb instruction
   bool thumb = false;
   if (const auto *definedAtom = dyn_cast<DefinedAtom>(ref.target())) {
     thumb = isThumbCode(definedAtom);

lld/trunk/test/elf/ARM/weak-branch.test

+# Check weak references fixup.
+
+# RUN: yaml2obj -format=elf -docnum 1 %s > %t-jmp11.o
+# RUN: lld -flavor gnu -target arm -m armelf_linux_eabi -Bstatic \
+# RUN: --noinhibit-exec %t-jmp11.o -o %t
+# RUN: llvm-objdump -s -t %t | FileCheck -check-prefix=JMP11-CHECK %s
+
+# JMP11-CHECK: Contents of section .text:
+# JMP11-CHECK: 400194 704700bf 01216be7 012100bf 002167e7
+#                                                 ^ NOP (thumb16)
+# 40019c: 2101  movs  r1, #1
+# 40019e: bf00  nop
+# JMP11-CHECK: 4001a4 002100bf
+#                                ^ NOP (thumb16)
+# 4001a4: 2100 movs r1, #0
+# 4001a6: bf00 nop
+# JMP11-CHECK: SYMBOL TABLE:
+# JMP11-CHECK: 00000000  w      *UND* 00000000 __gnu_h2f_internal
+
+# RUN: yaml2obj -format=elf -docnum 2 %s > %t-thm-call.o
+# RUN: lld -flavor gnu -target arm -m armelf_linux_eabi -Bstatic \
+# RUN: --noinhibit-exec %t-thm-call.o -o %t
+# RUN: llvm-objdump -s -t %t | FileCheck -check-prefix=THM-CALL-CHECK %s
+
+# THM-CALL-CHECK: Contents of section .text:
+# THM-CALL-CHECK: 400064 80b500af fff7f4ff aff30080 03461846
+#                                    ^ NOP.W
+# 400068: f7ff fff4   bl  400054 <my_fn>
+# 40006c: f3af 8000   nop.w
+# 400070: 4603        mov r3, r0 ; return value has not been changed after weak fn call
+#
+# THM-CALL-CHECK: SYMBOL TABLE:
+# THM-CALL-CHECK: 00000000  w      *UND*  00000000 weak_fn
+
+# RUN: yaml2obj -format=elf -docnum 3 %s > %t-arm-call.o
+# RUN: lld -flavor gnu -target arm -m armelf_linux_eabi -Bstatic \
+# RUN: --noinhibit-exec %t-arm-call.o -o %t
+# RUN: llvm-objdump -s -t %t | FileCheck -check-prefix=ARM-CALL-CHECK %s
+
+# ARM-CALL-CHECK: Contents of section .text:
+# ARM-CALL-CHECK: 400074 04b08de2 f5ffffeb 00f020e3 0030a0e1
+#                                             ^ NOP
+#  400078:  ebfffff5  bl  400054 <my_fn>
+#  40007c:  e320f000  nop {0}
+#  400080:  e1a03000  mov r3, r0 ; return value has not been changed after weak fn call
+#
+# ARM-CALL-CHECK: SYMBOL TABLE:
+# ARM-CALL-CHECK: 00000000  w      *UND*  00000000 weak_fn
+
+# jump11.o
+---
+FileHeader:      
+  Class:           ELFCLASS32
+  Data:            ELFDATA2LSB
+  Type:            ET_REL
+  Machine:         EM_ARM
+  Flags:           [ EF_ARM_EABI_VER5 ]
+Sections:        
+  - Name:            .text
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    AddressAlign:    0x0000000000000004
+    Content:         70B4020CC0F3C754FF2CC0F3160302F4004080B241D0002C08BF002B3BD0A4F17F0543F4000315F10E0FA8BF41F6FF7209DA15F1190FA3BFA4F166066FF07F42F2406FF07F4212EA03060CD001325208964208BF03EA42021344B3F1807F24BF5B08A4F17E0501B30F2D26DC15F1180F11DB15F10E0FB5BF4FF6F2710E35CFF6FF71AD02B7BF491BAAB2CB40002202EB5333034398B270BC70470029FBD040EA533370BC43F4FC407047102DDEDD6FEAD0336FEAC33398B2EDE740F4F84398B2E9E700BFC0F3842310B4A3F11F040029B4FA84F400F400424FEA541408BF0024C0F309002146140481B943B14203703302EBC35343EA04005DF8044B704760B1B0FA80F3153B98405B42EFE744EA40305DF8044B40F0FF40704720465DF8044B704700BF01216BE70121FEE7002167E70021FEE7
+  - Name:            .rel.text
+    Type:            SHT_REL
+    Link:            .symtab
+    AddressAlign:    0x0000000000000004
+    Info:            .text
+    Relocations:     
+      - Offset:          0x000000000000012A
+        Symbol:          __gnu_h2f_internal
+        Type:            R_ARM_THM_JUMP11
+      - Offset:          0x0000000000000132
+        Symbol:          __gnu_h2f_internal
+        Type:            R_ARM_THM_JUMP11
+  - Name:            .data
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_WRITE, SHF_ALLOC ]
+    AddressAlign:    0x0000000000000001
+    Content:         ''
+  - Name:            .bss
+    Type:            SHT_NOBITS
+    Flags:           [ SHF_WRITE, SHF_ALLOC ]
+    AddressAlign:    0x0000000000000001
+    Content:         ''
+Symbols:         
+  Local:           
+    - Name:            .text
+      Type:            STT_SECTION
+      Section:         .text
+    - Name:            .data
+      Type:            STT_SECTION
+      Section:         .data
+    - Name:            .bss
+      Type:            STT_SECTION
+      Section:         .bss
+    - Name:            '$t'
+      Section:         .text
+    - Name:            __gnu_f2h_internal
+      Type:            STT_FUNC
+      Section:         .text
+      Value:           0x0000000000000001
+      Size:            0x00000000000000C2
+  Global:          
+    - Name:            __gnu_f2h_ieee
+      Type:            STT_FUNC
+      Section:         .text
+      Value:           0x0000000000000125
+      Size:            0x0000000000000004
+      Visibility:      STV_HIDDEN
+    - Name:            __gnu_h2f_ieee
+      Type:            STT_FUNC
+      Section:         .text
+      Value:           0x0000000000000129
+      Size:            0x0000000000000004
+      Visibility:      STV_HIDDEN
+    - Name:            __gnu_f2h_alternative
+      Type:            STT_FUNC
+      Section:         .text
+      Value:           0x000000000000012D
+      Size:            0x0000000000000004
+      Visibility:      STV_HIDDEN
+    - Name:            __gnu_h2f_alternative
+      Type:            STT_FUNC
+      Section:         .text
+      Value:           0x0000000000000131
+      Size:            0x0000000000000004
+      Visibility:      STV_HIDDEN
+  Weak:
+    - Name:            __gnu_h2f_internal
+# thm-call.o
+---
+FileHeader:      
+  Class:           ELFCLASS32
+  Data:            ELFDATA2LSB
+  Type:            ET_REL
+  Machine:         EM_ARM
+  Flags:           [ EF_ARM_EABI_VER5 ]
+Sections:        
+  - Name:            .text
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    AddressAlign:    0x0000000000000004
+    Content:         80B400AF01231846BD465DF8047B704780B500AFFFF7FEFFFFF7FEFF0346184680BD00BF
+  - Name:            .rel.text
+    Type:            SHT_REL
+    Link:            .symtab
+    AddressAlign:    0x0000000000000004
+    Info:            .text
+    Relocations:     
+      - Offset:          0x0000000000000014
+        Symbol:          my_fn
+        Type:            R_ARM_THM_CALL
+      - Offset:          0x0000000000000018
+        Symbol:          weak_fn
+        Type:            R_ARM_THM_CALL
+Symbols:         
+  Local:           
+    - Name:            .text
+      Type:            STT_SECTION
+      Section:         .text
+    - Name:            '$t'
+      Section:         .text
+  Global:          
+    - Name:            my_fn
+      Type:            STT_FUNC
+      Section:         .text
+      Value:           0x0000000000000001
+      Size:            0x0000000000000010
+    - Name:            main
+      Type:            STT_FUNC
+      Section:         .text
+      Value:           0x0000000000000011
+      Size:            0x0000000000000012
+  Weak:            
+    - Name:            weak_fn
+# arm-call.o
+---
+FileHeader:      
+  Class:           ELFCLASS32
+  Data:            ELFDATA2LSB
+  Type:            ET_REL
+  Machine:         EM_ARM
+  Flags:           [ EF_ARM_EABI_VER5 ]
+Sections:        
+  - Name:            .text
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    AddressAlign:    0x0000000000000004
+    Content:         04B02DE500B08DE20130A0E30300A0E100D04BE204B09DE41EFF2FE100482DE904B08DE2FEFFFFEBFEFFFFEB0030A0E10300A0E10088BDE8
+  - Name:            .rel.text
+    Type:            SHT_REL
+    Link:            .symtab
+    AddressAlign:    0x0000000000000004
+    Info:            .text
+    Relocations:     
+      - Offset:          0x0000000000000024
+        Symbol:          my_fn
+        Type:            R_ARM_CALL
+      - Offset:          0x0000000000000028
+        Symbol:          weak_fn
+        Type:            R_ARM_CALL
+Symbols:         
+  Local:           
+    - Name:            .text
+      Type:            STT_SECTION
+      Section:         .text
+    - Name:            '$a'
+      Section:         .text
+  Global:          
+    - Name:            my_fn
+      Type:            STT_FUNC
+      Section:         .text
+      Size:            0x000000000000001C
+    - Name:            main
+      Type:            STT_FUNC
+      Section:         .text
+      Value:           0x000000000000001C
+      Size:            0x000000000000001C
+  Weak:            
+    - Name:            weak_fn
+...