Do the static-locals thing properly in the face of unions and

other things which might mess with the variable's type.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@153733 91177308-0d34-0410-b5e6-96231b3b80d8

lib/CodeGen/CGCXXABI.cpp

@@ -172,7 +172,7 @@ void CGCXXABI::ReadArrayCookie(CodeGenFunction &CGF, llvm::Value *Ptr,
 
 void CGCXXABI::EmitGuardedInit(CodeGenFunction &CGF,
                                const VarDecl &D,
-                               llvm::GlobalVariable *GV,
+                               llvm::Constant *GV,
                                bool PerformInit) {
   ErrorUnsupportedABI(CGF, "static local variable initialization");
 }

lib/CodeGen/CGCXXABI.h

@@ -246,8 +246,9 @@ public:
   /// The variable may be:
   ///   - a static local variable
   ///   - a static data member of a class template instantiation
+  /// In either case, it will be a (possibly casted) llvm::GlobalVariable.
   virtual void EmitGuardedInit(CodeGenFunction &CGF, const VarDecl &D,
-                               llvm::GlobalVariable *DeclPtr, bool PerformInit);
+                               llvm::Constant *addr, bool PerformInit);
 
 };
 

lib/CodeGen/CGDecl.cpp

@@ -169,7 +169,24 @@ static std::string GetStaticDeclName(CodeGenFunction &CGF, const VarDecl &D,
   return ContextName + Separator + D.getNameAsString();
 }
 
-llvm::GlobalVariable *
+/// We wanted to make a variable of one type, but the variable already
+/// exists with another.  Is that type good enough?
+///
+/// The problem we're working around here is that giving a global
+/// variable an initializer can require changing its type in some
+/// convoluted circumstances.
+static bool isExistingVarAdequate(CodeGenModule &CGM,
+                                  llvm::Type *existing, llvm::Type *desired) {
+  // Equality makes for a good fast path.
+  if (existing == desired) return true;
+
+  // Otherwise, just require them to have the same size.
+  return (CGM.getTargetData().getTypeStoreSize(existing)
+            == CGM.getTargetData().getTypeStoreSize(desired));
+}
+                                   
+
+llvm::Constant *
 CodeGenFunction::CreateStaticVarDecl(const VarDecl &D,
                                      const char *Separator,
                                      llvm::GlobalValue::LinkageTypes Linkage) {
@@ -184,20 +201,27 @@ CodeGenFunction::CreateStaticVarDecl(const VarDecl &D,
     Name = GetStaticDeclName(*this, D, Separator);
 
   llvm::Type *LTy = CGM.getTypes().ConvertTypeForMem(Ty);
+  unsigned addrspace = CGM.getContext().getTargetAddressSpace(Ty);
 
   // In C++, there are strange possibilities here involving the
   // double-emission of constructors and destructors.
   if (CGM.getLangOpts().CPlusPlus) {
     llvm::GlobalValue *value = CGM.getModule().getNamedValue(Name);
-    if (value && isa<llvm::GlobalVariable>(value) &&
-        value->getType() ==
-          LTy->getPointerTo(CGM.getContext().getTargetAddressSpace(Ty)))
-      return cast<llvm::GlobalVariable>(value);
+    if (value && isa<llvm::GlobalVariable>(value)) {
+      // Check that the type is compatible with the type we want.  The
+      // simple equality check isn't good enough because initializers
+      // can force the changing of a type (e.g.  with unions).
+      if (value->getType()->getAddressSpace() == addrspace &&
+          isExistingVarAdequate(CGM, value->getType()->getElementType(), LTy))
+        return llvm::ConstantExpr::getBitCast(value,
+                                              LTy->getPointerTo(addrspace));
+    }
 
     if (value) {
       CGM.Error(D.getLocation(),
-              "problem emitting static variable: already present as "
-              "different kind of symbol");
+                "problem emitting static variable '" + Name +
+                "': already present as different kind of symbol");
+
       // Fall through and implicitly give it a uniqued name.
     }
   }
@@ -207,7 +231,7 @@ CodeGenFunction::CreateStaticVarDecl(const VarDecl &D,
                              Ty.isConstant(getContext()), Linkage,
                              CGM.EmitNullConstant(D.getType()), Name, 0,
                              D.isThreadSpecified(),
-                             CGM.getContext().getTargetAddressSpace(Ty));
+                             addrspace);
   GV->setAlignment(getContext().getDeclAlign(&D).getQuantity());
   if (Linkage != llvm::GlobalValue::InternalLinkage)
     GV->setVisibility(CurFn->getVisibility());
