r281413 for the constructors.
On Tue, Sep 13, 2016 at 2:58 PM, Hans Wennborg <h...@chromium.org> wrote: > Good point. Constructors are also a problem, I'll try to fix. > > It's not exactly the same issue, because they do show up in the AST, > but they're referenced with a CXXConstructExpr, and not the > DeclRefExpr which DLLImportFunctionVisitor is looking for. > > There doesn't seem to be any problem with operator= though. > > On Tue, Sep 13, 2016 at 2:36 PM, Nico Weber via cfe-commits > <cfe-commits@lists.llvm.org> wrote: >> Could other implicit functions (operator=, ctors) have similar issues? >> >> On Tue, Sep 13, 2016 at 5:08 PM, Hans Wennborg via cfe-commits >> <cfe-commits@lists.llvm.org> wrote: >>> >>> Author: hans >>> Date: Tue Sep 13 16:08:20 2016 >>> New Revision: 281395 >>> >>> URL: http://llvm.org/viewvc/llvm-project?rev=281395&view=rev >>> Log: >>> Try harder to not inline dllimport functions referencing non-dllimport >>> functions >>> >>> In r246338, code was added to check for this, but it failed to take into >>> account implicit destructor invocations because those are not reflected >>> in the AST. This adds a separate check for them. >>> >>> Modified: >>> cfe/trunk/lib/CodeGen/CodeGenModule.cpp >>> cfe/trunk/test/CodeGenCXX/dllimport-members.cpp >>> cfe/trunk/test/CodeGenCXX/dllimport.cpp >>> >>> Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=281395&r1=281394&r2=281395&view=diff >>> >>> ============================================================================== >>> --- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original) >>> +++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Tue Sep 13 16:08:20 2016 >>> @@ -1740,8 +1740,17 @@ CodeGenModule::isTriviallyRecursive(cons >>> return Walker.Result; >>> } >>> >>> -bool >>> -CodeGenModule::shouldEmitFunction(GlobalDecl GD) { >>> +// Check if T is a class type with a destructor that's not dllimport. >>> +static bool HasNonDllImportDtor(QualType T) { >>> + if (const RecordType *RT = dyn_cast<RecordType>(T)) >>> + if (CXXRecordDecl *RD = dyn_cast<CXXRecordDecl>(RT->getDecl())) >>> + if (RD->getDestructor() && >>> !RD->getDestructor()->hasAttr<DLLImportAttr>()) >>> + return true; >>> + >>> + return false; >>> +} >>> + >>> +bool CodeGenModule::shouldEmitFunction(GlobalDecl GD) { >>> if (getFunctionLinkage(GD) != >>> llvm::Function::AvailableExternallyLinkage) >>> return true; >>> const auto *F = cast<FunctionDecl>(GD.getDecl()); >>> @@ -1754,6 +1763,18 @@ CodeGenModule::shouldEmitFunction(Global >>> Visitor.TraverseFunctionDecl(const_cast<FunctionDecl*>(F)); >>> if (!Visitor.SafeToInline) >>> return false; >>> + >>> + if (const CXXDestructorDecl *Dtor = dyn_cast<CXXDestructorDecl>(F)) { >>> + // Implicit destructor invocations aren't captured in the AST, so >>> the >>> + // check above can't see them. Check for them manually here. >>> + for (const Decl *Member : Dtor->getParent()->decls()) >>> + if (isa<FieldDecl>(Member)) >>> + if (HasNonDllImportDtor(cast<FieldDecl>(Member)->getType())) >>> + return false; >>> + for (const CXXBaseSpecifier &B : Dtor->getParent()->bases()) >>> + if (HasNonDllImportDtor(B.getType())) >>> + return false; >>> + } >>> } >>> >>> // PR9614. Avoid cases where the source code is lying to us. An >>> available >>> >>> Modified: cfe/trunk/test/CodeGenCXX/dllimport-members.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/dllimport-members.cpp?rev=281395&r1=281394&r2=281395&view=diff >>> >>> ============================================================================== >>> --- cfe/trunk/test/CodeGenCXX/dllimport-members.cpp (original) >>> +++ cfe/trunk/test/CodeGenCXX/dllimport-members.cpp Tue Sep 13 16:08:20 >>> 2016 >>> @@ -44,7 +44,7 @@ void useSpecials() { >>> } >>> >>> // Used to force non-trivial special members. >>> -struct ForceNonTrivial { >>> +struct __declspec(dllimport) ForceNonTrivial { >>> ForceNonTrivial(); >>> ~ForceNonTrivial(); >>> ForceNonTrivial(const ForceNonTrivial&); >>> >>> Modified: cfe/trunk/test/CodeGenCXX/dllimport.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/dllimport.cpp?rev=281395&r1=281394&r2=281395&view=diff >>> >>> ============================================================================== >>> --- cfe/trunk/test/CodeGenCXX/dllimport.cpp (original) >>> +++ cfe/trunk/test/CodeGenCXX/dllimport.cpp Tue Sep 13 16:08:20 2016 >>> @@ -347,6 +347,13 @@ __declspec(dllimport) inline int *Refere >>> // MO1-DAG: define available_externally dllimport i32* >>> @"\01?ReferencingImportedDelete@@YAPAHXZ" >>> USE(ReferencingImportedNew) >>> USE(ReferencingImportedDelete) >>> +struct ClassWithDtor { ~ClassWithDtor() {} }; >>> +struct __declspec(dllimport) ClassWithNonDllImportField { ClassWithDtor >>> t; }; >>> +struct __declspec(dllimport) ClassWithNonDllImportBase : public >>> ClassWithDtor { }; >>> +USECLASS(ClassWithNonDllImportField); >>> +USECLASS(ClassWithNonDllImportBase); >>> +// MO1-DAG: declare dllimport x86_thiscallcc void >>> @"\01??1ClassWithNonDllImportBase@@QAE@XZ"(%struct.ClassWithNonDllImportBase*) >>> +// MO1-DAG: declare dllimport x86_thiscallcc void >>> @"\01??1ClassWithNonDllImportField@@QAE@XZ"(%struct.ClassWithNonDllImportField*) >>> >>> // A dllimport function with a TLS variable must not be >>> available_externally. >>> __declspec(dllimport) inline void FunctionWithTLSVar() { static __thread >>> int x = 42; } >>> >>> >>> _______________________________________________ >>> cfe-commits mailing list >>> cfe-commits@lists.llvm.org >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> >> >> >> _______________________________________________ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
