From: John McCall Date: Fri, 30 Mar 2012 07:09:50 +0000 (+0000) Subject: Do the static-locals thing properly in the face of unions and X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=49d26d2817180ccde605c987f79cd3a5b57639cd;p=clang 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 --- diff --git a/lib/CodeGen/CGCXXABI.cpp b/lib/CodeGen/CGCXXABI.cpp index befebbecbd..998795d585 100644 --- a/lib/CodeGen/CGCXXABI.cpp +++ b/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"); } diff --git a/lib/CodeGen/CGCXXABI.h b/lib/CodeGen/CGCXXABI.h index 4e045f5f32..e357d8280c 100644 --- a/lib/CodeGen/CGCXXABI.h +++ b/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); }; diff --git a/lib/CodeGen/CGDecl.cpp b/lib/CodeGen/CGDecl.cpp index d6d6696894..7cd02d8d86 100644 --- a/lib/CodeGen/CGDecl.cpp +++ b/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(value) && - value->getType() == - LTy->getPointerTo(CGM.getContext().getTargetAddressSpace(Ty))) - return cast(value); + if (value && isa(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(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(addr->stripPointerCasts()); + var->setAlignment(getContext().getDeclAlign(&D).getQuantity()); if (D.hasAttr()) - CGM.AddGlobalAnnotations(&D, GV); + CGM.AddGlobalAnnotations(&D, var); if (const SectionAttr *SA = D.getAttr()) - GV->setSection(SA->getName()); + var->setSection(SA->getName()); if (D.hasAttr()) - 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(GV), &D); + DI->EmitGlobalVariable(var, &D); } } diff --git a/lib/CodeGen/CGDeclCXX.cpp b/lib/CodeGen/CGDeclCXX.cpp index dc52b11279..06ae035295 100644 --- a/lib/CodeGen/CGDeclCXX.cpp +++ b/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 * diff --git a/lib/CodeGen/CodeGenFunction.h b/lib/CodeGen/CodeGenFunction.h index 85cbd143d8..02346cf026 100644 --- a/lib/CodeGen/CodeGenFunction.h +++ b/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 diff --git a/lib/CodeGen/CodeGenModule.cpp b/lib/CodeGen/CodeGenModule.cpp index c9f1066032..4170c7bc30 100644 --- a/lib/CodeGen/CodeGenModule.cpp +++ b/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); } diff --git a/lib/CodeGen/CodeGenModule.h b/lib/CodeGen/CodeGenModule.h index 5719afb612..79247c6d47 100644 --- a/lib/CodeGen/CodeGenModule.h +++ b/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. diff --git a/lib/CodeGen/ItaniumCXXABI.cpp b/lib/CodeGen/ItaniumCXXABI.cpp index 738a1832cc..ce16c86b8d 100644 --- a/lib/CodeGen/ItaniumCXXABI.cpp +++ b/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(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(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. diff --git a/test/CodeGenCXX/static-init.cpp b/test/CodeGenCXX/static-init.cpp index ca47428fe2..7e840f5622 100644 --- a/test/CodeGenCXX/static-init.cpp +++ b/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( +}