[ELF] - Do not segfault when using --gc-sections with linker script

[grimar]

Summary

Patch fixes PR32024.

Sections that were not marked as Live has null output section.
Previously we tried to access that field and segfaulted.

[jhenderson]

I'm not yet familiar enough with LLD to know the policy with asserts, but would it make sense to assert after that if statement (possibly inside switchTo()) to check that IB->OutSec is not null? That way if there are any other cases either currently or in the future where OutSec is null we'll get a nicer failure, at least in Debug builds.

test/ELF/linkerscript/sections-gc.s

7 ↗

(On Diff #89170)

Do we really need these first two CHECKs?

[grimar]

In D30188#682028, @jhenderson wrote:

I'm not yet familiar enough with LLD to know the policy with asserts, but would it make sense to assert after that if statement (possibly inside switchTo()) to check that IB->OutSec is not null? That way if there are any other cases either currently or in the future where OutSec is null we'll get a nicer failure, at least in Debug builds.

We usually do not check arguments with asserts I think. Mostly using them to check logic flow conditions.

test/ELF/linkerscript/sections-gc.s

7 ↗

(On Diff #89170)

Thats for self-documenting the test.
It shows we check Sections and that "00000001" is a Size field.
I am checking a size to show that one of sections was collected.

[jhenderson]

test/ELF/linkerscript/sections-gc.s

7 ↗

(On Diff #89170)

I see, that makes sense. In that case, can we drop the "Idx" part of the string, since we don't check the section index? It would also make things line up nicer (i.e. ".text" appears underneath "Name").

[grimar]

test/ELF/linkerscript/sections-gc.s

7 ↗

(On Diff #89170)

Yeah, I dropped index from output, but forgot to dtop "Idx", you right.

[grimar]

• Addressed review comments

lld/trunk/ELF/LinkerScript.cpp

for (InputSectionData *ID : ICmd.Sections) {
     // empty ones afterwards (because there is no way to know whether they were
     // going be empty or not other than actually running linker scripts.)
     // We need to ignore remains of empty sections.
     if (auto *Sec = dyn_cast<SyntheticSection<ELFT>>(ID))
       if (Sec->empty())
         continue;
 
     auto *IB = static_cast<InputSectionBase<ELFT> *>(ID);
+    if (!IB->Live)
+      continue;
     switchTo(IB->OutSec);
     if (auto *I = dyn_cast<InputSection<ELFT>>(IB))
       output(I);
     else
       flush();
   }
 }
 

lld/trunk/test/ELF/linkerscript/sections-gc.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t
+# RUN: echo "SECTIONS { .text : { *(.text*) } }" > %t.script
+# RUN: ld.lld %t --gc-sections --script %t.script -o %t1
+# RUN: llvm-objdump -section-headers %t1 | FileCheck %s
+
+# CHECK:      Sections:
+# CHECK-NEXT:  Name      Size
+# CHECK:       .text     00000001
+
+.section .text.foo, "ax"
+.global _start
+_start:
+  nop
+
+.section .text.bar, "ax"
+.global bar
+bar:
+  nop