OSDN Git Service

[mingw] Fix GCC ABI compatibility for comdat things
authorReid Kleckner <rnk@google.com>
Thu, 21 Jun 2018 20:27:38 +0000 (20:27 +0000)
committerReid Kleckner <rnk@google.com>
Thu, 21 Jun 2018 20:27:38 +0000 (20:27 +0000)
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

include/llvm/CodeGen/TargetLoweringObjectFileImpl.h
include/llvm/MC/MCAsmInfo.h
lib/CodeGen/TargetLoweringObjectFileImpl.cpp
lib/MC/MCAsmInfoCOFF.cpp
lib/MC/MCStreamer.cpp
test/CodeGen/X86/mingw-comdats.ll [new file with mode: 0644]

index 626703e..98df861 100644 (file)
@@ -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;
 
index ac722a0..0a0f379 100644 (file)
@@ -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; }
index 03a1f62..8518ee7 100644 (file)
@@ -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,
index 8510448..8bb02c3 100644 (file)
@@ -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;
+}
index 491fec3..953aa05 100644 (file)
@@ -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) {
diff --git a/test/CodeGen/X86/mingw-comdats.ll b/test/CodeGen/X86/mingw-comdats.ll
new file mode 100644 (file)
index 0000000..083885a
--- /dev/null
@@ -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 }