Simplify processRelocAux

[• espindola]

Summary

It returns a different Expr only in the case of creating a function symbol pointing to its plt entry. We can just add a call to addPltEntry to avoid that and return null.

With this patch further simplifications of how we handle copy relocations are possible.

[grimar]

LGTM

[• espindola]

r330960

ELF/Relocations.cpp

 // * Write the dynamic relocations.
 // * Write the rest of the file.
 // This would have some drawbacks. For example, we would only know if .rela.dyn
 // is needed after applying relocations. If it is, it will go after rw and rx
 // sections. Given that it is ro, we will need an extra PT_LOAD. This
 // complicates things for the dynamic linker and means we would have to reserve
 // space for the extra PT_LOAD even if we end up not using it.
 template <class ELFT, class RelTy>
-static RelExpr processRelocAux(InputSectionBase &Sec, RelExpr Expr,
-                               RelType Type, uint64_t Offset, Symbol &Sym,
-                               const RelTy &Rel, int64_t Addend) {
+static void processRelocAux(InputSectionBase &Sec, RelExpr Expr, RelType Type,
+                            uint64_t Offset, Symbol &Sym, const RelTy &Rel,
+                            int64_t Addend) {
   if (isStaticLinkTimeConstant(Expr, Type, Sym, Sec, Offset)) {
     Sec.Relocations.push_back({Expr, Type, Offset, Addend, &Sym});
-    return Expr;
+    return;
   }
   bool CanWrite = (Sec.Flags & SHF_WRITE) || !Config->ZText;
   if (CanWrite) {
     // R_GOT refers to a position in the got, even if the symbol is preemptible.
     bool IsPreemptibleValue = Sym.IsPreemptible && Expr != R_GOT;
 
     if (!IsPreemptibleValue) {
       InX::RelaDyn->addReloc(Target->RelativeRel, &Sec, Offset, &Sym, Addend,
                              Expr, Type);
-      return Expr;
+      return;
     } else if (RelType Rel = Target->getDynRel(Type)) {
       InX::RelaDyn->addReloc(Rel, &Sec, Offset, &Sym, Addend, R_ADDEND, Type);
 
       // MIPS ABI turns using of GOT and dynamic relocations inside out.
       // While regular ABI uses dynamic relocations to fill up GOT entries
       // MIPS ABI requires dynamic linker to fills up GOT entries using
       // specially sorted dynamic symbol table. This affects even dynamic
       // relocations against symbols which do not require GOT entries
       // creation explicitly, i.e. do not have any GOT-relocations. So if
       // a preemptible symbol has a dynamic relocation we anyway have
       // to create a GOT entry for it.
       // If a non-preemptible symbol has a dynamic relocation against it,
       // dynamic linker takes it st_value, adds offset and writes down
       // result of the dynamic relocation. In case of preemptible symbol
       // dynamic linker performs symbol resolution, writes the symbol value
       // to the GOT entry and reads the GOT entry when it needs to perform
       // a dynamic relocation.
       // ftp://www.linux-mips.org/pub/linux/mips/doc/ABI/mipsabi.pdf p.4-19
       if (Config->EMachine == EM_MIPS)
         InX::MipsGot->addEntry(Sym, Addend, Expr);
-      return Expr;
+      return;
     }
   }
 
   // If the relocation is to a weak undef, and we are producing
   // executable, give up on it and produce a non preemptible 0.
   if (!Config->Shared && Sym.isUndefWeak()) {
     Sec.Relocations.push_back({Expr, Type, Offset, Addend, &Sym});
-    return Expr;
+    return;
   }
 
   if (!CanWrite && (Config->Pic && !isRelExpr(Expr))) {
     error(
         "can't create dynamic relocation " + toString(Type) + " against " +
         (Sym.getName().empty() ? "local symbol" : "symbol: " + toString(Sym)) +
         " in readonly segment; recompile object files with -fPIC "
         "or pass '-Wl,-z,notext' to allow text relocations in the output" +
         getLocation(Sec, Sym, Offset));
-    return Expr;
+    return;
   }
 
   // Copy relocations are only possible if we are creating an executable.
   if (Config->Shared) {
     errorOrWarn("relocation " + toString(Type) +
                 " cannot be used against symbol " + toString(Sym) +
                 "; recompile with -fPIC" + getLocation(Sec, Sym, Offset));
-    return Expr;
+    return;
   }
 
   // If the symbol is undefined we already reported any relevant errors.
   if (!Sym.isShared()) {
     assert(Sym.isUndefined());
-    return Expr;
+    return;
   }
 
   if (!canDefineSymbolInExecutable(Sym)) {
     error("cannot preempt symbol: " + toString(Sym) +
           getLocation(Sec, Sym, Offset));
-    return Expr;
+    return;
   }
 
   if (Sym.isObject()) {
     // Produce a copy relocation.
     auto &SS = cast<SharedSymbol>(Sym);
     if (!SS.CopyRelSec) {
       if (!Config->ZCopyreloc)
         error("unresolvable relocation " + toString(Type) +
               " against symbol '" + toString(SS) +
               "'; recompile with -fPIC or remove '-z nocopyreloc'" +
               getLocation(Sec, Sym, Offset));
       addCopyRelSymbol<ELFT>(SS);
     }
     Sec.Relocations.push_back({Expr, Type, Offset, Addend, &Sym});
-    return Expr;
+    return;
   }
 
   if (Sym.isFunc()) {
     // This handles a non PIC program call to function in a shared library. In
     // an ideal world, we could just report an error saying the relocation can
     // overflow at runtime. In the real world with glibc, crt1.o has a
     // R_X86_64_PC32 pointing to libc.so.
     //
if (Sym.isFunc()) {
     // * If some code has a direct reference to a function, it was probably
     //   compiled without -fPIE/-fPIC and doesn't maintain ebx.
     // * If a library definition gets preempted to the executable, it will have
     //   the wrong ebx value.
     if (Config->Pie && Config->EMachine == EM_386)
       errorOrWarn("symbol '" + toString(Sym) +
                   "' cannot be preempted; recompile with -fPIE" +
                   getLocation(Sec, Sym, Offset));
+    if (!Sym.isInPlt())
+      addPltEntry<ELFT>(InX::Plt, InX::GotPlt, InX::RelaPlt, Target->PltRel,
+                        Sym);
     Sym.NeedsPltAddr = true;
-    Expr = toPlt(Expr);
-    Sec.Relocations.push_back({Expr, Type, Offset, Addend, &Sym});
-    return Expr;
+    Sec.Relocations.push_back({toPlt(Expr), Type, Offset, Addend, &Sym});
+    return;
   }
 
   errorOrWarn("symbol '" + toString(Sym) + "' has no type" +
               getLocation(Sec, Sym, Offset));
-  return Expr;
 }
 
 template <class ELFT, class RelTy>
 static void scanReloc(InputSectionBase &Sec, OffsetGetter &GetOffset, RelTy *&I,
                       RelTy *End) {
   const RelTy &Rel = *I;
   Symbol &Sym = Sec.getFile<ELFT>()->getRelocTargetSym(Rel);
   RelType Type;
static void scanReloc(InputSectionBase &Sec, OffsetGetter &GetOffset, RelTy *&I,
   // Process some TLS relocations, including relaxing TLS relocations.
   // Note that this function does not handle all TLS relocations.
   if (unsigned Processed =
           handleTlsRelocation<ELFT>(Type, Sym, Sec, Offset, Addend, Expr)) {
     I += (Processed - 1);
     return;
   }
 
-  Expr = processRelocAux<ELFT>(Sec, Expr, Type, Offset, Sym, Rel, Addend);
   // If a relocation needs PLT, we create PLT and GOTPLT slots for the symbol.
   if (needsPlt(Expr) && !Sym.isInPlt()) {
     if (Sym.isGnuIFunc() && !Sym.IsPreemptible)
       addPltEntry<ELFT>(InX::Iplt, InX::IgotPlt, InX::RelaIplt,
                         Target->IRelativeRel, Sym);
     else
       addPltEntry<ELFT>(InX::Plt, InX::GotPlt, InX::RelaPlt, Target->PltRel,
                         Sym);
if (Config->EMachine == EM_MIPS) {
       InX::MipsGot->addEntry(Sym, Addend, Expr);
       if (Sym.isTls() && Sym.IsPreemptible)
         InX::RelaDyn->addReloc(Target->TlsGotRel, InX::MipsGot,
                                Sym.getGotOffset(), &Sym);
     } else if (!Sym.isInGot()) {
       addGotEntry<ELFT>(Sym);
     }
   }
+
+  processRelocAux<ELFT>(Sec, Expr, Type, Offset, Sym, Rel, Addend);
 }
 
 template <class ELFT, class RelTy>
 static void scanRelocs(InputSectionBase &Sec, ArrayRef<RelTy> Rels) {
   OffsetGetter GetOffset(Sec);
 
   // Not all relocations end up in Sec.Relocations, but a lot do.
   Sec.Relocations.reserve(Rels.size());

test/ELF/got32-i386-pie-rw.s

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=i686-pc-linux %s -o %t.o
 # RUN: ld.lld %t.o -o %t -pie
 # RUN: llvm-readelf -r -s %t | FileCheck %s
 
 # Unlike bfd and gold we accept this.
 
 # CHECK: .foobar           PROGBITS        00001000
 # CHECK: .got              PROGBITS        [[GOT:[0-9a-z]*]]
-# CHECK: 00001002  00000008 R_386_RELATIVE
-# CHECK: [[GOT]]   00000008 R_386_RELATIVE
+# CHECK-DAG: 00001002  00000008 R_386_RELATIVE
+# CHECK-DAG: [[GOT]]   00000008 R_386_RELATIVE
 foo:
 
 .section .foobar, "awx"
 .global _start
 _start:
  movl foo@GOT, %ebx