[ELF] Linkerscript: fix relocation offsets

[evgeny777]

Summary

Now, when we assign scripted sections offsets in assignAddresses, we should also fix relocation offsets there, because input section offset is used to calculate relocation offset.
When linker script is used scanRelocs adds relocation entries which offsets begin from the start of input section. We fix them in fixStaticRelocs() and fixDynamicRelocs() by means of adding input section offset, so relocation offset becomes an offset from beginning of output section - the way it should be.

[grimar]

This is PR28976.

[grimar]

Testcase ?

[evgeny777]

It's not very easy to write one due to the nature of the problem, but I'll think of it.

[Wallbraker]

This change gives me linking errors when I try to compile it on git head.

git: 722e076e650f0e43f2c31f32194507591a8d45c6
svn: 278819

error.txt4 KBDownload

Cheers, Jakob.

[evgeny777]

Jacob, I've added explicit template instantiations for DynamicReloc.
Please check, may be it works for you now.

[Wallbraker]

This commit compiles, fixes my test case and my full code now links correctly! Awesome thanks.

I'm not a stakeholder (nor do I know the code at all) in the code, so somebody else will need to sign off on the code itself.

Cheers, Jakob.

[silvas]

Together with fixes for the things mentioned in https://reviews.llvm.org/D23352#516156, with this patch applied we can link and boot the FreeBSD 10.3 kernel!

I'm not sure that this is the right implementation though. It seems weird that we have to fixup the relocations. Could you explain why we need to do that? Could we implement this in a more natural way?

[evgeny777]

Let me elaborate a little bit:
Roughly current workflow is like following:

Create output sections (createSections) -> Scan relocations (scanRelocs) -> Assign output section VA (assignAddresses)

The current implementation of scanRelocs assumes that output section sizes and input section offsets are already known. This is not always true when linker scripts are used.
For example this script creates output section .foo, which size depends on its own virtual address:

.foo : {
   *(.foo)
   . = ALIGN(0x1000); /* this line creates gap inside the output section .foo, which size depends on .foo virtual address */
   *(.bar)
}

This means that in case of linker script we only know exact size of the output and offset of the inputs after call to assignAddresses().
At the same time scanRelocs() cannot be called from assignAddresses(), because scanRelocs() creates and fills some predefined sections (.got, .rela.dyn, .got.plt),
and all sections must be created before assignAddresses() is called.

To my understanding some refactoring to the whole workflow is needed, which would probably look like this:

createSections() -> createRelocs() -> assignAddresses() -> assignRelocs()

This patch is just a quick fix for a specific problem.

[evgeny777]

Fixed bug in fixDynamicRelocs()

[ruiu]

Eugene,

Thank you for your description. I agree that the order we compute offset is the cause of the problem. In scanRelocs, we compute a symbol offset for each symbol and assign it to a local variable Offset -- but that value may change if linker scripts are in use. That value is computed too early, and we want to do it lazily.

As to this patch, I'd refactor instead of applying a quick fix. It doesn't seem hard to do. For C.Relocations in scanRelocs, we are able to not store Offset to them because it can be computed from Body.

[silvas]

In D23565#518608, @ruiu wrote:

Eugene,

Thank you for your description. I agree that the order we compute offset is the cause of the problem. In scanRelocs, we compute a symbol offset for each symbol and assign it to a local variable Offset -- but that value may change if linker scripts are in use. That value is computed too early, and we want to do it lazily.

As to this patch, I'd refactor instead of applying a quick fix. It doesn't seem hard to do. For C.Relocations in scanRelocs, we are able to not store Offset to them because it can be computed from Body.

This makes sense, but it may have performance implications to get the offset later (at least more pointer chasing for data that we already had in cache in scanRelocs); it will need to be measured to make sure that we implement it without costing too much performance.

[evgeny777]

Done here
https://reviews.llvm.org/D23655

ELF/LinkerScript.cpp

for (InputSectionBase<ELFT> *S : F->getSections()) {
       std::tie(OutSec, IsNew) = Factory.create(S, getOutputSectionName(S));
       if (IsNew)
         OutputSections->push_back(OutSec);
       OutSec->addSection(S);
     }
   }
 }
 
