[PATCH] D70020: [lld] Better support for group semantic wrt. notes

[serge via Phabricator via llvm-commits]

serge-sans-paille added a comment.

> I think this approach may have a problem when we consider /DISCARD/ in a linker script. I'll propose an alternative fix which I believe will improve the case and improve the compatibility with GNU ld, in the hope that you won't mind :)

The following patch is a frail tentative at supporting /DISCARD/ in linker script:

  diff --git a/lld/ELF/LinkerScript.cpp b/lld/ELF/LinkerScript.cpp
  index cebbd89..1d5c8d7 100644
  --- a/lld/ELF/LinkerScript.cpp
  +++ b/lld/ELF/LinkerScript.cpp
  @@ -442,6 +442,9 @@ LinkerScript::computeInputSections(const InputSectionDescription *cmd) {
   }
   
   void LinkerScript::discard(InputSectionBase *s) {
  +  if(!s->isLive())
  +    return;
  +
     if (s == in.shStrTab || s == mainPart->relaDyn || s == mainPart->relrDyn)
       error("discarding " + s->name + " section is not allowed");
   
  diff --git a/lld/test/ELF/linkerscript/discard-section-group.test b/lld/test/ELF/linkerscript/discard-section-group.test
  new file mode 100644
  index 0000000..e5b090f
  --- /dev/null
  +++ b/lld/test/ELF/linkerscript/discard-section-group.test
  @@ -0,0 +1,47 @@
  +# REQUIRES: x86
  +# RUN: yaml2obj %s -o %t.o
  +#
  +# RUN: echo "SECTIONS { /DISCARD/ : { *(.text.foo*) } }" > %t0.script
  +# RUN: ld.lld -o %t0 --script %t0.script %t.o
  +# RUN: llvm-readobj -S %t0 | FileCheck %s
  +#
  +# RUN: echo "SECTIONS { /DISCARD/ : { *(.text.foo*) } /DISCARD/ : { *(.note*) } }" > %t1.script
  +# RUN: ld.lld -o %t1 --script %t1.script %t.o
  +# RUN: llvm-readobj -S %t1 | FileCheck %s
  +#
  +# RUN: echo "SECTIONS { /DISCARD/ : { *(.note*) } }" > %t2.script
  +# RUN: ld.lld -o %t2 --script %t2.script %t.o
  +# RUN: llvm-readobj -S %t2 | FileCheck %s
  +
  +# CHECK-NOT: .text
  +# CHECK-NOT: .note
  +
  +--- !ELF
  +FileHeader:
  +  Class:           ELFCLASS64
  +  Data:            ELFDATA2LSB
  +  Type:            ET_REL
  +  Machine:         EM_X86_64
  +Sections:
  +  - Name:            .group
  +    Type:            SHT_GROUP
  +    Link:            .symtab
  +    Info:            foo
  +    Members:
  +      - SectionOrType:    GRP_COMDAT
  +      - SectionOrType:    .text.foo
  +      - SectionOrType:    .note.text.foo
  +  - Name:            .note.text.foo
  +    Type:            SHT_NOTE
  +    Flags:           [ SHF_GROUP ]
  +    Content:         "DEAD"
  +  - Name:            .text.foo
  +    Type:            SHT_PROGBITS
  +    Flags:           [ SHF_ALLOC, SHF_EXECINSTR, SHF_GROUP ]
  +    Content:         "00"
  +Symbols:
  +  - Name:            foo
  +    Binding:         STB_GLOBAL
  +    Type:            STT_FUNC
  +    Section:         .text.foo
  +    Size:            0x08

So at first glance it looks possible to apply a patch on top of this one to support /DISCARD/, and I'd be happier if we can do it that way rather than discard this review.

That being said, if you're alternative fix improves the case, let's put ego aside and go that way :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70020