From 947c1830e4a7e620de7991dea033755f0265b15b Mon Sep 17 00:00:00 2001 From: John McCall Date: Fri, 30 Mar 2012 04:25:14 +0000 Subject: [PATCH] When emitting a static local variable in C++, handle the case that the variable already exists. Partly this is just protection against people making crazy declarations with custom asm labels or extern "C" names that intentionally collide with the manglings of such variables, but the main reason is that we can actually emit a static local variable twice with the requirement that it match up. There may be other cases with (e.g.) the various nested functions, but the main exemplar is with constructor variants, where we can be forced into double-emitting the function body under certain circumstances like (currently) the presence of virtual bases. llvm-svn: 153723 --- clang/lib/CodeGen/CGDecl.cpp | 18 +++++++ clang/lib/CodeGen/ItaniumCXXABI.cpp | 78 ++++++++++++++++----------- clang/test/CodeGenCXX/static-init.cpp | 50 +++++++++++++++++ 3 files changed, 116 insertions(+), 30 deletions(-) diff --git a/clang/lib/CodeGen/CGDecl.cpp b/clang/lib/CodeGen/CGDecl.cpp index 970f0b2389f2..d6d669689480 100644 --- a/clang/lib/CodeGen/CGDecl.cpp +++ b/clang/lib/CodeGen/CGDecl.cpp @@ -184,6 +184,24 @@ CodeGenFunction::CreateStaticVarDecl(const VarDecl &D, Name = GetStaticDeclName(*this, D, Separator); llvm::Type *LTy = CGM.getTypes().ConvertTypeForMem(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) { + CGM.Error(D.getLocation(), + "problem emitting static variable: already present as " + "different kind of symbol"); + // Fall through and implicitly give it a uniqued name. + } + } + llvm::GlobalVariable *GV = new llvm::GlobalVariable(CGM.getModule(), LTy, Ty.isConstant(getContext()), Linkage, diff --git a/clang/lib/CodeGen/ItaniumCXXABI.cpp b/clang/lib/CodeGen/ItaniumCXXABI.cpp index 6fcf83d5ace1..8cce8c90a071 100644 --- a/clang/lib/CodeGen/ItaniumCXXABI.cpp +++ b/clang/lib/CodeGen/ItaniumCXXABI.cpp @@ -1051,7 +1051,7 @@ static llvm::Constant *getGuardAbortFn(CodeGenModule &CGM, namespace { struct CallGuardAbort : EHScopeStack::Cleanup { llvm::GlobalVariable *Guard; - CallGuardAbort(llvm::GlobalVariable *Guard) : Guard(Guard) {} + CallGuardAbort(llvm::GlobalVariable *guard) : Guard(guard) {} void Emit(CodeGenFunction &CGF, Flags flags) { CGF.Builder.CreateCall(getGuardAbortFn(CGF.CGM, Guard->getType()), Guard) @@ -1073,35 +1073,54 @@ void ItaniumCXXABI::EmitGuardedInit(CodeGenFunction &CGF, bool threadsafe = (getContext().getLangOpts().ThreadsafeStatics && D.isLocalVarDecl()); - llvm::IntegerType *GuardTy; + llvm::IntegerType *guardTy; // 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(); if (useInt8GuardVariable) { - GuardTy = CGF.Int8Ty; + guardTy = CGF.Int8Ty; } else { // Guard variables are 64 bits in the generic ABI and 32 bits on ARM. - GuardTy = (IsARM ? CGF.Int32Ty : CGF.Int64Ty); + guardTy = (IsARM ? CGF.Int32Ty : CGF.Int64Ty); } - llvm::PointerType *GuardPtrTy = GuardTy->getPointerTo(); + llvm::PointerType *guardPtrTy = guardTy->getPointerTo(); // Create the guard variable. - SmallString<256> GuardVName; - llvm::raw_svector_ostream Out(GuardVName); - getMangleContext().mangleItaniumGuardVariable(&D, Out); - Out.flush(); + SmallString<256> guardName; + { + llvm::raw_svector_ostream out(guardName); + getMangleContext().mangleItaniumGuardVariable(&D, out); + out.flush(); + } - // Just absorb linkage and visibility from the variable. - llvm::GlobalVariable *GuardVariable = - new llvm::GlobalVariable(CGM.getModule(), GuardTy, - false, GV->getLinkage(), - llvm::ConstantInt::get(GuardTy, 0), - GuardVName.str()); - GuardVariable->setVisibility(GV->getVisibility()); + // There are strange possibilities here involving the + // double-emission of constructors and destructors. + llvm::GlobalVariable *guard = nullptr; + if (llvm::GlobalValue *existingGuard + = CGM.getModule().getNamedValue(guardName.str())) { + if (isa(existingGuard) && + 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"); + // Fall through and implicitly give it a uniqued name. + } + } + if (!guard) { + // Just absorb linkage and visibility from the variable. + guard = new llvm::GlobalVariable(CGM.getModule(), guardTy, + false, GV->getLinkage(), + llvm::ConstantInt::get(guardTy, 0), + guardName.str()); + guard->setVisibility(GV->getVisibility()); + } + // Test whether the variable has completed initialization. - llvm::Value *IsInitialized; + llvm::Value *isInitialized; // ARM C++ ABI 3.2.3.1: // To support the potential use of initialization guard variables @@ -1115,9 +1134,9 @@ void ItaniumCXXABI::EmitGuardedInit(CodeGenFunction &CGF, // ... // } if (IsARM && !useInt8GuardVariable) { - llvm::Value *V = Builder.CreateLoad(GuardVariable); + llvm::Value *V = Builder.CreateLoad(guard); V = Builder.CreateAnd(V, Builder.getInt32(1)); - IsInitialized = Builder.CreateIsNull(V, "guard.uninitialized"); + isInitialized = Builder.CreateIsNull(V, "guard.uninitialized"); // Itanium C++ ABI 3.3.2: // The following is pseudo-code showing how these functions can be used: @@ -1135,10 +1154,9 @@ void ItaniumCXXABI::EmitGuardedInit(CodeGenFunction &CGF, // } } else { // Load the first byte of the guard variable. - llvm::Type *PtrTy = Builder.getInt8PtrTy(); - llvm::LoadInst *LI = - Builder.CreateLoad(Builder.CreateBitCast(GuardVariable, PtrTy)); - LI->setAlignment(1); + llvm::LoadInst *load = + Builder.CreateLoad(Builder.CreateBitCast(guard, CGM.Int8PtrTy)); + load->setAlignment(1); // Itanium ABI: // An implementation supporting thread-safety on multiprocessor @@ -1147,16 +1165,16 @@ void ItaniumCXXABI::EmitGuardedInit(CodeGenFunction &CGF, // // In LLVM, we do this by marking the load Acquire. if (threadsafe) - LI->setAtomic(llvm::Acquire); + load->setAtomic(llvm::Acquire); - IsInitialized = Builder.CreateIsNull(LI, "guard.uninitialized"); + isInitialized = Builder.CreateIsNull(load, "guard.uninitialized"); } llvm::BasicBlock *InitCheckBlock = CGF.createBasicBlock("init.check"); llvm::BasicBlock *EndBlock = CGF.createBasicBlock("init.end"); // Check if the first byte of the guard variable is zero. - Builder.CreateCondBr(IsInitialized, InitCheckBlock, EndBlock); + Builder.CreateCondBr(isInitialized, InitCheckBlock, EndBlock); CGF.EmitBlock(InitCheckBlock); @@ -1164,7 +1182,7 @@ void ItaniumCXXABI::EmitGuardedInit(CodeGenFunction &CGF, if (threadsafe) { // Call __cxa_guard_acquire. llvm::Value *V - = Builder.CreateCall(getGuardAcquireFn(CGM, GuardPtrTy), GuardVariable); + = Builder.CreateCall(getGuardAcquireFn(CGM, guardPtrTy), guard); llvm::BasicBlock *InitBlock = CGF.createBasicBlock("init"); @@ -1172,7 +1190,7 @@ void ItaniumCXXABI::EmitGuardedInit(CodeGenFunction &CGF, InitBlock, EndBlock); // Call __cxa_guard_abort along the exceptional edge. - CGF.EHStack.pushCleanup(EHCleanup, GuardVariable); + CGF.EHStack.pushCleanup(EHCleanup, guard); CGF.EmitBlock(InitBlock); } @@ -1185,9 +1203,9 @@ void ItaniumCXXABI::EmitGuardedInit(CodeGenFunction &CGF, CGF.PopCleanupBlock(); // Call __cxa_guard_release. This cannot throw. - Builder.CreateCall(getGuardReleaseFn(CGM, GuardPtrTy), GuardVariable); + Builder.CreateCall(getGuardReleaseFn(CGM, guardPtrTy), guard); } else { - Builder.CreateStore(llvm::ConstantInt::get(GuardTy, 1), GuardVariable); + Builder.CreateStore(llvm::ConstantInt::get(guardTy, 1), guard); } CGF.EmitBlock(EndBlock); diff --git a/clang/test/CodeGenCXX/static-init.cpp b/clang/test/CodeGenCXX/static-init.cpp index a96cb7ac8e02..ca47428fe2c6 100644 --- a/clang/test/CodeGenCXX/static-init.cpp +++ b/clang/test/CodeGenCXX/static-init.cpp @@ -79,3 +79,53 @@ namespace union_static_local { c::main(); } } + +// rdar://problem/11091093 +// Static variables should be consistent across constructor +// or destructor variants. +namespace test2 { + struct A { + A(); + ~A(); + }; + + struct B : virtual A { + B(); + ~B(); + }; + + // If we ever implement this as a delegate ctor call, just change + // this to take variadic arguments or something. + extern int foo(); + B::B() { + static int x = foo(); + } + // CHECK: define void @_ZN5test21BC1Ev + // CHECK: load atomic i8* bitcast (i64* @_ZGVZN5test21BC1EvE1x to i8*) acquire, + // CHECK: call i32 @__cxa_guard_acquire(i64* @_ZGVZN5test21BC1EvE1x) + // CHECK: [[T0:%.*]] = call i32 @_ZN5test23fooEv() + // CHECK: store i32 [[T0]], i32* @_ZZN5test21BC1EvE1x, + // CHECK: call void @__cxa_guard_release(i64* @_ZGVZN5test21BC1EvE1x) + + // CHECK: define void @_ZN5test21BC2Ev + // CHECK: load atomic i8* bitcast (i64* @_ZGVZN5test21BC1EvE1x to i8*) acquire, + // CHECK: call i32 @__cxa_guard_acquire(i64* @_ZGVZN5test21BC1EvE1x) + // CHECK: [[T0:%.*]] = call i32 @_ZN5test23fooEv() + // CHECK: store i32 [[T0]], i32* @_ZZN5test21BC1EvE1x, + // CHECK: call void @__cxa_guard_release(i64* @_ZGVZN5test21BC1EvE1x) + + // This is just for completeness, because we actually emit this + // using a delegate dtor call. + B::~B() { + static int y = foo(); + } + // CHECK: define void @_ZN5test21BD1Ev( + // CHECK: call void @_ZN5test21BD2Ev( + + // CHECK: define void @_ZN5test21BD2Ev( + // CHECK: load atomic i8* bitcast (i64* @_ZGVZN5test21BD1EvE1y to i8*) acquire, + // CHECK: call i32 @__cxa_guard_acquire(i64* @_ZGVZN5test21BD1EvE1y) + // CHECK: [[T0:%.*]] = call i32 @_ZN5test23fooEv() + // CHECK: store i32 [[T0]], i32* @_ZZN5test21BD1EvE1y, + // CHECK: call void @__cxa_guard_release(i64* @_ZGVZN5test21BD1EvE1y) +}