[JITLink][COFF] Handle COMDAT symbol with offset.

Handles COMDAT symbol with an offset and refactor the code to only generated symbol if the second symbol was encountered. This happens very infrequently but happens in recursive_mutex implementation of MSVC STL library.

Reviewed By: lhames

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

diff --git a/llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.cpp b/llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.cpp
index 4021415..4e823e6 100644 (file)
--- a/llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.cpp
+++ b/llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.cpp
@@ -579,7 +579,8 @@ Expected<Symbol *> COFFLinkGraphBuilder::createCOMDATExportRequest(
       dbgs() << "    " << SymIndex
              << ": Partially supported IMAGE_COMDAT_SELECT_LARGEST was used"
                 " in section "
-             << Symbol.getSectionNumber() << "\n";
+             << Symbol.getSectionNumber() << " (size: " << Definition->Length
+             << ")\n";
     });
     L = Linkage::Weak;
     break;
@@ -594,9 +595,9 @@ Expected<Symbol *> COFFLinkGraphBuilder::createCOMDATExportRequest(
                                     formatv("{0:d}", Definition->Selection));
   }
   }
-  PendingComdatExports[Symbol.getSectionNumber()] = {SymIndex, L};
-  return &G->addAnonymousSymbol(*B, Symbol.getValue(), Definition->Length,
-                                false, false);
+  PendingComdatExports[Symbol.getSectionNumber()] = {SymIndex, L,
+                                                     Definition->Length};
+  return nullptr;
 }
 
 // Process the second symbol of COMDAT sequence.
@@ -604,30 +605,26 @@ Expected<Symbol *>
 COFFLinkGraphBuilder::exportCOMDATSymbol(COFFSymbolIndex SymIndex,
                                          StringRef SymbolName,
                                          object::COFFSymbolRef Symbol) {
+  Block *B = getGraphBlock(Symbol.getSectionNumber());
   auto &PendingComdatExport = PendingComdatExports[Symbol.getSectionNumber()];
-  COFFSymbolIndex TargetIndex = PendingComdatExport->SymbolIndex;
-  Linkage L = PendingComdatExport->Linkage;
-  jitlink::Symbol *Target = getGraphSymbol(TargetIndex);
-  assert(Target && "COMDAT leaader is invalid.");
-  assert((llvm::count_if(G->defined_symbols(),
-                         [&](const jitlink::Symbol *Sym) {
-                           return Sym->getName() == SymbolName;
-                         }) == 0) &&
-         "Duplicate defined symbol");
-  Target->setName(SymbolName);
-  Target->setLinkage(L);
-  Target->setCallable(Symbol.getComplexType() ==
-                      COFF::IMAGE_SYM_DTYPE_FUNCTION);
-  Target->setScope(Scope::Default);
+  // NOTE: ComdatDef->Legnth is the size of "section" not size of symbol.
+  // We use zero symbol size to not reach out of bound of block when symbol
+  // offset is non-zero.
+  auto GSym = &G->addDefinedSymbol(
+      *B, Symbol.getValue(), SymbolName, 0, PendingComdatExport->Linkage,
+      Scope::Default, Symbol.getComplexType() == COFF::IMAGE_SYM_DTYPE_FUNCTION,
+      false);
   LLVM_DEBUG({
     dbgs() << "    " << SymIndex
            << ": Exporting COMDAT graph symbol for COFF symbol \"" << SymbolName
            << "\" in section " << Symbol.getSectionNumber() << "\n";
-    dbgs() << "      " << *Target << "\n";
+    dbgs() << "      " << *GSym << "\n";
   });
+  setGraphSymbol(Symbol.getSectionNumber(), PendingComdatExport->SymbolIndex,
+                 *GSym);
+  DefinedSymbols[SymbolName] = GSym;
   PendingComdatExport = None;
-  DefinedSymbols[SymbolName] = Target;
-  return Target;
+  return GSym;
 }
 
 } // namespace jitlink

