lld-link: Allow mixing 'discard' and 'largest' comdat selections

[thakis]

Summary

cl.exe and clang-cl.exe put vftables in a 'discard' comdat when building with RTTI disabled (/GR-) but in a 'largest' comdat when building with RTTI enabled. To be able to link /GR- code with /GR code, lld-link needs to accept comdats that have this type of comdat selection conflict.

For example, static libraries in the Visual Studio standard library are built with /GR, and without this it's impossible to build client code with /GR- and still link to the standard library.

link.exe also accepts merging 'discard' with 'largest', and it accepts merging 'largest' with any other selection type. lld-link is still a bit stricter since it only allows merging 'largest' with 'discard' for symmetry.

[hans]

lgtm

Maybe also make it clear in the commit message that link.exe allows this.

lld/COFF/InputFiles.cpp

488 ↗

(On Diff #184509)

ultra nit: maybe s/picks/uses/

[ruiu]

LGTM

lld/trunk/COFF/InputFiles.cpp

if (!Prevailing && Leader->isCOMDAT()) {
         LeaderChunk = Leader->getChunk();
         LeaderSelection = LeaderChunk->Selection;
       } else {
         // FIXME: comdats from LTO files don't know their selection; treat them
         // as "any".
         Selection = LeaderSelection;
       }
 
-      // Requiring selections to match exactly 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, and if we can get away with not allowing merging "any" and
-      // "largest" that keeps things more regular too.
-      // (ModuleLinker::getComdatResult() also does comdat type merging in a
-      // different way and it's also a bit more permissive.)
+      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);

lld/trunk/test/COFF/comdat-selection.s

 # REQUIRES: x86
 
 # Tests handling of the comdat selection type.
 # (Except associative which is tested in associative-comdat.s and
 # comdat-selection-associate-largest.s instead.)
 
 # Create obj files with each selection type.
 # RUN: sed -e s/SEL/discard/ %s | llvm-mc -triple x86_64-pc-win32 -filetype=obj -o %t.discard.obj
+# RUN: sed -e s/SEL/discard/ -e s/.long/.short/ -e s/1/2/ %s | llvm-mc -triple x86_64-pc-win32 -filetype=obj -o %t.discard.short.2.obj
 # RUN: sed -e s/SEL/one_only/ %s | llvm-mc -triple x86_64-pc-win32 -filetype=obj -o %t.one_only.obj
 # RUN: sed -e s/SEL/same_size/ %s | llvm-mc -triple x86_64-pc-win32 -filetype=obj -o %t.same_size.obj
 # RUN: sed -e s/SEL/same_size/ -e s/.long/.short/ %s | llvm-mc -triple x86_64-pc-win32 -filetype=obj -o %t.same_size.short.obj
 # RUN: sed -e s/SEL/same_contents/ %s | llvm-mc -triple x86_64-pc-win32 -filetype=obj -o %t.same_contents.obj
 # RUN: sed -e s/SEL/same_contents/ -e s/.long/.short/ %s | llvm-mc -triple x86_64-pc-win32 -filetype=obj -o %t.same_contents.short.obj
 # RUN: sed -e s/SEL/same_contents/ -e s/1/2/ %s | llvm-mc -triple x86_64-pc-win32 -filetype=obj -o %t.same_contents.2.obj
 # RUN: sed -e s/SEL/largest/ %s | llvm-mc -triple x86_64-pc-win32 -filetype=obj -o %t.largest.obj
 # RUN: sed -e s/SEL/largest/ -e s/.long/.short/ -e s/1/2/ %s | llvm-mc -triple x86_64-pc-win32 -filetype=obj -o %t.largest.short.2.obj

 # 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
 
+# 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.discard.obj %t.largest.obj 2>&1 | FileCheck --check-prefix=DISCARDLARGEST %s
-# DISCARDLARGEST: lld-link: error: conflicting comdat type for symbol: 2 in
 # 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