[PATCH] D39058: [ELF] - Simplify reporting of garbage collected sections.

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: ELF/LinkerScript.cpp:286
     for (InputSectionBase *Sec : InputSections) {
-      if (Sec->Assigned)
+      if (Sec->Assigned || !Sec->Live)
         continue;
----------------
ruiu wrote:
> grimar wrote:
> > ruiu wrote:
> > > grimar wrote:
> > > > ruiu wrote:
> > > > > Question: is it possible that a section is assigned to some output section but is not live? It sounds like a bad state to me.
> > > > Yes, it is possible for following case:
> > > > `/DISCARD/ : {*(.text*)}`
> > > > Here we set Assigned flag for .text. It is probably OK it seems ?
> > > > 
> > > > (I would probably try to completely get rid of 'Assigned' flag, I think we should be able to do that).
> > > No, I don't think that is OK. `/DISCARD/` is a keyword and not a real section. Sections discarded by `/DISCARD/` should be not Assigned and not Live.
> > I think that is fine because specification explicitly mentions that input sections are assigned:
> > 
> > > 
> > > The special output section name ‘/DISCARD/’ may be used to discard input sections. Any
> > > input sections which are assigned to an output section named ‘/DISCARD/’ are not included
> > > in the output file.
> > (https://sourceware.org/binutils/docs-2.16/ld/Output-Section-Discarding.html)
> > 
> > Anyways I believe nice way to solve this would be to remove Assigned flag, 
> > what should be possible and should be done in different patch.
> > 
> > 
> I don't think that is fine, please read my commit message for r316622. Can you rebase this patch?
Rebased, but we still need to check both conditions here:
1) We need to check that section is not dead, I think it is obvious.
2) We still need to check that it is not assigned. We could avoid that in theory and check `Sec->Parent` instead
    of `Sec->Assigned`, but it will not work so simple, because we can have script like:
    `SECTIONS {.foo : { *(.foo) *(.foo) } }` (input-sec-dup.s) and do not want to add `.foo` twice.
    We could assign `Sec->Parent` to some dummy non-zero value probably and get rid of `Assigned`, but that is story for
    another patch.


https://reviews.llvm.org/D39058