From 28ee2d1b0973df3295cd7eadb45473037e530ed9 Mon Sep 17 00:00:00 2001 From: Artem Dergachev Date: Wed, 30 Nov 2016 19:02:44 +0000 Subject: [PATCH] [analyzer] Construct temporary objects of correct types, destroy them properly. When constructing a temporary object region, which represents the result of MaterializeTemporaryExpr, track down the sub-expression for which the temporary is necessary with a trick similar to the approach used in CodeGen, namely by using Expr::skipRValueSubobjectAdjustments(). Then, create the temporary object region with type of that sub-expression. That type would propagate further in a path-sensitive manner. During destruction of lifetime-extened temporaries, consult the type of the temporary object region, rather than the type of the lifetime-extending variable, in order to call the correct destructor (fixes pr17001) and, at least, not to crash by trying to call a destructor of a plain type (fixes pr19539). rdar://problem/29131302 rdar://problem/29131576 Differential Revision: https://reviews.llvm.org/D26839 llvm-svn: 288263 --- clang/lib/StaticAnalyzer/Core/ExprEngine.cpp | 79 ++++++++++++-------- clang/test/Analysis/lifetime-extension.cpp | 46 ++++++++++++ 2 files changed, 95 insertions(+), 30 deletions(-) create mode 100644 clang/test/Analysis/lifetime-extension.cpp diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp index ebcc98af7719b..55e735693b7a2 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp @@ -202,25 +202,32 @@ ExprEngine::createTemporaryRegionIfNeeded(ProgramStateRef State, MemRegionManager &MRMgr = StateMgr.getRegionManager(); StoreManager &StoreMgr = StateMgr.getStoreManager(); - // We need to be careful about treating a derived type's value as - // bindings for a base type. Unless we're creating a temporary pointer region, - // start by stripping and recording base casts. - SmallVector Casts; - const Expr *Inner = Ex->IgnoreParens(); - if (!Loc::isLocType(Result->getType())) { - while (const CastExpr *CE = dyn_cast(Inner)) { - if (CE->getCastKind() == CK_DerivedToBase || - CE->getCastKind() == CK_UncheckedDerivedToBase) - Casts.push_back(CE); - else if (CE->getCastKind() != CK_NoOp) - break; + // MaterializeTemporaryExpr may appear out of place, after a few field and + // base-class accesses have been made to the object, even though semantically + // it is the whole object that gets materialized and lifetime-extended. + // + // For example: + // + // `-MaterializeTemporaryExpr + // `-MemberExpr + // `-CXXTemporaryObjectExpr + // + // instead of the more natural + // + // `-MemberExpr + // `-MaterializeTemporaryExpr + // `-CXXTemporaryObjectExpr + // + // Use the usual methods for obtaining the expression of the base object, + // and record the adjustments that we need to make to obtain the sub-object + // that the whole expression 'Ex' refers to. This trick is usual, + // in the sense that CodeGen takes a similar route. - Inner = CE->getSubExpr()->IgnoreParens(); - } - } + SmallVector CommaLHSs; + SmallVector Adjustments; + + const Expr *Init = Ex->skipRValueSubobjectAdjustments(CommaLHSs, Adjustments); - // Create a temporary object region for the inner expression (which may have - // a more derived type) and bind the value into it. const TypedValueRegion *TR = nullptr; if (const MaterializeTemporaryExpr *MT = dyn_cast(Result)) { @@ -228,25 +235,37 @@ ExprEngine::createTemporaryRegionIfNeeded(ProgramStateRef State, // If this object is bound to a reference with static storage duration, we // put it in a different region to prevent "address leakage" warnings. if (SD == SD_Static || SD == SD_Thread) - TR = MRMgr.getCXXStaticTempObjectRegion(Inner); + TR = MRMgr.getCXXStaticTempObjectRegion(Init); } if (!TR) - TR = MRMgr.getCXXTempObjectRegion(Inner, LC); + TR = MRMgr.getCXXTempObjectRegion(Init, LC); SVal Reg = loc::MemRegionVal(TR); + // Make the necessary adjustments to obtain the sub-object. + for (auto I = Adjustments.rbegin(), E = Adjustments.rend(); I != E; ++I) { + const SubobjectAdjustment &Adj = *I; + switch (Adj.Kind) { + case SubobjectAdjustment::DerivedToBaseAdjustment: + Reg = StoreMgr.evalDerivedToBase(Reg, Adj.DerivedToBase.BasePath); + break; + case SubobjectAdjustment::FieldAdjustment: + Reg = StoreMgr.getLValueField(Adj.Field, Reg); + break; + case SubobjectAdjustment::MemberPointerAdjustment: + // FIXME: Unimplemented. + State->bindDefault(Reg, UnknownVal()); + return State; + } + } + + // Try to recover some path sensitivity in case we couldn't compute the value. if (V.isUnknown()) V = getSValBuilder().conjureSymbolVal(Result, LC, TR->getValueType(), currBldrCtx->blockCount()); + // Bind the value of the expression to the sub-object region, and then bind + // the sub-object region to our expression. State = State->bindLoc(Reg, V); - - // Re-apply the casts (from innermost to outermost) for type sanity. - for (SmallVectorImpl::reverse_iterator I = Casts.rbegin(), - E = Casts.rend(); - I != E; ++I) { - Reg = StoreMgr.evalDerivedToBase(Reg, *I); - } - State = State->BindExpr(Result, LC, Reg); return State; } @@ -592,9 +611,9 @@ void ExprEngine::ProcessAutomaticObjDtor(const CFGAutomaticObjDtor Dtor, SVal dest = state->getLValue(varDecl, Pred->getLocationContext()); const MemRegion *Region = dest.castAs().getRegion(); - if (const ReferenceType *refType = varType->getAs()) { - varType = refType->getPointeeType(); - Region = state->getSVal(Region).getAsRegion(); + if (varType->isReferenceType()) { + Region = state->getSVal(Region).getAsRegion()->getBaseRegion(); + varType = cast(Region)->getValueType(); } VisitCXXDestructor(varType, Region, Dtor.getTriggerStmt(), /*IsBase=*/ false, diff --git a/clang/test/Analysis/lifetime-extension.cpp b/clang/test/Analysis/lifetime-extension.cpp new file mode 100644 index 0000000000000..124eef32fde37 --- /dev/null +++ b/clang/test/Analysis/lifetime-extension.cpp @@ -0,0 +1,46 @@ +// RUN: %clang_cc1 -Wno-unused -std=c++11 -analyze -analyzer-checker=debug.ExprInspection -verify %s + +void clang_analyzer_eval(bool); + +namespace pr17001_call_wrong_destructor { +bool x; +struct A { + int *a; + A() {} + ~A() {} +}; +struct B : public A { + B() {} + ~B() { x = true; } +}; + +void f() { + { + const A &a = B(); + } + clang_analyzer_eval(x); // expected-warning{{TRUE}} +} +} // end namespace pr17001_call_wrong_destructor + +namespace pr19539_crash_on_destroying_an_integer { +struct A { + int i; + int j[2]; + A() : i(1) { + j[0] = 2; + j[1] = 3; + } + ~A() {} +}; + +void f() { + const int &x = A().i; // no-crash + const int &y = A().j[1]; // no-crash + const int &z = (A().j[1], A().j[0]); // no-crash + + // FIXME: All of these should be TRUE, but constructors aren't inlined. + clang_analyzer_eval(x == 1); // expected-warning{{UNKNOWN}} + clang_analyzer_eval(y == 3); // expected-warning{{UNKNOWN}} + clang_analyzer_eval(z == 2); // expected-warning{{UNKNOWN}} +} +} // end namespace pr19539_crash_on_destroying_an_integer