diff options
| -rw-r--r-- | lld/COFF/InputFiles.cpp | 201 | ||||
| -rw-r--r-- | lld/COFF/InputFiles.h | 14 | ||||
| -rw-r--r-- | lld/test/COFF/comdat-selection.s | 27 |
3 files changed, 135 insertions, 107 deletions
diff --git a/lld/COFF/InputFiles.cpp b/lld/COFF/InputFiles.cpp index ba46184a62f..1ebb5da1b71 100644 --- a/lld/COFF/InputFiles.cpp +++ b/lld/COFF/InputFiles.cpp @@ -384,6 +384,107 @@ Symbol *ObjFile::createUndefined(COFFSymbolRef Sym) { return Symtab->addUndefined(Name, this, Sym.isWeakExternal()); } +void ObjFile::handleComdatSelection(COFFSymbolRef Sym, COMDATType &Selection, + bool &Prevailing, DefinedRegular *Leader) { + if (Prevailing) + return; + // There's already an existing comdat for this symbol: `Leader`. + // Use the comdats's selection field to determine if the new + // symbol in `Sym` should be discarded, produce a duplicate symbol + // error, etc. + + SectionChunk *LeaderChunk = nullptr; + COMDATType LeaderSelection = IMAGE_COMDAT_SELECT_ANY; + + if (Leader->Data) { + LeaderChunk = Leader->getChunk(); + LeaderSelection = LeaderChunk->Selection; + } else { + // FIXME: comdats from LTO files don't know their selection; treat them + // as "any". + Selection = LeaderSelection; + } + + if ((Selection == IMAGE_COMDAT_SELECT_ANY && + LeaderSelection == IMAGE_COMDAT_SELECT_LARGEST) || + (Selection == IMAGE_COMDAT_SELECT_LARGEST && + LeaderSelection == IMAGE_COMDAT_SELECT_ANY)) { + // cl.exe picks "any" for vftables when building with /GR- and + // "largest" when building with /GR. To be able to link object files + // compiled with each flag, "any" and "largest" are merged as "largest". + LeaderSelection = Selection = IMAGE_COMDAT_SELECT_LARGEST; + } + + // Other than that, comdat selections must match. This is a bit more + // strict than link.exe which allows merging "any" and "largest" if "any" + // is the first symbol the linker sees, and it allows merging "largest" + // with everything (!) if "largest" is the first symbol the linker sees. + // Making this symmetric independent of which selection is seen first + // seems better though. + // (This behavior matches ModuleLinker::getComdatResult().) + if (Selection != LeaderSelection) { + log(("conflicting comdat type for " + toString(*Leader) + ": " + + Twine((int)LeaderSelection) + " in " + toString(Leader->getFile()) + + " and " + Twine((int)Selection) + " in " + toString(this)) + .str()); + Symtab->reportDuplicate(Leader, this); + return; + } + + switch (Selection) { + case IMAGE_COMDAT_SELECT_NODUPLICATES: + Symtab->reportDuplicate(Leader, this); + break; + + case IMAGE_COMDAT_SELECT_ANY: + // Nothing to do. + break; + + case IMAGE_COMDAT_SELECT_SAME_SIZE: + if (LeaderChunk->getSize() != getSection(Sym)->SizeOfRawData) + Symtab->reportDuplicate(Leader, this); + break; + + case IMAGE_COMDAT_SELECT_EXACT_MATCH: { + SectionChunk NewChunk(this, getSection(Sym)); + // link.exe only compares section contents here and doesn't complain + // if the two comdat sections have e.g. different alignment. + // Match that. + if (LeaderChunk->getContents() != NewChunk.getContents()) + Symtab->reportDuplicate(Leader, this); + break; + } + + case IMAGE_COMDAT_SELECT_ASSOCIATIVE: + // createDefined() is never called for IMAGE_COMDAT_SELECT_ASSOCIATIVE. + // (This means lld-link doesn't produce duplicate symbol errors for + // associative comdats while link.exe does, but associate comdats + // are never extern in practice.) + llvm_unreachable("createDefined not called for associative comdats"); + + case IMAGE_COMDAT_SELECT_LARGEST: + if (LeaderChunk->getSize() < getSection(Sym)->SizeOfRawData) { + // Replace the existing comdat symbol with the new one. + StringRef Name; + COFFObj->getSymbolName(Sym, Name); + // FIXME: This is incorrect: With /opt:noref, the previous sections + // make it into the final executable as well. Correct handling would + // be to undo reading of the whole old section that's being replaced, + // or doing one pass that determines what the final largest comdat + // is for all IMAGE_COMDAT_SELECT_LARGEST comdats and then reading + // only the largest one. + replaceSymbol<DefinedRegular>(Leader, this, Name, /*IsCOMDAT*/ true, + /*IsExternal*/ true, Sym.getGeneric(), + nullptr); + Prevailing = true; + } + break; + + case IMAGE_COMDAT_SELECT_NEWEST: + llvm_unreachable("should have been rejected earlier"); + } +} + Optional<Symbol *> ObjFile::createDefined( COFFSymbolRef Sym, std::vector<const coff_aux_section_definition *> &ComdatDefs, @@ -463,104 +564,8 @@ Optional<Symbol *> ObjFile::createDefined( } COMDATType Selection = (COMDATType)Def->Selection; - if (!Prevailing && Leader->isCOMDAT()) { - // There's already an existing comdat for this symbol: `Leader`. - // Use the comdats's selection field to determine if the new - // symbol in `Sym` should be discarded, produce a duplicate symbol - // error, etc. - - SectionChunk *LeaderChunk = nullptr; - COMDATType LeaderSelection = IMAGE_COMDAT_SELECT_ANY; - - if (Leader->Data) { - LeaderChunk = Leader->getChunk(); - LeaderSelection = LeaderChunk->Selection; - } else { - // FIXME: comdats from LTO files don't know their selection; treat them - // as "any". - Selection = LeaderSelection; - } - - if ((Selection == IMAGE_COMDAT_SELECT_ANY && - LeaderSelection == IMAGE_COMDAT_SELECT_LARGEST) || - (Selection == IMAGE_COMDAT_SELECT_LARGEST && - LeaderSelection == IMAGE_COMDAT_SELECT_ANY)) { - // cl.exe picks "any" for vftables when building with /GR- and - // "largest" when building with /GR. To be able to link object files - // compiled with each flag, "any" and "largest" are merged as "largest". - LeaderSelection = Selection = IMAGE_COMDAT_SELECT_LARGEST; - } - - // Other than that, comdat selections must match. This is a bit more - // strict than link.exe which allows merging "any" and "largest" if "any" - // is the first symbol the linker sees, and it allows merging "largest" - // with everything (!) if "largest" is the first symbol the linker sees. - // Making this symmetric independent of which selection is seen first - // seems better though. - // (This behavior matches ModuleLinker::getComdatResult().) - if (Selection != LeaderSelection) { - std::string Msg = ("conflicting comdat type for " + toString(*Leader) + - ": " + Twine((int)LeaderSelection) + " in " + - toString(Leader->getFile()) + " and " + - Twine((int)Selection) + " in " + toString(this)) - .str(); - if (Config->ForceMultiple) - warn(Msg); - else - error(Msg); - } - - switch (Selection) { - case IMAGE_COMDAT_SELECT_NODUPLICATES: - Symtab->reportDuplicate(Leader, this); - break; - - case IMAGE_COMDAT_SELECT_ANY: - // Nothing to do. - break; - - case IMAGE_COMDAT_SELECT_SAME_SIZE: - if (LeaderChunk->getSize() != getSection(SectionNumber)->SizeOfRawData) - Symtab->reportDuplicate(Leader, this); - break; - - case IMAGE_COMDAT_SELECT_EXACT_MATCH: { - SectionChunk NewChunk(this, getSection(SectionNumber)); - // link.exe only compares section contents here and doesn't complain - // if the two comdat sections have e.g. different alignment. - // Match that. - if (LeaderChunk->getContents() != NewChunk.getContents()) - Symtab->reportDuplicate(Leader, this); - break; - } - - case IMAGE_COMDAT_SELECT_ASSOCIATIVE: - // createDefined() is never called for IMAGE_COMDAT_SELECT_ASSOCIATIVE. - // (This means lld-link doesn't produce duplicate symbol errors for - // associative comdats while link.exe does, but associate comdats - // are never extern in practice.) - llvm_unreachable("createDefined not called for associative comdats"); - - case IMAGE_COMDAT_SELECT_LARGEST: - if (LeaderChunk->getSize() < getSection(SectionNumber)->SizeOfRawData) { - // Replace the existing comdat symbol with the new one. - // FIXME: This is incorrect: With /opt:noref, the previous sections - // make it into the final executable as well. Correct handling would - // be to undo reading of the whole old section that's being replaced, - // or doing one pass that determines what the final largest comdat - // is for all IMAGE_COMDAT_SELECT_LARGEST comdats and then reading - // only the largest one. - replaceSymbol<DefinedRegular>( - Leader, this, GetName(), /*IsCOMDAT*/ true, - /*IsExternal*/ true, Sym.getGeneric(), nullptr); - Prevailing = true; - } - break; - - case IMAGE_COMDAT_SELECT_NEWEST: - llvm_unreachable("should have been rejected earlier"); - } - } + if (Leader->isCOMDAT()) + handleComdatSelection(Sym, Selection, Prevailing, Leader); if (Prevailing) { SectionChunk *C = readSection(SectionNumber, Def, GetName()); diff --git a/lld/COFF/InputFiles.h b/lld/COFF/InputFiles.h index 6bf0af3c424..f65f34d8f10 100644 --- a/lld/COFF/InputFiles.h +++ b/lld/COFF/InputFiles.h @@ -46,6 +46,7 @@ class Chunk; class Defined; class DefinedImportData; class DefinedImportThunk; +class DefinedRegular; class Lazy; class SectionChunk; class Symbol; @@ -157,6 +158,9 @@ public: private: const coff_section* getSection(uint32_t I); + const coff_section *getSection(COFFSymbolRef Sym) { + return getSection(Sym.getSectionNumber()); + } void initializeChunks(); void initializeSymbols(); @@ -183,6 +187,16 @@ private: COFFSymbolRef Sym, const llvm::object::coff_aux_section_definition *Def, const llvm::DenseMap<StringRef, uint32_t> &PrevailingSectionMap); + // Given a new symbol Sym with comdat selection Selection, if the new + // symbol is not (yet) Prevailing and the existing comdat leader set to + // Leader, emits a diagnostic if the new symbol and its selection doesn't + // match the existing symbol and its selection. If either old or new + // symbol have selection IMAGE_COMDAT_SELECT_LARGEST, Sym might replace + // the existing leader. In that case, Prevailing is set to true. + void handleComdatSelection(COFFSymbolRef Sym, + llvm::COFF::COMDATType &Selection, + bool &Prevailing, DefinedRegular *Leader); + llvm::Optional<Symbol *> createDefined(COFFSymbolRef Sym, std::vector<const llvm::object::coff_aux_section_definition *> diff --git a/lld/test/COFF/comdat-selection.s b/lld/test/COFF/comdat-selection.s index 4926173abd4..988d9bd9516 100644 --- a/lld/test/COFF/comdat-selection.s +++ b/lld/test/COFF/comdat-selection.s @@ -73,13 +73,21 @@ symbol: # lld-link rejects all comdat selection type mismatches. Spot-test just a few # combinations. -# RUN: not lld-link /dll /noentry /nodefaultlib %t.discard.obj %t.one_only.obj 2>&1 | FileCheck --check-prefix=DISCARDONE %s -# DISCARDONE: lld-link: error: conflicting comdat type for symbol: 2 in -# RUN: lld-link /force /dll /noentry /nodefaultlib %t.discard.obj %t.one_only.obj 2>&1 | FileCheck --check-prefix=DISCARDONEFORCE %s -# DISCARDONEFORCE: lld-link: warning: conflicting comdat type for symbol: 2 in - -# RUN: not lld-link /dll /noentry /nodefaultlib %t.same_contents.obj %t.same_size.obj 2>&1 | FileCheck --check-prefix=CONTENTSSIZE %s -# CONTENTSSIZE: lld-link: error: conflicting comdat type for symbol: 4 in +# RUN: not lld-link /verbose /dll /noentry /nodefaultlib %t.discard.obj %t.one_only.obj 2>&1 | FileCheck --check-prefix=DISCARDONE %s +# DISCARDONE: lld-link: conflicting comdat type for symbol: 2 in +# DISCARDONE: lld-link: error: duplicate symbol: symbol +# RUN: lld-link /verbose /force /dll /noentry /nodefaultlib %t.discard.obj %t.one_only.obj 2>&1 | FileCheck --check-prefix=DISCARDONEFORCE %s +# DISCARDONEFORCE: lld-link: conflicting comdat type for symbol: 2 in +# DISCARDONEFORCE: lld-link: warning: duplicate symbol: symbol + +# Make sure the error isn't depending on the order of .obj files. +# RUN: not lld-link /verbose /dll /noentry /nodefaultlib %t.one_only.obj %t.discard.obj 2>&1 | FileCheck --check-prefix=ONEDISCARD %s +# ONEDISCARD: lld-link: conflicting comdat type for symbol: 1 in +# ONEDISCARD: lld-link: error: duplicate symbol: symbol + +# RUN: not lld-link /verbose /dll /noentry /nodefaultlib %t.same_contents.obj %t.same_size.obj 2>&1 | FileCheck --check-prefix=CONTENTSSIZE %s +# CONTENTSSIZE: lld-link: conflicting comdat type for symbol: 4 in +# CONTENTSSIZE: lld-link: error: duplicate symbol: symbol # Check that linking one 'discard' and one 'largest' has the effect of # 'largest'. @@ -94,5 +102,6 @@ symbol: # These cases are accepted by link.exe but not by lld-link. -# RUN: not lld-link /dll /noentry /nodefaultlib %t.largest.obj %t.one_only.obj 2>&1 | FileCheck --check-prefix=LARGESTONE %s -# LARGESTONE: lld-link: error: conflicting comdat type for symbol: 6 in +# 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 |
