From 845b439bb4db7ba187e779dd51ea6ff77c24767a Mon Sep 17 00:00:00 2001 From: Reid Kleckner Date: Thu, 21 Jun 2018 20:27:38 +0000 Subject: [PATCH] [mingw] Fix GCC ABI compatibility for comdat things Summary: GCC and the binutils COFF linker do comdats differently from MSVC. If we want to be ABI compatible, we have to do what they do, which is to emit unique section names like ".text$_Z3foov" instead of short section names like ".text". Otherwise, the binutils linker gets confused and reports multiple definition errors when two object files from GCC and Clang containing the same inline function are linked together. The best description of the issue is probably at https://github.com/Alexpux/MINGW-packages/issues/1677, we don't seem to have a good one in our tracker. I fixed up the .pdata and .xdata sections needed everywhere other than 32-bit x86. GCC doesn't use associative comdats for those, it appears to rely on the section name. Reviewers: smeenai, compnerd, mstorsjo, martell, mati865 Subscribers: llvm-commits, hiraditya Differential Revision: https://reviews.llvm.org/D48402 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@335286 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../llvm/CodeGen/TargetLoweringObjectFileImpl.h | 5 ++ include/llvm/MC/MCAsmInfo.h | 6 +++ lib/CodeGen/TargetLoweringObjectFileImpl.cpp | 23 ++++++-- lib/MC/MCAsmInfoCOFF.cpp | 11 +++- lib/MC/MCStreamer.cpp | 21 ++++++-- test/CodeGen/X86/mingw-comdats.ll | 62 ++++++++++++++++++++++ 6 files changed, 119 insertions(+), 9 deletions(-) create mode 100644 test/CodeGen/X86/mingw-comdats.ll diff --git a/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h b/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h index 626703e46f6..98df861dea0 100644 --- a/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h +++ b/include/llvm/CodeGen/TargetLoweringObjectFileImpl.h @@ -134,6 +134,11 @@ public: class TargetLoweringObjectFileCOFF : public TargetLoweringObjectFile { mutable unsigned NextUniqueID = 0; + /// Append "$symbol" to the section name when targetting mingw. The ld.bfd + /// COFF linker will not properly handle comdats otherwise. + void appendComdatSymbolForMinGW(SmallVectorImpl &SecName, + StringRef Symbol, const DataLayout &DL) const; + public: ~TargetLoweringObjectFileCOFF() override = default; diff --git a/include/llvm/MC/MCAsmInfo.h b/include/llvm/MC/MCAsmInfo.h index ac722a007e9..0a0f37966fc 100644 --- a/include/llvm/MC/MCAsmInfo.h +++ b/include/llvm/MC/MCAsmInfo.h @@ -84,6 +84,11 @@ protected: /// directive for emitting thread local BSS Symbols. Default is false. bool HasMachoTBSSDirective = false; + /// True if this is a non-GNU COFF target. The COFF port of the GNU linker + /// doesn't handle associative comdats in the way that we would like to use + /// them. + bool HasCOFFAssociativeComdats = false; + /// This is the maximum possible length of an instruction, which is needed to /// compute the size of an inline asm. Defaults to 4. unsigned MaxInstLength = 4; @@ -463,6 +468,7 @@ public: bool hasMachoZeroFillDirective() const { return HasMachoZeroFillDirective; } bool hasMachoTBSSDirective() const { return HasMachoTBSSDirective; } + bool hasCOFFAssociativeComdats() const { return HasCOFFAssociativeComdats; } unsigned getMaxInstLength() const { return MaxInstLength; } unsigned getMinInstAlignment() const { return MinInstAlignment; } bool getDollarIsPC() const { return DollarIsPC; } diff --git a/lib/CodeGen/TargetLoweringObjectFileImpl.cpp b/lib/CodeGen/TargetLoweringObjectFileImpl.cpp index 03a1f6239f2..8518ee7def3 100644 --- a/lib/CodeGen/TargetLoweringObjectFileImpl.cpp +++ b/lib/CodeGen/TargetLoweringObjectFileImpl.cpp @@ -1060,7 +1060,7 @@ MCSection *TargetLoweringObjectFileCOFF::getExplicitSectionGlobal( Selection); } -static const char *getCOFFSectionNameForUniqueGlobal(SectionKind Kind) { +static StringRef getCOFFSectionNameForUniqueGlobal(SectionKind Kind) { if (Kind.isText()) return ".text"; if (Kind.isBSS()) @@ -1072,6 +1072,15 @@ static const char *getCOFFSectionNameForUniqueGlobal(SectionKind Kind) { return ".data"; } +void TargetLoweringObjectFileCOFF::appendComdatSymbolForMinGW( + SmallVectorImpl &SecName, StringRef Symbol, + const DataLayout &DL) const { + if (getTargetTriple().isWindowsGNUEnvironment()) { + SecName.push_back('$'); + getMangler().getNameWithPrefix(SecName, Symbol, DL); + } +} + MCSection *TargetLoweringObjectFileCOFF::SelectSectionForGlobal( const GlobalObject *GO, SectionKind Kind, const TargetMachine &TM) const { // If we have -ffunction-sections then we should emit the global value to a @@ -1083,7 +1092,8 @@ MCSection *TargetLoweringObjectFileCOFF::SelectSectionForGlobal( EmitUniquedSection = TM.getDataSections(); if ((EmitUniquedSection && !Kind.isCommon()) || GO->hasComdat()) { - const char *Name = getCOFFSectionNameForUniqueGlobal(Kind); + SmallString<256> Name = getCOFFSectionNameForUniqueGlobal(Kind); + unsigned Characteristics = getCOFFSectionFlags(Kind, TM); Characteristics |= COFF::IMAGE_SCN_LNK_COMDAT; @@ -1103,6 +1113,8 @@ MCSection *TargetLoweringObjectFileCOFF::SelectSectionForGlobal( if (!ComdatGV->hasPrivateLinkage()) { MCSymbol *Sym = TM.getSymbol(ComdatGV); StringRef COMDATSymName = Sym->getName(); + appendComdatSymbolForMinGW(Name, COMDATSymName, + GO->getParent()->getDataLayout()); return getContext().getCOFFSection(Name, Characteristics, Kind, COMDATSymName, Selection, UniqueID); } else { @@ -1160,13 +1172,14 @@ MCSection *TargetLoweringObjectFileCOFF::getSectionForJumpTable( StringRef COMDATSymName = Sym->getName(); SectionKind Kind = SectionKind::getReadOnly(); - const char *Name = getCOFFSectionNameForUniqueGlobal(Kind); + StringRef SecName = getCOFFSectionNameForUniqueGlobal(Kind); unsigned Characteristics = getCOFFSectionFlags(Kind, TM); Characteristics |= COFF::IMAGE_SCN_LNK_COMDAT; unsigned UniqueID = NextUniqueID++; - return getContext().getCOFFSection(Name, Characteristics, Kind, COMDATSymName, - COFF::IMAGE_COMDAT_SELECT_ASSOCIATIVE, UniqueID); + return getContext().getCOFFSection( + SecName, Characteristics, Kind, COMDATSymName, + COFF::IMAGE_COMDAT_SELECT_ASSOCIATIVE, UniqueID); } void TargetLoweringObjectFileCOFF::emitModuleMetadata(MCStreamer &Streamer, diff --git a/lib/MC/MCAsmInfoCOFF.cpp b/lib/MC/MCAsmInfoCOFF.cpp index 85104484fd4..8bb02c3f649 100644 --- a/lib/MC/MCAsmInfoCOFF.cpp +++ b/lib/MC/MCAsmInfoCOFF.cpp @@ -41,6 +41,10 @@ MCAsmInfoCOFF::MCAsmInfoCOFF() { // At least MSVC inline-asm does AShr. UseLogicalShr = false; + + // If this is a COFF target, assume that it supports associative comdats. It's + // part of the spec. + HasCOFFAssociativeComdats = true; } void MCAsmInfoMicrosoft::anchor() {} @@ -49,4 +53,9 @@ MCAsmInfoMicrosoft::MCAsmInfoMicrosoft() = default; void MCAsmInfoGNUCOFF::anchor() {} -MCAsmInfoGNUCOFF::MCAsmInfoGNUCOFF() = default; +MCAsmInfoGNUCOFF::MCAsmInfoGNUCOFF() { + // If this is a GNU environment (mingw or cygwin), don't use associative + // comdats for jump tables, unwind information, and other data associated with + // a function. + HasCOFFAssociativeComdats = false; +} diff --git a/lib/MC/MCStreamer.cpp b/lib/MC/MCStreamer.cpp index 491fec3e9ab..953aa0528fb 100644 --- a/lib/MC/MCStreamer.cpp +++ b/lib/MC/MCStreamer.cpp @@ -673,16 +673,31 @@ static MCSection *getWinCFISection(MCContext &Context, unsigned *NextWinCFIID, return MainCFISec; const auto *TextSecCOFF = cast(TextSec); + auto *MainCFISecCOFF = cast(MainCFISec); unsigned UniqueID = TextSecCOFF->getOrAssignWinCFISectionID(NextWinCFIID); // If this section is COMDAT, this unwind section should be COMDAT associative // with its group. const MCSymbol *KeySym = nullptr; - if (TextSecCOFF->getCharacteristics() & COFF::IMAGE_SCN_LNK_COMDAT) + if (TextSecCOFF->getCharacteristics() & COFF::IMAGE_SCN_LNK_COMDAT) { KeySym = TextSecCOFF->getCOMDATSymbol(); - return Context.getAssociativeCOFFSection(cast(MainCFISec), - KeySym, UniqueID); + // In a GNU environment, we can't use associative comdats. Instead, do what + // GCC does, which is to make plain comdat selectany section named like + // ".[px]data$_Z3foov". + if (!Context.getAsmInfo()->hasCOFFAssociativeComdats()) { + std::string SectionName = + (MainCFISecCOFF->getSectionName() + "$" + + TextSecCOFF->getSectionName().split('$').second) + .str(); + return Context.getCOFFSection( + SectionName, + MainCFISecCOFF->getCharacteristics() | COFF::IMAGE_SCN_LNK_COMDAT, + MainCFISecCOFF->getKind(), "", COFF::IMAGE_COMDAT_SELECT_ANY); + } + } + + return Context.getAssociativeCOFFSection(MainCFISecCOFF, KeySym, UniqueID); } MCSection *MCStreamer::getAssociatedPDataSection(const MCSection *TextSec) { diff --git a/test/CodeGen/X86/mingw-comdats.ll b/test/CodeGen/X86/mingw-comdats.ll new file mode 100644 index 00000000000..083885ae918 --- /dev/null +++ b/test/CodeGen/X86/mingw-comdats.ll @@ -0,0 +1,62 @@ +; RUN: llc -mtriple=x86_64-windows-itanium < %s | FileCheck %s +; RUN: llc -mtriple=x86_64-windows-msvc < %s | FileCheck %s +; RUN: llc -mtriple=x86_64-w64-windows-gnu < %s | FileCheck %s --check-prefix=GNU +; RUN: llc -mtriple=x86_64-w64-windows-gnu < %s -filetype=obj | llvm-objdump - -headers | FileCheck %s --check-prefix=GNUOBJ + +; GCC and MSVC handle comdats completely differently. Make sure we do the right +; thing for each. + +; Generated with this C++ source: +; int bar(int); +; __declspec(selectany) int gv = 42; +; inline int foo(int x) { return bar(x) + gv; } +; int main() { return foo(1); } + +$_Z3fooi = comdat any + +$gv = comdat any + +@gv = weak_odr dso_local global i32 42, comdat, align 4 + +; Function Attrs: norecurse uwtable +define dso_local i32 @main() #0 { +entry: + %call = tail call i32 @_Z3fooi(i32 1) + ret i32 %call +} + +; CHECK: main: +; GNU: main: + +; Function Attrs: inlinehint uwtable +define linkonce_odr dso_local i32 @_Z3fooi(i32 %x) #1 comdat { +entry: + %call = tail call i32 @_Z3bari(i32 %x) + %0 = load i32, i32* @gv, align 4 + %add = add nsw i32 %0, %call + ret i32 %add +} + +; CHECK: .section .text,"xr",discard,_Z3fooi +; CHECK: _Z3fooi: +; CHECK: .section .data,"dw",discard,gv +; CHECK: gv: +; CHECK: .long 42 + +; GNU: .section .text$_Z3fooi,"xr",discard,_Z3fooi +; GNU: _Z3fooi: +; GNU: .section .data$gv,"dw",discard,gv +; GNU: gv: +; GNU: .long 42 + +; Make sure the assembler puts the .xdata and .pdata in sections with the right +; names. +; GNUOBJ: .text$_Z3fooi +; GNUOBJ: .xdata$_Z3fooi +; GNUOBJ: .data$gv +; GNUOBJ: .pdata$_Z3fooi + +declare dso_local i32 @_Z3bari(i32) + +attributes #0 = { norecurse uwtable } +attributes #1 = { inlinehint uwtable } -- 2.11.0