[PATCH] D61186: [LLD][ELF] /DISCARD/ output sections should not be orphans

[Andrew Ng via Phabricator via llvm-commits]

andrewng marked 4 inline comments as done.
andrewng added inline comments.


================
Comment at: test/ELF/linkerscript/discard-phdr.s:44
+.section .baz,"aw"
+ .byte 0
----------------
grimar wrote:
> grimar wrote:
> > andrewng wrote:
> > > grimar wrote:
> > > > andrewng wrote:
> > > > > grimar wrote:
> > > > > > Does not seem you need all of this sections for the test? Please reduce the number.
> > > > > These are actually required to provoke the issue. It is a somewhat unusual situation (based on a real scenario), so whether this test should be "kept" is debatable.
> > > > No need to replicate the full real case. The test case should show the minimal reproducible scenario of the different output before/after the patch. That is how we do in LLD. 
> > > Just to be clear, you are saying that there is no need to demonstrate the issue being addressed? So the only requirement for a test is to show a change in the output?
> > I am saying that I think this test case can be reduced. For example,
> > I was able to remove 2 sections and 1 PT_LOAD and I think it still demonstrates
> > that the issue you're fixing is addressed:
> > 
> > ```
> > # RUN: echo "PHDRS { \
> > # RUN:   text PT_LOAD FLAGS(0x4 | 0x1); \
> > # RUN: } \
> > # RUN: SECTIONS { \
> > # RUN:  .text : { *(.text) } :text \
> > # RUN:  .bar : { *(.bar) } \
> > # RUN:  .baz : { *(.baz) } \
> > # RUN:  /DISCARD/ : { *(.comment) } :NONE \
> > # RUN: }" > %t.script
> > # RUN: ld.lld -o %t --script %t.script %t.o
> > # RUN: llvm-readelf -S -l %t | FileCheck %s dd
> > 
> > .....
> > 
> > .section .text,"ax"
> >  ret
> > 
> > .section .bar,"a"
> >  .byte 0
> > 
> > .section .baz,"ax"
> >  .byte 0
> > ```
>  Output before patch:
> 
> ```
> Section to Segment mapping:
>   Segment Sections...
>    00     .text
>    None   /DISCARD/ .bar .baz .symtab .shstrtab .strtab
> ```
> 
> Output after patch:
> 
> ```
>  Section to Segment mapping:
>   Segment Sections...
>   00     .text .bar .baz
>   None   /DISCARD/ .symtab .shstrtab .strtab
> ```
> 
> 
I have simplified as suggested, although this use case is even "stranger" than the original.


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

https://reviews.llvm.org/D61186