Skip to content

Commit

Permalink
[MSABI] Remove comdat attribute for inheriting ctor.
Browse files Browse the repository at this point in the history
Currently, for MS, the linkage for the inheriting constructors is set to
internal.  However, the comdat attribute is also set like:

define internal noundef ptr @"??0?$B@_N@@qeaa@AEBVF@@aebua@@@z"(ptr noundef nonnull returned align 1 dereferenceable(1) %this, ptr noundef nonnull align 1 dereferenceable(1) %0, ptr noundef nonnull align 1 dereferenceable(1) %1) unnamed_addr comdat

This could cause linker to fail.

The change is to remove comdat attribute for the inheriting constructor
to make linker happy.

Differential Revision: https://reviews.llvm.org/D158538
  • Loading branch information
jyu2-git committed Aug 28, 2023
1 parent 8885296 commit 1d0bd8e
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 9 deletions.
8 changes: 8 additions & 0 deletions clang/lib/AST/ASTContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11688,6 +11688,14 @@ static GVALinkage basicGVALinkageForFunction(const ASTContext &Context,
if (FD->isMSExternInline())
return GVA_StrongODR;

if (Context.getTargetInfo().getCXXABI().isMicrosoft() &&
isa<CXXConstructorDecl>(FD) &&
cast<CXXConstructorDecl>(FD)->isInheritingConstructor())
// Our approach to inheriting constructors is fundamentally different from
// that used by the MS ABI, so keep our inheriting constructor thunks
// internal rather than trying to pick an unambiguous mangling for them.
return GVA_Internal;

return GVA_DiscardableODR;
}

Expand Down
9 changes: 0 additions & 9 deletions clang/lib/CodeGen/CodeGenModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1970,15 +1970,6 @@ CodeGenModule::getFunctionLinkage(GlobalDecl GD) {
if (const auto *Dtor = dyn_cast<CXXDestructorDecl>(D))
return getCXXABI().getCXXDestructorLinkage(Linkage, Dtor, GD.getDtorType());

if (isa<CXXConstructorDecl>(D) &&
cast<CXXConstructorDecl>(D)->isInheritingConstructor() &&
Context.getTargetInfo().getCXXABI().isMicrosoft()) {
// Our approach to inheriting constructors is fundamentally different from
// that used by the MS ABI, so keep our inheriting constructor thunks
// internal rather than trying to pick an unambiguous mangling for them.
return llvm::GlobalValue::InternalLinkage;
}

return getLLVMLinkageForDeclarator(D, Linkage);
}

Expand Down
49 changes: 49 additions & 0 deletions clang/test/CodeGenCXX/ms-inheriting-ctor.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// RUN: %clang_cc1 -fcxx-exceptions -triple=x86_64-windows-msvc -emit-llvm %s -o - | FileCheck %s

class F {
public:
F(wchar_t *);
};
using a = F;
struct A {};
struct b {
b(a, F, A);
};
template <typename, typename> struct c : b {
c(const a &p1, const A &d) : b(p1, 0, d) {}
};
template <typename e> struct B : c<e, b> {
using c<e, b>::c;
};
class f {
public:
f(...);
}

typedef g;
class C {
public:
C(g, f);
};
static wchar_t h;
class D {
public:
static C E();
};

C D::E() {
C i(B<bool>(&h, {}), f());
return i;
}

// Inheriting ctor has internal linkage without comdat.

// CHECK-LABEL: define internal noundef ptr @"??0?$B@_N@@QEAA@AEBVF@@AEBUA@@@Z"
// CHECK-NOT:comdat
// CHECK-SAME: {{\{$}}

// non-inheriting ctro should has linkonce_odr with comdat attribute.

// CHECK-LABEL: define linkonce_odr dso_local noundef ptr @"??0?$c@_NUb@@@@QEAA@AEBVF@@AEBUA@@@Z"
// CHECK:comdat
// CHECK-SAME: {{\{$}}

0 comments on commit 1d0bd8e

Please sign in to comment.