ELF/ARM: Ignore R_ARM_V4BX 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.

[shankarke]

LGTM.

[ruiu]

It feels to me that creating a dangling reference having a null pointer is wrong in the first place. I think in most places we don't expect a reference having a null pointer. Conceptually it's a directed edge of a graph, so there must be a vertex when you follow an edge.

Does this really need to be treated that way? Is there any other way to handle this in the same manner as other symbols/references?

[zatrazz]

This was my very question in the first patch about it I sent:

I am not certainly that it is the right approach to handle this cases in arch-neutral
code, so I am more than willing to hear better suggestions.

And I am not aware of a similar relocation with same constraints,
so I am not sure which model will fit better. Some options I can
think is to make it reference to itself or to an special symbol.

[• shankar.easwaran]

A relocation to nothing has a symbol index which points to the local undef symbol, the first symbol in the symbol table entry. We could keep that symbol as the sentinel symbol and make the null reference point to that.

lib/ReaderWriter/ELF/ARM/ARMRelocationHandler.cpp

std::error_code ARMTargetRelocationHandler::applyRelocation(
   assert(ref.kindArch() == Reference::KindArch::ARM);
 
   // 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())) {
+  if (const auto *definedAtom = dyn_cast_or_null<DefinedAtom>(ref.target())) {
     thumb = isThumbCode(definedAtom);
   }
 
   switch (ref.kindValue()) {
   case R_ARM_NONE:
     break;
   case R_ARM_ABS32:
     relocR_ARM_ABS32(loc, reloc, target, addend, thumb);
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->definition() != Atom::definitionRegular)
+  if (!target || 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.

lib/ReaderWriter/ELF/SectionChunks.cpp

void AtomSection<ELFT>::printError(const std::string &errorStr,
                                    const AtomLayout &atom,
                                    const Reference &ref) const {
   StringRef kindValStr;
   if (!this->_ctx.registry().referenceKindToString(
           ref.kindNamespace(), ref.kindArch(), ref.kindValue(), kindValStr)) {
     kindValStr = "unknown";
   }
 
+  StringRef targetName = "undefined";
+  if (ref.target())
+    targetName = ref.target()->name();
   std::string errStr =
       (Twine(errorStr) + " in file " + atom._atom->file().path() +
        ": reference from " + atom._atom->name() + "+" +
-       Twine(ref.offsetInAtom()) + " to " + ref.target()->name() + "+" +
+       Twine(ref.offsetInAtom()) + " to " + targetName + "+" +
        Twine(ref.addend()) + " of type " + Twine(ref.kindValue()) + " (" +
        kindValStr + ")\n")
           .str();
 
   // Take the lock to prevent output getting interleaved between threads
   std::lock_guard<std::mutex> lock(_outputMutex);
   llvm::errs() << errStr;
 }

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
+...