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

Rafael EspĂ­ndola via llvm-commits llvm-commits at lists.llvm.org
Fri Aug 14 15:20:22 PDT 2015


I applied your patch on top of r245055.

I have git-clang-formated it.

It causes elf2/basic64be.s to fail with an unknown relocation. You
don't have to implement it now, just warn on unknown relocations for
no.

I converted your tests to assembly and that found a bug in your patch.
It thinks the llvm-mc produced file is invalid.

Attached.


On 14 August 2015 at 08:46, Rafael EspĂ­ndola <rafael.espindola at gmail.com> wrote:
>> I'm not sure how you guys have decided on divvying up the work, but as a
>> person that works within earshot of Michael, it seems that a significant
>> factor to that is that many of the changes you make are antagonistic to the
>> changes he is trying to make. Deleting dead code that is used by the patch
>> he is trying to land or
>
> The situation is perfectly symmetrical.
>
> If the code is dead, it should be deleted. As Rui pointed out, many
> were COFFism that he copied and failed to notice when s/COFF/ELF/.
>
> We would also be further along if I didn't have to be the one
> implementing code reviews on his code because he never replies to
> them.
>
>>>
>>> I am *totally* opposed to
>>> *ever* having a YAML test that can be written in assembly.
>>>
>>> If I can understand that entire YAML gunk is:
>>>
>>> foo:
>>> .section bar
>>>         call foo
>>>
>>
>> I like seeing bytes and symbolized ELF spec names. That is what I know best.
>> That is what anybody working on this linker will know. I don't know all the
>> weird corners of gas syntax (+ LLVM extensions apparently), nor would I
>> expect any linker developer to know that. I *would* expect any linker
>> developer to know the symbolic constants (or at least be able to easily
>> google them).
>>
>> The YAML file directly expresses the intent; yes there is lots of "gunk"
>> surrounding it, but any linker developer knows how to skim that based on
>> knowing the meaning of the symbolic constants. Do you find it hard to
>> navigate the YAML files? We can improve the format if necessary if there is
>> a particular pain point or two.
>>
>> To put it another way, every "gas directives" file I see in the test
>> directory, my immediate reaction is wanting to assemble it and readobj it to
>> see what it "actually" is feeding to the linker. I particularly like the
>> YAML because like readobj output, it is telling you pretty straight up what
>> is there. One doesn't need to guess what flags the assembler set on that
>> section, whether a particular relocation kind actually ends up in the
>> output, etc. You just see it immediately in your editor.
>
> Too bad. You have 0 lines of code on the new linker. You are most
> certainly not helping its progress and you are still very opinionated
> on how I should be coding.
>
> As the one that is actually writing this, the 3 line assembly I posted
> is way easier to understand. Use assembly for new tests.
>
> Cheeers,
> Rafael
-------------- next part --------------
diff --git a/ELF/Chunks.cpp b/ELF/Chunks.cpp
index 97c0429..8dec8ff 100644
--- a/ELF/Chunks.cpp
+++ b/ELF/Chunks.cpp
@@ -9,6 +9,7 @@
 
 #include "Chunks.h"
 #include "Error.h"
+#include "InputFiles.h"
 
 using namespace llvm;
 using namespace llvm::ELF;
@@ -17,9 +18,8 @@ using namespace lld;
 using namespace lld::elf2;
 
 template <class ELFT>
