r246338 - Allow TLS vars in dllimport/export functions; only inline dllimport functions when safe (PR24593)
Hans Wennborg via cfe-commits
cfe-commits at lists.llvm.org
Fri Aug 28 14:47:02 PDT 2015
Author: hans
Date: Fri Aug 28 16:47:01 2015
New Revision: 246338
URL: http://llvm.org/viewvc/llvm-project?rev=246338&view=rev
Log:
Allow TLS vars in dllimport/export functions; only inline dllimport functions when safe (PR24593)
This patch does two things:
1) Don't error about dllimport/export on thread-local static local variables.
We put those attributes on static locals in dllimport/export functions
implicitly in case the function gets inlined. Now, for TLS variables this
is a problem because we can't import such variables, but it's a benign
problem becase:
2) Make sure we never inline a dllimport function TLS static locals. In fact,
never inline a dllimport function that references a non-imported function
or variable (because these are not defined in the importing library). This
seems to match MSVC's behaviour.
Differential Revision: http://reviews.llvm.org/D12422
Modified:
cfe/trunk/lib/CodeGen/CodeGenModule.cpp
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/test/CodeGenCXX/dllimport.cpp
cfe/trunk/test/SemaCXX/dllexport.cpp
cfe/trunk/test/SemaCXX/dllimport.cpp
Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=246338&r1=246337&r2=246338&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Fri Aug 28 16:47:01 2015
@@ -1448,6 +1448,35 @@ namespace {
return true;
}
};
+
+ struct DLLImportFunctionVisitor
+ : public RecursiveASTVisitor<DLLImportFunctionVisitor> {
+ bool SafeToInline = true;
+
+ bool VisitVarDecl(VarDecl *VD) {
+ // A thread-local variable cannot be imported.
+ SafeToInline = !VD->getTLSKind();
+ return SafeToInline;
+ }
+
+ // Make sure we're not referencing non-imported vars or functions.
+ bool VisitDeclRefExpr(DeclRefExpr *E) {
+ ValueDecl *VD = E->getDecl();
+ if (isa<FunctionDecl>(VD))
+ SafeToInline = VD->hasAttr<DLLImportAttr>();
+ else if (VarDecl *V = dyn_cast<VarDecl>(VD))
+ SafeToInline = !V->hasGlobalStorage() || V->hasAttr<DLLImportAttr>();
+ return SafeToInline;
+ }
+ bool VisitCXXDeleteExpr(CXXDeleteExpr *E) {
+ SafeToInline = E->getOperatorDelete()->hasAttr<DLLImportAttr>();
+ return SafeToInline;
+ }
+ bool VisitCXXNewExpr(CXXNewExpr *E) {
+ SafeToInline = E->getOperatorNew()->hasAttr<DLLImportAttr>();
+ return SafeToInline;
+ }
+ };
}
// isTriviallyRecursive - Check if this function calls another
@@ -1478,6 +1507,15 @@ CodeGenModule::shouldEmitFunction(Global
const auto *F = cast<FunctionDecl>(GD.getDecl());
if (CodeGenOpts.OptimizationLevel == 0 && !F->hasAttr<AlwaysInlineAttr>())
return false;
+
+ if (F->hasAttr<DLLImportAttr>()) {
+ // Check whether it would be safe to inline this dllimport function.
+ DLLImportFunctionVisitor Visitor;
+ Visitor.TraverseFunctionDecl(const_cast<FunctionDecl*>(F));
+ if (!Visitor.SafeToInline)
+ return false;
+ }
+
// PR9614. Avoid cases where the source code is lying to us. An available
// externally function should have an equivalent function somewhere else,
// but a function that calls itself is clearly not equivalent to the real
Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=246338&r1=246337&r2=246338&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Fri Aug 28 16:47:01 2015
@@ -9967,9 +9967,17 @@ Sema::FinalizeDeclaration(Decl *ThisDecl
// dllimport/dllexport variables cannot be thread local, their TLS index
// isn't exported with the variable.
if (DLLAttr && VD->getTLSKind()) {
- Diag(VD->getLocation(), diag::err_attribute_dll_thread_local) << VD
- << DLLAttr;
- VD->setInvalidDecl();
+ FunctionDecl *F = dyn_cast<FunctionDecl>(VD->getDeclContext());
+ if (F && getDLLAttr(F)) {
+ assert(VD->isStaticLocal());
+ // But if this is a static local in a dlimport/dllexport function, the
+ // function will never be inlined, which means the var would never be
+ // imported, so having it marked import/export is safe.
+ } else {
+ Diag(VD->getLocation(), diag::err_attribute_dll_thread_local) << VD
+ << DLLAttr;
+ VD->setInvalidDecl();
+ }
}
if (UsedAttr *Attr = VD->getAttr<UsedAttr>()) {
Modified: cfe/trunk/test/CodeGenCXX/dllimport.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/dllimport.cpp?rev=246338&r1=246337&r2=246338&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/dllimport.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/dllimport.cpp Fri Aug 28 16:47:01 2015
@@ -86,7 +86,7 @@ USEVAR(GlobalRedecl3)
namespace ns { __declspec(dllimport) int ExternalGlobal; }
USEVAR(ns::ExternalGlobal)
-int f();
+int __declspec(dllimport) f();
// MO1-DAG: @"\01?x@?1??inlineStaticLocalsFunc@@YAHXZ at 4HA" = available_externally dllimport global i32 0
// MO1-DAG: @"\01??_B?1??inlineStaticLocalsFunc@@YAHXZ at 51" = available_externally dllimport global i32 0
inline int __declspec(dllimport) inlineStaticLocalsFunc() {
@@ -314,6 +314,46 @@ void UNIQ(use)() { ::operator new(42); }
namespace ns { __declspec(dllimport) void externalFunc(); }
USE(ns::externalFunc)
+// A dllimport function referencing non-imported vars or functions must not be available_externally.
+__declspec(dllimport) int ImportedVar;
+int NonImportedVar;
+__declspec(dllimport) int ImportedFunc();
+int NonImportedFunc();
+__declspec(dllimport) inline int ReferencingImportedVar() { return ImportedVar; }
+// MO1-DAG: define available_externally dllimport i32 @"\01?ReferencingImportedVar@@YAHXZ"
+__declspec(dllimport) inline int ReferencingNonImportedVar() { return NonImportedVar; }
+// MO1-DAG: declare dllimport i32 @"\01?ReferencingNonImportedVar@@YAHXZ"()
+__declspec(dllimport) inline int ReferencingImportedFunc() { return ImportedFunc(); }
+// MO1-DAG: define available_externally dllimport i32 @"\01?ReferencingImportedFunc@@YAHXZ"
+__declspec(dllimport) inline int ReferencingNonImportedFunc() { return NonImportedFunc(); }
+// MO1-DAG: declare dllimport i32 @"\01?ReferencingNonImportedFunc@@YAHXZ"()
+USE(ReferencingImportedVar)
+USE(ReferencingNonImportedVar)
+USE(ReferencingImportedFunc)
+USE(ReferencingNonImportedFunc)
+// References to operator new and delete count too, despite not being DeclRefExprs.
+__declspec(dllimport) inline int *ReferencingNonImportedNew() { return new int[2]; }
+// MO1-DAG: declare dllimport i32* @"\01?ReferencingNonImportedNew@@YAPAHXZ"
+__declspec(dllimport) inline int *ReferencingNonImportedDelete() { delete (int*)nullptr; }
+// MO1-DAG: declare dllimport i32* @"\01?ReferencingNonImportedDelete@@YAPAHXZ"
+USE(ReferencingNonImportedNew)
+USE(ReferencingNonImportedDelete)
+__declspec(dllimport) void* operator new[](__SIZE_TYPE__);
+__declspec(dllimport) void operator delete(void*);
+__declspec(dllimport) inline int *ReferencingImportedNew() { return new int[2]; }
+// MO1-DAG: define available_externally dllimport i32* @"\01?ReferencingImportedNew@@YAPAHXZ"
+__declspec(dllimport) inline int *ReferencingImportedDelete() { delete (int*)nullptr; }
+// MO1-DAG: define available_externally dllimport i32* @"\01?ReferencingImportedDelete@@YAPAHXZ"
+USE(ReferencingImportedNew)
+USE(ReferencingImportedDelete)
+
+// A dllimport function with a TLS variable must not be available_externally.
+__declspec(dllimport) inline void FunctionWithTLSVar() { static __thread int x = 42; }
+// MO1-DAG: declare dllimport void @"\01?FunctionWithTLSVar@@YAXXZ"
+__declspec(dllimport) inline void FunctionWithNormalVar() { static int x = 42; }
+// MO1-DAG: define available_externally dllimport void @"\01?FunctionWithNormalVar@@YAXXZ"
+USE(FunctionWithTLSVar)
+USE(FunctionWithNormalVar)
//===----------------------------------------------------------------------===//
Modified: cfe/trunk/test/SemaCXX/dllexport.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/dllexport.cpp?rev=246338&r1=246337&r2=246338&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/dllexport.cpp (original)
+++ cfe/trunk/test/SemaCXX/dllexport.cpp Fri Aug 28 16:47:01 2015
@@ -71,6 +71,15 @@ __declspec(dllexport) auto ExternalAutoT
// Thread local variables are invalid.
__declspec(dllexport) __thread int ThreadLocalGlobal; // expected-error{{'ThreadLocalGlobal' cannot be thread local when declared 'dllexport'}}
+inline void InlineWithThreadLocal() {
+ static __declspec(dllexport) __thread int ThreadLocalGlobal; // expected-error{{'ThreadLocalGlobal' cannot be thread local when declared 'dllexport'}}
+}
+
+// But if they're in a dllexport function, it's ok, because they will never get imported.
+inline void __declspec(dllexport) ExportedInlineWithThreadLocal() {
+ static __declspec(dllexport) __thread int OK1; // no-error
+ static __thread int OK2; // no-error
+}
// Export in local scope.
void functionScope() {
Modified: cfe/trunk/test/SemaCXX/dllimport.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/dllimport.cpp?rev=246338&r1=246337&r2=246338&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/dllimport.cpp (original)
+++ cfe/trunk/test/SemaCXX/dllimport.cpp Fri Aug 28 16:47:01 2015
@@ -93,6 +93,18 @@ __declspec(dllimport) auto InternalAutoT
// Thread local variables are invalid.
__declspec(dllimport) __thread int ThreadLocalGlobal; // expected-error{{'ThreadLocalGlobal' cannot be thread local when declared 'dllimport'}}
+inline void InlineWithThreadLocal() {
+ static __declspec(dllimport) __thread int ThreadLocalGlobal; // expected-error{{'ThreadLocalGlobal' cannot be thread local when declared 'dllimport'}}
+}
+
+// But if they're in a dllimported function, it's OK because we will not inline the function.
+// This doesn't work on MinGW, because there, dllimport on the inline function is ignored.
+#ifndef GNU
+inline void __declspec(dllimport) ImportedInlineWithThreadLocal() {
+ static __declspec(dllimport) __thread int OK1; // no-error
+ static __thread int OK2; // no-error
+}
+#endif
// Import in local scope.
__declspec(dllimport) float LocalRedecl1; // expected-note{{previous declaration is here}}
More information about the cfe-commits
mailing list