lld/coff: Simplify error message for comdat selection mismatches

[thakis]

Summary

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.

[ruiu]

lld/COFF/InputFiles.cpp

502–505

clang-format?

510

I'd return early instead of doing this to effectively disable the following code, but you can't do that because we want to execute the code after the following switch. Maybe it's time to split this into a new function?

[thakis]

pull out function

[thakis]

Extracted a function. Let me know if you want me to do the function extraction without other changes in a different CL, so that it's a behavior-preserving refactoring in one patch and a behavior change in a separate much smaller patch.

[ruiu]

LGTM

lld/COFF/InputFiles.cpp

387

Handle -> handle

lld/COFF/InputFiles.cpp

 }
 
 Symbol *ObjFile::createUndefined(COFFSymbolRef Sym) {
   StringRef Name;
   COFFObj->getSymbolName(Sym, Name);
   return Symtab->addUndefined(Name, this, Sym.isWeakExternal());
 }
 
 Optional<Symbol *> ObjFile::createDefined(
     COFFSymbolRef Sym,
     std::vector<const coff_aux_section_definition *> &ComdatDefs,
     bool &Prevailing) {
   Prevailing = false;
   auto GetName = [&]() {
     StringRef S;
     COFFObj->getSymbolName(Sym, S);
     return S;
if (!Prevailing && Leader->isCOMDAT()) {
       // 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) +
+        log(("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);
+                              .str());
+        Symtab->reportDuplicate(Leader, this);
+
+        // To make the switch below not emit another error.
+        Selection = IMAGE_COMDAT_SELECT_ANY;
       }
 
       switch (Selection) {
       case IMAGE_COMDAT_SELECT_NODUPLICATES:
         Symtab->reportDuplicate(Leader, this);
         break;
 
       case IMAGE_COMDAT_SELECT_ANY:

lld/test/COFF/comdat-selection.s

 
 
 # Test linking the same symbol with different comdat selection types.
 # link.exe generally rejects this, except for "largest" which is allowed to
 # combine with everything (https://bugs.llvm.org/show_bug.cgi?id=40094#c7).
 # 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'.
 # RUN: lld-link /dll /noentry /nodefaultlib %t.discard.short.2.obj %t.largest.obj
 # RUN: llvm-objdump -s %t.exe | FileCheck --check-prefix=DISCARDLARGEST %s
 # DISCARDLARGEST: Contents of section .text:
 # DISCARDLARGEST:   180001000 01000000 ....
 # RUN: lld-link /dll /noentry /nodefaultlib %t.largest.obj %t.discard.short.2.obj
 # RUN: llvm-objdump -s %t.exe | FileCheck --check-prefix=LARGESTDISCARD %s
 # LARGESTDISCARD: Contents of section .text:
 # LARGESTDISCARD:   180001000 01000000 ....
 
 
 # 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