[PATCH] D14955: [ELF] - Lazy relocations support for x86 target.

George Rimar via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 25 12:17:37 PST 2015


grimar added inline comments.

================
Comment at: ELF/Writer.cpp:248-250
@@ -247,2 +247,5 @@
           continue;
-        Out<ELFT>::Plt->addEntry(Body);
+        unsigned RelOff = Target->supportsLazyRelocations()
+                              ? Out<ELFT>::RelaPlt->getRelocOffset()
+                              : Out<ELFT>::RelaDyn->getRelocOffset();
+        Out<ELFT>::Plt->addEntry(Body, RelOff);
----------------
ruiu wrote:
> grimar wrote:
> > ruiu wrote:
> > > This piece of code does not depends on any local context of Writer.cpp, so I think you can move this into a function that handles i386 PLT entries instead of propagating this unsigned value all the way to the consumer.
> > What I need is a accordance of PLT entry and the offset in relocation table for its relocation. **It can be calculated only here I think.**
> > 
> > That its only needed for x86 now, thats true. But what is the way to move it from here ? Function that handles i386 PLT entries is 
> > 
> > ```
> > writePltEntry(uint8_t *Buf, uint64_t GotAddr, uint64_t GotEntryAddr, uint64_t PltEntryAddr, int32_t Index, unsigned RelOff).
> > ```
> > It does not know anything about conformity of relocations and PLT entries.
> > I added RelOff for it since I dont know the easier/shorter way to do what is needed.
> I think I don't understand what you mean. What I wanted to say is since no local variables are used in the following expression, you can move this code to somewhere else.
> 
>   unsigned RelOff = Target->supportsLazyRelocations()
>                         ? Out<ELFT>::RelaPlt->getRelocOffset()
>                         : Out<ELFT>::RelaDyn->getRelocOffset();
I understood what you want.
But I think it is only can be moved to 
```
template <class ELFT> void PltSection<ELFT>::addEntry(SymbolBody *Sym)
```
from here. It is anyways common code and not just i386 specific.

Point is that getRelocOffset() returns current offset in relocation table, so it needs to be remembered for each added Plt entry. Because 
each Out<ELFT>::RelaPlt->addReloc({&C, &RI}); call below changes it.


http://reviews.llvm.org/D14955





More information about the llvm-commits mailing list