Skip to content

Commit 6eaa832

Browse files
zmodemzmodem
committedAug 28, 2015
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 llvm-svn: 246338
1 parent e235d45 commit 6eaa832

File tree

5 files changed

+111
-4
lines changed

5 files changed

+111
-4
lines changed
 

‎clang/lib/CodeGen/CodeGenModule.cpp

+38
Original file line numberDiff line numberDiff line change
@@ -1448,6 +1448,35 @@ namespace {
14481448
return true;
14491449
}
14501450
};
1451+
1452+
struct DLLImportFunctionVisitor
1453+
: public RecursiveASTVisitor<DLLImportFunctionVisitor> {
1454+
bool SafeToInline = true;
1455+
1456+
bool VisitVarDecl(VarDecl *VD) {
1457+
// A thread-local variable cannot be imported.
1458+
SafeToInline = !VD->getTLSKind();
1459+
return SafeToInline;
1460+
}
1461+
1462+
// Make sure we're not referencing non-imported vars or functions.
1463+
bool VisitDeclRefExpr(DeclRefExpr *E) {
1464+
ValueDecl *VD = E->getDecl();
1465+
if (isa<FunctionDecl>(VD))
1466+
SafeToInline = VD->hasAttr<DLLImportAttr>();
1467+
else if (VarDecl *V = dyn_cast<VarDecl>(VD))
1468+
SafeToInline = !V->hasGlobalStorage() || V->hasAttr<DLLImportAttr>();
1469+
return SafeToInline;
1470+
}
1471+
bool VisitCXXDeleteExpr(CXXDeleteExpr *E) {
1472+
SafeToInline = E->getOperatorDelete()->hasAttr<DLLImportAttr>();
1473+
return SafeToInline;
1474+
}
1475+
bool VisitCXXNewExpr(CXXNewExpr *E) {
1476+
SafeToInline = E->getOperatorNew()->hasAttr<DLLImportAttr>();
1477+
return SafeToInline;
1478+
}
1479+
};
14511480
}
14521481