diff --git a/llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.h b/llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.h
index 1fd881c..f7f10bc 100644 (file)
--- a/llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.h
+++ b/llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.h
@@ -114,6 +114,7 @@ private:
   struct ComdatExportRequest {
     COFFSymbolIndex SymbolIndex;
     jitlink::Linkage Linkage;
+    orc::ExecutorAddrDiff Size;
   };
   std::vector<Optional<ComdatExportRequest>> PendingComdatExports;
 

diff --git a/llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_any.test b/llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_any.test
index 7133a10..10f1182 100644 (file)
--- a/llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_any.test
+++ b/llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_any.test
@@ -5,10 +5,8 @@
 # Check a weak symbol is created for a COMDAT symbol with IMAGE_COMDAT_SELECT_ANY selection type.
 #
 # CHECK: Creating graph symbols...
-# CHECK:      2: Creating defined graph symbol for COFF symbol ".text" in .text (index: 2)
-# CHECK-NEXT:   0x0 (block + 0x00000000): size: 0x00000001, linkage: strong, scope: local, dead  -   <anonymous symbol>
-# CHECK-NEXT: 4: Exporting COMDAT graph symbol for COFF symbol "func" in section 2
-# CHECK-NEXT:   0x0 (block + 0x00000000): size: 0x00000001, linkage: weak, scope: default, dead  -   func
+# CHECK: 4: Exporting COMDAT graph symbol for COFF symbol "func" in section 2
+# CHECK-NEXT:   0x0 (block + 0x00000000): size: 0x00000000, linkage: weak, scope: default, dead  -   func
 
 --- !COFF
 header:

diff --git a/llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_exact_match.test b/llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_exact_match.test
index 7407d42..f757271 100644 (file)
--- a/llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_exact_match.test
+++ b/llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_exact_match.test
@@ -6,10 +6,8 @@
 # Doesn't check the content validation.
 #
 # CHECK: Creating graph symbols...
-# CHECK:      2: Creating defined graph symbol for COFF symbol ".text" in .text (index: 2)
-# CHECK-NEXT:   0x0 (block + 0x00000000): size: 0x00000001, linkage: strong, scope: local, dead  -   <anonymous symbol>
-# CHECK-NEXT: 4: Exporting COMDAT graph symbol for COFF symbol "func" in section 2
-# CHECK-NEXT:   0x0 (block + 0x00000000): size: 0x00000001, linkage: weak, scope: default, dead  -   func
+# CHECK: 4: Exporting COMDAT graph symbol for COFF symbol "func" in section 2
+# CHECK-NEXT:   0x0 (block + 0x00000000): size: 0x00000000, linkage: weak, scope: default, dead  -   func
 
 --- !COFF
 header:

diff --git a/llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_intervene.test b/llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_intervene.test
index 6bf8de8..11a1825 100644 (file)
--- a/llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_intervene.test
+++ b/llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_intervene.test
@@ -6,9 +6,9 @@
 #
 # 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-NEXT:   0x0 (block + 0x00000000): size: 0x00000000, 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
+# CHECK-NEXT:   0x0 (block + 0x00000000): size: 0x00000000, linkage: weak, scope: default, dead  -   func
 
 --- !COFF
 header:

diff --git a/llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_largest.test b/llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_largest.test
index c6152a5..86d809d 100644 (file)
--- a/llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_largest.test
+++ b/llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_largest.test
@@ -5,10 +5,8 @@
 # Check jitlink handles largest selection type as plain weak symbol.
 #
 # CHECK: Creating graph symbols...
-# CHECK:      2: Creating defined graph symbol for COFF symbol ".text" in .text (index: 2)
-# CHECK-NEXT:   0x0 (block + 0x00000000): size: 0x00000001, linkage: strong, scope: local, dead  -   <anonymous symbol>
-# CHECK-NEXT: 4: Exporting COMDAT graph symbol for COFF symbol "func" in section 2
-# CHECK-NEXT:   0x0 (block + 0x00000000): size: 0x00000001, linkage: weak, scope: default, dead  -   func
+# CHECK: 4: Exporting COMDAT graph symbol for COFF symbol "func" in section 2
+# CHECK-NEXT:   0x0 (block + 0x00000000): size: 0x00000000, linkage: weak, scope: default, dead  -   func
 
 --- !COFF
 header:

