Hi rsmith,
Previously CodeGen assumed that static locals were emitted before they
could be accessed, which is true for automatic storage duration locals.
However, it is possible to have CodeGen emit a nested function that uses
a static local before emitting the function that defines the static
local, breaking that assumption.
Fix it by splitting static local creation up into a getAddrOf phase and
a definition phase, as is done for other globals.
Fixes PR18020. See also previous attempts to fix static locals in
PR6769 and PR7101.
http://reviews.llvm.org/D4787
Files:
lib/CodeGen/CGDecl.cpp
lib/CodeGen/CGExpr.cpp
lib/CodeGen/CGExprConstant.cpp
lib/CodeGen/CodeGenFunction.h
test/CodeGenCXX/static-local-in-local-class.cpp
Index: lib/CodeGen/CGDecl.cpp
===================================================================
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -171,28 +171,62 @@
return ContextName.str() + "." + D.getNameAsString();
}
-llvm::Constant *
-CodeGenFunction::CreateStaticVarDecl(const VarDecl &D,
- llvm::GlobalValue::LinkageTypes Linkage) {
- QualType Ty = D.getType();
+llvm::Constant *CodeGenFunction::GetAddrOfStaticLocal(const VarDecl &VD) {
+ // Static locals can be used inside nested functions, so they are tracked on
+ // the CodeGenModule.
+ llvm::Constant *Addr = CGM.getStaticLocalDeclAddress(&VD);
+ if (Addr)
+ return Addr;
+
+ QualType Ty = VD.getType();
assert(Ty->isConstantSizeType() && "VLAs can't be static");
// Use the label if the variable is renamed with the asm-label extension.
std::string Name;
- if (D.hasAttr<AsmLabelAttr>())
- Name = CGM.getMangledName(&D);
+ if (VD.hasAttr<AsmLabelAttr>())
+ Name = CGM.getMangledName(&VD);
else
- Name = GetStaticDeclName(*this, D);
+ Name = GetStaticDeclName(*this, VD);
llvm::Type *LTy = CGM.getTypes().ConvertTypeForMem(Ty);
- unsigned AddrSpace =
- CGM.GetGlobalVarAddressSpace(&D, CGM.getContext().getTargetAddressSpace(Ty));
+ unsigned AddrSpace = CGM.GetGlobalVarAddressSpace(
+ &VD, CGM.getContext().getTargetAddressSpace(Ty));
+ llvm::GlobalVariable *GV = new llvm::GlobalVariable(
+ CGM.getModule(), LTy, Ty.isConstant(getContext()),
+ llvm::GlobalValue::ExternalLinkage, nullptr, Name, nullptr,
+ llvm::GlobalVariable::NotThreadLocal, AddrSpace);
+
+ // Make sure the result is of the correct type.
+ unsigned ExpectedAddrSpace = CGM.getContext().getTargetAddressSpace(Ty);
+ if (AddrSpace != ExpectedAddrSpace) {
+ llvm::PointerType *PTy = llvm::PointerType::get(LTy, ExpectedAddrSpace);
+ Addr = llvm::ConstantExpr::getAddrSpaceCast(GV, PTy);
+ } else {
+ Addr = GV;
+ }
+
+ CGM.setStaticLocalDeclAddress(&VD, Addr);
+ return Addr;
+}
+
+llvm::Constant *
+CodeGenFunction::CreateStaticVarDecl(const VarDecl &D,
+ llvm::GlobalValue::LinkageTypes Linkage) {
+ llvm::Constant *Addr = GetAddrOfStaticLocal(D);
+
+ // The address has to be a cast of a global variable.
llvm::GlobalVariable *GV =
- new llvm::GlobalVariable(CGM.getModule(), LTy,
- Ty.isConstant(getContext()), Linkage,
- CGM.EmitNullConstant(D.getType()), Name, nullptr,
- llvm::GlobalVariable::NotThreadLocal,
- AddrSpace);
+ cast<llvm::GlobalVariable>(Addr->stripPointerCasts());
+
+ // Check to see if we already have a global variable for this declaration.
+ // This can happen when double-emitting function bodies, e.g. with complete
+ // and base constructors.
+ if (!GV->isDeclaration())
+ return Addr;
+
+ // Set initializer, linkage, etc, now that we have a definition.
+ GV->setInitializer(CGM.EmitNullConstant(D.getType()));
+ GV->setLinkage(Linkage);
GV->setAlignment(getContext().getDeclAlign(&D).getQuantity());
CGM.setGlobalVisibility(GV, &D);
@@ -206,14 +240,7 @@
GV->setDLLStorageClass(llvm::GlobalVariable::DLLExportStorageClass);
}
- // Make sure the result is of the correct type.
- unsigned ExpectedAddrSpace = CGM.getContext().getTargetAddressSpace(Ty);
- if (AddrSpace != ExpectedAddrSpace) {
- llvm::PointerType *PTy = llvm::PointerType::get(LTy, ExpectedAddrSpace);
- return llvm::ConstantExpr::getAddrSpaceCast(GV, PTy);
- }
-
- return GV;
+ return Addr;
}
/// hasNontrivialDestruction - Determine whether a type's destruction is
@@ -293,19 +320,11 @@
llvm::Value *&DMEntry = LocalDeclMap[&D];
assert(!DMEntry && "Decl already exists in localdeclmap!");
- // Check to see if we already have a global variable for this
- // declaration. This can happen when double-emitting function
- // bodies, e.g. with complete and base constructors.
- llvm::Constant *addr =
- CGM.getStaticLocalDeclAddress(&D);
-
- if (!addr)
- addr = CreateStaticVarDecl(D, Linkage);
+ llvm::Constant *addr = CreateStaticVarDecl(D, Linkage);
// Store into LocalDeclMap before generating initializer to handle
// circular references.
DMEntry = addr;
- CGM.setStaticLocalDeclAddress(&D, addr);
// We can't have a VLA here, but we can have a pointer to a VLA,
// even though that doesn't really make any sense.
Index: lib/CodeGen/CGExpr.cpp
===================================================================
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -1874,7 +1874,7 @@
llvm::Value *V = LocalDeclMap.lookup(VD);
if (!V && VD->isStaticLocal())
- V = CGM.getStaticLocalDeclAddress(VD);
+ V = GetAddrOfStaticLocal(*VD);
// Use special handling for lambdas.
if (!V) {
Index: lib/CodeGen/CGExprConstant.cpp
===================================================================
--- lib/CodeGen/CGExprConstant.cpp
+++ lib/CodeGen/CGExprConstant.cpp
@@ -868,8 +868,10 @@
if (!VD->hasLocalStorage()) {
if (VD->isFileVarDecl() || VD->hasExternalStorage())
return CGM.GetAddrOfGlobalVar(VD);
- else if (VD->isLocalVarDecl())
- return CGM.getStaticLocalDeclAddress(VD);
+ else if (VD->isLocalVarDecl()) {
+ assert(CGF && "cannot refer to static local outside function");
+ return CGF->GetAddrOfStaticLocal(*VD);
+ }
}
}
return nullptr;
Index: lib/CodeGen/CodeGenFunction.h
===================================================================
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -2422,6 +2422,9 @@
/// EmitLoadOfComplex - Load a complex number from the specified l-value.
ComplexPairTy EmitLoadOfComplex(LValue src, SourceLocation loc);
+ /// Get the address of a static local variabld declaration.
+ llvm::Constant *GetAddrOfStaticLocal(const VarDecl &VD);
+
/// CreateStaticVarDecl - Create a zero-initialized LLVM global for
/// a static local variable.
llvm::Constant *CreateStaticVarDecl(const VarDecl &D,
Index: test/CodeGenCXX/static-local-in-local-class.cpp
===================================================================
--- test/CodeGenCXX/static-local-in-local-class.cpp
+++ test/CodeGenCXX/static-local-in-local-class.cpp
@@ -1,6 +1,11 @@
-// RUN: %clang_cc1 -emit-llvm -o %t %s
-// PR6769
+// RUN: %clang_cc1 -triple x86_64-linux -emit-llvm -o - %s -std=c++1y | FileCheck %s
+
+// CHECK: @"_ZZZNK17pr18020_constexpr3$_1clEvENKUlvE_clEvE2l2" =
+// CHECK: internal global i32* @"_ZZNK17pr18020_constexpr3$_1clEvE2l1"
+
+
+namespace pr6769 {
struct X {
static void f();
};
@@ -19,8 +24,9 @@
}
(void)i;
}
+}
-// pr7101
+namespace pr7101 {
void foo() {
static int n = 0;
struct Helper {
@@ -30,4 +36,56 @@
};
Helper::Execute();
}
+}
+
+// These tests all break the assumption that the static var decl has to be
+// emitted before use of the var decl. This happens because we defer emission
+// of variables with internal linkage and no initialization side effects, such
+// as 'x'. Then we hit operator()() in 'f', and emit the callee before we emit
+// the arguments, so we emit the innermost function first.
+
+namespace pr18020_lambda {
+// Referring to l1 before emitting it used to crash.
+auto x = []() {
+ static int l1 = 0;
+ return [] { return l1; };
+};
+int f() { return x()(); }
+}
+
+// CHECK-LABEL: define internal i32 @"_ZZNK14pr18020_lambda3$_0clEvENKUlvE_clEv"
+// CHECK: load i32* @"_ZZNK14pr18020_lambda3$_0clEvE2l1"
+
+namespace pr18020_constexpr {
+// Taking the address of l1 in a constant expression used to crash.
+auto x = []() {
+ static int l1 = 0;
+ return [] {
+ static int *l2 = &l1;
+ return *l2;
+ };
+};
+int f() { return x()(); }
+}
+
+// CHECK-LABEL: define internal i32 @"_ZZNK17pr18020_constexpr3$_1clEvENKUlvE_clEv"
+// CHECK: load i32** @"_ZZZNK17pr18020_constexpr3$_1clEvENKUlvE_clEvE2l2"
+
+// Lambda-less reduction that references l1 before emitting it. This didn't
+// crash if you put it in a namespace.
+struct pr18020_class {
+ auto operator()() {
+ static int l1 = 0;
+ struct U {
+ int operator()() { return l1; }
+ };
+ return U();
+ }
+};
+static pr18020_class x;
+int pr18020_f() { return x()(); }
+
+// CHECK-LABEL: define linkonce_odr i32 @_ZZN13pr18020_classclEvEN1UclEv
+// CHECK: load i32* @_ZZN13pr18020_classclEvE2l1
+// FIXME: Surely we can exercise this edge case in C with -fblocks.
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
