[LLD][COFF] Fix ordering of CRT global initializers in COMDAT sections

aganea · aganea · commit 149de8de1906 · 2018-10-05T12:56:46.000Z
(patch by Benoit Rousseau) This patch fixes a bug where the global variable initializers were sometimes not invoked in the correct order when it involved a C++ template instantiation. Differential Revision: https://reviews.llvm.org/D52749 llvm-svn: 343847
diff --git a/lld/COFF/Chunks.cpp b/lld/COFF/Chunks.cpp
@@ -590,6 +590,13 @@ void SectionChunk::replace(SectionChunk *Other) {
   Other->Live = false;
 }
 
+uint32_t SectionChunk::getSectionNumber() const {
+  DataRefImpl R;
+  R.p = reinterpret_cast<uintptr_t>(Header);
+  SectionRef S(R, File->getCOFFObj());
+  return S.getIndex() + 1;
+}
+
 CommonChunk::CommonChunk(const COFFSymbolRef S) : Sym(S) {
   // Common symbols are aligned on natural boundaries up to 32 bytes.
   // This is what MSVC link.exe does.
diff --git a/lld/COFF/Chunks.h b/lld/COFF/Chunks.h
@@ -203,10 +203,13 @@ class SectionChunk final : public Chunk {
   // Allow iteration over the associated child chunks for this section.
   ArrayRef<SectionChunk *> children() const { return AssocChildren; }
 
+  // The section ID this chunk belongs to in its Obj.
+  uint32_t getSectionNumber() const;
+
   // A pointer pointing to a replacement for this chunk.
   // Initially it points to "this" object. If this chunk is merged
   // with other chunk by ICF, it points to another chunk,
-  // and this chunk is considrered as dead.
+  // and this chunk is considered as dead.
   SectionChunk *Repl;
 
   // The CRC of the contents as described in the COFF spec 4.5.5.
diff --git a/lld/COFF/Writer.cpp b/lld/COFF/Writer.cpp
@@ -192,6 +192,7 @@ class Writer {
   void writeSections();
   void writeBuildId();
   void sortExceptionTable();
+  void sortCRTSectionChunks(std::vector<Chunk *> &Chunks);
 
   llvm::Optional<coff_symbol16> createSymbol(Defined *D);
   size_t addEntryToStringTable(StringRef Str);
@@ -732,13 +733,18 @@ void Writer::createSections() {
     StringRef Name = getOutputSectionName(Pair.first.first);
     uint32_t OutChars = Pair.first.second;
 
-    // In link.exe, there is a special case for the I386 target where .CRT
-    // sections are treated as if they have output characteristics DATA | R if
-    // their characteristics are DATA | R | W. This implements the same special
-    // case for all architectures.
-    if (Name == ".CRT")
+    if (Name == ".CRT") {
+      // In link.exe, there is a special case for the I386 target where .CRT
+      // sections are treated as if they have output characteristics DATA | R if
+      // their characteristics are DATA | R | W. This implements the same
+      // special case for all architectures.
       OutChars = DATA | R;
 
+      log("Processing section " + Pair.first.first + " -> " + Name);
+
+      sortCRTSectionChunks(Pair.second);
+    }
+
     OutputSection *Sec = CreateSection(Name, OutChars);
     std::vector<Chunk *> &Chunks = Pair.second;
     for (Chunk *C : Chunks)
@@ -1577,6 +1583,42 @@ void Writer::sortExceptionTable() {
   errs() << "warning: don't know how to handle .pdata.\n";
 }
 
+// The CRT section contains, among other things, the array of function
+// pointers that initialize every global variable that is not trivially
+// constructed. The CRT calls them one after the other prior to invoking
+// main().
+//
+// As per C++ spec, 3.6.2/2.3,
+// "Variables with ordered initialization defined within a single
+// translation unit shall be initialized in the order of their definitions
+// in the translation unit"
+//
+// It is therefore critical to sort the chunks containing the function
+// pointers in the order that they are listed in the object file (top to
+// bottom), otherwise global objects might not be initialized in the
+// correct order.
+void Writer::sortCRTSectionChunks(std::vector<Chunk *> &Chunks) {
+  auto SectionChunkOrder = [](const Chunk *A, const Chunk *B) {
+    auto SA = dyn_cast<SectionChunk>(A);
+    auto SB = dyn_cast<SectionChunk>(B);
+    assert(SA && SB && "Non-section chunks in CRT section!");
+
+    StringRef SAObj = SA->File->MB.getBufferIdentifier();
+    StringRef SBObj = SB->File->MB.getBufferIdentifier();
+
+    return SAObj == SBObj && SA->getSectionNumber() < SB->getSectionNumber();
+  };
+  std::stable_sort(Chunks.begin(), Chunks.end(), SectionChunkOrder);
+
+  if (Config->Verbose) {
+    for (auto &C : Chunks) {
+      auto SC = dyn_cast<SectionChunk>(C);
+      log("  " + SC->File->MB.getBufferIdentifier().str() +
+          ", SectionID: " + Twine(SC->getSectionNumber()));
+    }
+  }
+}
+
 OutputSection *Writer::findSection(StringRef Name) {
   for (OutputSection *Sec : OutputSections)
     if (Sec->Name == Name)
diff --git a/lld/test/COFF/Inputs/crt-dyn-initializer-order_1.yaml b/lld/test/COFF/Inputs/crt-dyn-initializer-order_1.yaml
@@ -0,0 +1,15 @@
+--- !COFF
+header:
+  Machine:         IMAGE_FILE_MACHINE_AMD64
+  Characteristics: [  ]
+sections:
+  - Name:            '.CRT$XCU'
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+    Alignment:       1
+    SectionData:     55
+  - Name:            '.CRT$XCU'
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+    Alignment:       1
+    SectionData:     70
+symbols:
+...
diff --git a/lld/test/COFF/Inputs/crt-dyn-initializer-order_2.yaml b/lld/test/COFF/Inputs/crt-dyn-initializer-order_2.yaml
@@ -0,0 +1,19 @@
+--- !COFF
+header:
+  Machine:         IMAGE_FILE_MACHINE_AMD64
+  Characteristics: [  ]
+sections:
+  - Name:            '.CRT$XCU'
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+    Alignment:       1
+    SectionData:     10
+  - Name:            '.CRT$XCU'
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+    Alignment:       1
+    SectionData:     11
+  - Name:            '.CRT$XCU'
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+    Alignment:       1
+    SectionData:     12
+symbols:
+...
diff --git a/lld/test/COFF/crt-dyn-initializer-order.test b/lld/test/COFF/crt-dyn-initializer-order.test
@@ -0,0 +1,100 @@
+# // a.cpp
+# #include <iostream>
+# #include <vector>
+# 
+# template <int Magic> struct TemplatedObject {
+#   static std::vector<TemplatedObject<Magic> *> Instances;
+#   TemplatedObject() { Instances.push_back(this); }
+# };
+# 
+# using Object = TemplatedObject<0>;
+# template <> std::vector<Object *> Object::Instances{};
+# Object idle{};
+# 
+# int main() {
+#   if (Object::Instances.size() == 0)
+#     std::cout << "It's broken" << std::endl;
+#   else
+#     std::cout << "It works!" << std::endl;
+#   return 0;
+# }
+# // using `clang-cl /c a.cpp | lld-link a.obj` works
+# // using `cl /c a.cpp | lld-link a.obj` fails without lld/COFF/Writer.cpp/Writer::sortSectionChunks()
+
+# RUN: yaml2obj %s > %t.obj
+# RUN: yaml2obj %S/Inputs/crt-dyn-initializer-order_1.yaml > %t1.obj
+# RUN: yaml2obj %S/Inputs/crt-dyn-initializer-order_2.yaml > %t2.obj
+
+# CHECK: Name: .CRT
+# CHECK: Characteristics [
+# CHECK-NEXT: IMAGE_SCN_CNT_INITIALIZED_DATA
+# CHECK-NEXT: IMAGE_SCN_MEM_READ
+# CHECK-NEXT: ]
+# CHECK-NEXT: SectionData (
+
+# RUN: lld-link /out:%t.dll /entry:__ImageBase /dll %t.obj %t1.obj %t2.obj 
+# RUN: llvm-readobj -sections -section-data %t.dll | FileCheck %s --check-prefixes CHECK,CASE1
+# CASE1-NEXT: 01020304 55701011 1205
+
+# RUN: lld-link /out:%t.dll /entry:__ImageBase /dll %t.obj %t2.obj %t1.obj 
+# RUN: llvm-readobj -sections -section-data %t.dll | FileCheck %s --check-prefixes CHECK,CASE2
+# CASE2-NEXT: 01020304 10111255 7005
+
+# RUN: lld-link /out:%t.dll /entry:__ImageBase /dll %t1.obj %t2.obj %t.obj 
+# RUN: llvm-readobj -sections -section-data %t.dll | FileCheck %s --check-prefixes CHECK,CASE3
+# CASE3-NEXT: 01557010 11120203 0405
+
+# RUN: lld-link /out:%t.dll /entry:__ImageBase /dll %t1.obj %t.obj %t2.obj 
+# RUN: llvm-readobj -sections -section-data %t.dll | FileCheck %s --check-prefixes CHECK,CASE4
+# CASE4-NEXT: 01557002 03041011 1205
+
+# RUN: lld-link /out:%t.dll /entry:__ImageBase /dll %t2.obj %t1.obj %t.obj 
+# RUN: llvm-readobj -sections -section-data %t.dll | FileCheck %s --check-prefixes CHECK,CASE5
+# CASE5-NEXT: 01101112 55700203 0405
+
+# RUN: lld-link /out:%t.dll /entry:__ImageBase /dll %t2.obj %t.obj %t1.obj
+# RUN: llvm-readobj -sections -section-data %t.dll | FileCheck %s --check-prefixes CHECK,CASE6
+# CASE6-NEXT: 01101112 02030455 7005
+
+# CHECK-NEXT: )
+
+--- !COFF
+header:
+  Machine:         IMAGE_FILE_MACHINE_AMD64
+  Characteristics: [  ]
+sections:
+  - Name:            '.CRT$XCA'
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+    Alignment:       1
+    SectionData:     01
+  - Name:            '.CRT$XCU'
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+    Alignment:       1
+    SectionData:     02
+  - Name:            '.CRT$XCU'
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ, IMAGE_SCN_LNK_COMDAT ]
+    Alignment:       1
+    SectionData:     03
+  - Name:            '.CRT$XCU'
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+    Alignment:       1
+    SectionData:     04
+  - Name:            '.CRT$XCZ'
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+    Alignment:       1
+    SectionData:     05
+symbols:
+  - Name:            '.CRT$XCU'
+    Value:           0
+    SectionNumber:   3
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition:
+      Length:          1
+      NumberOfRelocations: 0
+      NumberOfLinenumbers: 0
+      CheckSum:        1
+      Number:          2
+      Selection:       IMAGE_COMDAT_SELECT_ASSOCIATIVE
+...
