[PATCH] D15235: [ELF] - Implemented R_*_IRELATIVE relocations for x86, x64 targets.

[Rui Ueyama via llvm-commits]

ruiu added inline comments.

================
Comment at: ELF/OutputSections.cpp:279-283
@@ -278,4 +278,7 @@
                             Config->Mips64EL);
     } else {
-      P->setSymbolAndType(0, Target->getRelativeReloc(), Config->Mips64EL);
+      unsigned RelReloc = (Body && isGnuIfunc<ELFT>(*Body))
+                              ? Target->getIRelativeReloc()
+                              : Target->getRelativeReloc();
+      P->setSymbolAndType(0, RelReloc, Config->Mips64EL);
     }
----------------
  } else if (Body && isGnuIfunc<ELFT>(*Body)) {
    P->setSymbolAndType(0, Target->getIRelativeReloc(), Config->Mips64EL);
  } else {
    P->setSymbolAndType(0, Target->getRelativeReloc(), Config->Mips64EL);
  }

================
Comment at: ELF/OutputSections.cpp:325-326
@@ -321,3 +324,4 @@
 template <class ELFT> void RelocationSection<ELFT>::finalize() {
-  this->Header.sh_link = Out<ELFT>::DynSymTab->SectionIndex;
+  this->Header.sh_link = Static ? Out<ELFT>::SymTab->SectionIndex
+                                : Out<ELFT>::DynSymTab->SectionIndex;
   this->Header.sh_size = Relocs.size() * this->Header.sh_entsize;
----------------
Is this change related to IRELATIVE relocations?

================
Comment at: ELF/Symbols.h:194
@@ +193,3 @@
+  // where R_[*]_IRELATIVE relocations do live.
+  static Elf_Sym RelaIpltStart, RelaIpltEnd;
+
----------------
  static Elf_Sym RelaIpltStart;
  static Elf_Sym RelaIpltEnd;


================
Comment at: ELF/Target.h:103
@@ -100,1 +102,3 @@
 
+template <class ELFT> bool isGnuIfunc(const SymbolBody &S);
+
----------------
 isGnuIfunc -> isGnuIFunc

================
Comment at: ELF/Writer.cpp:701
@@ +700,3 @@
+  // If RelaPlt is empty then there is no reason to create this symbols.
+  if (!isOutputDynamic() && Out<ELFT>::RelaPlt &&
+      Out<ELFT>::RelaPlt->hasRelocs()) {
----------------
Can you factor this new code out as a new function?

================
Comment at: ELF/Writer.cpp:704
@@ +703,3 @@
+    bool IsRela = Symtab.shouldUseRela();
+    StringRef IpltS = IsRela ? "__rela_iplt_start" : "__rel_iplt_start";
+    if (SymbolBody *B = Symtab.find(IpltS)) {
----------------
IpltS -> Start

================
Comment at: ELF/Writer.cpp:705
@@ +704,3 @@
+    StringRef IpltS = IsRela ? "__rela_iplt_start" : "__rel_iplt_start";
+    if (SymbolBody *B = Symtab.find(IpltS)) {
+      if (B->isUndefined())
----------------
Remove{}

================
Comment at: ELF/Writer.cpp:709
@@ +708,3 @@
+    }
+    StringRef IpltE = IsRela ? "__rela_iplt_end" : "__rel_iplt_end";
+    if (SymbolBody *B = Symtab.find(IpltE)) {
----------------
IpltE -> End

================
Comment at: ELF/Writer.cpp:710
@@ +709,3 @@
+    StringRef IpltE = IsRela ? "__rela_iplt_end" : "__rel_iplt_end";
+    if (SymbolBody *B = Symtab.find(IpltE)) {
+      if (B->isUndefined())
----------------
Remove{}

================
Comment at: ELF/Writer.cpp:771
@@ -743,1 +770,3 @@
 
+  // rel[a].plt can contain R_[*]_IRELATIVE relocations when static linking.
+  if (Out<ELFT>::RelaPlt && Out<ELFT>::RelaPlt->hasRelocs()) {
----------------
Is that only when static linking?

================
Comment at: ELF/Writer.cpp:993
@@ +992,3 @@
+  if (Out<ELFT>::RelaPlt) {
+    uintX_t RVA = Out<ELFT>::RelaPlt->getVA();
+    DefinedAbsolute<ELFT>::RelaIpltStart.st_value = RVA;
----------------
Is this relative? If not, please remove "R" from "RVA".


http://reviews.llvm.org/D15235