[ELF] - Linkerscript: disallow discarding COMMON.

[grimar]

Summary

This patch restricts following construction:

/DISCARD/ : { *(COMMON) }

Previously LLD would crash.

[davide]

What's the use case for discarding common symbols?
Also, when you say "ignores such scripts" you mean that gold accepts this syntax and doesn't discard the common?

[grimar]

In D36468#835412, @davide wrote:

What's the use case for discarding common symbols?

I do not know about real use case, I worked on a D36466 and tried to discard COMMON during work on that
patch to see what -Map will contain, but LLD crashed here :)
We can probably restrict discarding COMMON instead, though it will be 1 line fix too.

Also, when you say "ignores such scripts" you mean that gold accepts this syntax and doesn't discard the common?

Yes.

[ruiu]

I'm lukewarm about this change. Such linker script doesn't make sense, so doing something for such nonsense seems to be just increasing the complexity of the code. You might still want to fix a crash bug, but attempting to do "the right thing" for this doesn't seem to worth it.

[grimar]

In D36468#836637, @ruiu wrote:

I'm lukewarm about this change. Such linker script doesn't make sense, so doing something for such nonsense seems to be just increasing the complexity of the code. You might still want to fix a crash bug, but attempting to do "the right thing" for this doesn't seem to worth it.

Ok, I'll restrict discarding COMMON and update the diff then.

[ruiu]

LGTM

lld/trunk/ELF/LinkerScript.cpp

for (const SectionPattern &Pat : Cmd->SectionPatterns) {
     }
   }
   return Ret;
 }
 
 void LinkerScript::discard(ArrayRef<InputSectionBase *> V) {
   for (InputSectionBase *S : V) {
     S->Live = false;
-    if (S == InX::ShStrTab || S == InX::Dynamic || S == InX::DynSymTab ||
-        S == InX::DynStrTab)
+    if (S == InX::ShStrTab || S == InX::Common || S == InX::Dynamic ||
+        S == InX::DynSymTab || S == InX::DynStrTab)
       error("discarding " + S->Name + " section is not allowed");
     discard(S->DependentSections);
   }
 }
 
 std::vector<InputSectionBase *>
 LinkerScript::createInputSectionList(OutputSection &OutCmd) {
   std::vector<InputSectionBase *> Ret;

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

 # RUN: not ld.lld -pie -o %t --script %t.script %t.o 2>&1 | \
 # 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/ : { *(COMMON) } }" > %t.script
+# RUN: not ld.lld -pie -o %t --script %t.script %t.o 2>&1 | \
+# RUN:   FileCheck -check-prefix=COMMON %s
+# COMMON: discarding COMMON section is not allowed
+
+.comm foo,4,4