diff --git a/lld/COFF/InputFiles.h b/lld/COFF/InputFiles.h --- a/lld/COFF/InputFiles.h +++ b/lld/COFF/InputFiles.h @@ -261,11 +261,12 @@ bool &prevailing, DefinedRegular *leader, const llvm::object::coff_aux_section_definition *def); - std::optional - createDefined(COFFSymbolRef sym, - std::vector - &comdatDefs, - bool &prevailingComdat); + std::optional createDefined( + COFFSymbolRef sym, + std::vector> + &comdatDefs, + bool &prevailingComdat); Symbol *createRegular(COFFSymbolRef sym); Symbol *createUndefined(COFFSymbolRef sym); diff --git a/lld/COFF/InputFiles.cpp b/lld/COFF/InputFiles.cpp --- a/lld/COFF/InputFiles.cpp +++ b/lld/COFF/InputFiles.cpp @@ -385,8 +385,8 @@ pendingIndexes.reserve(numSymbols); DenseMap prevailingSectionMap; - std::vector comdatDefs( - coffObj->getNumberOfSections() + 1); + std::vector> + comdatDefs(coffObj->getNumberOfSections() + 1); for (uint32_t i = 0; i < numSymbols; ++i) { COFFSymbolRef coffSym = check(coffObj->getSymbol(i)); @@ -584,7 +584,8 @@ std::optional ObjFile::createDefined( COFFSymbolRef sym, - std::vector &comdatDefs, + std::vector> &comdatDefs, bool &prevailing) { prevailing = false; auto getName = [&]() { return check(coffObj->getSymbolName(sym)); }; @@ -623,51 +624,64 @@ " should not refer to non-existent section " + Twine(sectionNumber)); // Comdat handling. - // A comdat symbol consists of two symbol table entries. + // A comdat symbol consists of two symbol table entries, and a number of + // optional 'label' symbols. // The first symbol entry has the name of the section (e.g. .text), fixed // values for the other fields, and one auxiliary record. // The second symbol entry has the name of the comdat symbol, called the // "comdat leader". // When this function is called for the first symbol entry of a comdat, // it sets comdatDefs and returns std::nullopt, and when it's called for the - // second symbol entry it reads comdatDefs and then sets it back to nullptr. - - // Handle comdat leader. - if (const coff_aux_section_definition *def = comdatDefs[sectionNumber]) { - comdatDefs[sectionNumber] = nullptr; - DefinedRegular *leader; - - if (sym.isExternal()) { - std::tie(leader, prevailing) = - ctx.symtab.addComdat(this, getName(), sym.getGeneric()); + // second symbol entry it reads comdatDefs. + // Any optional label symbols might come after in the symbol table. + + DefinedRegular *leader{}; + const coff_aux_section_definition *def{}; + std::tie(leader, def) = comdatDefs[sectionNumber]; + if (def) { + if (leader) { + // The leader symbol has already been resolved at this point. We redirect + // the label to point to the leader' section. + if (sym.isLabel()) { + return make(this, /*Name*/ "", /*IsCOMDAT*/ false, + /*IsExternal*/ false, sym.getGeneric(), + leader->getChunk()); + } } else { - leader = make(this, /*Name*/ "", /*IsCOMDAT*/ false, - /*IsExternal*/ false, sym.getGeneric()); - prevailing = true; - } + // Handle comdat leader. + if (sym.isExternal()) { + std::tie(leader, prevailing) = + ctx.symtab.addComdat(this, getName(), sym.getGeneric()); + } else { + leader = make(this, /*Name*/ "", /*IsCOMDAT*/ false, + /*IsExternal*/ false, sym.getGeneric()); + prevailing = true; + } - if (def->Selection < (int)IMAGE_COMDAT_SELECT_NODUPLICATES || - // Intentionally ends at IMAGE_COMDAT_SELECT_LARGEST: link.exe - // doesn't understand IMAGE_COMDAT_SELECT_NEWEST either. - def->Selection > (int)IMAGE_COMDAT_SELECT_LARGEST) { - fatal("unknown comdat type " + std::to_string((int)def->Selection) + - " for " + getName() + " in " + toString(this)); - } - COMDATType selection = (COMDATType)def->Selection; + if (def->Selection < (int)IMAGE_COMDAT_SELECT_NODUPLICATES || + // Intentionally ends at IMAGE_COMDAT_SELECT_LARGEST: link.exe + // doesn't understand IMAGE_COMDAT_SELECT_NEWEST either. + def->Selection > (int)IMAGE_COMDAT_SELECT_LARGEST) { + fatal("unknown comdat type " + std::to_string((int)def->Selection) + + " for " + getName() + " in " + toString(this)); + } + COMDATType selection = (COMDATType)def->Selection; - if (leader->isCOMDAT) - handleComdatSelection(sym, selection, prevailing, leader, def); + if (leader->isCOMDAT) + handleComdatSelection(sym, selection, prevailing, leader, def); - if (prevailing) { - SectionChunk *c = readSection(sectionNumber, def, getName()); - sparseChunks[sectionNumber] = c; - c->sym = cast(leader); - c->selection = selection; - cast(leader)->data = &c->repl; - } else { - sparseChunks[sectionNumber] = nullptr; + if (prevailing) { + SectionChunk *c = readSection(sectionNumber, def, getName()); + sparseChunks[sectionNumber] = c; + c->sym = cast(leader); + c->selection = selection; + cast(leader)->data = &c->repl; + } else { + sparseChunks[sectionNumber] = nullptr; + } + comdatDefs[sectionNumber] = {leader, def}; + return leader; } - return leader; } // Prepare to handle the comdat leader symbol by setting the section's @@ -675,7 +689,7 @@ if (sparseChunks[sectionNumber] == pendingComdat) { if (const coff_aux_section_definition *def = sym.getSectionDefinition()) { if (def->Selection != IMAGE_COMDAT_SELECT_ASSOCIATIVE) - comdatDefs[sectionNumber] = def; + comdatDefs[sectionNumber] = {/*leader=*/nullptr, def}; } return std::nullopt; } diff --git a/lld/test/COFF/Inputs/comdat-malformed-assoc-a.yaml b/lld/test/COFF/Inputs/comdat-malformed-assoc-a.yaml new file mode 100644 --- /dev/null +++ b/lld/test/COFF/Inputs/comdat-malformed-assoc-a.yaml @@ -0,0 +1,127 @@ +--- !COFF +header: + Machine: IMAGE_FILE_MACHINE_AMD64 + Characteristics: [ ] +sections: + - Name: '.text$mn' + Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ] + Alignment: 16 + SectionData: 4883EC2833C9E8000000004883C428C3 + Relocations: + - VirtualAddress: 7 + SymbolName: '?func@@YAXPEAH@Z' + Type: IMAGE_REL_AMD64_REL32 + - Name: '.text$mn' + Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_LNK_COMDAT, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ] + Alignment: 16 + SectionData: 48894C24084883EC28488B442430C70000000000EB004883C428C3 + - Name: '.text$x' + Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ] + Alignment: 16 + SectionData: 4889542410554883EC20488BEA33D233C9E80000000090488D05000000004883C4205DC3CC + Relocations: + - VirtualAddress: 18 + SymbolName: _CxxThrowException + Type: IMAGE_REL_AMD64_REL32 + - VirtualAddress: 26 + SymbolName: '$LN7' + Type: IMAGE_REL_AMD64_REL32 +symbols: + - Name: '.text$mn' + Value: 0 + SectionNumber: 1 + SimpleType: IMAGE_SYM_TYPE_NULL + ComplexType: IMAGE_SYM_DTYPE_NULL + StorageClass: IMAGE_SYM_CLASS_STATIC + SectionDefinition: + Length: 16 + NumberOfRelocations: 1 + NumberOfLinenumbers: 0 + CheckSum: 221128940 + Number: 0 + - Name: '.text$mn' + Value: 0 + SectionNumber: 2 + SimpleType: IMAGE_SYM_TYPE_NULL + ComplexType: IMAGE_SYM_DTYPE_NULL + StorageClass: IMAGE_SYM_CLASS_STATIC + SectionDefinition: + Length: 27 + NumberOfRelocations: 0 + NumberOfLinenumbers: 0 + CheckSum: 928271883 + Number: 0 + Selection: IMAGE_COMDAT_SELECT_ANY + - Name: '.text$x' + Value: 0 + SectionNumber: 3 + SimpleType: IMAGE_SYM_TYPE_NULL + ComplexType: IMAGE_SYM_DTYPE_NULL + StorageClass: IMAGE_SYM_CLASS_STATIC + SectionDefinition: + Length: 37 + NumberOfRelocations: 2 + NumberOfLinenumbers: 0 + CheckSum: 39674339 + Number: 0 + - Name: '?func@@YAXPEAH@Z' + Value: 0 + SectionNumber: 2 + SimpleType: IMAGE_SYM_TYPE_NULL + ComplexType: IMAGE_SYM_DTYPE_FUNCTION + StorageClass: IMAGE_SYM_CLASS_EXTERNAL + - Name: '?a@@YAXXZ' + Value: 0 + SectionNumber: 1 + SimpleType: IMAGE_SYM_TYPE_NULL + ComplexType: IMAGE_SYM_DTYPE_FUNCTION + StorageClass: IMAGE_SYM_CLASS_EXTERNAL + - Name: '?catch$0@?0??func@@YAXPEAH@Z@4HA' + Value: 0 + SectionNumber: 3 + SimpleType: IMAGE_SYM_TYPE_NULL + ComplexType: IMAGE_SYM_DTYPE_FUNCTION + StorageClass: IMAGE_SYM_CLASS_STATIC + - Name: _CxxThrowException + Value: 0 + SectionNumber: 0 + SimpleType: IMAGE_SYM_TYPE_NULL + ComplexType: IMAGE_SYM_DTYPE_FUNCTION + StorageClass: IMAGE_SYM_CLASS_EXTERNAL + - Name: __CxxFrameHandler4 + Value: 0 + SectionNumber: 0 + SimpleType: IMAGE_SYM_TYPE_NULL + ComplexType: IMAGE_SYM_DTYPE_FUNCTION + StorageClass: IMAGE_SYM_CLASS_EXTERNAL + - Name: '$LN7' + Value: 22 + SectionNumber: 2 + SimpleType: IMAGE_SYM_TYPE_NULL + ComplexType: IMAGE_SYM_DTYPE_NULL + StorageClass: IMAGE_SYM_CLASS_LABEL + - Name: '$LN8' + Value: 27 + SectionNumber: 2 + SimpleType: IMAGE_SYM_TYPE_NULL + ComplexType: IMAGE_SYM_DTYPE_NULL + StorageClass: IMAGE_SYM_CLASS_LABEL + - Name: '__catch$?func@@YAXPEAH@Z$0' + Value: 13 + SectionNumber: 3 + SimpleType: IMAGE_SYM_TYPE_NULL + ComplexType: IMAGE_SYM_DTYPE_FUNCTION + StorageClass: IMAGE_SYM_CLASS_STATIC + - Name: '$LN10' + Value: 0 + SectionNumber: 2 + SimpleType: IMAGE_SYM_TYPE_NULL + ComplexType: IMAGE_SYM_DTYPE_NULL + StorageClass: IMAGE_SYM_CLASS_LABEL + - Name: '$LN3' + Value: 0 + SectionNumber: 1 + SimpleType: IMAGE_SYM_TYPE_NULL + ComplexType: IMAGE_SYM_DTYPE_NULL + StorageClass: IMAGE_SYM_CLASS_LABEL +... diff --git a/lld/test/COFF/Inputs/comdat-malformed-assoc-b.yaml b/lld/test/COFF/Inputs/comdat-malformed-assoc-b.yaml new file mode 100644 --- /dev/null +++ b/lld/test/COFF/Inputs/comdat-malformed-assoc-b.yaml @@ -0,0 +1,127 @@ +--- !COFF +header: + Machine: IMAGE_FILE_MACHINE_AMD64 + Characteristics: [ ] +sections: + - Name: '.text$mn' + Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ] + Alignment: 16 + SectionData: 4883EC2833C9E8000000004883C428C3 + Relocations: + - VirtualAddress: 7 + SymbolName: '?func@@YAXPEAH@Z' + Type: IMAGE_REL_AMD64_REL32 + - Name: '.text$mn' + Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_LNK_COMDAT, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ] + Alignment: 16 + SectionData: 48894C24084883EC28488B442430C70000000000EB004883C428C3 + - Name: '.text$x' + Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ] + Alignment: 16 + SectionData: 4889542410554883EC20488BEA33D233C9E80000000090488D05000000004883C4205DC3CC + Relocations: + - VirtualAddress: 18 + SymbolName: _CxxThrowException + Type: IMAGE_REL_AMD64_REL32 + - VirtualAddress: 26 + SymbolName: '$LN7' + Type: IMAGE_REL_AMD64_REL32 +symbols: + - Name: '.text$mn' + Value: 0 + SectionNumber: 1 + SimpleType: IMAGE_SYM_TYPE_NULL + ComplexType: IMAGE_SYM_DTYPE_NULL + StorageClass: IMAGE_SYM_CLASS_STATIC + SectionDefinition: + Length: 16 + NumberOfRelocations: 1 + NumberOfLinenumbers: 0 + CheckSum: 221128940 + Number: 0 + - Name: '.text$mn' + Value: 0 + SectionNumber: 2 + SimpleType: IMAGE_SYM_TYPE_NULL + ComplexType: IMAGE_SYM_DTYPE_NULL + StorageClass: IMAGE_SYM_CLASS_STATIC + SectionDefinition: + Length: 27 + NumberOfRelocations: 0 + NumberOfLinenumbers: 0 + CheckSum: 928271883 + Number: 0 + Selection: IMAGE_COMDAT_SELECT_ANY + - Name: '.text$x' + Value: 0 + SectionNumber: 3 + SimpleType: IMAGE_SYM_TYPE_NULL + ComplexType: IMAGE_SYM_DTYPE_NULL + StorageClass: IMAGE_SYM_CLASS_STATIC + SectionDefinition: + Length: 37 + NumberOfRelocations: 2 + NumberOfLinenumbers: 0 + CheckSum: 39674339 + Number: 0 + - Name: '?func@@YAXPEAH@Z' + Value: 0 + SectionNumber: 2 + SimpleType: IMAGE_SYM_TYPE_NULL + ComplexType: IMAGE_SYM_DTYPE_FUNCTION + StorageClass: IMAGE_SYM_CLASS_EXTERNAL + - Name: '?b@@YAXXZ' + Value: 0 + SectionNumber: 1 + SimpleType: IMAGE_SYM_TYPE_NULL + ComplexType: IMAGE_SYM_DTYPE_FUNCTION + StorageClass: IMAGE_SYM_CLASS_EXTERNAL + - Name: '?catch$0@?0??func@@YAXPEAH@Z@4HA' + Value: 0 + SectionNumber: 3 + SimpleType: IMAGE_SYM_TYPE_NULL + ComplexType: IMAGE_SYM_DTYPE_FUNCTION + StorageClass: IMAGE_SYM_CLASS_STATIC + - Name: _CxxThrowException + Value: 0 + SectionNumber: 0 + SimpleType: IMAGE_SYM_TYPE_NULL + ComplexType: IMAGE_SYM_DTYPE_FUNCTION + StorageClass: IMAGE_SYM_CLASS_EXTERNAL + - Name: __CxxFrameHandler4 + Value: 0 + SectionNumber: 0 + SimpleType: IMAGE_SYM_TYPE_NULL + ComplexType: IMAGE_SYM_DTYPE_FUNCTION + StorageClass: IMAGE_SYM_CLASS_EXTERNAL + - Name: '$LN7' + Value: 22 + SectionNumber: 2 + SimpleType: IMAGE_SYM_TYPE_NULL + ComplexType: IMAGE_SYM_DTYPE_NULL + StorageClass: IMAGE_SYM_CLASS_LABEL + - Name: '$LN8' + Value: 27 + SectionNumber: 2 + SimpleType: IMAGE_SYM_TYPE_NULL + ComplexType: IMAGE_SYM_DTYPE_NULL + StorageClass: IMAGE_SYM_CLASS_LABEL + - Name: '__catch$?func@@YAXPEAH@Z$0' + Value: 13 + SectionNumber: 3 + SimpleType: IMAGE_SYM_TYPE_NULL + ComplexType: IMAGE_SYM_DTYPE_FUNCTION + StorageClass: IMAGE_SYM_CLASS_STATIC + - Name: '$LN10' + Value: 0 + SectionNumber: 2 + SimpleType: IMAGE_SYM_TYPE_NULL + ComplexType: IMAGE_SYM_DTYPE_NULL + StorageClass: IMAGE_SYM_CLASS_LABEL + - Name: '$LN3' + Value: 0 + SectionNumber: 2 + SimpleType: IMAGE_SYM_TYPE_NULL + ComplexType: IMAGE_SYM_DTYPE_NULL + StorageClass: IMAGE_SYM_CLASS_LABEL +... diff --git a/lld/test/COFF/comdat-selection.s b/lld/test/COFF/comdat-selection.s --- a/lld/test/COFF/comdat-selection.s +++ b/lld/test/COFF/comdat-selection.s @@ -105,3 +105,12 @@ # RUN: not lld-link /verbose /dll /noentry /nodefaultlib %t.largest.obj %t.one_only.obj 2>&1 | FileCheck --check-prefix=LARGESTONE %s # LARGESTONE: lld-link: conflicting comdat type for symbol: 6 in # LARGESTONE: lld-link: error: duplicate symbol: symbol + + +# Check a peculiar case where the .text$mn is a COMDAT but not the associated .text$x. +# This is properly handled by link.exe, but it really looks like a bug in MSVC. +# See PR62182. +# RUN: yaml2obj %p/Inputs/comdat-malformed-assoc-a.yaml -o %t.a.obj +# RUN: yaml2obj %p/Inputs/comdat-malformed-assoc-b.yaml -o %t.b.obj +# RUN: lld-link %t.a.obj %t.b.obj /entry:a /subsystem:console /force:unresolved 2>&1 | FileCheck --check-prefix=MISSING-ASSOC %s +# MISSING-ASSOC-NOT: error: relocation against symbol in discarded section: diff --git a/llvm/include/llvm/Object/COFF.h b/llvm/include/llvm/Object/COFF.h --- a/llvm/include/llvm/Object/COFF.h +++ b/llvm/include/llvm/Object/COFF.h @@ -378,6 +378,10 @@ return getStorageClass() == COFF::IMAGE_SYM_CLASS_EXTERNAL; } + bool isLabel() const { + return getStorageClass() == COFF::IMAGE_SYM_CLASS_LABEL; + } + bool isCommon() const { return (isExternal() || isSection()) && getSectionNumber() == COFF::IMAGE_SYM_UNDEFINED && getValue() != 0;