14531482
// isTriviallyRecursive - Check if this function calls another
@@ -1478,6 +1507,15 @@ CodeGenModule::shouldEmitFunction(GlobalDecl GD) {
14781507
const auto *F = cast<FunctionDecl>(GD.getDecl());
14791508
if (CodeGenOpts.OptimizationLevel == 0 && !F->hasAttr<AlwaysInlineAttr>())
14801509
return false;
1510+
1511+
if (F->hasAttr<DLLImportAttr>()) {
1512+
// Check whether it would be safe to inline this dllimport function.
1513+
DLLImportFunctionVisitor Visitor;
1514+
Visitor.TraverseFunctionDecl(const_cast<FunctionDecl*>(F));
1515+
if (!Visitor.SafeToInline)
1516+
return false;
1517+
}
1518+
14811519
// PR9614. Avoid cases where the source code is lying to us. An available
14821520
// externally function should have an equivalent function somewhere else,
14831521
// but a function that calls itself is clearly not equivalent to the real

‎clang/lib/Sema/SemaDecl.cpp

+11-3
Original file line numberDiff line numberDiff line change
@@ -9967,9 +9967,17 @@ Sema::FinalizeDeclaration(Decl *ThisDecl) {
99679967
// dllimport/dllexport variables cannot be thread local, their TLS index
99689968
// isn't exported with the variable.
99699969
if (DLLAttr && VD->getTLSKind()) {
9970-
Diag(VD->getLocation(), diag::err_attribute_dll_thread_local) << VD
9971-
<< DLLAttr;
9972-
VD->setInvalidDecl();
9970+
FunctionDecl *F = dyn_cast<FunctionDecl>(VD->getDeclContext());
9971+
if (F && getDLLAttr(F)) {
9972+
assert(VD->isStaticLocal());
9973+
// But if this is a static local in a dlimport/dllexport function, the
9974+
// function will never be inlined, which means the var would never be
9975+
// imported, so having it marked import/export is safe.
9976+
} else {
9977+
Diag(VD->getLocation(), diag::err_attribute_dll_thread_local) << VD
9978+
<< DLLAttr;
9979+
VD->setInvalidDecl();
9980+
}
99739981
}
99749982

99759983
if (UsedAttr *Attr = VD->getAttr<UsedAttr>()) {

‎clang/test/CodeGenCXX/dllimport.cpp

+41-1
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ USEVAR(GlobalRedecl3)
8686
namespace ns { __declspec(dllimport) int ExternalGlobal; }
8787
USEVAR(ns::ExternalGlobal)
8888

89-
int f();
89+
int __declspec(dllimport) f();
9090
// MO1-DAG: @"\01?x@?1??inlineStaticLocalsFunc@@YAHXZ@4HA" = available_externally dllimport global i32 0
9191
// MO1-DAG: @"\01??_B?1??inlineStaticLocalsFunc@@YAHXZ@51" = available_externally dllimport global i32 0
9292
inline int __declspec(dllimport) inlineStaticLocalsFunc() {
@@ -314,6 +314,46 @@ void UNIQ(use)() { ::operator new(42); }
314314
namespace ns { __declspec(dllimport) void externalFunc(); }
315315
USE(ns::externalFunc)
316316

317+
// A dllimport function referencing non-imported vars or functions must not be available_externally.
318+
__declspec(dllimport) int ImportedVar;
319+
int NonImportedVar;
320+
__declspec(dllimport) int ImportedFunc();
321+
int NonImportedFunc();
322+
__declspec(dllimport) inline int ReferencingImportedVar() { return ImportedVar; }
323+
// MO1-DAG: define available_externally dllimport i32 @"\01?ReferencingImportedVar@@YAHXZ"
324+
__declspec(dllimport) inline int ReferencingNonImportedVar() { return NonImportedVar; }
325+
// MO1-DAG: declare dllimport i32 @"\01?ReferencingNonImportedVar@@YAHXZ"()
326+
__declspec(dllimport) inline int ReferencingImportedFunc() { return ImportedFunc(); }
327+
// MO1-DAG: define available_externally dllimport i32 @"\01?ReferencingImportedFunc@@YAHXZ"
328+
__declspec(dllimport) inline int ReferencingNonImportedFunc() { return NonImportedFunc(); }
329+
// MO1-DAG: declare dllimport i32 @"\01?ReferencingNonImportedFunc@@YAHXZ"()
330+
USE(ReferencingImportedVar)
331+
USE(ReferencingNonImportedVar)
332+
USE(ReferencingImportedFunc)
333+
USE(ReferencingNonImportedFunc)
334+
// References to operator new and delete count too, despite not being DeclRefExprs.
335+
__declspec(dllimport) inline int *ReferencingNonImportedNew() { return new int[2]; }
336+
// MO1-DAG: declare dllimport i32* @"\01?ReferencingNonImportedNew@@YAPAHXZ"
337+
__declspec(dllimport) inline int *ReferencingNonImportedDelete() { delete (int*)nullptr; }
338+
// MO1-DAG: declare dllimport i32* @"\01?ReferencingNonImportedDelete@@YAPAHXZ"
339+
USE(ReferencingNonImportedNew)
340+
USE(ReferencingNonImportedDelete)
341+
__declspec(dllimport) void* operator new[](__SIZE_TYPE__);
342+
__declspec(dllimport) void operator delete(void*);
343+
__declspec(dllimport) inline int *ReferencingImportedNew() { return new int[2]; }
344+
// MO1-DAG: define available_externally dllimport i32* @"\01?ReferencingImportedNew@@YAPAHXZ"
345+
__declspec(dllimport) inline int *ReferencingImportedDelete() { delete (int*)nullptr; }
346+
// MO1-DAG: define available_externally dllimport i32* @"\01?ReferencingImportedDelete@@YAPAHXZ"
347+
USE(ReferencingImportedNew)
348+
USE(ReferencingImportedDelete)
349+
350+
// A dllimport function with a TLS variable must not be available_externally.
351+
__declspec(dllimport) inline void FunctionWithTLSVar() { static __thread int x = 42; }
352+
// MO1-DAG: declare dllimport void @"\01?FunctionWithTLSVar@@YAXXZ"
353+
__declspec(dllimport) inline void FunctionWithNormalVar() { static int x = 42; }
354+
// MO1-DAG: define available_externally dllimport void @"\01?FunctionWithNormalVar@@YAXXZ"
355+
USE(FunctionWithTLSVar)
356+
USE(FunctionWithNormalVar)
317357

318358

319359
//===----------------------------------------------------------------------===//

‎clang/test/SemaCXX/dllexport.cpp

+9
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,15 @@ __declspec(dllexport) auto ExternalAutoTypeGlobal = External();
7171

7272
// Thread local variables are invalid.
7373
__declspec(dllexport) __thread int ThreadLocalGlobal; // expected-error{{'ThreadLocalGlobal' cannot be thread local when declared 'dllexport'}}
74+
inline void InlineWithThreadLocal() {
75+
static __declspec(dllexport) __thread int ThreadLocalGlobal; // expected-error{{'ThreadLocalGlobal' cannot be thread local when declared 'dllexport'}}
76+
}
77+
78+
// But if they're in a dllexport function, it's ok, because they will never get imported.
79+
inline void __declspec(dllexport) ExportedInlineWithThreadLocal() {
80+
static __declspec(dllexport) __thread int OK1; // no-error
81+
static __thread int OK2; // no-error
82+
}
7483

7584
// Export in local scope.
7685
void functionScope() {

‎clang/test/SemaCXX/dllimport.cpp

+12
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,18 @@ __declspec(dllimport) auto InternalAutoTypeGlobal = Internal(); // expected-erro
9393

9494
// Thread local variables are invalid.
9595
__declspec(dllimport) __thread int ThreadLocalGlobal; // expected-error{{'ThreadLocalGlobal' cannot be thread local when declared 'dllimport'}}
96+
inline void InlineWithThreadLocal() {
97+
static __declspec(dllimport) __thread int ThreadLocalGlobal; // expected-error{{'ThreadLocalGlobal' cannot be thread local when declared 'dllimport'}}
98+
}
99+
100+
// But if they're in a dllimported function, it's OK because we will not inline the function.
101+
// This doesn't work on MinGW, because there, dllimport on the inline function is ignored.
102+
#ifndef GNU
103+
inline void __declspec(dllimport) ImportedInlineWithThreadLocal() {
104+
static __declspec(dllimport) __thread int OK1; // no-error
105+
static __thread int OK2; // no-error
106+
}
107+
#endif
96108

97109
// Import in local scope.
98110
__declspec(dllimport) float LocalRedecl1; // expected-note{{previous declaration is here}}

0 commit comments

Comments
 (0)
Please sign in to comment.