@@ -222,80 +246,85 @@ static bool hasNontrivialDestruction(QualType T) {
   return RD && !RD->hasTrivialDestructor();
 }
 
-/// 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
-/// one.  Otherwise it just returns GV.
-llvm::GlobalVariable *
+/// 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
+/// force the underlying variable to change.  Otherwise it just
+/// returns it.
+///
+/// The argument must be a (potentially casted) global variable,
+/// and the result will be one, too.
+llvm::Constant *
 CodeGenFunction::AddInitializerToStaticVarDecl(const VarDecl &D,
-                                               llvm::GlobalVariable *GV) {
-  llvm::Constant *Init = CGM.EmitConstantInit(D, this);
+                                               llvm::Constant *addr) {
+  llvm::Constant *init = CGM.EmitConstantInit(D, this);
+
+  llvm::GlobalVariable *var =
+    cast<llvm::GlobalVariable>(addr->stripPointerCasts());
 
   // If constant emission failed, then this should be a C++ static
   // initializer.
-  if (!Init) {
+  if (!init) {
     if (!getContext().getLangOpts().CPlusPlus)
       CGM.ErrorUnsupported(D.getInit(), "constant l-value expression");
     else if (Builder.GetInsertBlock()) {
       // Since we have a static initializer, this global variable can't
       // be constant.
-      GV->setConstant(false);
+      var->setConstant(false);
 
-      EmitCXXGuardedInit(D, GV, /*PerformInit*/true);
+      EmitCXXGuardedInit(D, addr, /*PerformInit*/true);
     }
-    return GV;
+    return addr;
   }
 
   // The initializer may differ in type from the global. Rewrite
   // the global to match the initializer.  (We have to do this
   // because some types, like unions, can't be completely represented
   // in the LLVM type system.)
-  if (GV->getType()->getElementType() != Init->getType()) {
-    llvm::GlobalVariable *OldGV = GV;
-
-    GV = new llvm::GlobalVariable(CGM.getModule(), Init->getType(),
-                                  OldGV->isConstant(),
-                                  OldGV->getLinkage(), Init, "",
-                                  /*InsertBefore*/ OldGV,
-                                  D.isThreadSpecified(),
-                           CGM.getContext().getTargetAddressSpace(D.getType()));
-    GV->setVisibility(OldGV->getVisibility());
+  if (var->getType()->getElementType() != init->getType()) {
+    llvm::GlobalVariable *newVar
+      = new llvm::GlobalVariable(CGM.getModule(), init->getType(),
+                                 var->isConstant(),
+                                 var->getLinkage(), init, "",
+                                 /*InsertBefore*/ var,
+                                 D.isThreadSpecified(),
+                                 var->getType()->getAddressSpace());
+    newVar->setVisibility(var->getVisibility());
 
     // Steal the name of the old global
-    GV->takeName(OldGV);
+    newVar->takeName(var);
 
     // Replace all uses of the old global with the new global
-    llvm::Constant *NewPtrForOldDecl =
-    llvm::ConstantExpr::getBitCast(GV, OldGV->getType());
-    OldGV->replaceAllUsesWith(NewPtrForOldDecl);
+    addr = llvm::ConstantExpr::getBitCast(newVar, addr->getType());
+    var->replaceAllUsesWith(addr);
 
     // Erase the old global, since it is no longer used.
-    OldGV->eraseFromParent();
+    var->eraseFromParent();
+    var = newVar;
   }
 
-  GV->setConstant(CGM.isTypeConstant(D.getType(), true));
-  GV->setInitializer(Init);
+  var->setConstant(CGM.isTypeConstant(D.getType(), true));
+  var->setInitializer(init);
 
   if (hasNontrivialDestruction(D.getType())) {
     // We have a constant initializer, but a nontrivial destructor. We still
     // need to perform a guarded "initialization" in order to register the
     // destructor.
-    EmitCXXGuardedInit(D, GV, /*PerformInit*/false);
+    EmitCXXGuardedInit(D, addr, /*PerformInit*/false);
   }
 
-  return GV;
+  return addr;
 }
 
 void CodeGenFunction::EmitStaticVarDecl(const VarDecl &D,
                                       llvm::GlobalValue::LinkageTypes Linkage) {
-  llvm::Value *&DMEntry = LocalDeclMap[&D];
-  assert(DMEntry == 0 && "Decl already exists in localdeclmap!");
 
-  llvm::GlobalVariable *GV = CreateStaticVarDecl(D, ".", Linkage);
+  llvm::Constant *addr = CreateStaticVarDecl(D, ".", Linkage);
 
   // Store into LocalDeclMap before generating initializer to handle
   // circular references.
-  DMEntry = GV;
+  assert(!LocalDeclMap.count(&D) && "Decl already exists in localdeclmap!");
+  LocalDeclMap[&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.
@@ -305,40 +334,34 @@ void CodeGenFunction::EmitStaticVarDecl(const VarDecl &D,
 
   // Local static block variables must be treated as globals as they may be
   // referenced in their RHS initializer block-literal expresion.
-  CGM.setStaticLocalDeclAddress(&D, GV);
+  CGM.setStaticLocalDeclAddress(&D, addr);
 
   // If this value has an initializer, emit it.
+  // This can leave us with a casted pointer.
   if (D.getInit())
-    GV = AddInitializerToStaticVarDecl(D, GV);
+    addr = AddInitializerToStaticVarDecl(D, addr);
 
-  GV->setAlignment(getContext().getDeclAlign(&D).getQuantity());
+  llvm::GlobalVariable *var =
+    cast<llvm::GlobalVariable>(addr->stripPointerCasts());
+  var->setAlignment(getContext().getDeclAlign(&D).getQuantity());
 
   if (D.hasAttr<AnnotateAttr>())
-    CGM.AddGlobalAnnotations(&D, GV);
+    CGM.AddGlobalAnnotations(&D, var);
 
   if (const SectionAttr *SA = D.getAttr<SectionAttr>())
-    GV->setSection(SA->getName());
+    var->setSection(SA->getName());
 
   if (D.hasAttr<UsedAttr>())
-    CGM.AddUsedGlobal(GV);
-
-  // We may have to cast the constant because of the initializer
-  // mismatch above.
-  //
-  // FIXME: It is really dangerous to store this in the map; if anyone
-  // RAUW's the GV uses of this constant will be invalid.
-  llvm::Type *LTy = CGM.getTypes().ConvertTypeForMem(D.getType());
-  llvm::Type *LPtrTy =
-    LTy->getPointerTo(CGM.getContext().getTargetAddressSpace(D.getType()));
-  llvm::Constant *CastedVal = llvm::ConstantExpr::getBitCast(GV, LPtrTy);
-  DMEntry = CastedVal;
-  CGM.setStaticLocalDeclAddress(&D, CastedVal);
+    CGM.AddUsedGlobal(var);
+
+  LocalDeclMap[&D] = addr;
+  CGM.setStaticLocalDeclAddress(&D, addr);
 
   // Emit global variable debug descriptor for static vars.
   CGDebugInfo *DI = getDebugInfo();
   if (DI) {
     DI->setLocation(D.getLocation());
-    DI->EmitGlobalVariable(static_cast<llvm::GlobalVariable *>(GV), &D);
+    DI->EmitGlobalVariable(var, &D);
   }
 }
 

lib/CodeGen/CGDeclCXX.cpp

@@ -179,7 +179,7 @@ CodeGenFunction::EmitCXXGlobalDtorRegistration(llvm::Constant *DtorFn,
 }
 
 void CodeGenFunction::EmitCXXGuardedInit(const VarDecl &D,
-                                         llvm::GlobalVariable *DeclPtr,
+                                         llvm::Constant *addr,
                                          bool PerformInit) {
   // If we've been asked to forbid guard variables, emit an error now.
   // This diagnostic is hard-coded for Darwin's use case;  we can find
@@ -189,7 +189,7 @@ void CodeGenFunction::EmitCXXGuardedInit(const VarDecl &D,
               "this initialization requires a guard variable, which "
               "the kernel does not support");
 
-  CGM.getCXXABI().EmitGuardedInit(*this, D, DeclPtr, PerformInit);
+  CGM.getCXXABI().EmitGuardedInit(*this, D, addr, PerformInit);
 }
 
 static llvm::Function *

lib/CodeGen/CodeGenFunction.h

@@ -2392,17 +2392,17 @@ public:
 
   /// CreateStaticVarDecl - Create a zero-initialized LLVM global for
   /// a static local variable.
-  llvm::GlobalVariable *CreateStaticVarDecl(const VarDecl &D,
-                                            const char *Separator,
-                                       llvm::GlobalValue::LinkageTypes Linkage);
+  llvm::Constant *CreateStaticVarDecl(const VarDecl &D,
+                                      const char *Separator,
+                                      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
-  /// one.  Otherwise it just returns GV.
-  llvm::GlobalVariable *
-  AddInitializerToStaticVarDecl(const VarDecl &D,
-                                llvm::GlobalVariable *GV);
+  /// 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
+  /// force the underlying variable to change.  Otherwise it just
+  /// returns it.
+  llvm::Constant *
+  AddInitializerToStaticVarDecl(const VarDecl &D, llvm::Constant *GV);
 
 
   /// EmitCXXGlobalVarDeclInit - Create the initializer for a C++
@@ -2420,7 +2420,7 @@ public:
   /// possible to prove that an initialization will be done exactly
   /// once, e.g. with a static local variable or a static data member
   /// of a class template.
-  void EmitCXXGuardedInit(const VarDecl &D, llvm::GlobalVariable *DeclPtr,
+  void EmitCXXGuardedInit(const VarDecl &D, llvm::Constant *addr,
                           bool PerformInit);
 
   /// GenerateCXXGlobalInitFunc - Generates code for initializing global

lib/CodeGen/CodeGenModule.cpp

@@ -197,8 +197,9 @@ bool CodeGenModule::isTargetDarwin() const {
   return getContext().getTargetInfo().getTriple().isOSDarwin();
 }
 
-void CodeGenModule::Error(SourceLocation loc, StringRef error) {
-  unsigned diagID = getDiags().getCustomDiagID(DiagnosticsEngine::Error, error);
+void CodeGenModule::Error(SourceLocation loc, const Twine &error) {
+  unsigned diagID = getDiags().getCustomDiagID(DiagnosticsEngine::Error,
+                                               error.str());
   getDiags().Report(Context.getFullLoc(loc), diagID);
 }
 

lib/CodeGen/CodeGenModule.h

@@ -733,7 +733,7 @@ public:
   llvm::Constant *EmitNullConstantForBase(const CXXRecordDecl *Record);
 
   /// Error - Emit a general error that something can't be done.
-  void Error(SourceLocation loc, StringRef error);
+  void Error(SourceLocation loc, const Twine &error);
 
   /// ErrorUnsupported - Print out an error that codegen doesn't support the
   /// specified stmt yet.

lib/CodeGen/ItaniumCXXABI.cpp

@@ -123,7 +123,7 @@ public:
                        llvm::Value *&AllocPtr, CharUnits &CookieSize);
 
   void EmitGuardedInit(CodeGenFunction &CGF, const VarDecl &D,
-                       llvm::GlobalVariable *DeclPtr, bool PerformInit);
+                       llvm::Constant *addr, bool PerformInit);
 };
 
 class ARMCXXABI : public ItaniumCXXABI {
@@ -1064,7 +1064,7 @@ namespace {
 /// just special-case it at particular places.
 void ItaniumCXXABI::EmitGuardedInit(CodeGenFunction &CGF,
                                     const VarDecl &D,
-                                    llvm::GlobalVariable *GV,
+                                    llvm::Constant *varAddr,
                                     bool PerformInit) {
   CGBuilderTy &Builder = CGF.Builder;
 
@@ -1075,9 +1075,14 @@ void ItaniumCXXABI::EmitGuardedInit(CodeGenFunction &CGF,
 
   llvm::IntegerType *guardTy;
 
+  // Find the underlying global variable for linkage purposes.
+  // This may not have the right type for actual evaluation purposes.
+  llvm::GlobalVariable *var =
+    cast<llvm::GlobalVariable>(varAddr->stripPointerCasts());
+
   // If we have a global variable with internal linkage and thread-safe statics
   // are disabled, we can just let the guard variable be of type i8.
-  bool useInt8GuardVariable = !threadsafe && GV->hasInternalLinkage();
+  bool useInt8GuardVariable = !threadsafe && var->hasInternalLinkage();
   if (useInt8GuardVariable) {
     guardTy = CGF.Int8Ty;
   } else {
@@ -1103,9 +1108,10 @@ void ItaniumCXXABI::EmitGuardedInit(CodeGenFunction &CGF,
         existingGuard->getType() == guardPtrTy) {
       guard = cast<llvm::GlobalVariable>(existingGuard); // okay
     } else {
-      CGM.Error(D.getLocation(),
-              "problem emitting guard for static variable: "
-              "already present as different kind of symbol");
+      CGM.Error(D.getLocation(), "problem emitting static variable '"
+                                 + guardName.str() +
+                "': already present as different kind of symbol");
+
       // Fall through and implicitly give it a uniqued name.
     }
   }
@@ -1113,10 +1119,10 @@ void ItaniumCXXABI::EmitGuardedInit(CodeGenFunction &CGF,
   if (!guard) {
     // Just absorb linkage and visibility from the variable.
     guard = new llvm::GlobalVariable(CGM.getModule(), guardTy,
-                                     false, GV->getLinkage(),
+                                     false, var->getLinkage(),
                                      llvm::ConstantInt::get(guardTy, 0),
                                      guardName.str());
-    guard->setVisibility(GV->getVisibility());
+    guard->setVisibility(var->getVisibility());
   }
     
   // Test whether the variable has completed initialization.
@@ -1196,7 +1202,7 @@ void ItaniumCXXABI::EmitGuardedInit(CodeGenFunction &CGF,
   }
 
   // Emit the initializer and add a global destructor if appropriate.
-  CGF.EmitCXXGlobalVarDeclInit(D, GV, PerformInit);
+  CGF.EmitCXXGlobalVarDeclInit(D, varAddr, PerformInit);
 
   if (threadsafe) {
     // Pop the guard-abort cleanup if we pushed one.

test/CodeGenCXX/static-init.cpp

@@ -3,6 +3,7 @@
 // CHECK: @_ZZ1hvE1i = internal global i32 0, align 4
 // CHECK: @base_req = global [4 x i8] c"foo\00", align 1
 
+// CHECK: @_ZZN5test31BC1EvE1u = internal global { i8, [3 x i8] } { i8 97, [3 x i8] undef }, align 4
 // CHECK: @_ZZN5test1L6getvarEiE3var = internal constant [4 x i32] [i32 1, i32 0, i32 2, i32 4], align 16
 // CHECK: @_ZZ2h2vE1i = linkonce_odr global i32 0
 // CHECK: @_ZGVZ2h2vE1i = linkonce_odr global i64 0
@@ -129,3 +130,23 @@ namespace test2 {
   // CHECK:   store i32 [[T0]], i32* @_ZZN5test21BD1EvE1y,
   // CHECK:   call void @__cxa_guard_release(i64* @_ZGVZN5test21BD1EvE1y)
 }
+
+// This shouldn't error out.
+namespace test3 {
+  struct A {
+    A();
+    ~A();
+  };
+
+  struct B : virtual A {
+    B();
+    ~B();
+  };
+
+  B::B() {
+    union U { char x; int i; };
+    static U u = { 'a' };
+  }
+  // CHECK: define void @_ZN5test31BC1Ev(
+  // CHECK: define void @_ZN5test31BC2Ev(
+}