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

[Eugene Leviant via llvm-commits]

evgeny777 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;
----------------
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?

================
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));
 }
----------------
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.


================
Comment at: ELF/LinkerScript.h:155
@@ -153,2 +154,3 @@
   uintX_t Dot;
+  std::vector<std::unique_ptr<InputSectionBase<ELFT>>> Synthetics;
 };
----------------
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.

================
Comment at: ELF/Writer.cpp:737
@@ +736,3 @@
+  // Should be called after assignOffsets().
+  Script<ELFT>::X->addSyntheticSymbols();
+
----------------
ruiu wrote:
> So, does this work? Adding new symbols changes the size of .symtab and .strtab, no?
You're right, this is a bug. But I think this can be fixed easily by calling addSynthetic in createSections(), and setting symbol values in addSyntheticSymbols().
If the whole approach is okay and there is no need for radical changes then I'll certainly do this.


https://reviews.llvm.org/D22683