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

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: test/ELF/linkerscript/discard-phdr.s:44
+.section .baz,"aw"
+ .byte 0
----------------
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
```


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

https://reviews.llvm.org/D61186