<div dir="auto">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.<div dir="auto"><br></div><div dir="auto">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.)</div><div dir="auto"><br></div><div dir="auto">Peter</div><div dir="auto"><div dir="auto"><br><div class="gmail_extra"><br><div class="gmail_quote">On Jan 6, 2017 8:08 PM, "Rafael Avila de Espindola" <<a href="mailto:rafael.espindola@gmail.com">rafael.espindola@gmail.com</a>> wrote:<br type="attribution"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
Do we really need this? If there is only one use left it seems better to<br>
fix the use:<br>
<br>
<a href="https://sourceware.org/bugzilla/show_bug.cgi?id=20543" rel="noreferrer" target="_blank">https://sourceware.org/<wbr>bugzilla/show_bug.cgi?id=20543</a><br>
<br>
Cheers,<br>
Rafael<br>
<div class="elided-text"><br>
Peter Collingbourne via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> writes:<br>
<br>
> pcc created this revision.<br>
> pcc added reviewers: rafael, ruiu.<br>
> pcc added subscribers: llvm-commits, eugenis, grimar.<br>
><br>
> The linkonce feature is a sort of proto-comdat. bfd and gold appear to<br>
> treat linkonce sections as if they were single-element comdat groups named<br>
> after the section name suffix (which means they can cause comdat groups to<br>
> be discarded), and this feature is necessary to link parts of the x86-32<br>
> CRT on Linux.<br>
><br>
> We only need support for .gnu.linkonce.t.* for the CRT (and I cannot find<br>
> any other uses of linkonce in the x86-32 prebuilt libraries on my system),<br>
> so this patch does only that.<br>
><br>
> Fixes PR31215.<br>
><br>
><br>
> <a href="https://reviews.llvm.org/D28430" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D28430</a><br>
><br>
> Files:<br>
> lld/ELF/InputFiles.cpp<br>
> lld/ELF/InputFiles.h<br>
> lld/test/ELF/comdat-linkonce.s<br>
><br>
><br>
> Index: lld/test/ELF/comdat-linkonce.s<br>
> ==============================<wbr>==============================<wbr>=======<br>
> --- /dev/null<br>
> +++ lld/test/ELF/comdat-linkonce.s<br>
> @@ -0,0 +1,9 @@<br>
> +// RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o<br>
> +// RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %p/Inputs/comdat.s -o %t2.o<br>
> +// RUN: ld.lld -shared %t.o %t2.o -o %t<br>
> +// RUN: ld.lld -shared %t2.o %t.o -o %t<br>
> +<br>
> +.section .gnu.linkonce.t.zed<br>
> +.globl abc<br>
> +abc:<br>
> +nop<br>
> Index: lld/ELF/InputFiles.h<br>
> ==============================<wbr>==============================<wbr>=======<br>
> --- lld/ELF/InputFiles.h<br>
> +++ lld/ELF/InputFiles.h<br>
> @@ -184,8 +184,9 @@<br>
> void initializeSymbols();<br>
> void initializeDwarfLine();<br>
> InputSectionBase<ELFT> *getRelocTarget(const Elf_Shdr &Sec);<br>
> - InputSectionBase<ELFT> *createInputSection(const Elf_Shdr &Sec,<br>
> - StringRef SectionStringTable);<br>
> + InputSectionBase<ELFT> *<br>
> + createInputSection(const Elf_Shdr &Sec, StringRef SectionStringTable,<br>
> + llvm::DenseSet<llvm::<wbr>CachedHashStringRef> &ComdatGroups);<br>
><br>
> bool shouldMerge(const Elf_Shdr &Sec);<br>
> SymbolBody *createSymbolBody(const Elf_Sym *Sym);<br>
> Index: lld/ELF/InputFiles.cpp<br>
> ==============================<wbr>==============================<wbr>=======<br>
> --- lld/ELF/InputFiles.cpp<br>
> +++ lld/ELF/InputFiles.cpp<br>
> @@ -294,7 +294,7 @@<br>
> case SHT_NULL:<br>
> break;<br>
> default:<br>
> - Sections[I] = createInputSection(Sec, SectionStringTable);<br>
> + Sections[I] = createInputSection(Sec, SectionStringTable, ComdatGroups);<br>
> }<br>
><br>
> // .ARM.exidx sections have a reverse dependency on the InputSection they<br>
> @@ -329,12 +329,11 @@<br>
> }<br>
><br>
> template <class ELFT><br>
> -InputSectionBase<ELFT> *<br>
> -elf::ObjectFile<ELFT>::<wbr>createInputSection(const Elf_Shdr &Sec,<br>
> - StringRef SectionStringTable) {<br>
> +InputSectionBase<ELFT> *elf::ObjectFile<ELFT>::<wbr>createInputSection(<br>
> + const Elf_Shdr &Sec, StringRef SectionStringTable,<br>
> + DenseSet<CachedHashStringRef> &ComdatGroups) {<br>
> StringRef Name =<br>
> check(this->getObj().<wbr>getSectionName(&Sec, SectionStringTable));<br>
> -<br>
> switch (Sec.sh_type) {<br>
> case SHT_ARM_ATTRIBUTES:<br>
> // FIXME: ARM meta-data section. Retain the first attribute section<br>
> @@ -399,6 +398,15 @@<br>
> if (Config->Strip != StripPolicy::None && Name.startswith(".debug"))<br>
> return &InputSection<ELFT>::<wbr>Discarded;<br>
><br>
> + // The linkonce feature is a sort of proto-comdat. bfd and gold appear to<br>
> + // treat linkonce sections as if they were single-element comdat groups named<br>
> + // after the section name suffix (which means they can cause comdat groups to<br>
> + // be discarded), and this feature is necessary to link parts of the x86-32<br>
> + // CRT on Linux.<br>
> + if (Name.startswith(".gnu.<wbr>linkonce.t.") &&<br>
> + !ComdatGroups.insert(<wbr>CachedHashStringRef(Name.<wbr>substr(16))).second)<br>
> + return &InputSection<ELFT>::<wbr>Discarded;<br>
> +<br>
> // The linker merges EH (exception handling) frames and creates a<br>
> // .eh_frame_hdr section for runtime. So we handle them with a special<br>
> // class. For relocatable outputs, they are just passed through.<br>
><br>
><br>
> Index: lld/test/ELF/comdat-linkonce.s<br>
> ==============================<wbr>==============================<wbr>=======<br>
> --- /dev/null<br>
> +++ lld/test/ELF/comdat-linkonce.s<br>
> @@ -0,0 +1,9 @@<br>
> +// RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o<br>
> +// RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %p/Inputs/comdat.s -o %t2.o<br>
> +// RUN: ld.lld -shared %t.o %t2.o -o %t<br>
> +// RUN: ld.lld -shared %t2.o %t.o -o %t<br>
> +<br>
> +.section .gnu.linkonce.t.zed<br>
> +.globl abc<br>
> +abc:<br>
> +nop<br>
> Index: lld/ELF/InputFiles.h<br>
> ==============================<wbr>==============================<wbr>=======<br>
> --- lld/ELF/InputFiles.h<br>
> +++ lld/ELF/InputFiles.h<br>
> @@ -184,8 +184,9 @@<br>
> void initializeSymbols();<br>
> void initializeDwarfLine();<br>
> InputSectionBase<ELFT> *getRelocTarget(const Elf_Shdr &Sec);<br>
> - InputSectionBase<ELFT> *createInputSection(const Elf_Shdr &Sec,<br>
> - StringRef SectionStringTable);<br>
> + InputSectionBase<ELFT> *<br>
> + createInputSection(const Elf_Shdr &Sec, StringRef SectionStringTable,<br>
> + llvm::DenseSet<llvm::<wbr>CachedHashStringRef> &ComdatGroups);<br>
><br>
> bool shouldMerge(const Elf_Shdr &Sec);<br>
> SymbolBody *createSymbolBody(const Elf_Sym *Sym);<br>
> Index: lld/ELF/InputFiles.cpp<br>
> ==============================<wbr>==============================<wbr>=======<br>
> --- lld/ELF/InputFiles.cpp<br>
> +++ lld/ELF/InputFiles.cpp<br>
> @@ -294,7 +294,7 @@<br>
> case SHT_NULL:<br>
> break;<br>
> default:<br>
> - Sections[I] = createInputSection(Sec, SectionStringTable);<br>
> + Sections[I] = createInputSection(Sec, SectionStringTable, ComdatGroups);<br>
> }<br>
><br>
> // .ARM.exidx sections have a reverse dependency on the InputSection they<br>
> @@ -329,12 +329,11 @@<br>
> }<br>
><br>
> template <class ELFT><br>
> -InputSectionBase<ELFT> *<br>
> -elf::ObjectFile<ELFT>::<wbr>createInputSection(const Elf_Shdr &Sec,<br>
> - StringRef SectionStringTable) {<br>
> +InputSectionBase<ELFT> *elf::ObjectFile<ELFT>::<wbr>createInputSection(<br>
> + const Elf_Shdr &Sec, StringRef SectionStringTable,<br>
> + DenseSet<CachedHashStringRef> &ComdatGroups) {<br>
> StringRef Name =<br>
> check(this->getObj().<wbr>getSectionName(&Sec, SectionStringTable));<br>
> -<br>
> switch (Sec.sh_type) {<br>
> case SHT_ARM_ATTRIBUTES:<br>
> // FIXME: ARM meta-data section. Retain the first attribute section<br>
> @@ -399,6 +398,15 @@<br>
> if (Config->Strip != StripPolicy::None && Name.startswith(".debug"))<br>
> return &InputSection<ELFT>::<wbr>Discarded;<br>
><br>
> + // The linkonce feature is a sort of proto-comdat. bfd and gold appear to<br>
> + // treat linkonce sections as if they were single-element comdat groups named<br>
> + // after the section name suffix (which means they can cause comdat groups to<br>
> + // be discarded), and this feature is necessary to link parts of the x86-32<br>
> + // CRT on Linux.<br>
> + if (Name.startswith(".gnu.<wbr>linkonce.t.") &&<br>
> + !ComdatGroups.insert(<wbr>CachedHashStringRef(Name.<wbr>substr(16))).second)<br>
> + return &InputSection<ELFT>::<wbr>Discarded;<br>
> +<br>
> // The linker merges EH (exception handling) frames and creates a<br>
> // .eh_frame_hdr section for runtime. So we handle them with a special<br>
> // class. For relocatable outputs, they are just passed through.<br>
</div></blockquote></div><br></div></div></div></div>