[PATCH] D22683: [ELF] Symbol assignment within input section list

[Rui Ueyama via llvm-commits]

ruiu added inline comments.

================
Comment at: ELF/LinkerScript.cpp:409-410
@@ +408,4 @@
+    if ((!B && !SS->Cmd->Provide) || (B && B->isUndefined())) {
+      Symbol *S = Symtab<ELFT>::X->addSynthetic(
+          SS->Cmd->Name, SS->OutSec, SS->Cmd->Expression(SS->OutSecOff));
+      S->Visibility = SS->Cmd->Hidden ? STV_HIDDEN : STV_DEFAULT;
----------------
evgeny777 wrote:
> ruiu wrote:
> > Now you have a dummy input section. Is there any reason to use Synthetic symbols instead of Regular symbols?
> I think its possible to add regular symbol to this fake input section and this might work as well. But is this any easier?
I guess it makes things easier because if you attach symbols to input sections, their final offsets are automatically adjusted as input sections get final offsets. That means you can add symbols earlier.

================
Comment at: ELF/LinkerScript.cpp:813-816
@@ -755,1 +812,6 @@
+  else
+    // The section definition contains only symbols or nothing at all.
+    // Make all symbols global and discard the section.
+    std::move(Cmd->Commands.begin(), Cmd->Commands.end(),
+              std::back_inserter(Opt.Commands));
 }
----------------
evgeny777 wrote:
> ruiu wrote:
> > Why do you need to handle this case as a special case?
> You can have script like this:
> 
> ```
> SECTIONS { .dummy { a = .; b = . + 2; } }
> ```
> 
> There are no input sections, so output section will never be created, so you can't add symbols anywhere.
> 
Is it a real use case? I'm inclined not to add code to handle special cases.

================
Comment at: ELF/LinkerScript.h:155
@@ -153,2 +154,3 @@
   uintX_t Dot;
+  std::vector<std::unique_ptr<InputSectionBase<ELFT>>> Synthetics;
 };
----------------
evgeny777 wrote:
> ruiu wrote:
> > It seems you are storing only unique_ptr<SymbolInputSection> to this vector. Why don't you use SymbolInputSection instead of InputSectionBase?
> Just because I wanted to keep this class local to LinkerScript without any forward declarations in header file. But can change this of course.
I think adding a forward declaration would be better.


https://reviews.llvm.org/D22683