ELF/ARM: Ignore R_ARM_V4BX for ARMv4 but allow linking

[zatrazz]

Summary

This patch allow the ARM relocation R_ARM_V4BX to be processed by lld,
although it is not really handled in the static relocation code. The
relocation is in the form:

Relocation section '.rel.text' at offset 0x428 contains 4 entries:
Offset Info Type Sym.Value Sym. Name
00000014 00000028 R_ARM_V4BX

Meaning it does have a direct target, but rather references to an absolute
section *ABS* (in this exemple to the .text segment itself). It makes the
target Atom after file parse to not have a associated pointer and thus
generating a derrefence NULL point in ELFFile<ELFT>::findAtom. Current
approach is just ignore and return nullptr in such cases.

The problem relies that default GCC configuration
for arm-linux-gnueabi{hf} emits the relocation for the asm:

.syntax unified
.arm

.p2align 2
.type fn, %function
fn:

ldr r3, .LGOT
ldr r2, .LGOT+4

.LPIC:

add r3, pc, r3
ldr r2, [r3, r2]
cmp r2, #0
bxeq lr
b __start__

.LGOT:
.word _GLOBAL_OFFSET_TABLE_-(.LPIC+8)

.word start(GOT)

But only with the option -march=armv4 (which is the default GCC configuration).
For arm5 and forward the relocation is not created. This a special relocation
(defined miscellaneous for ARM) that instruct the linker to replace the bx
instruction into a mov. GNU linker has some options related to which substitution
it can create for such cases.

With this patch I can dynamically link an application against a GLIBC
arm-linux-gnueabi system configured with default GCC.

[ruiu]

lib/ReaderWriter/ELF/ELFFile.cpp

41

