[PATCH] [lld][ELF] Add support for IFUNC.

[Michael Spencer]

================
Comment at: lib/ReaderWriter/ELF/DefaultELFLayout.h:255-263
@@ -253,1 +254,11 @@
 
+  ELFRelocationTable<ELFT> *getRelocationTable() {
+    // Only create the relocation table if it is needed.
+    if (!_relocationTable) {
+      _relocationTable =
+          new (_allocator) ELFRelocationTable<ELFT>(".rela.plt", ORDER_REL);
+      addSection(_relocationTable);
+    }
+    return _relocationTable;
+  }
+
----------------
Shankar Kalpathi Easwaran wrote:
> This would be in the targetHandler after WriterELF adds code to call the targetHandler right ?
Yes. We also need to add some support for selecting which relocation table to get for dynamic libraries.

================
Comment at: lib/ReaderWriter/ELF/DefaultELFLayout.h:329-332
@@ -310,2 +328,6 @@
     return ".bss";
+  if (contentType == DefinedAtom::typeGOT)
+    return ".got";
+  if (contentType == DefinedAtom::typeStub)
+    return ".plt";
   if (name.startswith(".text"))
----------------
Shankar Kalpathi Easwaran wrote:
> Shankar Kalpathi Easwaran wrote:
> > Is there a need to distinguish the .got/.plt for IRELATIVE relocations from actual .got/.plt ?
> If the GOTAtom/PLT atom have defined a function to return SectionName, this wouldnt be necessary. I think that would be cleaner.
K

================
Comment at: lib/ReaderWriter/ELF/DefaultELFLayout.h:352
@@ -329,2 +351,3 @@
   case ORDER_DYNAMIC_STRINGS:
+  case ORDER_REL:
   case ORDER_INIT:
----------------
Shankar Kalpathi Easwaran wrote:
> Does binutils, create a seperate segment for ORDER_REL ?
No. It sticks it in front of the .init section and loads it as executable. I assume this is for performance so it's near all the initialization code. I didn't feel like making it executable so I load it with read only data.

================
Comment at: lib/ReaderWriter/ELF/ELFSectionChunks.h:304-305
@@ -296,1 +303,4 @@
 Section<ELFT>::type() {
+  if (_sectionKind == K_SymbolTable)
+    return llvm::ELF::SHT_SYMTAB;
+
----------------
Shankar Kalpathi Easwaran wrote:
> Couldnt follow on why is this needed ?
Because typeResolver is 2. _contentType is being used for both the content type and llvm::ELF::SHT_ values.

================
Comment at: lib/ReaderWriter/ELF/ELFSectionChunks.h:725-726
@@ -702,1 +724,4 @@
 
+template <class ELFT> class ELFRelocationTable : public Section<ELFT> {
+public:
+  typedef llvm::object::Elf_Rel_Impl<ELFT, true> Elf_Rela;
----------------
Shankar Kalpathi Easwaran wrote:
> Should this class be renamed differently, since its main purpose is to support IRELATIVE relocations ?
That's currently its only purpose. It it will need to support other dynamic relocation types once we get to that.

================
Comment at: lib/ReaderWriter/ELF/FileELF.h:55-56
@@ -54,1 +54,4 @@
       : File(MB->getBufferIdentifier()), _elfTargetInfo(ti) {
+    static uint32_t lastOrdinal = 0;
+    _ordinal = lastOrdinal++;
+
----------------
Shankar Kalpathi Easwaran wrote:
> Wasnt ordinals supposed to be set by OrderPass ?
Not sure. Without this (and without adding the OrderPass) it hits an assert trying to access the ordinal.

================
Comment at: lib/ReaderWriter/ELF/WriterELF.cpp:159-160
@@ -158,2 +158,4 @@
   _runtimeFile.addAbsoluteAtom("__init_array_end");
+  _runtimeFile.addAbsoluteAtom("__rela_iplt_start");
+  _runtimeFile.addAbsoluteAtom("__rela_iplt_end");
 }
----------------
Shankar Kalpathi Easwaran wrote:
> I think even this will go into each Target once Writer hooks are being called into the target right ?
Yes.

================
Comment at: lib/ReaderWriter/ELF/WriterELF.cpp:179-181
@@ -176,12 +178,5 @@
  auto initArrayStartIter = _layout->findAbsoluteAtom("__init_array_start");
  auto initArrayEndIter = _layout->findAbsoluteAtom("__init_array_end");
-
- auto section = _layout->findOutputSection(".init_array");
- if (section) {
-   initArrayStartIter->setValue(section->virtualAddr());
-   initArrayEndIter->setValue(section->virtualAddr() +
-                              section->memSize());
- } else {
-   initArrayStartIter->setValue(0);
-   initArrayEndIter->setValue(0);
- }
+  auto realIpltStartIter = _layout->findAbsoluteAtom("__rela_iplt_start");
+  auto realIpltEndIter = _layout->findAbsoluteAtom("__rela_iplt_end");
+
----------------
Shankar Kalpathi Easwaran wrote:
> Is there an indentation issue ?
Fixed.

================
Comment at: lib/ReaderWriter/ELF/X86_64/X86_64TargetInfo.cpp:89-91
@@ +88,5 @@
+
+class PLTPass : public Pass {
+public:
+  PLTPass(const ELFTargetInfo &ti) : _file(ti) {}
+
----------------
Shankar Kalpathi Easwaran wrote:
> should the class be changed to IPLT to refer to IFUNC ?
It will be expanded to handle other PLT accesses.

================
Comment at: lib/ReaderWriter/ELF/X86_64/X86_64TargetInfo.cpp:131
@@ -16,2 +130,3 @@
+
 #define LLD_CASE(name) .Case(#name, llvm::ELF::name)
 
----------------
Shankar Kalpathi Easwaran wrote:
> Should this go into a generic header ? I think every other code for relcations are going to use the same.
It would need a better name.

================
Comment at: lib/ReaderWriter/ELF/X86_64Reference.cpp:115-120
@@ -114,2 +114,8 @@
                                    uint64_t targetAddress) {
+  if (reloc == llvm::ELF::R_X86_64_IRELATIVE ||
+      reloc == llvm::ELF::R_X86_64_GOTPCREL ||
+      reloc == llvm::ELF::R_X86_64_PLT32 ||
+      reloc == llvm::ELF::R_X86_64_GOTTPOFF ||
+      reloc == llvm::ELF::R_X86_64_TPOFF32)
+    return;
   if (_fixupHandler[reloc])
----------------
Shankar Kalpathi Easwaran wrote:
> should you have an assert here, than return 0 ?
This wasn't supposed to make it into this patch. This was just stuff to make glibc "work".

The IRELATIVE check is supposed to be here, as it's handled elsewhere.


http://llvm-reviews.chandlerc.com/D324