r219265 - Fix IRGen for referencing a static local before emitting its decl

Reid Kleckner reid at kleckner.net
Tue Oct 7 18:07:54 PDT 2014


Author: rnk
Date: Tue Oct  7 20:07:54 2014
New Revision: 219265

URL: http://llvm.org/viewvc/llvm-project?rev=219265&view=rev
Log:
Fix IRGen for referencing a static local before emitting its decl

Summary:
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 creating the static local upon access and ensuring that the
deferred function body gets emitted. We may not be able to emit the
initializer properly from outside the function body, so don't try.

Fixes PR18020.  See also previous attempts to fix static locals in
PR6769 and PR7101.

Reviewers: rsmith

Subscribers: cfe-commits

Differential Revision: http://reviews.llvm.org/D4787

Modified:
    cfe/trunk/lib/CodeGen/CGDecl.cpp
    cfe/trunk/lib/CodeGen/CGExpr.cpp
    cfe/trunk/lib/CodeGen/CGExprConstant.cpp
    cfe/trunk/lib/CodeGen/CodeGenFunction.h
    cfe/trunk/lib/CodeGen/CodeGenModule.h
    cfe/trunk/test/CodeGenCXX/static-local-in-local-class.cpp

Modified: cfe/trunk/lib/CodeGen/CGDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDecl.cpp?rev=219265&r1=219264&r2=219265&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGDecl.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDecl.cpp Tue Oct  7 20:07:54 2014
@@ -146,58 +146,60 @@ void CodeGenFunction::EmitVarDecl(const
   return EmitAutoVarDecl(D);
 }
 
-static std::string GetStaticDeclName(CodeGenFunction &CGF, const VarDecl &D) {
-  CodeGenModule &CGM = CGF.CGM;
-
-  if (CGF.getLangOpts().CPlusPlus)
+static std::string getStaticDeclName(CodeGenModule &CGM, const VarDecl &D) {
+  if (CGM.getLangOpts().CPlusPlus)
     return CGM.getMangledName(&D).str();
 
-  StringRef ContextName;
-  if (!CGF.CurFuncDecl) {
-    // Better be in a block declared in global scope.
-    const NamedDecl *ND = cast<NamedDecl>(&D);
-    const DeclContext *DC = ND->getDeclContext();
-    if (const BlockDecl *BD = dyn_cast<BlockDecl>(DC))
-      ContextName = CGM.getBlockMangledName(GlobalDecl(), BD);
-    else
-      llvm_unreachable("Unknown context for block static var decl");
-  } else if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(CGF.CurFuncDecl))
+  // If this isn't C++, we don't need a mangled name, just a pretty one.
+  assert(!D.isExternallyVisible() && "name shouldn't matter");
+  std::string ContextName;
+  const DeclContext *DC = D.getDeclContext();
+  if (const auto *FD = dyn_cast<FunctionDecl>(DC))
     ContextName = CGM.getMangledName(FD);
-  else if (isa<ObjCMethodDecl>(CGF.CurFuncDecl))
-    ContextName = CGF.CurFn->getName();
+  else if (const auto *BD = dyn_cast<BlockDecl>(DC))
+    ContextName = CGM.getBlockMangledName(GlobalDecl(), BD);
+  else if (const auto *OMD = dyn_cast<ObjCMethodDecl>(DC))
+    ContextName = OMD->getSelector().getAsString();
   else
     llvm_unreachable("Unknown context for static var decl");
 
-  return ContextName.str() + "." + D.getNameAsString();
+  ContextName += "." + D.getNameAsString();
+  return ContextName;
 }
 
