[LLD][COFF] Fix ordering of CRT global initializers in COMDAT sections
authorAlexandre Ganea <alexandre.ganea@ubisoft.com>
Fri, 5 Oct 2018 12:56:46 +0000 (12:56 +0000)
committerAlexandre Ganea <alexandre.ganea@ubisoft.com>
Fri, 5 Oct 2018 12:56:46 +0000 (12:56 +0000)
(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

lld/COFF/Chunks.cpp
lld/COFF/Chunks.h
lld/COFF/Writer.cpp
lld/test/COFF/Inputs/crt-dyn-initializer-order_1.yaml [new file with mode: 0644]
lld/test/COFF/Inputs/crt-dyn-initializer-order_2.yaml [new file with mode: 0644]
lld/test/COFF/crt-dyn-initializer-order.test [new file with mode: 0644]

index eb90bd7..5fb33e1 100644 (file)
@@ -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.
index e9facb8..bdfff7c 100644 (file)
@@ -203,10 +203,13 @@ public:
   // 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.
index 23de466..d42c82c 100644 (file)
@@ -192,6 +192,7 @@ private:
   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
new file mode 100644 (file)
index 0000000..302f6f2
--- /dev/null
@@ -0,0 +1,15 @@
+--- !COFF\r
+header:\r
+  Machine:         IMAGE_FILE_MACHINE_AMD64\r
+  Characteristics: [  ]\r
+sections:\r
+  - Name:            '.CRT$XCU'\r
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]\r
+    Alignment:       1\r
+    SectionData:     55\r
+  - Name:            '.CRT$XCU'\r
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]\r
+    Alignment:       1\r
+    SectionData:     70\r
+symbols:\r
+...\r
diff --git a/lld/test/COFF/Inputs/crt-dyn-initializer-order_2.yaml b/lld/test/COFF/Inputs/crt-dyn-initializer-order_2.yaml
new file mode 100644 (file)
index 0000000..a2d0e5e
--- /dev/null
@@ -0,0 +1,19 @@
+--- !COFF\r
+header:\r
+  Machine:         IMAGE_FILE_MACHINE_AMD64\r
+  Characteristics: [  ]\r
+sections:\r
+  - Name:            '.CRT$XCU'\r
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]\r
+    Alignment:       1\r
+    SectionData:     10\r
+  - Name:            '.CRT$XCU'\r
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]\r
+    Alignment:       1\r
+    SectionData:     11\r
+  - Name:            '.CRT$XCU'\r
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]\r
+    Alignment:       1\r
+    SectionData:     12\r
+symbols:\r
+...\r
diff --git a/lld/test/COFF/crt-dyn-initializer-order.test b/lld/test/COFF/crt-dyn-initializer-order.test
new file mode 100644 (file)
index 0000000..963b065
--- /dev/null
@@ -0,0 +1,100 @@
+# // a.cpp\r
+# #include <iostream>\r
+# #include <vector>\r
+# \r
+# template <int Magic> struct TemplatedObject {\r
+#   static std::vector<TemplatedObject<Magic> *> Instances;\r
+#   TemplatedObject() { Instances.push_back(this); }\r
+# };\r
+# \r
+# using Object = TemplatedObject<0>;\r
+# template <> std::vector<Object *> Object::Instances{};\r
+# Object idle{};\r
+# \r
+# int main() {\r
+#   if (Object::Instances.size() == 0)\r
+#     std::cout << "It's broken" << std::endl;\r
+#   else\r
+#     std::cout << "It works!" << std::endl;\r
+#   return 0;\r
+# }\r
+# // using `clang-cl /c a.cpp | lld-link a.obj` works\r
+# // using `cl /c a.cpp | lld-link a.obj` fails without lld/COFF/Writer.cpp/Writer::sortSectionChunks()\r
+\r
+# RUN: yaml2obj %s > %t.obj\r
+# RUN: yaml2obj %S/Inputs/crt-dyn-initializer-order_1.yaml > %t1.obj\r
+# RUN: yaml2obj %S/Inputs/crt-dyn-initializer-order_2.yaml > %t2.obj\r
+\r
+# CHECK: Name: .CRT\r
+# CHECK: Characteristics [\r
+# CHECK-NEXT: IMAGE_SCN_CNT_INITIALIZED_DATA\r
+# CHECK-NEXT: IMAGE_SCN_MEM_READ\r
+# CHECK-NEXT: ]\r
+# CHECK-NEXT: SectionData (\r
+\r
+# RUN: lld-link /out:%t.dll /entry:__ImageBase /dll %t.obj %t1.obj %t2.obj \r
+# RUN: llvm-readobj -sections -section-data %t.dll | FileCheck %s --check-prefixes CHECK,CASE1\r
+# CASE1-NEXT: 01020304 55701011 1205\r
+\r
+# RUN: lld-link /out:%t.dll /entry:__ImageBase /dll %t.obj %t2.obj %t1.obj \r
+# RUN: llvm-readobj -sections -section-data %t.dll | FileCheck %s --check-prefixes CHECK,CASE2\r
+# CASE2-NEXT: 01020304 10111255 7005\r
+\r
+# RUN: lld-link /out:%t.dll /entry:__ImageBase /dll %t1.obj %t2.obj %t.obj \r
+# RUN: llvm-readobj -sections -section-data %t.dll | FileCheck %s --check-prefixes CHECK,CASE3\r
+# CASE3-NEXT: 01557010 11120203 0405\r
+\r
+# RUN: lld-link /out:%t.dll /entry:__ImageBase /dll %t1.obj %t.obj %t2.obj \r
+# RUN: llvm-readobj -sections -section-data %t.dll | FileCheck %s --check-prefixes CHECK,CASE4\r
+# CASE4-NEXT: 01557002 03041011 1205\r
+\r
+# RUN: lld-link /out:%t.dll /entry:__ImageBase /dll %t2.obj %t1.obj %t.obj \r
+# RUN: llvm-readobj -sections -section-data %t.dll | FileCheck %s --check-prefixes CHECK,CASE5\r
+# CASE5-NEXT: 01101112 55700203 0405\r
+\r
+# RUN: lld-link /out:%t.dll /entry:__ImageBase /dll %t2.obj %t.obj %t1.obj\r
+# RUN: llvm-readobj -sections -section-data %t.dll | FileCheck %s --check-prefixes CHECK,CASE6\r
+# CASE6-NEXT: 01101112 02030455 7005\r
+\r
+# CHECK-NEXT: )\r
+\r
+--- !COFF\r
+header:\r
+  Machine:         IMAGE_FILE_MACHINE_AMD64\r
+  Characteristics: [  ]\r
+sections:\r
+  - Name:            '.CRT$XCA'\r
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]\r
+    Alignment:       1\r
+    SectionData:     01\r
+  - Name:            '.CRT$XCU'\r
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]\r
+    Alignment:       1\r
+    SectionData:     02\r
+  - Name:            '.CRT$XCU'\r
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ, IMAGE_SCN_LNK_COMDAT ]\r
+    Alignment:       1\r
+    SectionData:     03\r
+  - Name:            '.CRT$XCU'\r
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]\r
+    Alignment:       1\r
+    SectionData:     04\r
+  - Name:            '.CRT$XCZ'\r
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]\r
+    Alignment:       1\r
+    SectionData:     05\r
+symbols:\r
+  - Name:            '.CRT$XCU'\r
+    Value:           0\r
+    SectionNumber:   3\r
+    SimpleType:      IMAGE_SYM_TYPE_NULL\r
+    ComplexType:     IMAGE_SYM_DTYPE_NULL\r
+    StorageClass:    IMAGE_SYM_CLASS_STATIC\r
+    SectionDefinition:\r
+      Length:          1\r
+      NumberOfRelocations: 0\r
+      NumberOfLinenumbers: 0\r
+      CheckSum:        1\r
+      Number:          2\r
+      Selection:       IMAGE_COMDAT_SELECT_ASSOCIATIVE\r
+...\r