r288263 - [analyzer] Construct temporary objects of correct types, destroy them properly.
Artem Dergachev via cfe-commits
cfe-commits at lists.llvm.org
Wed Nov 30 11:02:44 PST 2016
Author: dergachev
Date: Wed Nov 30 13:02:44 2016
New Revision: 288263
URL: http://llvm.org/viewvc/llvm-project?rev=288263&view=rev
Log:
[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
Added:
cfe/trunk/test/Analysis/lifetime-extension.cpp
Modified:
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp?rev=288263&r1=288262&r2=288263&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp Wed Nov 30 13:02:44 2016
@@ -202,25 +202,32 @@ ExprEngine::createTemporaryRegionIfNeede
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<const CastExpr *, 4> Casts;
- const Expr *Inner = Ex->IgnoreParens();
- if (!Loc::isLocType(Result->getType())) {
- while (const CastExpr *CE = dyn_cast<CastExpr>(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<const Expr *, 2> CommaLHSs;
+ SmallVector<SubobjectAdjustment, 2> 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<MaterializeTemporaryExpr>(Result)) {
@@ -228,25 +235,37 @@ ExprEngine::createTemporaryRegionIfNeede
// 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<const CastExpr *>::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
SVal dest = state->getLValue(varDecl, Pred->getLocationContext());
const MemRegion *Region = dest.castAs<loc::MemRegionVal>().getRegion();
- if (const ReferenceType *refType = varType->getAs<ReferenceType>()) {
- varType = refType->getPointeeType();
- Region = state->getSVal(Region).getAsRegion();
+ if (varType->isReferenceType()) {
+ Region = state->getSVal(Region).getAsRegion()->getBaseRegion();
+ varType = cast<TypedValueRegion>(Region)->getValueType();
}
VisitCXXDestructor(varType, Region, Dtor.getTriggerStmt(), /*IsBase=*/ false,
Added: cfe/trunk/test/Analysis/lifetime-extension.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/lifetime-extension.cpp?rev=288263&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/lifetime-extension.cpp (added)
+++ cfe/trunk/test/Analysis/lifetime-extension.cpp Wed Nov 30 13:02:44 2016
@@ -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
More information about the cfe-commits
mailing list