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

[grimar]

Summary

Currently when we build input sections list in linker script
we ignore all rel[a] sections. That was done to support
scripts like .rela.dyn : { *(.rela.data) } for emit relocs.

Though as a result following scripts were also silently ignored:

/DISCARD/ : { *(.rela.plt)
/DISCARD/ : { *(.rela.dyn)

and we produced output with this sections. That is not ideal.
Solution this patch suggests is simple: do not ignore synthetic
rel[a] sections. That way we can enable common discarding logic
for them and report proper error.

[ruiu]

ELF/LinkerScript.cpp

293 ↗

(On Diff #128356)

Obviously this needs comment because without it it is very hard to figure out what this isa for.

[grimar]

• Addressed review comment.

[ruiu]

ELF/LinkerScript.cpp

293–295 ↗

(On Diff #129062)

I think I don't understand this comment. How is /DISCARD/ related to the synthetic section?

[ruiu]

ELF/LinkerScript.cpp

293–295 ↗

(On Diff #129062)

Then please update the comment. This comment can be understood only when you know ".rela.dyn" is a linker-synthesized section, but describing that is not the point of this particular comment.

[grimar]

• Changed comment.
• Added testcase showing we are able to do custom layout for synthetic .rel[a] sections with this patch.

[ruiu]

ELF/LinkerScript.cpp

293–294 ↗

(On Diff #129228)

It is not still clear to me what this comment means. It's because you cannot discard linker-synthesized sections, right?

320–321 ↗

(On Diff #129228)

Can this just be isa<SyntheticSection>?

[grimar]

ELF/LinkerScript.cpp

293–294 ↗

(On Diff #129228)

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).

320–321 ↗

(On Diff #129228)

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).

[grimar]

Ping.

[grimar]

Ping.

[grimar]

Ping?

[• espindola]

LGTM with a nit.

test/ELF/linkerscript/synthetic-relsec-layout.s

4 ↗

(On Diff #129228)

Do you need --emit-relocs in this test?

lld/trunk/ELF/LinkerScript.cpp

for (const SectionPattern &Pat : Cmd->SectionPatterns) {
 
     for (InputSectionBase *Sec : InputSections) {
       if (!Sec->Live || Sec->Assigned)
         continue;
 
       // For -emit-relocs we have to ignore entries like
       //   .rela.dyn : { *(.rela.data) }
       // 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) &&
+          (Sec->Type == SHT_REL || Sec->Type == SHT_RELA))
         continue;
 
       std::string Filename = getFilename(Sec->File);
       if (!Cmd->FilePat.match(Filename) ||
           Pat.ExcludedFilePat.match(Filename) ||
           !Pat.SectionPat.match(Sec->Name))
         continue;
 
       // It is safe to assume that Sec is an InputSection
       // because mergeable or EH input sections have already been
       // handled and eliminated.
       Ret.push_back(cast<InputSection>(Sec));
       Sec->Assigned = true;
     }
 
     sortInputSections(MutableArrayRef<InputSection *>(Ret).slice(SizeBefore),
                       Pat);
   }
   return Ret;
 }
 
 void LinkerScript::discard(ArrayRef<InputSection *> V) {
   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");
 
     // You can discard .hash and .gnu.hash sections by linker scripts. Since
     // they are synthesized sections, we need to handle them differently than
     // other regular sections.
     if (S == InX::GnuHashTab)
       InX::GnuHashTab = nullptr;
     if (S == InX::HashTab)

lld/trunk/test/ELF/linkerscript/discard-section-err.s

 # RUN:   FileCheck -check-prefix=DYNSYM %s
 # DYNSYM: discarding .dynsym section is not allowed
 
 # RUN: echo "SECTIONS { /DISCARD/ : { *(.dynstr) } }" > %t.script
 # RUN: not ld.lld -pie -o %t --script %t.script %t.o 2>&1 | \
 # RUN:   FileCheck -check-prefix=DYNSTR %s
 # DYNSTR: discarding .dynstr section is not allowed
 
+# RUN: echo "SECTIONS { /DISCARD/ : { *(.rela.plt) } }" > %t.script
+# RUN: not ld.lld -pie -o %t --script %t.script %t.o 2>&1 | \
+# RUN:   FileCheck -check-prefix=RELAPLT %s
+# RELAPLT: discarding .rela.plt section is not allowed
+
+# RUN: echo "SECTIONS { /DISCARD/ : { *(.rela.dyn) } }" > %t.script
+# RUN: not ld.lld -pie -o %t --script %t.script %t.o 2>&1 | \
+# RUN:   FileCheck -check-prefix=RELADYN %s
+# RELADYN: discarding .rela.dyn section is not allowed
+
 .comm foo,4,4

lld/trunk/test/ELF/linkerscript/synthetic-relsec-layout.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
+# RUN: echo "SECTIONS { .foo : { *(.rela.dyn) } }" > %t.script
+# RUN: ld.lld -T %t.script %t.o -o %t.so -shared
+# RUN: llvm-readobj -r %t.so | FileCheck %s
+
+# Check we are able to do custom layout for synthetic sections.
+# (here we check we can place synthetic .rela.dyn into .foo).
+
+# CHECK: Relocations [
+# CHECK:   Section ({{.*}}) .foo {
+# CHECK:     R_X86_64_64 .foo 0x0
+# CHECK:   }
+
+.data
+.quad .foo