[ELF] Improve error reporting for relocations

[phosek]

Summary

We should always include symbol name when reporting relocations error to simplify debugging of these issues. Without symbol names users have to manually investigate which of the libraries contain invalid relocations which can be cumbersome when linking multiple libraries.

[ruiu]

Thank you for doing this!

ELF/Relocations.cpp

425–430 ↗

(On Diff #68591)

You can call getName() on a local symbol, can't you?

[phosek]

ELF/Relocations.cpp

425–430 ↗

(On Diff #68591)

There is an assert(!isLocal()) in SymbolBody::getName(). The name for local symbols is empty, but it I could use the name offset and find name in the File string table (if it's there), does that sound fine to you?

[ruiu]

ELF/Relocations.cpp

425–430 ↗

(On Diff #68591)

Ah, right. Please define something like this and use it.

template <class ELFT>
static StringRef getLocalSymName(elf::ObjectFile<ELFT> &F, SymbolBody &B) {
  return F.getStringTable().data() + B.getNameOffset();
}

[ruiu]

LGTM

lld/trunk/ELF/Relocations.cpp

 // Reserve space in .bss for copy relocation.
 template <class ELFT> static void addCopyRelSymbol(SharedSymbol<ELFT> *SS) {
   typedef typename ELFT::uint uintX_t;
   typedef typename ELFT::Sym Elf_Sym;
 
   // Copy relocation against zero-sized symbol doesn't make sense.
   uintX_t SymSize = SS->template getSize<ELFT>();
   if (SymSize == 0)
-    fatal("cannot create a copy relocation for " + SS->getName());
+    fatal("cannot create a copy relocation for symbol " + SS->getName());
 
   uintX_t Alignment = getAlignment(SS);
   uintX_t Off = alignTo(Out<ELFT>::Bss->getSize(), Alignment);
   Out<ELFT>::Bss->setSize(Off + SymSize);
   Out<ELFT>::Bss->updateAlignment(Alignment);
   uintX_t Shndx = SS->Sym.st_shndx;
   uintX_t Value = SS->Sym.st_value;
   // Look through the DSO's dynamic symbol table for aliases and create a
for (const Elf_Sym &S : SS->file()->getElfSymbols(true)) {
     Alias->NeedsCopyOrPltAddr = true;
     Alias->symbol()->IsUsedInRegularObj = true;
   }
   Out<ELFT>::RelaDyn->addReloc(
       {Target->CopyRel, Out<ELFT>::Bss, SS->OffsetInBss, false, SS, 0});
 }
 
 template <class ELFT>
+static StringRef getLocalSymbolName(const elf::ObjectFile<ELFT> &File,
+                                    SymbolBody &Body) {
+  return File.getStringTable().data() + Body.getNameOffset();
+}
+
+template <class ELFT>
 static RelExpr adjustExpr(const elf::ObjectFile<ELFT> &File, SymbolBody &Body,
                           bool IsWrite, RelExpr Expr, uint32_t Type,
                           const uint8_t *Data) {
   bool Preemptible = isPreemptible(Body, Type);
   if (Body.isGnuIFunc()) {
     Expr = toPlt(Expr);
   } else if (!Preemptible) {
     if (needsPlt(Expr))
       Expr = fromPlt(Expr);
     if (Expr == R_GOT_PC)
       Expr = Target->adjustRelaxExpr(Type, Data, Expr);
   }
   Expr = Target->getThunkExpr(Expr, Type, File, Body);
 
   if (IsWrite || isStaticLinkTimeConstant<ELFT>(Expr, Type, Body))
     return Expr;
 
   // This relocation would require the dynamic linker to write a value to read
   // only memory. We can hack around it if we are producing an executable and
   // the refered symbol can be preemepted to refer to the executable.
   if (Config->Shared || (Config->Pic && !isRelExpr(Expr))) {
+    StringRef Name = Body.isLocal() ? getLocalSymbolName(File, Body)
+                                    : Body.getName();
     error("can't create dynamic relocation " + getRelName(Type) +
-          " against readonly segment");
+          " against symbol " + Name);
     return Expr;
   }
   if (Body.getVisibility() != STV_DEFAULT) {
-    error("cannot preempt symbol");
+    error("cannot preempt symbol " + Body.getName());
     return Expr;
   }
   if (Body.isObject()) {
     // Produce a copy relocation.
     auto *B = cast<SharedSymbol<ELFT>>(&Body);
     if (!B->needsCopy())
       addCopyRelSymbol(B);
     return Expr;
if (Body.isFunc()) {
     // the value of the symbol we created. This is true even for got entries, so
     // pointer equality is maintained. To avoid an infinite loop, the only entry
     // that points to the real function is a dedicated got entry used by the
     // plt. That is identified by special relocation types (R_X86_64_JUMP_SLOT,
     // R_386_JMP_SLOT, etc).
     Body.NeedsCopyOrPltAddr = true;
     return toPlt(Expr);
   }
-  error("symbol is missing type");
+  error("symbol " + Body.getName() + " is missing type");
 
   return Expr;
 }
 
 template <class ELFT, class RelTy>
 static typename ELFT::uint computeAddend(const elf::ObjectFile<ELFT> &File,
                                          const uint8_t *SectionData,
                                          const RelTy *End, const RelTy &RI,

lld/trunk/test/ELF/aarch64-fpic-add_abs_lo12_nc.s

 // REQUIRES: aarch64
 // RUN: llvm-mc -filetype=obj -triple=aarch64-none-freebsd %s -o %t.o
 // RUN: not ld.lld -shared %t.o -o %t.so 2>&1 | FileCheck %s
-// CHECK: can't create dynamic relocation R_AARCH64_ADD_ABS_LO12_NC against readonly segment
+// CHECK: can't create dynamic relocation R_AARCH64_ADD_ABS_LO12_NC against symbol dat
 
   add x0, x0, :lo12:dat
 .data
 .globl dat
 dat:
   .word 0

lld/trunk/test/ELF/aarch64-fpic-adr_prel_lo21.s

 // REQUIRES: aarch64
 // RUN: llvm-mc -filetype=obj -triple=aarch64-none-freebsd %s -o %t.o
 // RUN: not ld.lld -shared %t.o -o %t.so 2>&1 | FileCheck %s
-// CHECK: can't create dynamic relocation R_AARCH64_ADR_PREL_LO21 against readonly segment
+// CHECK: can't create dynamic relocation R_AARCH64_ADR_PREL_LO21 against symbol dat
 
   adr x0, dat
 .data
 .globl dat
 dat:
   .word 0

lld/trunk/test/ELF/aarch64-fpic-adr_prel_pg_hi21.s

 // REQUIRES: aarch64
 // RUN: llvm-mc -filetype=obj -triple=aarch64-none-freebsd %s -o %t.o
 // RUN: not ld.lld -shared %t.o -o %t.so 2>&1 | FileCheck %s
-// CHECK: can't create dynamic relocation R_AARCH64_ADR_PREL_PG_HI21 against readonly segment
+// CHECK: can't create dynamic relocation R_AARCH64_ADR_PREL_PG_HI21 against symbol dat
 
   adrp x0, dat
 .data
 .globl dat
 dat:
   .word 0

lld/trunk/test/ELF/aarch64-fpic-ldst32_abs_lo12_nc.s

 // REQUIRES: aarch64
 // RUN: llvm-mc -filetype=obj -triple=aarch64-none-freebsd %s -o %t.o
 // RUN: not ld.lld -shared %t.o -o %t.so 2>&1 | FileCheck %s
-// CHECK: can't create dynamic relocation R_AARCH64_LDST32_ABS_LO12_NC against readonly segment
+// CHECK: can't create dynamic relocation R_AARCH64_LDST32_ABS_LO12_NC against symbol dat
 
   ldr s4, [x0, :lo12:dat]
 .data
 .globl dat
 dat:
   .word 0

lld/trunk/test/ELF/aarch64-fpic-ldst64_abs_lo12_nc.s

 // REQUIRES: aarch64
 // RUN: llvm-mc -filetype=obj -triple=aarch64-none-freebsd %s -o %t.o
 // RUN: not ld.lld -shared %t.o -o %t.so 2>&1 | FileCheck %s
-// CHECK: can't create dynamic relocation R_AARCH64_LDST64_ABS_LO12_NC against readonly segment
+// CHECK: can't create dynamic relocation R_AARCH64_LDST64_ABS_LO12_NC against symbol dat
 
   ldr x0, [x0, :lo12:dat]
 .data
 .globl dat
 dat:
   .word 0

lld/trunk/test/ELF/aarch64-fpic-ldst8_abs_lo12_nc.s

 // REQUIRES: aarch64
 // RUN: llvm-mc -filetype=obj -triple=aarch64-none-freebsd %s -o %t.o
 // RUN: not ld.lld -shared %t.o -o %t.so 2>&1 | FileCheck %s
-// CHECK: can't create dynamic relocation R_AARCH64_LDST8_ABS_LO12_NC against readonly segment
+// CHECK: can't create dynamic relocation R_AARCH64_LDST8_ABS_LO12_NC against symbol dat
 
   ldrsb x0, [x1, :lo12:dat]
 .data
 .globl dat
 dat:
   .word 0

lld/trunk/test/ELF/arm-target1.s

.text
   patatino:
 
 // RELATIVE: Disassembly of section .text:
 // RELATIVE: $d.0:
 // RELATIVE:     1000:       04 00 00 00     andeq   r0, r0, r4
 // RELATIVE: SYMBOL TABLE:
 // RELATIVE: 00001004         .text           00000000 patatino
 
-// ABS: can't create dynamic relocation R_ARM_TARGET1 against readonly segment
+// ABS: can't create dynamic relocation R_ARM_TARGET1 against symbol patatino

lld/trunk/test/ELF/copy-errors.s

 // REQUIRES: x86
 // RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
 // RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %p/Inputs/protected-shared.s -o %t2.o
 // RUN: ld.lld %t2.o -o %t2.so -shared
 // RUN: not ld.lld %t.o %t2.so -o %t 2>&1 | FileCheck %s
 
 .global _start
 _start:
 
 
 call bar
-// CHECK: cannot preempt symbol
+// CHECK: cannot preempt symbol bar
 
 call zed
-// CHECK: symbol is missing type
+// CHECK: symbol zed is missing type

lld/trunk/test/ELF/copy-in-shared.s

 // REQUIRES: x86
 // RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %p/Inputs/copy-in-shared.s -o %t1.o
 // RUN: ld.lld -shared %t1.o -o %t1.so
 // RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t2.o
 // RUN: not ld.lld %t2.o %t1.so -o %t2.so -shared 2>&1 | FileCheck %s
 
 
 .quad foo
 
-// CHECK: can't create dynamic relocation R_X86_64_64 against readonly segment
+// CHECK: can't create dynamic relocation R_X86_64_64 against symbol foo

lld/trunk/test/ELF/copy-rel-corrupted.s

 // RUN: llvm-mc %s -o %t.o -filetype=obj -triple=x86_64-pc-linux
 // RUN: llvm-mc %p/Inputs/copy-rel-corrupted.s -o %t2.o -filetype=obj -triple=x86_64-pc-linux
 // RUN: ld.lld %t2.o -o %t2.so -shared
 // RUN: not ld.lld %t.o %t2.so -o %t.exe 2>&1 | FileCheck %s
 
-// CHECK: cannot create a copy relocation for x
+// CHECK: cannot create a copy relocation for symbol x
 
 .global _start
 _start:
         call x

lld/trunk/test/ELF/copy-rel-pie-error.s

 // RUN: llvm-mc %s -o %t.o -filetype=obj -triple=x86_64-pc-linux
 // RUN: llvm-mc %p/Inputs/copy-rel-pie.s -o %t2.o -filetype=obj -triple=x86_64-pc-linux
 // RUN: ld.lld %t2.o -o %t2.so -shared
 // RUN: not ld.lld %t.o %t2.so -o %t.exe -pie 2>&1 | FileCheck %s
 
-// CHECK: can't create dynamic relocation R_X86_64_64 against readonly segment
-// CHECK: can't create dynamic relocation R_X86_64_64 against readonly segment
+// CHECK: can't create dynamic relocation R_X86_64_64 against symbol bar
+// CHECK: can't create dynamic relocation R_X86_64_64 against symbol foo
 
 .global _start
 _start:
         .quad bar
         .quad foo

lld/trunk/test/ELF/dynamic-reloc-in-ro.s

 // REQUIRES: x86
 // RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
 // RUN: not ld.lld %t.o -o %t.so -shared 2>&1 | FileCheck %s
 
 foo:
 .quad foo
 
-// CHECK: can't create dynamic relocation R_X86_64_64 against readonly segment
+// CHECK: can't create dynamic relocation R_X86_64_64 against symbol

lld/trunk/test/ELF/eh-frame-dyn-rel.s

 // REQUIRES: x86
 // RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
 // RUN: not ld.lld %t.o %t.o -o %t -shared 2>&1 | FileCheck %s
 
         .section        bar,"axG",@progbits,foo,comdat
         .cfi_startproc
         .cfi_personality 0x8c, foo
         .cfi_endproc
 
-// CHECK: can't create dynamic relocation R_X86_64_64 against readonly segment
+// CHECK: can't create dynamic relocation R_X86_64_64 against symbol foo