[PATCH] D65018: [LLD] [COFF] Unbreak sorting of mingw comdat .tls sections after SVN r363457

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo updated this revision to Diff 211188.
mstorsjo edited the summary of this revision.
mstorsjo added a comment.

Applied Reid's suggestions.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65018/new/

https://reviews.llvm.org/D65018

Files:
  COFF/Writer.cpp
  test/COFF/tls_suffix_sorting.s


Index: test/COFF/tls_suffix_sorting.s
===================================================================
--- /dev/null
+++ test/COFF/tls_suffix_sorting.s
@@ -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
Index: COFF/Writer.cpp
===================================================================
--- COFF/Writer.cpp
+++ COFF/Writer.cpp
@@ -776,6 +776,28 @@
   }
 }
 
+// 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.
@@ -821,10 +843,7 @@
       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());


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D65018.211188.patch
Type: text/x-patch
Size: 3192 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190722/0b441d3f/attachment.bin>