[JITLink][COFF] Handle out-of-order COMDAT second symbol.

Handle out-of-order COMDAT second symbols. In llvm codegen, the second symbol of COMDAT sequence always follows the first symbol in the global symbol list. But, when the object file came from MSVC compiler, these can come in out of order.

Reviewed By: lhames

Differential Revision: https://reviews.llvm.org/D129721

diff --git a/llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.cpp b/llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.cpp
index 9638f49..88951e9 100644 (file)
--- a/llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.cpp
+++ b/llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.cpp
@@ -190,6 +190,7 @@ Error COFFLinkGraphBuilder::graphifySymbols() {
   LLVM_DEBUG(dbgs() << "  Creating graph symbols...\n");
 
   SymbolSets.resize(Obj.getNumberOfSections() + 1);
+  PendingComdatExports.resize(Obj.getNumberOfSections() + 1);
   GraphSymbols.resize(Obj.getNumberOfSymbols());
 
   for (COFFSymbolIndex SymIndex = 0;
@@ -396,18 +397,16 @@ Expected<Symbol *> COFFLinkGraphBuilder::createDefinedSymbol(
   Block *B = getGraphBlock(Symbol.getSectionNumber());
   if (Symbol.isExternal()) {
     // This is not a comdat sequence, export the symbol as it is
-    if (!isComdatSection(Section))
+    if (!isComdatSection(Section)) {
+
       return &G->addDefinedSymbol(
           *B, Symbol.getValue(), SymbolName, 0, Linkage::Strong, Scope::Default,
           Symbol.getComplexType() == COFF::IMAGE_SYM_DTYPE_FUNCTION, false);
-    else {
-      if (!PendingComdatExport)
+    } else {
+      if (!PendingComdatExports[Symbol.getSectionNumber()])
         return make_error<JITLinkError>("No pending COMDAT export for symbol " +
                                         formatv("{0:d}", SymIndex));
-      if (PendingComdatExport->SectionIndex != Symbol.getSectionNumber())
-        return make_error<JITLinkError>(
-            "COMDAT export section number mismatch for symbol " +
-            formatv("{0:d}", SymIndex));
+
       return exportCOMDATSymbol(SymIndex, SymbolName, Symbol);
     }
   }
@@ -429,7 +428,7 @@ Expected<Symbol *> COFFLinkGraphBuilder::createDefinedSymbol(
       getGraphBlock(Target)->addEdge(Edge::KeepAlive, 0, *GSym, 0);
       return GSym;
     }
-    if (PendingComdatExport)
+    if (PendingComdatExports[Symbol.getSectionNumber()])
       return make_error<JITLinkError>(
           "COMDAT export request already exists before symbol " +
           formatv("{0:d}", SymIndex));
@@ -491,7 +490,7 @@ Expected<Symbol *> COFFLinkGraphBuilder::createCOMDATExportRequest(
                                     formatv("{0:d}", Definition->Selection));
   }
   }
-  PendingComdatExport = {SymIndex, Symbol.getSectionNumber(), L};
+  PendingComdatExports[Symbol.getSectionNumber()] = {SymIndex, L};
   return &G->addAnonymousSymbol(*B, Symbol.getValue(), Definition->Length,
                                 false, false);
 }
@@ -501,6 +500,7 @@ Expected<Symbol *>
 COFFLinkGraphBuilder::exportCOMDATSymbol(COFFSymbolIndex SymIndex,
                                          StringRef SymbolName,
                                          object::COFFSymbolRef Symbol) {
+  auto &PendingComdatExport = PendingComdatExports[Symbol.getSectionNumber()];
   COFFSymbolIndex TargetIndex = PendingComdatExport->SymbolIndex;
   Linkage L = PendingComdatExport->Linkage;
   jitlink::Symbol *Target = getGraphSymbol(TargetIndex);

diff --git a/llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.h b/llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.h
index 4dc1b14..72e2214 100644 (file)
--- a/llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.h
+++ b/llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.h
@@ -111,10 +111,9 @@ private:
   // COMDAT sequence.
   struct ComdatExportRequest {
     COFFSymbolIndex SymbolIndex;
-    COFFSectionIndex SectionIndex;
     jitlink::Linkage Linkage;
   };
-  Optional<ComdatExportRequest> PendingComdatExport;
+  std::vector<Optional<ComdatExportRequest>> PendingComdatExports;
 
   // This represents a pending request to create a weak external symbol with a
   // name.

diff --git a/llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_intervene.test b/llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_intervene.test
new file mode 100644 (file)
index 0000000..6bf8de8
--- /dev/null
+++ b/llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_intervene.test
@@ -0,0 +1,87 @@
+# REQUIRES: asserts
+# RUN: yaml2obj %s -o %t
+# RUN: llvm-jitlink -noexec --debug-only=jitlink -noexec %t 2>&1 | FileCheck %s
+# 
+# Check a comdat export is done correctly even if second symbol of comdat sequences appear out of order
+#
+# CHECK: Creating graph symbols...
+# CHECK:      6: Exporting COMDAT graph symbol for COFF symbol "func2" in section 3
+# CHECK-NEXT:   0x0 (block + 0x00000000): size: 0x00000001, linkage: weak, scope: default, dead  -   func2
+# CHECK:      7: Exporting COMDAT graph symbol for COFF symbol "func" in section 2
+# CHECK-NEXT:   0x0 (block + 0x00000000): size: 0x00000001, linkage: weak, scope: default, dead  -   func
+
+--- !COFF
+header:
+  Machine:         IMAGE_FILE_MACHINE_AMD64
+  Characteristics: [  ]
+sections:
+  - Name:            .text
+    Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+    Alignment:       16
+    SectionData:     C3
+  - Name:            .text
+    Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_LNK_COMDAT, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+    Alignment:       16
+    SectionData:     C3
+  - Name:            .text
+    Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_LNK_COMDAT, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+    Alignment:       16
+    SectionData:     C3
+symbols:
+  - Name:            .text
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition:
+      Length:          1
+      NumberOfRelocations: 0
+      NumberOfLinenumbers: 0
+      CheckSum:        40735498
+      Number:          1
+  - Name:            .text
+    Value:           0
+    SectionNumber:   2
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition:
+      Length:          1
+      NumberOfRelocations: 0
+      NumberOfLinenumbers: 0
+      CheckSum:        40735498
+      Number:          2
+      Selection:       IMAGE_COMDAT_SELECT_ANY
+  - Name:            .text
+    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:        40735498
+      Number:          3
+      Selection:       IMAGE_COMDAT_SELECT_ANY
+  - Name:            func2
+    Value:           0
+    SectionNumber:   3
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_FUNCTION
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+  - Name:            func
+    Value:           0
+    SectionNumber:   2
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_FUNCTION
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+  - Name:            main
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_FUNCTION
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+...