[ELF] Only print symbol name when it is available

[phosek]

Summary

Not only symbols (like sections) have names, in case where we fail to create relocation against such symbol, we should not print out an empty string, instead we should print a generic message.

[ruiu]

ELF/Relocations.cpp

431 ↗

(On Diff #68751)

This is a bit scattered. What we want to do here is (1) to get a symbol name, and (2) if it fails use "readonly sement" instead. So how about defining StringRef getSymbolName(elf::ObjectFile<ELFT> &File, SymbolBody &Body) which returns the symbol name or the empty string?

[ruiu]

LGTM with nits.

ELF/Relocations.cpp

413 ↗

(On Diff #68796)

Now that this is the only place where getLocalSymbolName is called, so let's inline that function and remove getLocalSymbolName.

414 ↗

(On Diff #68796)

Remove else after return.

lld/trunk/ELF/Relocations.cpp

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) {
+static StringRef getSymbolName(const elf::ObjectFile<ELFT> &File,
+  if (Body.isLocal() && Body.getNameOffset())
     return File.getStringTable().data() + Body.getNameOffset();
+  if (!Body.isLocal())
+    return Body.getName();
+  return "";
 }
 
 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();
+    StringRef Name = getSymbolName(File, Body);
     error("can't create dynamic relocation " + getRelName(Type) +
-          " against symbol " + Name);
+          " against " + (Name.empty() ? "readonly segment" : "symbol " + Name));
     return Expr;
   }
   if (Body.getVisibility() != STV_DEFAULT) {
     error("cannot preempt symbol " + Body.getName());
     return Expr;
   }
   if (Body.isObject()) {
     // Produce a copy relocation.

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 symbol
+// CHECK: can't create dynamic relocation R_X86_64_64 against readonly segment