diff --git a/llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_noduplicate.test b/llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_noduplicate.test
index 900851c..53b2c81 100644 (file)
--- a/llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_noduplicate.test
+++ b/llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_noduplicate.test
@@ -5,10 +5,8 @@
 # Check a strong symbol is created for a COMDAT symbol with IMAGE_COMDAT_SELECT_NODUPLICATES selection type.
 #
 # CHECK: Creating graph symbols...
-# CHECK:      2: Creating defined graph symbol for COFF symbol ".text" in .text (index: 2)
-# CHECK-NEXT:   0x0 (block + 0x00000000): size: 0x00000001, linkage: strong, scope: local, dead  -   <anonymous symbol>
-# CHECK-NEXT: 4: Exporting COMDAT graph symbol for COFF symbol "func" in section 2
-# CHECK-NEXT:   0x0 (block + 0x00000000): size: 0x00000001, linkage: strong, scope: default, dead  -   func
+# CHECK: 4: Exporting COMDAT graph symbol for COFF symbol "func" in section 2
+# CHECK-NEXT:   0x0 (block + 0x00000000): size: 0x00000000, linkage: strong, scope: default, dead  -   func
 
 --- !COFF
 header:

diff --git a/llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_offset.test b/llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_offset.test
new file mode 100644 (file)
index 0000000..97467fd
--- /dev/null
+++ b/llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_offset.test
@@ -0,0 +1,62 @@
+# REQUIRES: asserts
+# RUN: yaml2obj %s -o %t
+# RUN: llvm-jitlink -noexec --debug-only=jitlink -noexec %t 2>&1 | FileCheck %s
+# 
+# Check a COMDAT symbol with an offset is handled correctly.
+#
+# CHECK: Creating graph symbols...
+# CHECK: 4: Exporting COMDAT graph symbol for COFF symbol "func" in section 2
+# CHECK-NEXT:   0x1 (block + 0x00000001): size: 0x00000000, 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
+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:            func
+    Value:           1
+    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
+...

diff --git a/llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_same_size.test b/llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_same_size.test
index 0c18bf2..ef0f84a 100644 (file)
--- a/llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_same_size.test
+++ b/llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_same_size.test
@@ -6,10 +6,8 @@
 # Doesn't check the size validation.
 #
 # CHECK: Creating graph symbols...
-# CHECK:      2: Creating defined graph symbol for COFF symbol ".text" in .text (index: 2)
-# CHECK-NEXT:   0x0 (block + 0x00000000): size: 0x00000001, linkage: strong, scope: local, dead  -   <anonymous symbol>
-# CHECK-NEXT: 4: Exporting COMDAT graph symbol for COFF symbol "func" in section 2
-# CHECK-NEXT:   0x0 (block + 0x00000000): size: 0x00000001, linkage: weak, scope: default, dead  -   func
+# CHECK: 4: Exporting COMDAT graph symbol for COFF symbol "func" in section 2
+# CHECK-NEXT:   0x0 (block + 0x00000000): size: 0x00000000, linkage: weak, scope: default, dead  -   func
 
 --- !COFF
 header:

diff --git a/llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_weak.s b/llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_weak.s
index fbb9f6c..79ac75f 100644 (file)
--- a/llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_weak.s
+++ b/llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_weak.s
@@ -5,10 +5,8 @@
 # Check a COMDAT any symbol is exported as a weak symbol.
 #
 # CHECK: Creating graph symbols...
-# CHECK:      6: Creating defined graph symbol for COFF symbol ".text" in .text (index: 4)
-# CHECK-NEXT:   0x0 (block + 0x00000000): size: 0x00000001, linkage: strong, scope: local, dead  -   <anonymous symbol>
-# CHECK-NEXT: 8: Exporting COMDAT graph symbol for COFF symbol "func" in section 4
-# CHECK-NEXT:   0x0 (block + 0x00000000): size: 0x00000001, linkage: weak, scope: default, dead  -   func
+# CHECK: 8: Exporting COMDAT graph symbol for COFF symbol "func" in section 4
+# CHECK-NEXT:   0x0 (block + 0x00000000): size: 0x00000000, linkage: weak, scope: default, dead  -   func
 
        .text