[llvm] b501770 - [JITLink][COFF] Handle COMDAT symbol with offset.
Sunho Kim via llvm-commits
llvm-commits at lists.llvm.org
Sat Jul 30 17:09:57 PDT 2022
Author: Sunho Kim
Date: 2022-07-31T09:09:48+09:00
New Revision: b501770aef85008089ceff708e1471c3eeec1f3a
URL: https://github.com/llvm/llvm-project/commit/b501770aef85008089ceff708e1471c3eeec1f3a
DIFF: https://github.com/llvm/llvm-project/commit/b501770aef85008089ceff708e1471c3eeec1f3a.diff
LOG: [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
Added:
llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_offset.test
Modified:
llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.cpp
llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.h
llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_any.test
llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_exact_match.test
llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_intervene.test
llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_largest.test
llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_noduplicate.test
llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_same_size.test
llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_weak.s
Removed:
################################################################################
diff --git a/llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.cpp b/llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.cpp
index 402141589034..4e823e6f2498 100644
--- 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 1fd881c59558..f7f10bcce208 100644
--- a/llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.h
+++ b/llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.h
@@ -114,6 +114,7 @@ class COFFLinkGraphBuilder {
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 7133a102aeca..10f118280b1d 100644
--- 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 7407d42b33cf..f7572714bae1 100644
--- 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 6bf8de8ca8a2..11a182578379 100644
--- 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 c6152a50f6f6..86d809d63ed2 100644
--- 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 900851c83f70..53b2c81b5ec7 100644
--- 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
index 000000000000..97467fdb5ee9
--- /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 0c18bf277a65..ef0f84a584c3 100644
--- 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 fbb9f6ca58dc..79ac75ffe441 100644
--- 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
More information about the llvm-commits
mailing list