[PATCH] D40454: [ELF] - Handle EhInputSection Live bit in MarkLive.cpp

[Rafael Avila de Espindola via llvm-commits]

LGTM

Thanks,
Rafael

George Rimar via Phabricator <reviews at reviews.llvm.org> writes:

> grimar created this revision.
> Herald added subscribers: arichardson, emaste.
>
> I suggest to so this refactoring change. Since MarkLive.cpp is
> the place where we set Live flags for other sections, it looks correct
> to do that there.
> Benefit is that we stop spreading GC logic outsize of MarkLive.cpp.
>
>
> https://reviews.llvm.org/D40454
>
> Files:
>   ELF/InputSection.cpp
>   ELF/MarkLive.cpp
>
>
> Index: ELF/MarkLive.cpp
> ===================================================================
> --- ELF/MarkLive.cpp
> +++ ELF/MarkLive.cpp
> @@ -235,11 +235,15 @@
>    // Preserve special sections and those which are specified in linker
>    // script KEEP command.
>    for (InputSectionBase *Sec : InputSections) {
> -    // .eh_frame is always marked as live now, but also it can reference to
> -    // sections that contain personality. We preserve all non-text sections
> -    // referred by .eh_frame here.
> -    if (auto *EH = dyn_cast_or_null<EhInputSection>(Sec))
> +    // Mark .eh_frame sections as live because there are usually no relocations
> +    // that point to .eh_frames. Otherwise, the garbage collector would drop
> +    // all of them. We also want to preserve personality routines and LSDA
> +    // referenced by .eh_frame sections, so we scan them for that here.
> +    if (auto *EH = dyn_cast_or_null<EhInputSection>(Sec)) {
> +      EH->Live = true;
>        scanEhFrameSection<ELFT>(*EH, Enqueue);
> +    }
> +
>      if (Sec->Flags & SHF_LINK_ORDER)
>        continue;
>      if (isReserved<ELFT>(Sec) || Script->shouldKeep(Sec))
> Index: ELF/InputSection.cpp
> ===================================================================
> --- ELF/InputSection.cpp
> +++ ELF/InputSection.cpp
> @@ -807,12 +807,7 @@
>  EhInputSection::EhInputSection(ObjFile<ELFT> *F,
>                                 const typename ELFT::Shdr *Header,
>                                 StringRef Name)
> -    : InputSectionBase(F, Header, Name, InputSectionBase::EHFrame) {
> -  // Mark .eh_frame sections as live by default because there are
> -  // usually no relocations that point to .eh_frames. Otherwise,
> -  // the garbage collector would drop all .eh_frame sections.
> -  this->Live = true;
> -}
> +    : InputSectionBase(F, Header, Name, InputSectionBase::EHFrame) {}
>  
>  SyntheticSection *EhInputSection::getParent() const {
>    return cast_or_null<SyntheticSection>(Parent);
>
>
> Index: ELF/MarkLive.cpp
> ===================================================================
> --- ELF/MarkLive.cpp
> +++ ELF/MarkLive.cpp
> @@ -235,11 +235,15 @@
>    // Preserve special sections and those which are specified in linker
>    // script KEEP command.
>    for (InputSectionBase *Sec : InputSections) {
> -    // .eh_frame is always marked as live now, but also it can reference to
> -    // sections that contain personality. We preserve all non-text sections
> -    // referred by .eh_frame here.
> -    if (auto *EH = dyn_cast_or_null<EhInputSection>(Sec))
> +    // Mark .eh_frame sections as live because there are usually no relocations
> +    // that point to .eh_frames. Otherwise, the garbage collector would drop
> +    // all of them. We also want to preserve personality routines and LSDA
> +    // referenced by .eh_frame sections, so we scan them for that here.
> +    if (auto *EH = dyn_cast_or_null<EhInputSection>(Sec)) {
> +      EH->Live = true;
>        scanEhFrameSection<ELFT>(*EH, Enqueue);
> +    }
> +
>      if (Sec->Flags & SHF_LINK_ORDER)
>        continue;
>      if (isReserved<ELFT>(Sec) || Script->shouldKeep(Sec))
> Index: ELF/InputSection.cpp
> ===================================================================
> --- ELF/InputSection.cpp
> +++ ELF/InputSection.cpp
> @@ -807,12 +807,7 @@
>  EhInputSection::EhInputSection(ObjFile<ELFT> *F,
>                                 const typename ELFT::Shdr *Header,
>                                 StringRef Name)
> -    : InputSectionBase(F, Header, Name, InputSectionBase::EHFrame) {
> -  // Mark .eh_frame sections as live by default because there are
> -  // usually no relocations that point to .eh_frames. Otherwise,
> -  // the garbage collector would drop all .eh_frame sections.
> -  this->Live = true;
> -}
> +    : InputSectionBase(F, Header, Name, InputSectionBase::EHFrame) {}
>  
>  SyntheticSection *EhInputSection::getParent() const {
>    return cast_or_null<SyntheticSection>(Parent);