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

Sunho Kim via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 25 07:05:25 PDT 2022


Author: Sunho Kim
Date: 2022-07-25T23:02:31+09:00
New Revision: 07aa8fc8db6b4b8581e0ba8ef4a66274023c0b59

URL: https://github.com/llvm/llvm-project/commit/07aa8fc8db6b4b8581e0ba8ef4a66274023c0b59
DIFF: https://github.com/llvm/llvm-project/commit/07aa8fc8db6b4b8581e0ba8ef4a66274023c0b59.diff

LOG: [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

Added: 
    llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_intervene.test

Modified: 
    llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.cpp
    llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.h

Removed: 
    


################################################################################
diff  --git a/llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.cpp b/llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.cpp
index 9638f498598f8..88951e96dc690 100644
--- 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 4dc1b14dc4a28..72e221489090f 100644
--- a/llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.h
+++ b/llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.h
@@ -111,10 +111,9 @@ class COFFLinkGraphBuilder {
   // 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
index 0000000000000..6bf8de8ca8a22
--- /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
+...


        


More information about the llvm-commits mailing list