[PATCH] D28430: ELF: Implement support for .gnu.linkonce.t.*.

[Peter Collingbourne via llvm-commits]

Agreed that we should fix, but I suppose that even if we fix it now we'll
still encounter bad objects in the wild for a few years.

One observation we made was that the linkonce was conflicting with a real
comdat, so maybe a simpler thing we could do is to discard all linkonce
sections? (Away from computer, can't check if that works.)

Peter


On Jan 6, 2017 8:08 PM, "Rafael Avila de Espindola" <
rafael.espindola at gmail.com> wrote:


Do we really need this? If there is only one use left it seems better to
fix the use:

https://sourceware.org/bugzilla/show_bug.cgi?id=20543

Cheers,
Rafael

Peter Collingbourne via Phabricator <reviews at reviews.llvm.org> writes:

> pcc created this revision.
> pcc added reviewers: rafael, ruiu.
> pcc added subscribers: llvm-commits, eugenis, grimar.
>
> The linkonce feature is a sort of proto-comdat. bfd and gold appear to
> treat linkonce sections as if they were single-element comdat groups named
> after the section name suffix (which means they can cause comdat groups to
> be discarded), and this feature is necessary to link parts of the x86-32
> CRT on Linux.
>
> We only need support for .gnu.linkonce.t.* for the CRT (and I cannot find
> any other uses of linkonce in the x86-32 prebuilt libraries on my system),
> so this patch does only that.
>
> Fixes PR31215.
>
>
> https://reviews.llvm.org/D28430
>
> Files:
>   lld/ELF/InputFiles.cpp
>   lld/ELF/InputFiles.h
>   lld/test/ELF/comdat-linkonce.s
>
>
> Index: lld/test/ELF/comdat-linkonce.s
> ===================================================================
> --- /dev/null
> +++ lld/test/ELF/comdat-linkonce.s
> @@ -0,0 +1,9 @@
> +// RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
> +// RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %p/Inputs/comdat.s
-o %t2.o
> +// RUN: ld.lld -shared %t.o %t2.o -o %t
> +// RUN: ld.lld -shared %t2.o %t.o -o %t
> +
> +.section .gnu.linkonce.t.zed
> +.globl abc
> +abc:
> +nop
> Index: lld/ELF/InputFiles.h
> ===================================================================
> --- lld/ELF/InputFiles.h
> +++ lld/ELF/InputFiles.h
> @@ -184,8 +184,9 @@
>    void initializeSymbols();
>    void initializeDwarfLine();
>    InputSectionBase<ELFT> *getRelocTarget(const Elf_Shdr &Sec);
> -  InputSectionBase<ELFT> *createInputSection(const Elf_Shdr &Sec,
> -                                             StringRef
SectionStringTable);
> +  InputSectionBase<ELFT> *
> +  createInputSection(const Elf_Shdr &Sec, StringRef SectionStringTable,
> +                     llvm::DenseSet<llvm::CachedHashStringRef>
&ComdatGroups);
>
>    bool shouldMerge(const Elf_Shdr &Sec);
>    SymbolBody *createSymbolBody(const Elf_Sym *Sym);
> Index: lld/ELF/InputFiles.cpp
> ===================================================================
> --- lld/ELF/InputFiles.cpp
> +++ lld/ELF/InputFiles.cpp
> @@ -294,7 +294,7 @@
>      case SHT_NULL:
>        break;
>      default:
> -      Sections[I] = createInputSection(Sec, SectionStringTable);
> +      Sections[I] = createInputSection(Sec, SectionStringTable,
ComdatGroups);
>      }
>
>      // .ARM.exidx sections have a reverse dependency on the InputSection
they
> @@ -329,12 +329,11 @@
>  }
>
>  template <class ELFT>
> -InputSectionBase<ELFT> *
> -elf::ObjectFile<ELFT>::createInputSection(const Elf_Shdr &Sec,
> -                                          StringRef SectionStringTable) {
> +InputSectionBase<ELFT> *elf::ObjectFile<ELFT>::createInputSection(
> +    const Elf_Shdr &Sec, StringRef SectionStringTable,
> +    DenseSet<CachedHashStringRef> &ComdatGroups) {
>    StringRef Name =
>        check(this->getObj().getSectionName(&Sec, SectionStringTable));
> -
>    switch (Sec.sh_type) {
>    case SHT_ARM_ATTRIBUTES:
>      // FIXME: ARM meta-data section. Retain the first attribute section
> @@ -399,6 +398,15 @@
>    if (Config->Strip != StripPolicy::None && Name.startswith(".debug"))
>      return &InputSection<ELFT>::Discarded;
>
> +  // The linkonce feature is a sort of proto-comdat. bfd and gold appear
to
> +  // treat linkonce sections as if they were single-element comdat
groups named
> +  // after the section name suffix (which means they can cause comdat
groups to
> +  // be discarded), and this feature is necessary to link parts of the
x86-32
> +  // CRT on Linux.
> +  if (Name.startswith(".gnu.linkonce.t.") &&
> +      !ComdatGroups.insert(CachedHashStringRef(Name.substr(16))).second)
> +    return &InputSection<ELFT>::Discarded;
> +
>    // The linker merges EH (exception handling) frames and creates a
>    // .eh_frame_hdr section for runtime. So we handle them with a special
>    // class. For relocatable outputs, they are just passed through.
>
>
> Index: lld/test/ELF/comdat-linkonce.s
> ===================================================================
> --- /dev/null
> +++ lld/test/ELF/comdat-linkonce.s
> @@ -0,0 +1,9 @@
> +// RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
> +// RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %p/Inputs/comdat.s
-o %t2.o
> +// RUN: ld.lld -shared %t.o %t2.o -o %t
> +// RUN: ld.lld -shared %t2.o %t.o -o %t
> +
> +.section .gnu.linkonce.t.zed
> +.globl abc
> +abc:
> +nop
> Index: lld/ELF/InputFiles.h
> ===================================================================
> --- lld/ELF/InputFiles.h
> +++ lld/ELF/InputFiles.h
> @@ -184,8 +184,9 @@
>    void initializeSymbols();
>    void initializeDwarfLine();
>    InputSectionBase<ELFT> *getRelocTarget(const Elf_Shdr &Sec);
> -  InputSectionBase<ELFT> *createInputSection(const Elf_Shdr &Sec,
> -                                             StringRef
SectionStringTable);
> +  InputSectionBase<ELFT> *
> +  createInputSection(const Elf_Shdr &Sec, StringRef SectionStringTable,
> +                     llvm::DenseSet<llvm::CachedHashStringRef>
&ComdatGroups);
>
>    bool shouldMerge(const Elf_Shdr &Sec);
>    SymbolBody *createSymbolBody(const Elf_Sym *Sym);
> Index: lld/ELF/InputFiles.cpp
> ===================================================================
> --- lld/ELF/InputFiles.cpp
> +++ lld/ELF/InputFiles.cpp
> @@ -294,7 +294,7 @@
>      case SHT_NULL:
>        break;
>      default:
> -      Sections[I] = createInputSection(Sec, SectionStringTable);
> +      Sections[I] = createInputSection(Sec, SectionStringTable,
ComdatGroups);
>      }
>
>      // .ARM.exidx sections have a reverse dependency on the InputSection
they
> @@ -329,12 +329,11 @@
>  }
>
>  template <class ELFT>
> -InputSectionBase<ELFT> *
> -elf::ObjectFile<ELFT>::createInputSection(const Elf_Shdr &Sec,
> -                                          StringRef SectionStringTable) {
> +InputSectionBase<ELFT> *elf::ObjectFile<ELFT>::createInputSection(
> +    const Elf_Shdr &Sec, StringRef SectionStringTable,
> +    DenseSet<CachedHashStringRef> &ComdatGroups) {
>    StringRef Name =
>        check(this->getObj().getSectionName(&Sec, SectionStringTable));
> -
>    switch (Sec.sh_type) {
>    case SHT_ARM_ATTRIBUTES:
>      // FIXME: ARM meta-data section. Retain the first attribute section
> @@ -399,6 +398,15 @@
>    if (Config->Strip != StripPolicy::None && Name.startswith(".debug"))
>      return &InputSection<ELFT>::Discarded;
>
> +  // The linkonce feature is a sort of proto-comdat. bfd and gold appear
to
> +  // treat linkonce sections as if they were single-element comdat
groups named
> +  // after the section name suffix (which means they can cause comdat
groups to
> +  // be discarded), and this feature is necessary to link parts of the
x86-32
> +  // CRT on Linux.
> +  if (Name.startswith(".gnu.linkonce.t.") &&
> +      !ComdatGroups.insert(CachedHashStringRef(Name.substr(16))).second)
> +    return &InputSection<ELFT>::Discarded;
> +
>    // The linker merges EH (exception handling) frames and creates a
>    // .eh_frame_hdr section for runtime. So we handle them with a special
>    // class. For relocatable outputs, they are just passed through.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170106/86f276a4/attachment.html>