[PATCH] D27040: [ELF] - Add support for access to most of synthetic sections from linkerscript.

[George Rimar via llvm-commits]

grimar added inline comments.


================
Comment at: ELF/Writer.cpp:230
+  Symtab<ELFT>::X->Sections.push_back(Targ);
+};
+
----------------
evgeny777 wrote:
> This looks better
> 
> ```
> auto add = [](InputSection<ELFT> *IS) {
>   Symtab<ELFT>::X->Sections.push_back(IS);
>   return IS;
> };
> ```
> 
Unfortunately this will not work. You can not assign base class pointer to derived.
I applied the idea of your suggestion though.


================
Comment at: ELF/Writer.cpp:723
   for (InputSectionBase<ELFT> *IS : Symtab<ELFT>::X->Sections) {
-    if (!IS->Live)
+    if (!IS || !IS->Live)
       continue;
----------------
evgeny777 wrote:
> Why are you checking IS for nullptr?
Forgot to remove after experiments, thanks.


================
Comment at: ELF/Writer.cpp:883
+    SyntheticSection<ELFT> *SS = dyn_cast<SyntheticSection<ELFT>>(*I);
+    if (!SS)
+      return;
----------------
evgeny777 wrote:
> Why are you checking for null?
Because Symtab<ELFT>::X->Sections contains all sections, not only synthetic.
And that works like optimization.

I am iterating from the end to start and stop doing that when met non-synthetic section,
which will be null after cast.


================
Comment at: ELF/Writer.cpp:889
+    OutputSection<ELFT> *OutSec = cast<OutputSection<ELFT>>(SS->OutSec);
+    OutSec->Sections.erase(
+        std::find(OutSec->Sections.begin(), OutSec->Sections.end(), SS),
----------------
evgeny777 wrote:
> Looks like you're removing all sections starting from unused synthetic till the end of the input section array. May be you actually wanted to use std::remove() ?
It is a mistake definetely. Wanted to use std::erase(Iterator) form, like below.


https://reviews.llvm.org/D27040