[lld] r366780 - [COFF] Unbreak sorting of mingw comdat .tls sections after SVN r363457

[Hans Wennborg via llvm-commits]

Merged together with r366784 to the lld 9 branch in r366816.

On Mon, Jul 22, 2019 at 11:37 PM Martin Storsjo via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
>
> Author: mstorsjo
> Date: Mon Jul 22 23:38:04 2019
> New Revision: 366780
>
> URL: http://llvm.org/viewvc/llvm-project?rev=366780&view=rev
> Log:
> [COFF] Unbreak sorting of mingw comdat .tls sections after SVN r363457
>
> Code built for mingw with -fdata-sections will store each TLS variable
> in a comdat section, named .tls$$<varname>. Normal TLS variables are
> stored in sections named .tls$ with a trailing dollar, which are
> sorted after a starter marker (in a later linked object file) in a
> section named ".tls" (with no dollar suffix), before an ending marker
> in a section named ".tls$ZZZ".
>
> The mingw comdat section suffix stripping introduced in SVN r363457
> broke sorting of such tls sections, ending up sorting the stripped
> .tls$$<varname> sections (stripped to ".tls") before the start marker
> in the section named ".tls".
>
> We could add exceptions to the section name suffix stripping for
> .tls (and .CRT, where suffixes always should be honored), but the
> more conservative option is probably the reverse; to only apply the
> stripping for the normal sections where sorting shouldn't have any
> effect.
>
> Differential Revision: https://reviews.llvm.org/D65018
>
> Added:
>     lld/trunk/test/COFF/tls_suffix_sorting.s
> Modified:
>     lld/trunk/COFF/Writer.cpp
>
> Modified: lld/trunk/COFF/Writer.cpp
> URL: http://llvm.org/viewvc/llvm-project/lld/trunk/COFF/Writer.cpp?rev=366780&r1=366779&r2=366780&view=diff
> ==============================================================================
> --- lld/trunk/COFF/Writer.cpp (original)
> +++ lld/trunk/COFF/Writer.cpp Mon Jul 22 23:38:04 2019
> @@ -762,6 +762,28 @@ void Writer::locateImportTables() {
>    }
>  }
>
> +// Return whether a SectionChunk's suffix (the dollar and any trailing
> +// suffix) should be removed and sorted into the main suffixless
> +// PartialSection.
> +static bool shouldStripSectionSuffix(SectionChunk *sc, StringRef name) {
> +  // On MinGW, comdat groups are formed by putting the comdat group name
> +  // after the '$' in the section name. For .eh_frame$<symbol>, that must
> +  // still be sorted before the .eh_frame trailer from crtend.o, thus just
> +  // strip the section name trailer. For other sections, such as
> +  // .tls$$<symbol> (where non-comdat .tls symbols are otherwise stored in
> +  // ".tls$"), they must be strictly sorted after .tls. And for the
> +  // hypothetical case of comdat .CRT$XCU, we definitely need to keep the
> +  // suffix for sorting. Thus, to play it safe, only strip the suffix for
> +  // the standard sections.
> +  if (!config->mingw)
> +    return false;
> +  if (!sc || !sc->isCOMDAT())
> +    return false;
> +  return name.startswith(".text$") || name.startswith(".data$") ||
> +         name.startswith(".rdata$") || name.startswith(".pdata$") ||
> +         name.startswith(".xdata$") || name.startswith(".eh_frame$");
> +}
> +
>  // Create output section objects and add them to OutputSections.
>  void Writer::createSections() {
>    // First, create the builtin sections.
> @@ -807,10 +829,7 @@ void Writer::createSections() {
>        continue;
>      }
>      StringRef name = c->getSectionName();
> -    // On MinGW, comdat groups are formed by putting the comdat group name
> -    // after the '$' in the section name. Such a section name suffix shouldn't
> -    // imply separate alphabetical sorting of those section chunks though.
> -    if (config->mingw && sc && sc->isCOMDAT())
> +    if (shouldStripSectionSuffix(sc, name))
>        name = name.split('$').first;
>      PartialSection *pSec = createPartialSection(name,
>                                                  c->getOutputCharacteristics());
>
> Added: lld/trunk/test/COFF/tls_suffix_sorting.s
> URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/COFF/tls_suffix_sorting.s?rev=366780&view=auto
> ==============================================================================
> --- lld/trunk/test/COFF/tls_suffix_sorting.s (added)
> +++ lld/trunk/test/COFF/tls_suffix_sorting.s Mon Jul 22 23:38:04 2019
> @@ -0,0 +1,28 @@
> +# REQUIRES: x86
> +
> +# RUN: echo -e ".section .tls,\"dw\"\n .byte 0xaa\n .section .tls\$ZZZ,\"dw\"\n .byte 0xff\n .globl _tls_index\n .data\n _tls_index:\n .int 0" > %t.tlssup.s
> +# RUN: llvm-mc -triple=x86_64-windows-gnu %t.tlssup.s -filetype=obj -o %t.tlssup.o
> +# RUN: llvm-mc -triple=x86_64-windows-gnu %s -filetype=obj -o %t.main.o
> +
> +# RUN: lld-link -lldmingw -entry:main %t.main.o %t.tlssup.o -out:%t.exe
> +# RUN: llvm-objdump -s %t.exe | FileCheck %s
> +
> +# Check that .tls$$foo is sorted after the start marker (aa) and before the
> +# end marker (ff).
> +
> +# CHECK: Contents of section .tls:
> +# CHECK:  140004000 aabbff
> +
> +        .text
> +        .globl          main
> +main:
> +        movl            _tls_index(%rip), %eax
> +        movq            %gs:88, %rcx
> +        movq            (%rcx,%rax,8), %rax
> +        movb            foo at SECREL32(%rax), %al
> +        ret
> +
> +        .section        .tls$$foo,"dw"
> +        .linkonce       discard
> +foo:
> +        .byte           0xbb
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits