r281395 - Try harder to not inline dllimport functions referencing non-dllimport functions

[Nico Weber via cfe-commits]

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 at 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 at XZ"(%struct.ClassWithNonDllImportBase*)
> +// MO1-DAG: declare dllimport x86_thiscallcc void @"\01??
> 1ClassWithNonDllImportField@@QAE at 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 at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160913/26cec910/attachment-0001.html>