+template <class ELFT> static void fixStaticRelocs(InputSection<ELFT> *I) {
+  for (Relocation<ELFT> &R : I->Relocations)
+    R.Offset += I->OutSecOff;
+}
+
+template <class ELFT>
+static void fixDynamicRelocs(OutputSectionBase<ELFT> *OutSec) {
+  for (DynamicReloc<ELFT> &D : Out<ELFT>::RelaDyn->Relocs)
+    if (D.getOutputSec() == OutSec)
+      D.setSectionOffset(D.getOffset() - D.getOutputSec()->getVA() +
+                         cast<InputSection<ELFT>>(D.getInputSec())->OutSecOff);
+}
+
 template <class ELFT> void assignOffsets(OutputSectionBase<ELFT> *Sec) {
   auto *OutSec = dyn_cast<OutputSection<ELFT>>(Sec);
   if (!OutSec) {
     Sec->assignOffsets();
     return;
   }
 
   typedef typename ELFT::uint uintX_t;
if (auto *L = dyn_cast<LayoutInputSection<ELFT>>(I)) {
         // for non-null value.
         Sym->Section = OutSec;
         Sym->Value = Value;
       }
     } else {
       Off = alignTo(Off, I->Alignment);
       I->OutSecOff = Off;
       Off += I->getSize();
+      fixStaticRelocs(I);
     }
     // Update section size inside for-loop, so that SIZEOF
     // works correctly in the case below:
     // .foo { *(.aaa) a = SIZEOF(.foo); *(.bbb) }
     Sec->setSize(Off);
   }
+  fixDynamicRelocs(Sec);
 }
 
 template <class ELFT>
 static OutputSectionBase<ELFT> *
 findSection(OutputSectionCommand &Cmd,
             ArrayRef<OutputSectionBase<ELFT> *> Sections) {
   for (OutputSectionBase<ELFT> *Sec : Sections) {
     if (Sec->getName() != Cmd.Name)

ELF/OutputSections.h

public:
 
   DynamicReloc(uint32_t Type, const OutputSectionBase<ELFT> *OutputSec,
                uintX_t OffsetInSec, bool UseSymVA, SymbolBody *Sym,
                uintX_t Addend)
       : Type(Type), Sym(Sym), OutputSec(OutputSec), OffsetInSec(OffsetInSec),
         UseSymVA(UseSymVA), Addend(Addend) {}
 
   uintX_t getOffset() const;
+  void setSectionOffset(uintX_t Offset) { OffsetInSec = Offset; }
   uintX_t getAddend() const;
   uint32_t getSymIndex() const;
-  const OutputSectionBase<ELFT> *getOutputSec() const { return OutputSec; }
+  const OutputSectionBase<ELFT> *getOutputSec() const {
+    return OutputSec ? OutputSec : InputSec->OutSec;
+  }
+  const InputSectionBase<ELFT> *getInputSec() const { return InputSec; }
 
   uint32_t Type;
 
 private:
   SymbolBody *Sym;
   const InputSectionBase<ELFT> *InputSec = nullptr;
   const OutputSectionBase<ELFT> *OutputSec = nullptr;
   uintX_t OffsetInSec;
public:
   void addReloc(const DynamicReloc<ELFT> &Reloc);
   unsigned getRelocOffset();
   void finalize() override;
   void writeTo(uint8_t *Buf) override;
   bool hasRelocs() const { return !Relocs.empty(); }
   typename Base::Kind getKind() const override { return Base::Reloc; }
   static bool classof(const Base *B) { return B->getKind() == Base::Reloc; }
 
+  std::vector<DynamicReloc<ELFT>> Relocs;
+
 private:
   bool Sort;
-  std::vector<DynamicReloc<ELFT>> Relocs;
 };
 
 template <class ELFT>
 class OutputSection final : public OutputSectionBase<ELFT> {
   typedef OutputSectionBase<ELFT> Base;
 
 public:
   typedef typename ELFT::Shdr Elf_Shdr;

ELF/OutputSections.cpp

 template class BuildIdSha1<ELF64LE>;
 template class BuildIdSha1<ELF64BE>;
 
 template class BuildIdHexstring<ELF32LE>;
 template class BuildIdHexstring<ELF32BE>;
 template class BuildIdHexstring<ELF64LE>;
 template class BuildIdHexstring<ELF64BE>;
 
+template class DynamicReloc<ELF32LE>;
+template class DynamicReloc<ELF32BE>;
+template class DynamicReloc<ELF64LE>;
+template class DynamicReloc<ELF64BE>;
+
 template class OutputSectionFactory<ELF32LE>;
 template class OutputSectionFactory<ELF32BE>;
 template class OutputSectionFactory<ELF64LE>;
 template class OutputSectionFactory<ELF64BE>;
 }
 }