[COFF] Implement constructor priorities

[mstorsjo]

Summary

The priorities in the section name suffixes are zero padded, allowing the linker to just do a lexical sort.

Add zero padding for .ctors sections in ELF as well.

[ruiu]

Could you give me a context of this patch? .ctors and .dtors are essentially an ELF stuff, so I'm wondering how you are going to use it in COFF.

[mstorsjo]

I noticed that the .ctors.1234 numerical suffixes are zero padded by GCC when targeting MinGW, updated this patch accordingly and expanded the test to showcase it.

It doesn't seem like any other modern platform actually defaults to using .ctors at all, so I'm not sure if other platforms should have zero padding or not. If linking an object file that contains unpadded .ctors with GNU ld, they are interpreted numerically (and end up in .init_array). If linking for MinGW with GNU ld though, the zero padding is needed for them to be ordered properly.

[ruiu]

lib/CodeGen/TargetLoweringObjectFileImpl.cpp

1225–1229 ↗

(On Diff #124312)

Maybe std::string Name = IsCtor ? ".ctors" : ".dtors" is slightly more readable?

test/CodeGen/X86/constructor.ll

33 ↗

(On Diff #124312)

Is this correct? I thought it would be .ctors.09980.

[mstorsjo]

lib/CodeGen/TargetLoweringObjectFileImpl.cpp

1225–1229 ↗

(On Diff #124312)

Sure, I can change that.

1233 ↗

(On Diff #124312)

Do you have any suggestion on a more idiomatic way of doing the equivalent of snprintf("%05u", 65535 - Priority) here? This feels quite roundabout.

test/CodeGen/X86/constructor.ll

33 ↗

(On Diff #124312)

I'm actually not sure. My GCC reference doesn't output .ctors on linux (and can't easily be made to output it either, it seems). GNU ld does handle it correctly with .ctors without the zero padding on linux, but not for mingw though. But I found this GCC bug report, which discussed similar matters, and it does seem like it did do zero padding there as well: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46770

So I guess it makes sense to make it consistent and add the zero padding there as well. If the linker does interpret it numerically, it will still work, and if it doesn't, zero padding is the only way it can work.

[ruiu]

lib/CodeGen/TargetLoweringObjectFileImpl.cpp

1233 ↗

(On Diff #124312)

I think snprintf isn't actually bad. Alternatively, you could include llvm/Support/Format.h and do

Number << format("%05u", 65535 - Priority);

. Or you could keep the current code. I don't have a strong preference.

[mstorsjo]

lib/CodeGen/TargetLoweringObjectFileImpl.cpp

1233 ↗

(On Diff #124312)

I did find llvm/Support/Format.h, but that doesn't work with the << operator directly to a std::string. After yet some more grepping around the source base, I found raw_string_ostream, which together with format() probably is the best match of how I want to write it.

[mstorsjo]

Added zero padding for ELF as well, rewrote the zero padding in a more idiomatic way, applied Rui's style suggestion.

[ruiu]

LGTM

lib/CodeGen/TargetLoweringObjectFileImpl.cpp

535–536 ↗

(On Diff #124473)

nit: I believe you could write this as

raw_string_ostream(Name) << format(".%05u", 65535 - Priority);

1226–1227 ↗

(On Diff #124473)

Ditto

[mstorsjo]

lib/CodeGen/TargetLoweringObjectFileImpl.cpp

535–536 ↗

(On Diff #124473)

Oh, yes - thanks!

llvm/trunk/lib/CodeGen/TargetLoweringObjectFileImpl.cpp

 #include "llvm/MC/MCStreamer.h"
 #include "llvm/MC/MCSymbol.h"
 #include "llvm/MC/MCSymbolELF.h"
 #include "llvm/MC/MCValue.h"
 #include "llvm/MC/SectionKind.h"
 #include "llvm/ProfileData/InstrProf.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/CodeGen.h"
+#include "llvm/Support/Format.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Target/TargetMachine.h"
 #include <cassert>
 #include <string>
 
 using namespace llvm;
 using namespace dwarf;
if (UseInitArray) {
     }
   } else {
     // The default scheme is .ctor / .dtor, so we have to invert the priority
     // numbering.
     if (IsCtor)
       Name = ".ctors";
     else
       Name = ".dtors";
-    if (Priority != 65535) {
-      Name += '.';
-      Name += utostr(65535 - Priority);
-    }
+    if (Priority != 65535)
+      raw_string_ostream(Name) << format(".%05u", 65535 - Priority);
     Type = ELF::SHT_PROGBITS;
   }
 
   return Ctx.getELFSection(Name, Type, Flags, 0, COMDAT);
 }
 
 MCSection *TargetLoweringObjectFileELF::getStaticCtorSection(
     unsigned Priority, const MCSymbol *KeySym) const {
StaticCtorSection = Ctx.getCOFFSection(
         SectionKind::getData());
     StaticDtorSection = Ctx.getCOFFSection(
         ".dtors", COFF::IMAGE_SCN_CNT_INITIALIZED_DATA |
                       COFF::IMAGE_SCN_MEM_READ | COFF::IMAGE_SCN_MEM_WRITE,
         SectionKind::getData());
   }
 }
 
+static MCSectionCOFF *getCOFFStaticStructorSection(MCContext &Ctx,
+                                                   const Triple &T, bool IsCtor,
+                                                   unsigned Priority,
+                                                   const MCSymbol *KeySym,
+                                                   MCSectionCOFF *Default) {
+  if (T.isKnownWindowsMSVCEnvironment() || T.isWindowsItaniumEnvironment())
+    return Ctx.getAssociativeCOFFSection(Default, KeySym, 0);
+
+  std::string Name = IsCtor ? ".ctors" : ".dtors";
+  if (Priority != 65535)
+    raw_string_ostream(Name) << format(".%05u", 65535 - Priority);
+
+  return Ctx.getAssociativeCOFFSection(
+      Ctx.getCOFFSection(Name, COFF::IMAGE_SCN_CNT_INITIALIZED_DATA |
+                                   COFF::IMAGE_SCN_MEM_READ |
+                                   COFF::IMAGE_SCN_MEM_WRITE,
+                         SectionKind::getData()),
+      KeySym, 0);
+}
+
 MCSection *TargetLoweringObjectFileCOFF::getStaticCtorSection(
     unsigned Priority, const MCSymbol *KeySym) const {
-  return getContext().getAssociativeCOFFSection(
-      cast<MCSectionCOFF>(StaticCtorSection), KeySym, 0);
+  return getCOFFStaticStructorSection(getContext(), getTargetTriple(), true,
+                                      Priority, KeySym,
+                                      cast<MCSectionCOFF>(StaticCtorSection));
 }
 
 MCSection *TargetLoweringObjectFileCOFF::getStaticDtorSection(
     unsigned Priority, const MCSymbol *KeySym) const {
-  return getContext().getAssociativeCOFFSection(
-      cast<MCSectionCOFF>(StaticDtorSection), KeySym, 0);
+  return getCOFFStaticStructorSection(getContext(), getTargetTriple(), false,
+                                      Priority, KeySym,
+                                      cast<MCSectionCOFF>(StaticDtorSection));
 }
 
 void TargetLoweringObjectFileCOFF::emitLinkerFlagsForGlobal(
     raw_ostream &OS, const GlobalValue *GV) const {
   emitLinkerFlagsForGlobalCOFF(OS, GV, getTargetTriple(), getMangler());
 }
 
 //===----------------------------------------------------------------------===//

llvm/trunk/test/CodeGen/X86/constructor.ll

 ; RUN: llc -mtriple x86_64-pc-linux -use-ctors < %s | FileCheck --check-prefix=CTOR %s
 ; RUN: llc -mtriple x86_64-unknown-freebsd -use-ctors < %s | FileCheck --check-prefix=CTOR %s
 ; RUN: llc -mtriple x86_64-pc-solaris2.11 -use-ctors < %s | FileCheck --check-prefix=CTOR %s
 ; RUN: llc -mtriple x86_64-pc-linux < %s | FileCheck --check-prefix=INIT-ARRAY %s
 ; RUN: llc -mtriple x86_64-unknown-freebsd < %s | FileCheck --check-prefix=INIT-ARRAY %s
 ; RUN: llc -mtriple x86_64-pc-solaris2.11 < %s | FileCheck --check-prefix=INIT-ARRAY %s
 ; RUN: llc -mtriple x86_64-unknown-nacl < %s | FileCheck --check-prefix=NACL %s
 ; RUN: llc -mtriple i586-intel-elfiamcu -use-ctors < %s | FileCheck %s --check-prefix=MCU-CTORS
 ; RUN: llc -mtriple i586-intel-elfiamcu < %s | FileCheck %s --check-prefix=MCU-INIT-ARRAY
[e-mail]_ctors = appending global [2 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @f, i8* null}, { i32, void ()*, i8* } { i32 15, void ()* @g, i8* @v }]
+; RUN: llc -mtriple x86_64-win32-gnu < %s | FileCheck --check-prefix=COFF-CTOR %s
[e-mail]_ctors = appending global [3 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @f, i8* null}, { i32, void ()*, i8* } { i32 15, void ()* @g, i8* @v }, { i32, void ()*, i8* } { i32 55555, void ()* @h, i8* @v }]
 
 @v = weak_odr global i8 0
 
 define void @f() {
 entry:
   ret void
 }
 
 define void @g() {
 entry:
   ret void
 }
 
+define void @h() {
+entry:
+  ret void
+}
+
 ; CTOR:   .section  .ctors.65520,"aGw",@progbits,v,comdat
 ; CTOR-NEXT:  .p2align  3
 ; CTOR-NEXT:  .quad g
+; CTOR-NEXT:  .section  .ctors.09980,"aGw",@progbits,v,comdat
+; CTOR-NEXT:  .p2align  3
+; CTOR-NEXT:  .quad h
 ; CTOR-NEXT:  .section  .ctors,"aw",@progbits
 ; CTOR-NEXT:  .p2align  3
 ; CTOR-NEXT:  .quad f
 
 ; INIT-ARRAY:   .section  .init_array.15,"aGw",@init_array,v,comdat
 ; INIT-ARRAY-NEXT:  .p2align  3
 ; INIT-ARRAY-NEXT:  .quad g
+; INIT-ARRAY-NEXT:  .section  .init_array.55555,"aGw",@init_array,v,comdat
+; INIT-ARRAY-NEXT:  .p2align  3
+; INIT-ARRAY-NEXT:  .quad h
 ; INIT-ARRAY-NEXT:  .section  .init_array,"aw",@init_array
 ; INIT-ARRAY-NEXT:  .p2align  3
 ; INIT-ARRAY-NEXT:  .quad f
 
 ; NACL:   .section  .init_array.15,"aGw",@init_array,v,comdat
 ; NACL-NEXT:  .p2align  2
 ; NACL-NEXT:  .long g
+; NACL-NEXT:  .section  .init_array.55555,"aGw",@init_array,v,comdat
+; NACL-NEXT:  .p2align  2
+; NACL-NEXT:  .long h
 ; NACL-NEXT:  .section  .init_array,"aw",@init_array
 ; NACL-NEXT:  .p2align  2
 ; NACL-NEXT:  .long f
 
 ; MCU-CTORS:         .section        .ctors,"aw",@progbits
 ; MCU-INIT-ARRAY:    .section        .init_array,"aw",@init_array
+
+; COFF-CTOR:    .section  .ctors.65520,"dw",associative,v
+; COFF-CTOR-NEXT: .p2align  3
+; COFF-CTOR-NEXT: .quad g
+; COFF-CTOR-NEXT: .section  .ctors.09980,"dw",associative,v
+; COFF-CTOR-NEXT: .p2align  3
+; COFF-CTOR-NEXT: .quad h
+; COFF-CTOR-NEXT: .section  .ctors,"dw"
+; COFF-CTOR-NEXT: .p2align  3
+; COFF-CTOR-NEXT: .quad f