[PATCH] D17934: [ELF] Implement infrastructure for thunk code creation

[Simon Atanasyan via llvm-commits]

atanasyan added inline comments.

================
Comment at: ELF/OutputSections.cpp:806
@@ +805,3 @@
+                                   SymbolBody &S) {
+  ThunkChunk<ELFT> *&TC = ThunkChunks[&C];
+  if (!TC)
----------------
ruiu wrote:
> atanasyan wrote:
> > ruiu wrote:
> > > Using a hash table feels a bit tricky. Can you add a pointer to SymbolBody to point to a thunk?
> > I need a link between input section and corresponding set of thunks to put the thunks in the right place during a "write" phase. I can add pointer to `ThunkChunk` to the `InputSectionBase` class. That will allow to remove this `DenseMap`. Is it acceptable to you?
> I think that instead of attaching a chunk to an InputSection, we may be able to define a new type of (synthesized) InputSection for a thunk. Then we can keep OutputSection to have a flat list of InputSections instead of InputSections + their attachment. It may be cleaner approach IMO.
> 
> In that design, we can make a symbol have a pointer to a thunk input section. getThunkVA() returns the VA of the thunk input section, so it just have to call getVA on the thunk section.
> 
> What do you think? I'm not sure if this is really cleaner, but I'd think it's worth to try.
Now the root of input section hierarchy `InputSectionBase` class is tightly related to ELF input section. Its constructor requires reference to `ObjectFile` and `Elf_Shdr`. If we want to introduce something like `SyntheticInputSection` which is unrelated to and file and ELF header we have to create new root base class and inherit `SyntheticInputSection` and `InputSectionBase`. One more problem is that `OutputSection::Sections` container contains pointers to `InputSection` objects and it is not clear how to mix `InputSection` and `SyntheticInputSection` in the same container.

================
Comment at: ELF/Writer.cpp:1068-1072
@@ -1056,2 +1067,7 @@
         CopyRelSymbols.push_back(SC);
+    if (auto *SS = dyn_cast<DefinedSynthetic<ELFT>>(Body))
+      // Setup values of synthetic symbols that need
+      // to point to end of a section.
+      if (SS->Value == DefinedSynthetic<ELFT>::End)
+        SS->Value = SS->Section.getSize();
 
----------------
ruiu wrote:
> atanasyan wrote:
> > ruiu wrote:
> > > I don't understand what this is supposed to do?
> > This and the next change are related. Now we add synthetic symbols pointing to beginning and end of output sections before relocation scanning phase. In this patch output section size might change during the relocation scanning. So synthetic symbols point to the end of sections need to be updated. There is a couple of possible solutions:
> >   - Add synthetic symbols after the relocation scanning. Not sure that this does not affect something in relocation scanning phase.
> >   - Use special value `DefinedSynthetic<ELFT>::End` to mark symbol that need to point to the end of sections. Update such symbols with the correct value later.
> > In this patch I use the second solution. Do you think it is safe to add synthetic symbols after the relocation scanning?
> OK. So if we keep the output section to have a flat list of input sections instead of input sections + their chunks, the size of the output section wouldn't change during relocation scanning phase, right?
Before the scanning phase we know nothing about thunks. During the scanning we either add chunks to OutputSections or create synthetic input sections and add them to OutputSections too. In any case the size will change.


Repository:
  rL LLVM

http://reviews.llvm.org/D17934