-llvm::Constant *
-CodeGenFunction::CreateStaticVarDecl(const VarDecl &D,
-                                     llvm::GlobalValue::LinkageTypes Linkage) {
+llvm::Constant *CodeGenModule::getOrCreateStaticVarDecl(
+    const VarDecl &D, llvm::GlobalValue::LinkageTypes Linkage) {
+  // In general, we don't always emit static var decls once before we reference
+  // them. It is possible to reference them before emitting the function that
+  // contains them, and it is possible to emit the containing function multiple
+  // times.
+  if (llvm::Constant *ExistingGV = StaticLocalDeclMap[&D])
+    return ExistingGV;
+
   QualType Ty = D.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);
+    Name = getMangledName(&D);
   else
-    Name = GetStaticDeclName(*this, D);
+    Name = getStaticDeclName(*this, D);
 
-  llvm::Type *LTy = CGM.getTypes().ConvertTypeForMem(Ty);
+  llvm::Type *LTy = getTypes().ConvertTypeForMem(Ty);
   unsigned AddrSpace =
-   CGM.GetGlobalVarAddressSpace(&D, CGM.getContext().getTargetAddressSpace(Ty));
+      GetGlobalVarAddressSpace(&D, getContext().getTargetAddressSpace(Ty));
   llvm::GlobalVariable *GV =
-    new llvm::GlobalVariable(CGM.getModule(), LTy,
+    new llvm::GlobalVariable(getModule(), LTy,
                              Ty.isConstant(getContext()), Linkage,
-                             CGM.EmitNullConstant(D.getType()), Name, nullptr,
+                             EmitNullConstant(D.getType()), Name, nullptr,
                              llvm::GlobalVariable::NotThreadLocal,
                              AddrSpace);
   GV->setAlignment(getContext().getDeclAlign(&D).getQuantity());
-  CGM.setGlobalVisibility(GV, &D);
+  setGlobalVisibility(GV, &D);
 
   if (D.getTLSKind())
-    CGM.setTLSMode(GV, D);
+    setTLSMode(GV, D);
 
   if (D.isExternallyVisible()) {
     if (D.hasAttr<DLLImportAttr>())
@@ -207,13 +209,44 @@ CodeGenFunction::CreateStaticVarDecl(con
   }
 
   // Make sure the result is of the correct type.
-  unsigned ExpectedAddrSpace = CGM.getContext().getTargetAddressSpace(Ty);
+  unsigned ExpectedAddrSpace = getContext().getTargetAddressSpace(Ty);
+  llvm::Constant *Addr = GV;
   if (AddrSpace != ExpectedAddrSpace) {
     llvm::PointerType *PTy = llvm::PointerType::get(LTy, ExpectedAddrSpace);
-    return llvm::ConstantExpr::getAddrSpaceCast(GV, PTy);
+    Addr = llvm::ConstantExpr::getAddrSpaceCast(GV, PTy);
   }
 
-  return GV;
+  setStaticLocalDeclAddress(&D, Addr);
+
+  // Ensure that the static local gets initialized by making sure the parent
+  // function gets emitted eventually.
+  const Decl *DC = cast<Decl>(D.getDeclContext());
+
+  // We can't name blocks or captured statements directly, so try to emit their
+  // parents.
+  if (isa<BlockDecl>(DC) || isa<CapturedDecl>(DC)) {
+    DC = DC->getNonClosureContext();
+    // FIXME: Ensure that global blocks get emitted.
+    if (!DC)
+      return Addr;
+  }
+
+  GlobalDecl GD;
+  if (const auto *CD = dyn_cast<CXXConstructorDecl>(DC))
+    GD = GlobalDecl(CD, Ctor_Base);
+  else if (const auto *DD = dyn_cast<CXXDestructorDecl>(DC))
+    GD = GlobalDecl(DD, Dtor_Base);
+  else if (const auto *FD = dyn_cast<FunctionDecl>(DC))
+    GD = GlobalDecl(FD);
+  else {
+    // Don't do anything for Obj-C method decls or global closures. We should
+    // never defer them.
+    assert(isa<ObjCMethodDecl>(DC) && "unexpected parent code decl");
+  }
+  if (GD.getDecl())
+    (void)GetAddrOfGlobal(GD);
+
+  return Addr;
 }
 
 /// hasNontrivialDestruction - Determine whether a type's destruction is
@@ -296,16 +329,11 @@ void CodeGenFunction::EmitStaticVarDecl(
   // 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 = CGM.getOrCreateStaticVarDecl(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.
@@ -1139,7 +1167,7 @@ void CodeGenFunction::EmitAutoVarInit(co
   } else {
     // Otherwise, create a temporary global with the initializer then
     // memcpy from the global to the alloca.
-    std::string Name = GetStaticDeclName(*this, D);
+    std::string Name = getStaticDeclName(CGM, D);
     llvm::GlobalVariable *GV =
       new llvm::GlobalVariable(CGM.getModule(), constant->getType(), true,
                                llvm::GlobalValue::PrivateLinkage,

Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=219265&r1=219264&r2=219265&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGExpr.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp Tue Oct  7 20:07:54 2014
@@ -1941,7 +1941,8 @@ LValue CodeGenFunction::EmitDeclRefLValu
 
     llvm::Value *V = LocalDeclMap.lookup(VD);
     if (!V && VD->isStaticLocal())
-      V = CGM.getStaticLocalDeclAddress(VD);
+      V = CGM.getOrCreateStaticVarDecl(
+          *VD, CGM.getLLVMLinkageVarDefinition(VD, /*isConstant=*/false));
 
     // Use special handling for lambdas.
     if (!V) {

Modified: cfe/trunk/lib/CodeGen/CGExprConstant.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprConstant.cpp?rev=219265&r1=219264&r2=219265&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGExprConstant.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExprConstant.cpp Tue Oct  7 20:07:54 2014
@@ -869,7 +869,8 @@ public:
           if (VD->isFileVarDecl() || VD->hasExternalStorage())
             return CGM.GetAddrOfGlobalVar(VD);
           else if (VD->isLocalVarDecl())
-            return CGM.getStaticLocalDeclAddress(VD);
+            return CGM.getOrCreateStaticVarDecl(
+                *VD, CGM.getLLVMLinkageVarDefinition(VD, /*isConstant=*/false));
         }
       }
       return nullptr;

Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.h?rev=219265&r1=219264&r2=219265&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CodeGenFunction.h (original)
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.h Tue Oct  7 20:07:54 2014
@@ -2495,11 +2495,6 @@ public:
   /// EmitLoadOfComplex - Load a complex number from the specified l-value.
   ComplexPairTy EmitLoadOfComplex(LValue src, SourceLocation loc);
 
-  /// CreateStaticVarDecl - Create a zero-initialized LLVM global for
-  /// a static local variable.
-  llvm::Constant *CreateStaticVarDecl(const VarDecl &D,
-                                      llvm::GlobalValue::LinkageTypes Linkage);
-
   /// AddInitializerToStaticVarDecl - Add the initializer for 'D' to the
   /// global variable that has already been created for it.  If the initializer
   /// has a different type than GV does, this may free GV and return a different

Modified: cfe/trunk/lib/CodeGen/CodeGenModule.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.h?rev=219265&r1=219264&r2=219265&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CodeGenModule.h (original)
+++ cfe/trunk/lib/CodeGen/CodeGenModule.h Tue Oct  7 20:07:54 2014
@@ -549,6 +549,10 @@ public:
     StaticLocalDeclMap[D] = C;
   }
 
+  llvm::Constant *
+  getOrCreateStaticVarDecl(const VarDecl &D,
+                           llvm::GlobalValue::LinkageTypes Linkage);
+
   llvm::GlobalVariable *getStaticLocalDeclGuardAddress(const VarDecl *D) {
     return StaticLocalDeclGuardMap[D];
   }

Modified: cfe/trunk/test/CodeGenCXX/static-local-in-local-class.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/static-local-in-local-class.cpp?rev=219265&r1=219264&r2=219265&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/static-local-in-local-class.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/static-local-in-local-class.cpp Tue Oct  7 20:07:54 2014
@@ -1,6 +1,13 @@
-// RUN: %clang_cc1 -emit-llvm -o %t %s
-// PR6769
+// RUN: %clang_cc1 -triple x86_64-linux -fblocks -emit-llvm -o - %s -std=c++1y | FileCheck %s
 
+// CHECK: @"_ZZNK3$_2clEvE1x" = internal global i32 42
+// CHECK: @_ZZ18static_local_labelPvE1q = linkonce_odr global i8* blockaddress(@_Z18static_local_labelPv, %{{.*}})
+// CHECK: @_ZZZL20block_deduced_returnvEUb_E1n = internal global i32 42
+// CHECK: @_ZZL14deduced_returnvE1n = internal global i32 42
+// CHECK: @"_ZZZNK17pr18020_constexpr3$_1clEvENKUlvE_clEvE2l2" =
+// CHECK: internal global i32* @"_ZZNK17pr18020_constexpr3$_1clEvE2l1"
+
+namespace pr6769 {
 struct X {
   static void f();
 };
@@ -19,8 +26,9 @@ void X::f() {
   }
   (void)i;
 }
+}
 
-// pr7101
+namespace pr7101 {
 void foo() {
     static int n = 0;
     struct Helper {
@@ -30,4 +38,123 @@ void foo() {
     };
     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
+
+// In this test case, the function containing the static local will not be
+// emitted because it is unneeded. However, the operator call of the inner class
+// is called, and the static local is referenced and must be emitted.
+static auto deduced_return() {
+  static int n = 42;
+  struct S { int *operator()() { return &n; } };
+  return S();
+}
+extern "C" int call_deduced_return_operator() {
+  return *decltype(deduced_return())()();
+}
+
+// CHECK-LABEL: define i32 @call_deduced_return_operator()
+// CHECK: call i32* @_ZZL14deduced_returnvEN1SclEv(%struct.S* %tmp)
+// CHECK: load i32* %
+// CHECK: ret i32 %
+
+// CHECK-LABEL: define internal i32* @_ZZL14deduced_returnvEN1SclEv(%struct.S* %this)
+// CHECK: ret i32* @_ZZL14deduced_returnvE1n
+
+static auto block_deduced_return() {
+  auto (^b)() = ^() {
+    static int n = 42;
+    struct S { int *operator()() { return &n; } };
+    return S();
+  };
+  return b();
+}
+extern "C" int call_block_deduced_return() {
+  return *decltype(block_deduced_return())()();
+}
+
+// CHECK-LABEL: define i32 @call_block_deduced_return()
+// CHECK: call i32* @_ZZZL20block_deduced_returnvEUb_EN1SclEv(%struct.S.6* %tmp)
+// CHECK: load i32* %
+// CHECK: ret i32 %
+
+// CHECK-LABEL: define internal i32* @_ZZZL20block_deduced_returnvEUb_EN1SclEv(%struct.S.6* %this) #0 align 2 {
+// CHECK: ret i32* @_ZZZL20block_deduced_returnvEUb_E1n
+
+inline auto static_local_label(void *p) {
+  if (p)
+    goto *p;
+  static void *q = &&label;
+  struct S { static void *get() { return q; } };
+  return S();
+label:
+  __builtin_abort();
+}
+void *global_label = decltype(static_local_label(0))::get();
+
+// CHECK-LABEL: define linkonce_odr i8* @_ZZ18static_local_labelPvEN1S3getEv()
+// CHECK: %[[lbl:[^ ]*]] = load i8** @_ZZ18static_local_labelPvE1q
+// CHECK: ret i8* %[[lbl]]
+
+auto global_lambda = []() {
+  static int x = 42;
+  struct S { static int *get() { return &x; } };
+  return S();
+};
+extern "C" int use_global_lambda() {
+  return *decltype(global_lambda())::get();
+}
+// CHECK-LABEL: define i32 @use_global_lambda()
+// CHECK: call i32* @"_ZZNK3$_2clEvEN1S3getEv"()
 
+// CHECK-LABEL: define internal i32* @"_ZZNK3$_2clEvEN1S3getEv"()
+// CHECK: ret i32* @"_ZZNK3$_2clEvE1x"





More information about the cfe-commits mailing list