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