[Lldb-commits] [PATCH] D54747: Discard debuginfo for object files empty after GC

[Robert O'Callahan via Phabricator via lldb-commits]

rocallahan marked an inline comment as done.
rocallahan added inline comments.


================
Comment at: lld/ELF/MarkLive.cpp:190-191
+  // case we still need to mark the file.
+  if (S && !IsLSDA && Sec->File)
+    if (isa<InputSection>(Sec) || isa<MergeInputSection>(Sec))
+      Sec->getFile<ELFT>()->HasLiveCodeOrData = true;
----------------
ruiu wrote:
> ruiu wrote:
> > rocallahan wrote:
> > > rocallahan wrote:
> > > > ruiu wrote:
> > > > > ruiu wrote:
> > > > > > S is true only when it is an InputSection, so you have duplicate tests for this. It also looks like you don't have to care about whether a section is an input section or a merged section. Isn't this condition sufficient?
> > > > > > 
> > > > > >   if (Sec->File && !IsLSDA)
> > > > > >     Sec->getFile<ELFT>()->HasLiveCodeOrData = true;
> > > > > Move this code after `Sec->Live = true` so that when we visit the same section more than once, this piece of code runs only once.
> > > > We can't do that. The comment I added tries to explain why. Is it unclear?
> > > I want to skip `EhInputSection` and `SyntheticSection` here. So I guess the advice to use `isa` here was wrong and I should revert to checking `kind() == Regular || kind() == Merge`?
> > But you don't really care even if it is `EhInputSection` or `SyntheticSection`, no? I mean an assigned value would not be used but doesn't harm.
> Ah, OK, got it.
I'm assuming that `.eh_frame` sections don't have associated debuginfo so even if such a section is enqueued somehow, it should not cause debuginfo to be included for that object file. Is it impossible for a `.eh_frame` section to be enqueued here? Ditto for a `SyntheticSection`?

To put it another way, the logic here is supposed to be: "debuginfo can only be associated with Regular or Merge sections, so it only makes sense to mark files as having 'live code or data' possibly associated with debuginfo for those section types."


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

https://reviews.llvm.org/D54747