From 07aa8fc8db6b4b8581e0ba8ef4a66274023c0b59 Mon Sep 17 00:00:00 2001 From: Sunho Kim Date: Mon, 25 Jul 2022 23:02:31 +0900 Subject: [PATCH] [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 --- .../JITLink/COFFLinkGraphBuilder.cpp | 18 ++--- .../ExecutionEngine/JITLink/COFFLinkGraphBuilder.h | 3 +- .../JITLink/X86/COFF_comdat_intervene.test | 87 ++++++++++++++++++++++ 3 files changed, 97 insertions(+), 11 deletions(-) create mode 100644 llvm/test/ExecutionEngine/JITLink/X86/COFF_comdat_intervene.test diff --git a/llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.cpp b/llvm/lib/ExecutionEngine/JITLink/COFFLinkGraphBuilder.cpp index 9638f49..88951e9 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 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("No pending COMDAT export for symbol " + formatv("{0:d}", SymIndex)); - if (PendingComdatExport->SectionIndex != Symbol.getSectionNumber()) - return make_error( - "COMDAT export section number mismatch for symbol " + - formatv("{0:d}", SymIndex)); + return exportCOMDATSymbol(SymIndex, SymbolName, Symbol); } } @@ -429,7 +428,7 @@ Expected COFFLinkGraphBuilder::createDefinedSymbol( getGraphBlock(Target)->addEdge(Edge::KeepAlive, 0, *GSym, 0); return GSym; } - if (PendingComdatExport) + if (PendingComdatExports[Symbol.getSectionNumber()]) return make_error( "COMDAT export request already exists before symbol " + formatv("{0:d}", SymIndex)); @@ -491,7 +490,7 @@ Expected 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 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 --- 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 PendingComdatExport; + std::vector> 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 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 +... -- 2.7.4