From 9aa55d3c668cea6542d6cb746c51ac7038feefff Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Thu, 31 Jan 2019 16:14:33 +0000 Subject: [PATCH] lld-link: Allow mixing 'discard' and 'largest' comdat selections 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. Differential Revision: https://reviews.llvm.org/D57515 llvm-svn: 352765 --- lld/COFF/InputFiles.cpp | 26 +++++++++++++++++--------- lld/test/COFF/comdat-selection.s | 15 +++++++++++++-- 2 files changed, 30 insertions(+), 11 deletions(-) diff --git a/lld/COFF/InputFiles.cpp b/lld/COFF/InputFiles.cpp index 86871029afd3..ba46184a62f2 100644 --- a/lld/COFF/InputFiles.cpp +++ b/lld/COFF/InputFiles.cpp @@ -481,15 +481,23 @@ Optional ObjFile::createDefined( 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 " + diff --git a/lld/test/COFF/comdat-selection.s b/lld/test/COFF/comdat-selection.s index b9d03014c773..4926173abd42 100644 --- a/lld/test/COFF/comdat-selection.s +++ b/lld/test/COFF/comdat-selection.s @@ -6,6 +6,7 @@ # 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 @@ -80,8 +81,18 @@ 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 +# 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