[PATCH] D13589: [ELF2] - Fixed PLT rellocations types for X86 and X86_64 targets

[Rui Ueyama via llvm-commits]

ruiu accepted this revision.
ruiu added a comment.
This revision is now accepted and ready to land.

LGTM with a nit.


================
Comment at: ELF/OutputSections.cpp:172-201
@@ -171,28 +171,32 @@
 
     bool CanBePreempted = canBePreempted(Body);
     uintX_t Addend = 0;
     if (!CanBePreempted) {
       if (IsRela) {
         if (Body)
           Addend += getSymVA<ELFT>(cast<ELFSymbolBody<ELFT>>(*Body));
         else
           Addend += getLocalSymVA(
               Obj.getRelocationSymbol(&RI, File.getSymbolTable()), File);
       }
       P->setSymbolAndType(0, Target->getRelativeReloc(), IsMips64EL);
     }
 
     if (Body && Target->relocNeedsGot(Type, *Body)) {
       P->r_offset = Out<ELFT>::Got->getEntryAddr(*Body);
-      if (CanBePreempted)
+      if (CanBePreempted) {
+        bool NeedsPlt = Target->relocNeedsPlt(Type, *Body);
         P->setSymbolAndType(Body->getDynamicSymbolTableIndex(),
-                            Target->getGotReloc(), IsMips64EL);
+                            NeedsPlt ? Target->getPltReloc()
+                                     : Target->getGotReloc(),
+                            IsMips64EL);
+      }
     } else {
       if (IsRela)
         Addend += static_cast<const Elf_Rela &>(RI).r_addend;
       P->r_offset = RI.r_offset + C.getOutputSectionOff() + OutSec->getVA();
       if (CanBePreempted)
         P->setSymbolAndType(Body->getDynamicSymbolTableIndex(), Type,
                             IsMips64EL);
     }
 
----------------
Overall the conditions in this function are getting complicated over time. We need to simplify that. (It's a TODO for me, so this change is okay.)

================
Comment at: ELF/Target.cpp:206
@@ -203,2 +205,3 @@
   // GotReloc = FIXME
+  // PltReloc = FIXME
   PltEntrySize = 32;
----------------
Don't have to add fixmes. I'd remove them instead (but not in this patch).


http://reviews.llvm.org/D13589