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

[Rui Ueyama via llvm-commits]

ruiu added inline comments.

================
Comment at: ELF/OutputSections.cpp:806
@@ +805,3 @@
+                                   SymbolBody &S) {
+  ThunkChunk<ELFT> *&TC = ThunkChunks[&C];
+  if (!TC)
----------------
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.

================
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();
 
----------------
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?


Repository:
  rL LLVM

http://reviews.llvm.org/D17934