[PATCH] D11612: [lld][ELF2] Apply relocations.

[Rafael Ávila de Espíndola via llvm-commits]

rafael added a comment.

I do want us to be able to do a single pass over the relocations. On other ELF linkers at least relocation processing shows up in profiles.

Having said that, I am not sure that forces where the code lives.

Rui, are you ok with this going in with the code in writer.cpp, we add GOT/PLT support and then try to refactor it to some other file? It seems easier than starting with 2 passes and trying to then combine them.


================
Comment at: ELF/Chunks.h:19
@@ -18,3 +18,2 @@
 
-class Defined;
 template <class ELFT> class ObjectFile;
----------------
Independent cleanup, please commit it first.

================
Comment at: ELF/Chunks.h:81
@@ -81,3 +80,3 @@
 public:
-  SectionChunk(llvm::object::ELFFile<ELFT> *Obj, const Elf_Shdr *Header);
+  SectionChunk(ObjectFile<ELFT> *F, const Elf_Shdr *Header);
   size_t getSize() const override { return Header->sh_size; }
----------------
Why do you need to pass an ObjectFile here?

================
Comment at: ELF/InputFiles.cpp:35
@@ -34,2 +34,3 @@
   Chunks.reserve(Size);
+  SparseChunks.reserve(Size);
   for (const Elf_Shdr &Sec : ELFObj->sections()) {
----------------
For SparseChunks you can use resize and drop the push_backs below.

================
Comment at: ELF/InputFiles.cpp:42
@@ +41,3 @@
+    } else if (Sec.sh_type == SHT_RELA || Sec.sh_type == SHT_REL) {
+      RelocMap.emplace_back(*ELFObj->getSection(Sec.sh_info), &Sec);
+      SparseChunks.push_back(nullptr);
----------------
This is not checking if getSection fails.

================
Comment at: ELF/InputFiles.h:57
@@ +56,3 @@
+
+    bool operator<(const RelocMapInfo &Other) const {
+      return Section < Other.Section;
----------------
Add a comment on why it is deterministic to use < on pointers (they are all from one file).

================
Comment at: ELF/InputFiles.h:73
@@ +72,3 @@
+    if (SymbolIndex >= SymbolBodies.size())
+      llvm::report_fatal_error("Invalid symbol table index.");
+    return SymbolBodies[SymbolIndex]->getReplacement();
----------------
don't use report_fatal_error.

================
Comment at: ELF/SymbolTable.h:19
@@ -18,3 +18,2 @@
 namespace elf2 {
-class Defined;
 struct Symbol;
----------------
independent cleanup. Please commit first.

================
Comment at: ELF/Symbols.h:105
@@ -99,1 +104,3 @@
 
+  const Elf_Sym *getELFSymbol() const { return Sym; }
+
----------------
This is dead.

================
Comment at: ELF/Symbols.h:108
@@ -100,1 +107,3 @@
+  uint64_t getVA() const { return Chunk->getVA() + Sym->st_value; }
+  SectionChunk<ELFT> *getChunk() const { return Chunk; }
 private:
----------------
This is dead.

================
Comment at: ELF/Symbols.h:125
@@ -114,1 +124,3 @@
 
+template <class ELFT>
+inline uint64_t Defined<ELFT>::getVA() const {
----------------
Seems better to just cast to DefinedRegular at use.

================
Comment at: ELF/Writer.cpp:189
@@ +188,3 @@
+           I != E && I->Section == Hdr; ++I) {
+        for (auto RI = EObj->rela_begin(I->RelocSection),
+                  RE = EObj->rela_end(I->RelocSection);
----------------
Can you use a range loop.

================
Comment at: test/elf2/relocation.test:1
@@ +1,2 @@
+# RUN: yaml2obj -format elf %s -docnum 1 -o %t1obj
+# RUN: yaml2obj -format elf %s -docnum 2 -o %t2obj
----------------
llvm-mc


http://reviews.llvm.org/D11612