[PATCH] D41640: [ELF] - Do not ignore discarding of .rela.plt/.rela.dyn, allow doing custom layout for them.

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: ELF/LinkerScript.cpp:293-294
       // which are common because they are in the default bfd script.
-      if (Sec->Type == SHT_REL || Sec->Type == SHT_RELA)
+      // We do not ignore SHT_REL[A] linker-synthesized sections here because
+      // want to support scripts that do custom layout for them.
+      if (!isa<SyntheticSection>(Sec) &&
----------------
ruiu wrote:
> It is not still clear to me what this comment means. It's because you cannot discard linker-synthesized sections, right?
Generally it should be safe to discard some of synthesized sections.

But problem is that with current code we silently ignore script lines that do `/DISCARD/ : { *(.rela.plt)` for example,
because `rela.plt` is synthetic SHT_REL[A] section , and all SHT_REL[A] sections currently are always proccessed as orphans.
What also means code like `.foo : { *(.rela.dyn) }` silently does not work for them either.

Here in comment I mean that we should not ignore doing custom layout for synthetic sections. By custom layout I mean
actions like above (placing to different output section, discarding, etc manipulations allowed for other sections).

Given above should we allow discarding of .rela.plt/.rela.dyn or not is a bit different question. I think it does not makes much sence
to discard them in general, so I think logic implemented is OK. At the other hand we could still allow doing that if script requests it explicitly.
(so we could remove changes from `discard-section-err.s` and from `LinkerScript::discard` introduced, leaving all others).


================
Comment at: ELF/LinkerScript.cpp:320-321
   for (InputSection *S : V) {
     if (S == InX::ShStrTab || S == InX::Dynamic || S == InX::DynSymTab ||
-        S == InX::DynStrTab)
+        S == InX::DynStrTab || S == InX::RelaPlt || S == InX::RelaDyn)
       error("discarding " + S->Name + " section is not allowed");
----------------
ruiu wrote:
> Can this just be isa<SyntheticSection>?
I think no. Because for example with -strip-all we discard .symtab and .strtab during regular link.
I think that means we should be able to `/DISCARD/` them from script and so should have white list here.

(The same applies for InX::BuildId, for example, it should be safe to discard it, so probably not useful to disallow it).


https://reviews.llvm.org/D41640