[PATCH] D29981: [ELF] - Allow section to have multiple dependent sections.

[Rafael Avila de Espindola via llvm-commits]

LGTM. Thanks!

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

> grimar created this revision.
>
> That fixes a case when section has more than one metadata section.
> Previously GC would collect on of such section because we had implementation
> that stored ony last one as dependent.
>
>
> https://reviews.llvm.org/D29981
>
> Files:
>   ELF/InputFiles.cpp
>   ELF/InputSection.h
>   ELF/MarkLive.cpp
>   test/ELF/gc-sections-metadata2.s
>
>
> Index: test/ELF/gc-sections-metadata2.s
> ===================================================================
> --- test/ELF/gc-sections-metadata2.s
> +++ test/ELF/gc-sections-metadata2.s
> @@ -0,0 +1,19 @@
> +# REQUIRES: x86
> +# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
> +# RUN: ld.lld --gc-sections %t.o -o %t
> +# RUN: llvm-objdump -section-headers %t | FileCheck %s
> +
> +# CHECK: .foo
> +# CHECK: .bar
> +# CHECK: .zed
> +
> +.globl _start
> +_start:
> +.quad .foo
> +
> +.section .foo,"a"
> +.quad 0
> +.section .bar,"am", at progbits,.foo
> +.quad 0
> +.section .zed,"am", at progbits,.foo
> +.quad 0
> Index: ELF/MarkLive.cpp
> ===================================================================
> --- ELF/MarkLive.cpp
> +++ ELF/MarkLive.cpp
> @@ -87,8 +87,8 @@
>      for (const typename ELFT::Rel &Rel : Sec.rels())
>        Fn(resolveReloc(Sec, Rel));
>    }
> -  if (Sec.DependentSection)
> -    Fn({Sec.DependentSection, 0});
> +  for (InputSectionBase<ELFT> *IS : Sec.DependentSections)
> +    Fn({IS, 0});
>  }
>  
>  // The .eh_frame section is an unfortunate special case.
> Index: ELF/InputSection.h
> ===================================================================
> --- ELF/InputSection.h
> +++ ELF/InputSection.h
> @@ -281,8 +281,8 @@
>    // to. The writer sets a value.
>    uint64_t OutSecOff = 0;
>  
> -  // InputSection that is dependent on us (reverse dependency for GC)
> -  InputSectionBase<ELFT> *DependentSection = nullptr;
> +  // InputSections that are dependent on us (reverse dependency for GC)
> +  llvm::TinyPtrVector<InputSectionBase<ELFT> *> DependentSections;
>  
>    static bool classof(const InputSectionData *S);
>  
> Index: ELF/InputFiles.cpp
> ===================================================================
> --- ELF/InputFiles.cpp
> +++ ELF/InputFiles.cpp
> @@ -323,7 +323,7 @@
>          fatal(toString(this) + ": invalid sh_link index: " +
>                Twine(Sec.sh_link));
>        auto *IS = cast<InputSection<ELFT>>(Sections[Sec.sh_link]);
> -      IS->DependentSection = Sections[I];
> +      IS->DependentSections.push_back(Sections[I]);
>      }
>    }
>  }
>
>
> Index: test/ELF/gc-sections-metadata2.s
> ===================================================================
> --- test/ELF/gc-sections-metadata2.s
> +++ test/ELF/gc-sections-metadata2.s
> @@ -0,0 +1,19 @@
> +# REQUIRES: x86
> +# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
> +# RUN: ld.lld --gc-sections %t.o -o %t
> +# RUN: llvm-objdump -section-headers %t | FileCheck %s
> +
> +# CHECK: .foo
> +# CHECK: .bar
> +# CHECK: .zed
> +
> +.globl _start
> +_start:
> +.quad .foo
> +
> +.section .foo,"a"
> +.quad 0
> +.section .bar,"am", at progbits,.foo
> +.quad 0
> +.section .zed,"am", at progbits,.foo
> +.quad 0
> Index: ELF/MarkLive.cpp
> ===================================================================
> --- ELF/MarkLive.cpp
> +++ ELF/MarkLive.cpp
> @@ -87,8 +87,8 @@
>      for (const typename ELFT::Rel &Rel : Sec.rels())
>        Fn(resolveReloc(Sec, Rel));
>    }
> -  if (Sec.DependentSection)
> -    Fn({Sec.DependentSection, 0});
> +  for (InputSectionBase<ELFT> *IS : Sec.DependentSections)
> +    Fn({IS, 0});
>  }
>  
>  // The .eh_frame section is an unfortunate special case.
> Index: ELF/InputSection.h
> ===================================================================
> --- ELF/InputSection.h
> +++ ELF/InputSection.h
> @@ -281,8 +281,8 @@
>    // to. The writer sets a value.
>    uint64_t OutSecOff = 0;
>  
> -  // InputSection that is dependent on us (reverse dependency for GC)
> -  InputSectionBase<ELFT> *DependentSection = nullptr;
> +  // InputSections that are dependent on us (reverse dependency for GC)
> +  llvm::TinyPtrVector<InputSectionBase<ELFT> *> DependentSections;
>  
>    static bool classof(const InputSectionData *S);
>  
> Index: ELF/InputFiles.cpp
> ===================================================================
> --- ELF/InputFiles.cpp
> +++ ELF/InputFiles.cpp
> @@ -323,7 +323,7 @@
>          fatal(toString(this) + ": invalid sh_link index: " +
>                Twine(Sec.sh_link));
>        auto *IS = cast<InputSection<ELFT>>(Sections[Sec.sh_link]);
> -      IS->DependentSection = Sections[I];
> +      IS->DependentSections.push_back(Sections[I]);
>      }
>    }
>  }