[ELF][test] Add test for --gc-sections + many sections

[jhenderson]

Summary

This test covers the case where --gc-sections is used when there are many sections. In particular, it ensures that there is no adverse interaction with common and absolute symbols.

[MaskRay]

An -O0 llvm-mc can take ~3 seconds to produce %t.o, we shall be careful adding such a test.

many-sections.s tests how e_shnum/sh_size of SHN_UNDEF is represented when there are more than SHN_LOPRESERVE sections. Did you find something interesting with lld's --gc-sections? Does it improve coverage? If it is still needed, we can add --gc-sections into many-sections.s, but I want to understand we need it at first.

[jhenderson]

In D76003#1918434, @MaskRay wrote:

An -O0 llvm-mc can take ~3 seconds to produce %t.o, we shall be careful adding such a test.

many-sections.s tests how e_shnum/sh_size of SHN_UNDEF is represented when there are more than SHN_LOPRESERVE sections. Did you find something interesting with lld's --gc-sections? Does it improve coverage? If it is still needed, we can add --gc-sections into many-sections.s, but I want to understand we need it at first.

I'm not aware of any specific issue with lld's --gc-sections, but there is a conceptual area of coverage that this fills, namely that common and absolute symbols don't get accidentally misattributed to a real section (because their indexes conflict with such sections). We have an old testsuite that picked up a bug in another linker in relation to this issue, which we're trying to migrate to lit, hence why I'm adding this test (this same coverage was the thing that picked up the large file handling issue in the archive library that I fixed recently). I'm happy to try modifying many-sections.s (or more likely many-alloc-sections.s) instead of introducing a new test.

To expand on the conceptual coverage aspect, this is essentially a black-box test. It prevents the naive approach of implementing --gc-sections, whereby something like the following pseudo code exists:

for (Symbol S : Symbols) {
  if (!Sections[S->st_shndx]->Referenced)
    removeSymbol(S);
}

and ensures that there is a check first of all for st_shndx == SHN_COMMON etc. Of course, we may not have this code right now, but there's nothing stopping a future refactoring effort changing to something more like this.

[jhenderson]

Merged new test into many-alloc-sections. This increased the duration of that test by about 50-60% in a debug build. Given the testsuite parallelism, I'm not convinced it wouldn't be better overall to put it in a separate test, but I'm okay doing it this way.

[jhenderson]

Ping?

[grimar]

This LGTM, please wait for @MaskRay since he had concerns before.

[jhenderson]

Ping @MaskRay?

[MaskRay]

Looks great!

lld/test/ELF/many-alloc-sections.s

 // REQUIRES: x86
 // RUN: llvm-mc -filetype=obj -triple x86_64-pc-linux-gnu %s -o %t.o
 // RUN: echo "SECTIONS { . = SIZEOF_HEADERS; .text : { *(.text) } }" > %t.script
 // RUN: ld.lld -T %t.script %t.o -o %t
 // RUN: llvm-readobj --symbols %t | FileCheck %s
 
 // Test that _start is in the correct section.
 // CHECK:      Name: _start
 // CHECK-NEXT: Value:
 // CHECK-NEXT: Size: 0
 // CHECK-NEXT: Binding: Global
 // CHECK-NEXT: Type: None
 // CHECK-NEXT: Other: 0
-// CHECK-NEXT: Section: dm
+// CHECK-NEXT: Section: dn
+
+// Show that --gc-sections works when there are many sections, and that
+// referenced common and absolute symbols in such cases are not removed, nor are
+// they incorrectly attributed to the sections with index 0xFFF1 or 0xFFF2.
+// RUN: ld.lld %t.o -T %t.script -o %t --gc-sections
+// RUN: llvm-readobj --symbols --sections %t | FileCheck %s --check-prefix=GC
+
+// GC:      Sections [
+// GC-NEXT:   Section {
+// GC-NEXT:     Index: 0
+// GC-NEXT:     Name:  (0)
+// GC:        Section {
+// GC-NEXT:     Index: 1
+// GC-NEXT:     Name: dn
+// GC:        Section {
+// GC-NEXT:     Index: 2
+// GC-NEXT:     Name: .bss
+// GC:        Section {
+// GC-NEXT:     Index: 3
+// GC-NEXT:     Name: .comment
+// GC:        Section {
+// GC-NEXT:     Index: 4
+// GC-NEXT:     Name: .symtab
+// GC:        Section {
+// GC-NEXT:     Index: 5
+// GC-NEXT:     Name: .shstrtab
+// GC:        Section {
+// GC-NEXT:     Index: 6
+// GC-NEXT:     Name: .strtab
+// GC-NOT:    Section {
+
+// GC:      Symbols [
+// GC-NEXT:   Symbol {
+// GC-NEXT:     Name:  (0)
+// GC:        Symbol {
+// GC-NEXT:     Name: sdn
+// GC:        Symbol {
+// GC-NEXT:     Name: _start
+// GC:        Symbol {
+// GC-NEXT:     Name: abs
+// GC:        Symbol {
+// GC-NEXT:     Name: common
+// GC-NOT:    Symbol {
 
 .macro gen_sections4 x
         .section a\x,"a"
+        .global sa\x
+        sa\x:
         .section b\x,"a"
+        .global sa\x
+        sb\x:
         .section c\x,"a"
+        .global sa\x
+        sc\x:
         .section d\x,"a"
+        .global sa\x
+        sd\x:
 .endm
 
 .macro gen_sections8 x
         gen_sections4 a\x
         gen_sections4 b\x
 .endm
 
 .macro gen_sections16 x
.macro gen_sections16384 x
         gen_sections8192 b\x
 .endm
 
 .macro gen_sections32768 x
         gen_sections16384 a\x
         gen_sections16384 b\x
 .endm
 
-        .bss
-        .section bar
-
 gen_sections32768 a
 gen_sections16384 b
 gen_sections8192 c
 gen_sections4096 d
 gen_sections2048 e
 gen_sections1024 f
 gen_sections512 g
-gen_sections128 h
-gen_sections64 i
-gen_sections32 j
-gen_sections16 k
-gen_sections8 l
-gen_sections4 m
+gen_sections256 h
+gen_sections128 i
+gen_sections64 j
+gen_sections32 k
+gen_sections16 l
+gen_sections8 m
+gen_sections4 n
+
+.global abs
+abs = 0x12345678
+
+.comm common,4,4
 
 .global _start
 _start:
+  .quad abs
+  .quad common