forked from OSchip/llvm-project
lld/coff: Simplify error message for comdat selection mismatches
Turns out nobody understands what "conflicting comdat type" is supposed to mean, so just emit a regular "duplicate symbol" error and move the comdat selection information into /verbose output. This also fixes a problem where the error output would depend on the order of .obj files passed. Before this patch: - If passed `one_only.obj discard.obj`, lld-link would only err "conflicting comdat type" - If passed `discard.obj one_only.obj`, lld-link would err "conflicting comdat type" and then "duplicate symbol" Now lld-link only errs "duplicate symbol" in both cases. I considered adding a "Detail" parameter to reportDuplicate() that's printed in parens at the end of the "duplicate symbol" diag if present, and then put the comdat selection mismatch details there, but since users don't know what it's supposed to mean decided against it. I also considered special-casing the Detail message for one_only/discard mismatches, which in practice means "function defined as inline in TU 1 but as out-of-line in TU 2", but I wasn't sure how useful it is so I omitted that too. Differential Revision: https://reviews.llvm.org/D58180 llvm-svn: 354006
This commit is contained in:
Nico Weber
parent
9a370ccca2
commit
04db8cb92b
|
|
@ -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());
|
||||
|
|
|
|||
|
|
@ -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 *>
|
||||
|
|
|
|||
|
|
@ -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 /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
|
||||
|
||||
# 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
|
||||
# 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
|
||||
|
|
|
|||
Loading…
Reference in New Issue