[ELF] - Fix for PR28976 - Corrupted section contents when using linker scripts

[grimar]

Summary

This is alternative solution to fix PR28976.

Problem is that in scanRelocs, we computed relocation offset too early for case when linkerscript was used.
Patch fixes the issue delaying the calculation.

Testcase is provided.

[Wallbraker]

This works for my test case as well, nice work! :D

[ruiu]

ELF/Relocations.cpp

559

This seems a cool fix, but is it safe? Specifically, my concern is on this line.

We used to call getOffset only in the following path. If PieceI != PieceE, getOffset was not called. Now you call getOffset on both paths. Didn't change the behavior?

[grimar]

ELF/Relocations.cpp

559

Yep, I think it is safe. See:

If PieceI != PieceE that means C is EhInputSection, right ? (see line 528-529 above)

if (auto *Eh = dyn_cast<EhInputSection<ELFT>>(&C))
  Pieces = Eh->Pieces;

and getOffset() for EHFrame always returns just an Offset given:

template <class ELFT>
typename ELFT::uint InputSectionBase<ELFT>::getOffset(uintX_t Offset) const {
  switch (SectionKind) {
...
  case EHFrame:
    // The file crtbeginT.o has relocations pointing to the start of an empty
    // .eh_frame that is known to be the first in the link. It does that to
    // identify the start of the output .eh_frame.
    return Offset;

So in fact I call getOffset on both paths but that does not change anything I believe.

[ruiu]

Makes sense. LGTM.

ELF/InputSection.cpp

for (const Elf_Shdr *RelSec : IS->RelocSections) {
       else
         IS->relocateNonAlloc(Buf, IS->File->getObj().rels(RelSec));
     }
     return;
   }
 
   const unsigned Bits = sizeof(uintX_t) * 8;
   for (const Relocation<ELFT> &Rel : Relocations) {
-    uintX_t Offset = Rel.Offset;
+    uintX_t Offset = getOffset(Rel.Offset);
     uint8_t *BufLoc = Buf + Offset;
     uint32_t Type = Rel.Type;
     uintX_t A = Rel.Addend;
 
     uintX_t AddrLoc = OutSec->getVA() + Offset;
     RelExpr Expr = Rel.Expr;
     uint64_t SymVA =
         SignExtend64<Bits>(getSymVA<ELFT>(Type, A, AddrLoc, *Rel.Sym, Expr));

ELF/OutputSections.cpp

for (StringRef S : Strings) {
     Buf += S.size() + 1;
   }
 }
 
 template <class ELFT>
 typename ELFT::uint DynamicReloc<ELFT>::getOffset() const {
   if (OutputSec)
     return OutputSec->getVA() + OffsetInSec;
-  return InputSec->OutSec->getVA() + OffsetInSec;
+  return InputSec->OutSec->getVA() + InputSec->getOffset(OffsetInSec);
 }
 
 template <class ELFT>
 typename ELFT::uint DynamicReloc<ELFT>::getAddend() const {
   if (UseSymVA)
     return Sym->getVA<ELFT>(Addend);
   return Addend;
 }

ELF/Relocations.cpp

for (auto I = Rels.begin(), E = Rels.end(); I != E; ++I) {
 
     // Compute the offset of this section in the output section. We do it here
     // to try to compute it only once.
     uintX_t Offset;
     if (PieceI != PieceE) {
       assert(PieceI->InputOff <= RI.r_offset && "Relocation not in any piece");
       if (PieceI->OutputOff == (size_t)-1)
         continue;
       Offset = PieceI->OutputOff + RI.r_offset - PieceI->InputOff;
     } else {
-      Offset = C.getOffset(RI.r_offset);
+      Offset = RI.r_offset;
     }
 
     // This relocation does not require got entry, but it is relative to got and
     // needs it to be created. Here we request for that.
     if (Expr == R_GOTONLY_PC || Expr == R_GOTREL || Expr == R_PPC_TOC)
       Out<ELFT>::Got->HasGotOffRel = true;
 
     uintX_t Addend = computeAddend(File, Buf, E, RI, Expr, Body);

test/ELF/linkerscript/Inputs/linkerscript-merge-sections-reloc.s

+.globl _start
+_start:
+ .quad 0x11223344

test/ELF/linkerscript/linkerscript-merge-sections-reloc.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %p/Inputs/linkerscript-merge-sections-reloc.s -o %t1.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t2.o
+# RUN: echo "SECTIONS {}" > %t.script
+# RUN: ld.lld -o %t --script %t.script %t1.o %t2.o
+# RUN: llvm-objdump -s %t | FileCheck %s
+
+## Check that sections content is not corrupted.
+# CHECK:      Contents of section .text:
+# CHECK-NEXT:  44332211 00000000 44332211 00000000
+# CHECK-NEXT:  f0ffffff ffffffff
+
+.globl _start
+_foo:
+ .quad 0x11223344
+ .quad _start - .