From a9b21d22bb9337649723a8477b5cb15f83451e7d Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Fri, 17 Feb 2012 06:48:11 +0000 Subject: [PATCH] Bug fix: do not emit static const local variables with mutable members as constants. Refactor and simplify all the separate checks for whether a type can be emitted as a constant. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@150793 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/CodeGen/CGDecl.cpp | 23 ++++++++------------- lib/CodeGen/CodeGenModule.cpp | 33 ++++++++++++++++-------------- lib/CodeGen/CodeGenModule.h | 2 ++ test/CodeGenCXX/static-mutable.cpp | 12 +++++++++++ 4 files changed, 41 insertions(+), 29 deletions(-) create mode 100644 test/CodeGenCXX/static-mutable.cpp diff --git a/lib/CodeGen/CGDecl.cpp b/lib/CodeGen/CGDecl.cpp index 0ee3fcdae4..2467d991d9 100644 --- a/lib/CodeGen/CGDecl.cpp +++ b/lib/CodeGen/CGDecl.cpp @@ -255,14 +255,13 @@ CodeGenFunction::AddInitializerToStaticVarDecl(const VarDecl &D, OldGV->eraseFromParent(); } + GV->setConstant(CGM.isTypeConstant(D.getType(), true)); GV->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. Since we're running a destructor on this variable, it can't - // be a constant even if it's const. - GV->setConstant(false); + // destructor. EmitCXXGuardedInit(D, GV, /*PerformInit*/false); } @@ -775,7 +774,7 @@ CodeGenFunction::EmitAutoVarAlloca(const VarDecl &D) { // If this value is a POD array or struct with a statically // determinable constant initializer, there are optimizations we can do. // - // TODO: we should constant-evaluate any variable of literal type + // TODO: We should constant-evaluate the initializer of any variable, // as long as it is initialized by a constant expression. Currently, // isConstantInitializer produces wrong answers for structs with // reference or bitfield members, and a few other cases, and checking @@ -789,17 +788,13 @@ CodeGenFunction::EmitAutoVarAlloca(const VarDecl &D) { // If the variable's a const type, and it's neither an NRVO // candidate nor a __block variable and has no mutable members, // emit it as a global instead. - if (CGM.getCodeGenOpts().MergeAllConstants && Ty.isConstQualified() && - !NRVO && !isByRef && Ty->isLiteralType()) { - CXXRecordDecl *RD = - Ty->getBaseElementTypeUnsafe()->getAsCXXRecordDecl(); - if (!RD || !RD->hasMutableFields()) { - EmitStaticVarDecl(D, llvm::GlobalValue::InternalLinkage); + if (CGM.getCodeGenOpts().MergeAllConstants && !NRVO && !isByRef && + CGM.isTypeConstant(Ty, true)) { + EmitStaticVarDecl(D, llvm::GlobalValue::InternalLinkage); - emission.Address = 0; // signal this condition to later callbacks - assert(emission.wasEmittedAsGlobal()); - return emission; - } + emission.Address = 0; // signal this condition to later callbacks + assert(emission.wasEmittedAsGlobal()); + return emission; } // Otherwise, tell the initialization code that we're in this case. diff --git a/lib/CodeGen/CodeGenModule.cpp b/lib/CodeGen/CodeGenModule.cpp index 418b1449d3..f39e20741b 100644 --- a/lib/CodeGen/CodeGenModule.cpp +++ b/lib/CodeGen/CodeGenModule.cpp @@ -1101,18 +1101,23 @@ CodeGenModule::CreateRuntimeFunction(llvm::FunctionType *FTy, ExtraAttrs); } -static bool DeclIsConstantGlobal(ASTContext &Context, const VarDecl *D, - bool ConstantInit) { - if (!D->getType().isConstant(Context) && !D->getType()->isReferenceType()) +/// isTypeConstant - Determine whether an object of this type can be emitted +/// as a constant. +/// +/// If ExcludeCtor is true, the duration when the object's constructor runs +/// will not be considered. The caller will need to verify that the object is +/// not written to during its construction. +bool CodeGenModule::isTypeConstant(QualType Ty, bool ExcludeCtor) { + if (!Ty.isConstant(Context) && !Ty->isReferenceType()) return false; - + if (Context.getLangOptions().CPlusPlus) { - if (const RecordType *Record - = Context.getBaseElementType(D->getType())->getAs()) - return ConstantInit && - !cast(Record->getDecl())->hasMutableFields(); + if (const CXXRecordDecl *Record + = Context.getBaseElementType(Ty)->getAsCXXRecordDecl()) + return ExcludeCtor && !Record->hasMutableFields() && + Record->hasTrivialDestructor(); } - + return true; } @@ -1169,7 +1174,7 @@ CodeGenModule::GetOrCreateLLVMGlobal(StringRef MangledName, if (D) { // FIXME: This code is overly simple and should be merged with other global // handling. - GV->setConstant(DeclIsConstantGlobal(Context, D, false)); + GV->setConstant(isTypeConstant(D->getType(), false)); // Set linkage and visibility in case we never see a definition. NamedDecl::LinkageInfo LV = D->getLinkageAndVisibility(); @@ -1450,13 +1455,11 @@ void CodeGenModule::EmitGlobalVarDefinition(const VarDecl *D) { GV->setInitializer(Init); // If it is safe to mark the global 'constant', do so now. - GV->setConstant(false); - if (!NeedsGlobalCtor && !NeedsGlobalDtor && - DeclIsConstantGlobal(Context, D, true)) - GV->setConstant(true); + GV->setConstant(!NeedsGlobalCtor && !NeedsGlobalDtor && + isTypeConstant(D->getType(), true)); GV->setAlignment(getContext().getDeclAlign(D).getQuantity()); - + // Set the llvm linkage type as appropriate. llvm::GlobalValue::LinkageTypes Linkage = GetLLVMLinkageVarDefinition(D, GV); diff --git a/lib/CodeGen/CodeGenModule.h b/lib/CodeGen/CodeGenModule.h index 9b6931d5aa..fa1830a386 100644 --- a/lib/CodeGen/CodeGenModule.h +++ b/lib/CodeGen/CodeGenModule.h @@ -448,6 +448,8 @@ public: llvm::MDNode *getTBAAInfo(QualType QTy); + bool isTypeConstant(QualType QTy, bool ExcludeCtorDtor); + static void DecorateInstruction(llvm::Instruction *Inst, llvm::MDNode *TBAAInfo); diff --git a/test/CodeGenCXX/static-mutable.cpp b/test/CodeGenCXX/static-mutable.cpp new file mode 100644 index 0000000000..6d51f241d1 --- /dev/null +++ b/test/CodeGenCXX/static-mutable.cpp @@ -0,0 +1,12 @@ +// RUN: %clang_cc1 %s -triple=i686-linux-gnu -emit-llvm -o - | FileCheck %s + +struct S { + mutable int n; +}; +int f() { + // The purpose of this test is to ensure that this variable is a global + // not a constant. + // CHECK: @_ZZ1fvE1s = internal global {{.*}} { i32 12 } + static const S s = { 12 }; + return ++s.n; +}