[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

llvm-svn: 335286

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<char> &SecName,
+                                  StringRef Symbol, const DataLayout &DL) const;
+
 public:
   ~TargetLoweringObjectFileCOFF() override = default;
 

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; }

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<char> &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,

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;
+}

lib/MC/MCStreamer.cpp

@@ -673,16 +673,31 @@ static MCSection *getWinCFISection(MCContext &Context, unsigned *NextWinCFIID,
     return MainCFISec;
 
   const auto *TextSecCOFF = cast<MCSectionCOFF>(TextSec);
+  auto *MainCFISecCOFF = cast<MCSectionCOFF>(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<MCSectionCOFF>(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) {

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 }