-SectionChunk<ELFT>::SectionChunk(object::ELFFile<ELFT> *Obj,
-                                 const Elf_Shdr *Header)
-    : Obj(Obj), Header(Header) {
+SectionChunk<ELFT>::SectionChunk(ObjectFile<ELFT> *F, const Elf_Shdr *Header)
+    : File(F), OS(nullptr), Header(Header) {
   Align = Header->sh_addralign;
 }
 
@@ -27,14 +27,14 @@ template <class ELFT> void SectionChunk<ELFT>::writeTo(uint8_t *Buf) {
   if (Header->sh_type == SHT_NOBITS)
     return;
   // Copy section contents from source object file to output file.
-  ArrayRef<uint8_t> Data = *Obj->getSectionContents(Header);
+  ArrayRef<uint8_t> Data = *File->getObj()->getSectionContents(Header);
   memcpy(Buf + OutputSectionOff, Data.data(), Data.size());
 
   // FIXME: Relocations
 }
 
 template <class ELFT> StringRef SectionChunk<ELFT>::getSectionName() const {
-  ErrorOr<StringRef> Name = Obj->getSectionName(Header);
+  ErrorOr<StringRef> Name = File->getObj()->getSectionName(Header);
   error(Name);
   return *Name;
 }
diff --git a/ELF/Chunks.h b/ELF/Chunks.h
index 41a8cc1..f7bec41 100644
--- a/ELF/Chunks.h
+++ b/ELF/Chunks.h
@@ -17,6 +17,7 @@ namespace lld {
 namespace elf2 {
 
 template <class ELFT> class ObjectFile;
+template <class ELFT> class OutputSection;
 
 // A chunk corresponding a section of an input file.
 template <class ELFT> class SectionChunk {
@@ -25,7 +26,7 @@ template <class ELFT> class SectionChunk {
   typedef llvm::object::Elf_Rel_Impl<ELFT, false> Elf_Rel;
 
 public:
-  SectionChunk(llvm::object::ELFFile<ELFT> *Obj, const Elf_Shdr *Header);
+  SectionChunk(ObjectFile<ELFT> *F, const Elf_Shdr *Header);
 
   // Returns the size of this chunk (even if this is a common or BSS.)
   size_t getSize() const { return Header->sh_size; }
@@ -36,6 +37,9 @@ public:
 
   StringRef getSectionName() const;
   const Elf_Shdr *getSectionHdr() const { return Header; }
+  ObjectFile<ELFT> *getFile() { return File; }
+  void setOutputSection(OutputSection<ELFT> *S) { OS = S; }
+  OutputSection<ELFT> *getOutputSection() { return OS; }
 
   // The writer sets and uses the addresses.
   uint64_t getOutputSectionOff() { return OutputSectionOff; }
@@ -50,8 +54,11 @@ private:
   // The alignment of this chunk. The writer uses the value.
   uint32_t Align = 1;
 
-  // A file this chunk was created from.
-  llvm::object::ELFFile<ELFT> *Obj;
+  // The file this chunk was created from.
+  ObjectFile<ELFT> *File;
+
+  // The OutputSection this is assigned to.
+  OutputSection<ELFT> *OS;
 
   const Elf_Shdr *Header;
 };
diff --git a/ELF/InputFiles.cpp b/ELF/InputFiles.cpp
index e319fcd..4820716 100644
--- a/ELF/InputFiles.cpp
+++ b/ELF/InputFiles.cpp
@@ -40,22 +40,33 @@ template <class ELFT> void elf2::ObjectFile<ELFT>::parse() {
 template <class ELFT> void elf2::ObjectFile<ELFT>::initializeChunks() {
   uint64_t Size = ELFObj->getNumSections();
   Chunks.reserve(Size);
+  SparseChunks.reserve(Size);
   for (const Elf_Shdr &Sec : ELFObj->sections()) {
+    SectionChunk<ELFT> *SC = nullptr;
     switch (Sec.sh_type) {
     case SHT_SYMTAB:
       Symtab = &Sec;
       break;
     case SHT_STRTAB:
     case SHT_NULL:
+      break;
     case SHT_RELA:
-    case SHT_REL:
+    case SHT_REL: {
+      auto RelocatedSec = ELFObj->getSection(Sec.sh_info);
+      if (!RelocatedSec)
+        error(RelocatedSec);
+      RelocMap.emplace_back(*RelocatedSec, &Sec);
       break;
+    }
     default:
-      auto *C = new (Alloc) SectionChunk<ELFT>(this->getObj(), &Sec);
-      Chunks.push_back(C);
+      SC = new (Alloc) SectionChunk<ELFT>(this, &Sec);
+      Chunks.push_back(SC);
       break;
     }
+    SparseChunks.push_back(SC);
   }
+  assert(SparseChunks.size() == Size && "Not all chunks added to SparseChunks");
+  std::sort(RelocMap.begin(), RelocMap.end());
 }
 
 template <class ELFT> void elf2::ObjectFile<ELFT>::initializeSymbols() {
@@ -87,7 +98,8 @@ SymbolBody *elf2::ObjectFile<ELFT>::createSymbolBody(StringRef StringTable,
   case STB_GLOBAL:
     if (Sym->isUndefined())
       return new (Alloc) Undefined(Name);
-    return new (Alloc) DefinedRegular(Name);
+    return new (Alloc)
+        DefinedRegular<ELFT>(Name, Sym, SparseChunks[Sym->st_shndx]);
   case STB_WEAK:
     if (Sym->isUndefined())
       return new (Alloc) UndefinedWeak(Name);
diff --git a/ELF/InputFiles.h b/ELF/InputFiles.h
index b916789..873194b 100644
--- a/ELF/InputFiles.h
+++ b/ELF/InputFiles.h
@@ -11,6 +11,7 @@
 #define LLD_ELF_INPUT_FILES_H
 
 #include "Chunks.h"
+#include "Error.h"
 #include "lld/Core/LLVM.h"
 #include "llvm/Object/ELF.h"
 
@@ -67,6 +68,21 @@ template <class ELFT> class ObjectFile : public ObjectFileBase {
   typedef typename llvm::object::ELFFile<ELFT>::Elf_Sym_Range Elf_Sym_Range;
 
 public:
+  struct RelocMapInfo {
+    RelocMapInfo(const Elf_Shdr *S, const Elf_Shdr *RS)
+        : Section(S), RelocSection(RS) {}
+
+    const Elf_Shdr *Section;
+    const Elf_Shdr *RelocSection;
+
+    bool operator<(const RelocMapInfo &Other) const {
+      // Pointer comparison is stable here as all pointers in a given RelocMap
+      // point to a single memory mapped file.
+      return std::tie(Section, RelocSection) <
+             std::tie(Other.Section, Other.RelocSection);
+    }
+  };
+
   bool isCompatibleWith(const ObjectFileBase &Other) const override;
 
   static Kind getKind() {
@@ -84,12 +100,23 @@ public:
 
   explicit ObjectFile(MemoryBufferRef M) : ObjectFileBase(getKind(), M) {}
   void parse() override;
+  const std::vector<RelocMapInfo> &getRelocations() const { return RelocMap; }
+
+  SymbolBody *getSymbolBody(uint32_t SymbolIndex) {
+    // Skip null symbol.
+    SymbolIndex -= 1;
+    if (SymbolIndex >= SymbolBodies.size())
+      error("Invalid symbol table index.");
+    return SymbolBodies[SymbolIndex]->getReplacement();
+  }
 
   // Returns the underying ELF file.
   llvm::object::ELFFile<ELFT> *getObj() const { return ELFObj.get(); }
 
   ArrayRef<SectionChunk<ELFT> *> getChunks() { return Chunks; }
 
+  const Elf_Shdr *getSymtab() const { return Symtab; }
+
 private:
   void initializeChunks();
   void initializeSymbols();
@@ -102,6 +129,13 @@ private:
   std::vector<SectionChunk<ELFT> *> Chunks;
 
   const Elf_Shdr *Symtab = nullptr;
+  // Section index to SectionChunk map. Unmapped sections are nullptr.
+  std::vector<SectionChunk<ELFT> *> SparseChunks;
+
+  // This vector contains a sorted list of <section, relocation section that
+  // applies to section> pairs. This is used to quickly find and iterate over
+  // the relocations for a given section.
+  std::vector<RelocMapInfo> RelocMap;
 };
 
 } // namespace elf2
diff --git a/ELF/Symbols.cpp b/ELF/Symbols.cpp
index ecf6d23..3b404f7 100644
--- a/ELF/Symbols.cpp
+++ b/ELF/Symbols.cpp
@@ -17,6 +17,11 @@ using namespace llvm::object;
 using namespace lld;
 using namespace lld::elf2;
 
+template <class ELFT>
+DefinedRegular<ELFT>::DefinedRegular(StringRef N, const Elf_Sym *S,
+                                     SectionChunk<ELFT> *C)
+    : Defined(DefinedRegularKind, N), Sym(S), Chunk(C) {}
+
 // Returns 1, 0 or -1 if this symbol should take precedence
 // over the Other, tie or lose, respectively.
 int SymbolBody::compare(SymbolBody *Other) {
@@ -42,3 +47,12 @@ int SymbolBody::compare(SymbolBody *Other) {
   }
   llvm_unreachable("unknown symbol kind");
 }
+
+namespace lld {
+namespace elf2 {
+template class DefinedRegular<llvm::object::ELF32LE>;
+template class DefinedRegular<llvm::object::ELF32BE>;
+template class DefinedRegular<llvm::object::ELF64LE>;
+template class DefinedRegular<llvm::object::ELF64BE>;
+}
+}
diff --git a/ELF/Symbols.h b/ELF/Symbols.h
index 9a3f578..707522d 100644
--- a/ELF/Symbols.h
+++ b/ELF/Symbols.h
@@ -10,6 +10,8 @@
 #ifndef LLD_ELF_SYMBOLS_H
 #define LLD_ELF_SYMBOLS_H
 
+#include "Chunks.h"
+
 #include "lld/Core/LLVM.h"
 #include "llvm/Object/ELF.h"
 
@@ -84,13 +86,25 @@ public:
 };
 
 // Regular defined symbols read from object file symbol tables.
-class DefinedRegular : public Defined {
+template <class ELFT> class DefinedRegular : public Defined {
+  typedef typename llvm::object::ELFFile<ELFT>::Elf_Sym Elf_Sym;
+
 public:
-  explicit DefinedRegular(StringRef N) : Defined(DefinedRegularKind, N) {}
+  explicit DefinedRegular(StringRef N, const Elf_Sym *S, SectionChunk<ELFT> *C);
 
   static bool classof(const SymbolBody *S) {
     return S->kind() == DefinedRegularKind;
   }
+
+  typename llvm::object::ELFFile<ELFT>::uintX_t getValue() const {
+    return Sym->st_value;
+  }
+
+  SectionChunk<ELFT> *getChunk() const { return Chunk; }
+
+private:
+  const Elf_Sym *Sym;
+  SectionChunk<ELFT> *Chunk;
 };
 
 class DefinedWeak : public Defined {
diff --git a/ELF/Writer.cpp b/ELF/Writer.cpp
index 9902cc4..03e0102 100644
--- a/ELF/Writer.cpp
+++ b/ELF/Writer.cpp
@@ -10,6 +10,7 @@
 #include "Chunks.h"
 #include "Config.h"
 #include "Error.h"
+#include "Symbols.h"
 #include "SymbolTable.h"
 #include "Writer.h"
 #include "Symbols.h"
@@ -26,7 +27,8 @@ using namespace lld::elf2;
 
 static const int PageSize = 4096;
 
-namespace {
+namespace lld {
+namespace elf2 {
 // OutputSection represents a section in an output file. It's a
 // container of chunks. OutputSection and Chunk are 1:N relationship.
 // Chunks cannot belong to more than one OutputSections. The writer
@@ -45,6 +47,7 @@ public:
     Header.sh_flags = sh_flags;
   }
   void setVA(uintX_t VA) { Header.sh_addr = VA; }
+  uintX_t getVA() { return Header.sh_addr; }
   void setFileOffset(uintX_t Off) { Header.sh_offset = Off; }
   template <endianness E>
   void writeHeaderTo(typename ELFFile<ELFType<E, Is64Bits>>::Elf_Shdr *SHdr);
@@ -79,7 +82,10 @@ public:
 private:
   std::vector<SectionChunk<ELFT> *> Chunks;
 };
+} // end namespace elf2
+} // end namespace lld
 
+namespace {
 template <bool Is64Bits>
 class StringTableSection final : public OutputSectionBase<Is64Bits> {
   llvm::StringTableBuilder &StrTabBuilder;
@@ -199,9 +205,50 @@ void OutputSection<ELFT>::addChunk(SectionChunk<ELFT> *C) {
   this->Header.sh_size = Off;
 }
 
+template <class ELFT>
+typename llvm::object::ELFFile<ELFT>::uintX_t
+getSymVA(DefinedRegular<ELFT> *DR) {
+  SectionChunk<ELFT> *SC = DR->getChunk();
+  OutputSection<ELFT> *OS = SC->getOutputSection();
+  return OS->getVA() + SC->getOutputSectionOff() + DR->getValue();
+}
+
 template <class ELFT> void OutputSection<ELFT>::writeTo(uint8_t *Buf) {
-  for (SectionChunk<ELFT> *C : Chunks)
+  for (SectionChunk<ELFT> *C : Chunks) {
     C->writeTo(Buf);
+    const std::vector<typename ObjectFile<ELFT>::RelocMapInfo> &Relocs =
+        C->getFile()->getRelocations();
+    auto Hdr = C->getSectionHdr();
+    auto EObj = C->getFile()->getObj();
+    uint8_t *Base = Buf + C->getOutputSectionOff();
+    // Iterate over all relocation sections that apply to this section.
+    for (auto I = std::lower_bound(
+             Relocs.begin(), Relocs.end(),
+             typename ObjectFile<ELFT>::RelocMapInfo(Hdr, nullptr)),
+              E = Relocs.end();
+         I != E && I->Section == Hdr; ++I) {
+      // Only support RELA for now.
+      if (I->RelocSection->sh_type != SHT_RELA)
+        continue;
+      for (auto &RI : EObj->relas(I->RelocSection)) {
+        SymbolBody *Body = C->getFile()
+                               ->getSymbolBody(RI.getSymbol(EObj->isMips64EL()))
+                               ->getReplacement();
+        uintX_t Offset = RI.r_offset;
+        uint32_t Type = RI.getType(EObj->isMips64EL());
+        uintX_t P = this->getVA() + C->getOutputSectionOff();
+        uintX_t SymVA = getSymVA<ELFT>(cast<DefinedRegular<ELFT>>(Body));
+        switch (Type) {
+        case llvm::ELF::R_X86_64_PC32:
+          support::endian::write32le(Base + Offset,
+                                     SymVA + (RI.r_addend - (P + Offset)));
+          break;
+        default:
+          error(Twine("unrecognized reloc ") + Twine(Type));
+        }
+      }
+    }
+  }
 }
 
 template <bool Is64Bits>
@@ -297,6 +344,7 @@ template <class ELFT> void Writer<ELFT>::createSections() {
         OutputSections.push_back(Sec);
       }
       Sec->addChunk(C);
+      C->setOutputSection(Sec);
     }
   }
 }
diff --git a/test/elf2/relocation.s b/test/elf2/relocation.s
new file mode 100644
index 0000000..37c4431
--- /dev/null
+++ b/test/elf2/relocation.s
@@ -0,0 +1,15 @@
+// RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t
+// RUN: lld -flavor gnu2 %t -o %t2
+// RUN: llvm-objdump -d %t2 | FileCheck %s
+// REQUIRES: x86
+
+
+.section       .text,"ax", at progbits,unique,1
+.global _start
+_start:
+  call lulz
+
+.section       .text,"ax", at progbits,unique,2
+lulz:
+
+// CHECK: e8 00 00 00 00  callq   0


More information about the llvm-commits mailing list