[PATCH] D76003: [ELF][test] Add test for --gc-sections + many sections

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

In D76003#1918434 <https://reviews.llvm.org/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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76003/new/

https://reviews.llvm.org/D76003