llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Chinmay Deshpande (chinmaydd) <details> <summary>Changes</summary> Current `clang-cl` behavior differs from `msvc` in the case of dllexport-ed inherited constructors from base classes - `msvc` exports the inherited constructor symbol whereas `clang-cl` does not. This PR bridges that gap by forcing creation of inherited constructors for dllexport-ed classes and then propagating the attribute to them. The standard does not specify behavior for this since `dllexport` is a Microsoft-specific extension. This aims to resolve https://github.com/llvm/llvm-project/issues/162640. --- Full diff: https://github.com/llvm/llvm-project/pull/182706.diff 3 Files Affected: - (modified) clang/lib/AST/ASTContext.cpp (+4-1) - (modified) clang/lib/Sema/SemaDeclCXX.cpp (+30) - (added) clang/test/CodeGenCXX/dllexport-inherited-ctor.cpp (+58) ``````````diff diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp index d97a97091e4eb..f9234c03db932 100644 --- a/clang/lib/AST/ASTContext.cpp +++ b/clang/lib/AST/ASTContext.cpp @@ -13010,10 +13010,13 @@ static GVALinkage basicGVALinkageForFunction(const ASTContext &Context, if (Context.getTargetInfo().getCXXABI().isMicrosoft() && isa<CXXConstructorDecl>(FD) && - cast<CXXConstructorDecl>(FD)->isInheritingConstructor()) + cast<CXXConstructorDecl>(FD)->isInheritingConstructor() && + !FD->hasAttr<DLLExportAttr>()) // 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. + // However, dllexport inherited constructors must be externally visible + // to match MSVC's behavior. return GVA_Internal; return GVA_DiscardableODR; diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index 5837ecd6b9163..34f9e159b4355 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -6312,6 +6312,12 @@ static void ReferenceDllExportedMembers(Sema &S, CXXRecordDecl *Class) { S.MarkFunctionReferenced(Class->getLocation(), MD); + // Inherited constructors are synthesized, not written in source, so + // their definitions won't be encountered later. + if (auto *CD = dyn_cast<CXXConstructorDecl>(MD)) + if (CD->getInheritedConstructor()) + S.Consumer.HandleTopLevelDecl(DeclGroupRef(MD)); + // The function will be passed to the consumer when its definition is // encountered. } else if (MD->isExplicitlyDefaulted()) { @@ -6588,6 +6594,19 @@ void Sema::checkClassLevelDLLAttribute(CXXRecordDecl *Class) { // Force declaration of implicit members so they can inherit the attribute. ForceDeclarationOfImplicitMembers(Class); + // Inherited constructors are created lazily; force their creation now so the + // loop below can propagate the DLL attribute to them. + if (ClassExported) { + SmallVector<ConstructorUsingShadowDecl *, 4> Shadows; + for (auto *D : Class->decls()) + if (auto *S = dyn_cast<ConstructorUsingShadowDecl>(D)) + Shadows.push_back(S); + for (auto *S : Shadows) + if (auto *BC = dyn_cast<CXXConstructorDecl>(S->getTargetDecl()); + BC && !BC->isDeleted()) + findInheritingConstructor(Class->getLocation(), BC, S); + } + // FIXME: MSVC's docs say all bases must be exportable, but this doesn't // seem to be true in practice? @@ -14367,6 +14386,17 @@ Sema::findInheritingConstructor(SourceLocation Loc, DerivedCtor->setParams(ParamDecls); Derived->addDecl(DerivedCtor); + // Propagate the class-level DLLExport attribute to the new inherited + // constructor so it gets exported. DLLImport is not propagated because the + // inherited ctor is an inline definition synthesized by the compiler. + if (Derived->hasAttr<DLLExportAttr>() && + !DerivedCtor->hasAttr<DLLExportAttr>()) { + auto *NewAttr = ::new (getASTContext()) + DLLExportAttr(getASTContext(), *Derived->getAttr<DLLExportAttr>()); + NewAttr->setInherited(true); + DerivedCtor->addAttr(NewAttr); + } + if (ShouldDeleteSpecialMember(DerivedCtor, CXXSpecialMemberKind::DefaultConstructor, &ICI)) SetDeclDeleted(DerivedCtor, UsingLoc); diff --git a/clang/test/CodeGenCXX/dllexport-inherited-ctor.cpp b/clang/test/CodeGenCXX/dllexport-inherited-ctor.cpp new file mode 100644 index 0000000000000..6317b18886f2d --- /dev/null +++ b/clang/test/CodeGenCXX/dllexport-inherited-ctor.cpp @@ -0,0 +1,58 @@ +// RUN: %clang_cc1 -no-enable-noundef-analysis -triple x86_64-windows-msvc -emit-llvm -std=c++17 -fms-extensions -O0 -o - %s | FileCheck --check-prefix=MSVC %s +// RUN: %clang_cc1 -no-enable-noundef-analysis -triple i686-windows-msvc -emit-llvm -std=c++17 -fms-extensions -O0 -o - %s | FileCheck --check-prefix=M32 %s +// RUN: %clang_cc1 -no-enable-noundef-analysis -triple x86_64-windows-gnu -emit-llvm -std=c++17 -fms-extensions -O0 -o - %s | FileCheck --check-prefix=GNU %s + +// Test that inherited constructors via 'using Base::Base' in a dllexport +// class are properly exported (https://github.com/llvm/llvm-project/issues/162640). + +struct __declspec(dllexport) Base { + Base(int); + Base(double); +}; + +struct __declspec(dllexport) Child : public Base { + using Base::Base; +}; + +// The inherited constructors Child(int) and Child(double) should be exported. + +// MSVC-DAG: define weak_odr dso_local dllexport {{.*}} @"??0Child@@QEAA@H@Z" +// MSVC-DAG: define weak_odr dso_local dllexport {{.*}} @"??0Child@@QEAA@N@Z" + +// M32-DAG: define weak_odr dso_local dllexport {{.*}} @"??0Child@@QAE@H@Z" +// M32-DAG: define weak_odr dso_local dllexport {{.*}} @"??0Child@@QAE@N@Z" + +// GNU-DAG: define {{.*}}dso_local dllexport {{.*}} @_ZN5ChildCI14BaseEi( +// GNU-DAG: define {{.*}}dso_local dllexport {{.*}} @_ZN5ChildCI14BaseEd( + +// Also test that a non-exported derived class does not export inherited ctors. +struct NonExportedBase { + NonExportedBase(int); +}; + +struct NonExportedChild : public NonExportedBase { + using NonExportedBase::NonExportedBase; +}; + +// MSVC-NOT: dllexport{{.*}}NonExportedChild +// M32-NOT: dllexport{{.*}}NonExportedChild +// GNU-NOT: dllexport{{.*}}NonExportedChild + +// Test that only the derived class is dllexport, base is not. +struct PlainBase { + PlainBase(int); + PlainBase(float); +}; + +struct __declspec(dllexport) ExportedChild : public PlainBase { + using PlainBase::PlainBase; +}; + +// MSVC-DAG: define weak_odr dso_local dllexport {{.*}} @"??0ExportedChild@@QEAA@H@Z" +// MSVC-DAG: define weak_odr dso_local dllexport {{.*}} @"??0ExportedChild@@QEAA@M@Z" + +// M32-DAG: define weak_odr dso_local dllexport {{.*}} @"??0ExportedChild@@QAE@H@Z" +// M32-DAG: define weak_odr dso_local dllexport {{.*}} @"??0ExportedChild@@QAE@M@Z" + +// GNU-DAG: define {{.*}}dso_local dllexport {{.*}} @_ZN13ExportedChildCI19PlainBaseEi( +// GNU-DAG: define {{.*}}dso_local dllexport {{.*}} @_ZN13ExportedChildCI19PlainBaseEf( `````````` </details> https://github.com/llvm/llvm-project/pull/182706 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
