[LLD] [COFF] Interpret a period as a separator for section suffix just like '$'

[mstorsjo]

Summary

This allows grouping all sections like ".ctors.12345" into ".ctors".

For MinGW, the numerical values for such ctors are all zero-padded, so a lexical sort is good enough.

[ruiu]

Ah, okay, this is the lld-side patch. I wonder if you can't just create .ctors$12345 instead of .ctors.12345 for COFF. Do you need this?

[mstorsjo]

In D40408#934167, @ruiu wrote:

Ah, okay, this is the lld-side patch. I wonder if you can't just create .ctors$12345 instead of .ctors.12345 for COFF. Do you need this?

On the LLVM side, I'm trying to match what GCC does, to allow using this with clang as compiler while using the GNU binutils in MinGW.

(Even if we'd make LLVM produce .ctors$12345, we need to sort them numerically so that .ctors$5 would end up before .ctors$12345.)

[ruiu]

COFF/Writer.cpp

329 ↗

(On Diff #124134)

Please add a comment saying that this is for MinGW. I'd also mention that this could have been technically avoided because we could have used '$' instead of '.', but we do this for compatibility with GNU.

329 ↗

(On Diff #124134)

Passing a StringRef by reference seems a bit odd. The class is designed to be passed by value, because it is essentially a pointer with size.

329 ↗

(On Diff #124134)

Can you describe the rule that this function is trying to implement briefly?

[mstorsjo]

COFF/Writer.cpp

329 ↗

(On Diff #124134)

Please add a comment saying that this is for MinGW.

Sure

I'd also mention that this could have been technically avoided because we could have used '$' instead of '.',

No, this still couldn't have been avoided by using '$' instead of '.', since half of the point is to order sections according to the numerical value, not alphabetically. Alphabetically, 123 would be ordered before 5.

Passing a StringRef by reference seems a bit odd. The class is designed to be passed by value, because it is essentially a pointer with size.

Yes, that's obviously better. I had tried that, but missed that I needed to update the prototype in the declaration of std::map<..> Map as well.

Can you describe the rule that this function is trying to implement briefly?

Order sections alphabetically, as before, with two exceptions:

• If the base section name is followed by a period, it should be sorted before section names followed by a dollar sign. An alphabetical sort would give the order .ctors, .ctors$zzz, .ctors.123.
• If both base section names are followed by a period, order them according to the numerical value following the period, instead of the lexical sort of the strings.

I can update with the redundant const StringRef& removed and some comments added.

[mstorsjo]

Added comments, removed redundant checking for periods (replaced with asserts), removed the redundant StringRef references.

[mstorsjo]

I noticed that GCC actually does zero pad these, and GNU ld expects them zero padded, so this simplifies this patch quite significantly. The .ctors$zzz part is only used in the llvm/lld specific parts of mingw-w64, and can be replaced with .ctors.99999 or something similar once this goes in, to avoid the need for the special sorting of them.

[mstorsjo]

Added a comment in the source to clarify that this is for MinGW.

[ruiu]

Nice! LGTM

COFF/Writer.cpp

322–324 ↗

(On Diff #124327)

nit: add blank lines before and after this code block so that MinGW code looks separated from other code.

lld/trunk/COFF/Writer.cpp

void Writer::run() {
   writeMapFile(OutputSections);
 
   if (auto E = Buffer->commit())
     fatal("failed to write the output file: " + toString(std::move(E)));
 }
 
 static StringRef getOutputSection(StringRef Name) {
   StringRef S = Name.split('$').first;
+
+  // Treat a later period as a separator for MinGW, for sections like
+  // ".ctors.01234".
+  S = S.substr(0, S.find('.', 1));
+
   auto It = Config->Merge.find(S);
   if (It == Config->Merge.end())
     return S;
   return It->second;
 }
 
 // Create output section objects and add them to OutputSections.
 void Writer::createSections() {

lld/trunk/test/COFF/ctors_dtors_priority.s

+# REQUIRES: x86
+# RUN: llvm-mc -triple=x86_64-windows-gnu -filetype=obj -o %t.obj %s
+# RUN: lld-link -entry:main %t.obj -out:%t.exe
+# RUN: llvm-objdump -s %t.exe | FileCheck %s
+
+.globl main
+main:
+  nop
+
+.section .ctors.00005, "w"
+  .quad 2
+.section .ctors, "w"
+  .quad 1
+.section .ctors.00100, "w"
+  .quad 3
+
+.section .dtors, "w"
+  .quad 1
+.section .dtors.00100, "w"
+  .quad 3
+.section .dtors.00005, "w"
+  .quad 2
+
+# CHECK:      Contents of section .ctors:
+# CHECK-NEXT: 140001000 01000000 00000000 02000000 00000000
+# CHECK-NEXT: 140001010 03000000 00000000
+
+# CHECK:      Contents of section .dtors:
+# CHECK-NEXT: 140002000 01000000 00000000 02000000 00000000
+# CHECK-NEXT: 140002010 03000000 00000000