if (Atom *target = _symbolToAtomMapping.lookup(targetSym)) {

52

When it reaches here, target can be null, no?

[zatrazz]

Indeed, I though the testcase should catch the same case I was originally testing, but seems it is not stressing correctly this code path. I see the best approach is in fact to make the target Atom reference to point to itself instead to an undefined one (which will trigger an assert of nameless undefined Atom and later failure of linking):

diff --git a/lib/ReaderWriter/ELF/ELFFile.cpp b/lib/ReaderWriter/ELF/ELFFile.cpp
index ab2ec60..8fe2d30 100644

• a/lib/ReaderWriter/ELF/ELFFile.cpp

+++ b/lib/ReaderWriter/ELF/ELFFile.cpp
@@ -36,8 +36,15 @@ std::error_code ELFFile<ELFT>::isCompatible(const MemoryBuffer &mb,
template <typename ELFT>
Atom *ELFFile<ELFT>::findAtom(const Elf_Sym *sourceSym,

const Elf_Sym *targetSym) {

+ StringRef targetName;
+

// Return the atom for targetSym if we can do so.
Atom *target = _symbolToAtomMapping.lookup(targetSym);

+ if (!target)
+ Some realocations (R_ARM_V4BX) do not have a defined
+ target. For this cases make it points to itself.
+ target = _symbolToAtomMapping.lookup(sourceSym);
+

if (target->definition() != Atom::definitionRegular)
  return target;
Atom::Scope scope = llvm::cast<DefinedAtom>(target)->scope();

@@ -47,7 +54,7 @@ Atom *ELFFile<ELFT>::findAtom(const Elf_Sym *sourceSym,

  return target;

// Otherwise, create a new undefined symbol and returns it.

• StringRef targetName = target->name();

+ targetName = target->name();

auto it = _undefAtomsForGroupChild.find(targetName);
if (it != _undefAtomsForGroupChild.end())
  return it->getValue();

What do you think?

[zatrazz]

I will repost with the previous changes.

lib/ReaderWriter/ELF/ARM/ARMRelocationHandler.cpp

case R_ARM_ALU_PC_G1_NC:
     break;
   case R_ARM_LDR_PC_G2:
     relocR_ARM_LDR_PC_G2(loc, reloc, target, addend);
     break;
   case R_ARM_JUMP_SLOT:
   case R_ARM_IRELATIVE:
     // Runtime only relocations. Ignore here.
     break;
+  case R_ARM_V4BX:
+    // TODO implement
+    break;
   default:
     return make_unhandled_reloc_error();
   }
 
   return std::error_code();
 }

lib/ReaderWriter/ELF/ELFFile.cpp

std::error_code ELFFile<ELFT>::isCompatible(const MemoryBuffer &mb,
   return elf::isCompatible<ELFT>(mb, ctx);
 }
 
 template <typename ELFT>
 Atom *ELFFile<ELFT>::findAtom(const Elf_Sym *sourceSym,
                               const Elf_Sym *targetSym) {
   // Return the atom for targetSym if we can do so.
   Atom *target = _symbolToAtomMapping.lookup(targetSym);
+  if (target) {
     if (target->definition() != Atom::definitionRegular)
       return target;
     Atom::Scope scope = llvm::cast<DefinedAtom>(target)->scope();
     if (scope == DefinedAtom::scopeTranslationUnit)
       return target;
     if (!redirectReferenceUsingUndefAtom(sourceSym, targetSym))
       return target;
+  }
 
   // Otherwise, create a new undefined symbol and returns it.
   StringRef targetName = target->name();
   auto it = _undefAtomsForGroupChild.find(targetName);
   if (it != _undefAtomsForGroupChild.end())
     return it->getValue();
   auto atom = new (_readerStorage) SimpleUndefinedAtom(*this, targetName);
   _undefAtomsForGroupChild[targetName] = atom;
   addAtom(*atom);
   return atom;
 }

test/elf/ARM/rel-v4bx.test

+# REQUIRES: arm
+
+# Check handling of R_ARM_V4BX relocation.
+# RUN: yaml2obj -format=elf %s > %t-o.o
+# RUN: lld -flavor gnu -target arm -m armelf_linux_eabi \
+# RUN: --noinhibit-exec %t-o.o -o %t
+# RUN: llvm-objdump -d -triple=armv4t %t | FileCheck %s
+
+# CHECK: Disassembly of section .text:
+# CHECK: main:
+# CHECK:   400114:       03 30 8f e0     add     r3, pc, r3
+# CHECK:   400118:       02 20 93 e7     ldr     r2, [r3, r2]
+# CHECK:   40011c:       00 00 52 e3     cmp     r2, #0
+# The following instruction creates a RM_ARM_V4BX relocation that
+# allows the linker to replace the bx instruction to a ARMv4
+# compatible one.  Currently lld does not change it.
+# CHECK:   400120:       1e ff 2f 01     bxeq    lr 
+
+---
+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:         03308FE0022093E7000052E31EFF2F01
+  - Name:            .rel.text
+    Type:            SHT_REL
+    Flags:           [ SHF_INFO_LINK ]
+    Link:            .symtab
+    AddressAlign:    0x0000000000000004
+    Info:            .text
+    Relocations:     
+      - Offset:          0x000000000000000C
+        Symbol:          ''
+        Type:            R_ARM_V4BX
+  - 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:         ''
+  - Name:            .ARM.attributes
+    Type:            SHT_ARM_ATTRIBUTES
+    AddressAlign:    0x0000000000000001
+    Content:         4115000000616561626900010B000000060208010901
+Symbols:         
+  Local:           
+    - Name:            .text
+      Type:            STT_SECTION
+      Section:         .text
+    - Name:            .data
+      Type:            STT_SECTION
+      Section:         .data
+    - Name:            .bss
+      Type:            STT_SECTION
+      Section:         .bss
+    - Name:            '$a'
+      Section:         .text
+    - Name:            .ARM.attributes
+      Type:            STT_SECTION
+      Section:         .ARM.attributes
+  Global:          
+    - Name:            main
+      Type:            STT_FUNC
+      Section:         .text
+...