[analyzer] Make sure a materialized temporary matches its bindings.

This is a follow-up to r175830, which made sure a temporary object region
created for, say, a struct rvalue matched up with the initial bindings
being stored into it. This does the same for the case in which the AST
actually tells us that we need to create a temporary via a
MaterializeObjectExpr. I've unified the two code paths and moved a static
helper function onto ExprEngine.

This also caused a bit of test churn, causing us to go back to describing
temporary regions without a 'const' qualifier. This seems acceptable; it's
our behavior from a few months ago.

<rdar://problem/13265460> (part 2)

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@175854 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h b/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
index 959c1dc6aa54a6fb1904b7f98ad78dafcc952947..6910c299cc2e400c279c6a8f6d0147194ac99468 100644 (file)
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -552,6 +552,17 @@ private:
   /// Models a trivial copy or move constructor call with a simple bind.
   void performTrivialCopy(NodeBuilder &Bldr, ExplodedNode *Pred,
                           const CXXConstructorCall &Call);
+
+  /// If the value of the given expression is a NonLoc, copy it into a new
+  /// temporary object region, and replace the value of the expression with
+  /// that.
+  ///
+  /// If \p ResultE is provided, the new region will be bound to this expression
+  /// instead of \p E.
+  ProgramStateRef createTemporaryRegionIfNeeded(ProgramStateRef State,
+                                                const LocationContext *LC,
+                                                const Expr *E,
+                                                const Expr *ResultE = 0);
 };
 
 /// Traits for storing the call processing policy inside GDM.

diff --git a/lib/StaticAnalyzer/Core/ExprEngine.cpp b/lib/StaticAnalyzer/Core/ExprEngine.cpp
index f15a02be2b029e75410e3107e8221e8cafebb52f..5d5b9b70cb297e1680b2c35297dd696414cfb67e 100644 (file)
--- a/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -165,47 +165,46 @@ ProgramStateRef ExprEngine::getInitialState(const LocationContext *InitLoc) {
   return state;
 }
 
-/// If the value of the given expression is a NonLoc, copy it into a new
-/// temporary region, and replace the value of the expression with that.
-static ProgramStateRef createTemporaryRegionIfNeeded(ProgramStateRef State,
-                                                     const LocationContext *LC,
-                                                     const Expr *Ex) {
+ProgramStateRef
+ExprEngine::createTemporaryRegionIfNeeded(ProgramStateRef State,
+                                          const LocationContext *LC,
+                                          const Expr *Ex,
+                                          const Expr *Result) {
   SVal V = State->getSVal(Ex, LC);
+  if (!Result && !V.getAs<NonLoc>())
+    return State;
 
-  if (V.getAs<NonLoc>()) {
-    ProgramStateManager &StateMgr = State->getStateManager();
-    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. Start by stripping and recording base casts.
-    SmallVector<const CastExpr *, 4> Casts;
-    const Expr *Inner = Ex->IgnoreParens();
-    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;
-
-      Inner = CE->getSubExpr()->IgnoreParens();
-    }
+  ProgramStateManager &StateMgr = State->getStateManager();
+  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. Start by stripping and recording base casts.
+  SmallVector<const CastExpr *, 4> Casts;
+  const Expr *Inner = Ex->IgnoreParens();
+  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;
 
-    // Create a temporary object region for the inner expression (which may have
-    // a more derived type) and bind the NonLoc value into it.
-    SVal Reg = loc::MemRegionVal(MRMgr.getCXXTempObjectRegion(Inner, LC));
-    State = State->bindLoc(Reg, V);
+    Inner = CE->getSubExpr()->IgnoreParens();
+  }
 
-    // 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);
-    }
+  // Create a temporary object region for the inner expression (which may have
+  // a more derived type) and bind the NonLoc value into it.
+  SVal Reg = loc::MemRegionVal(MRMgr.getCXXTempObjectRegion(Inner, LC));
+  State = State->bindLoc(Reg, V);
 
-    State = State->BindExpr(Ex, LC, Reg);
+  // 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 ? Result : Ex, LC, Reg);
   return State;
 }
 

diff --git a/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
index 43184cfd8218e9cc5bf0d4796d4e426c15e25f66..5d04340a9713a9fca399f1c362a7f9f31d8b260d 100644 (file)
--- a/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -30,23 +30,17 @@ void ExprEngine::CreateCXXTemporaryObject(const MaterializeTemporaryExpr *ME,
   ProgramStateRef state = Pred->getState();
   const LocationContext *LCtx = Pred->getLocationContext();
 
-  // Bind the temporary object to the value of the expression. Then bind
-  // the expression to the location of the object.
   SVal V = state->getSVal(tempExpr, LCtx);
 
   // If the value is already a CXXTempObjectRegion, it is fine as it is.
   // Otherwise, create a new CXXTempObjectRegion, and copy the value into it.
   const MemRegion *MR = V.getAsRegion();
-  if (!MR || !isa<CXXTempObjectRegion>(MR)) {
-    const MemRegion *R =
-      svalBuilder.getRegionManager().getCXXTempObjectRegion(ME, LCtx);
+  if (MR && isa<CXXTempObjectRegion>(MR))
+    state = state->BindExpr(ME, LCtx, V);
+  else
+    state = createTemporaryRegionIfNeeded(state, LCtx, tempExpr, ME);
 
-    SVal L = loc::MemRegionVal(R);
-    state = state->bindLoc(L, V);
-    V = L;
-  }
-
-  Bldr.generateNode(ME, Pred, state->BindExpr(ME, LCtx, V));
+  Bldr.generateNode(ME, Pred, state);
 }
 
 void ExprEngine::performTrivialCopy(NodeBuilder &Bldr, ExplodedNode *Pred,

diff --git a/test/Analysis/stack-addr-ps.cpp b/test/Analysis/stack-addr-ps.cpp
index 9dd63f83c674d0d9b5b7b4ce498961eff8600406..7aefea5095c78640ac5d3966e2d5bf443dc832f4 100644 (file)
--- a/test/Analysis/stack-addr-ps.cpp
+++ b/test/Analysis/stack-addr-ps.cpp
@@ -22,17 +22,17 @@ const int& g3() {
 
 int get_value();
 
-const int &get_reference1() { return get_value(); } // expected-warning{{Address of stack memory associated with temporary object of type 'const int' returned}} expected-warning {{returning reference to local temporary}}
+const int &get_reference1() { return get_value(); } // expected-warning{{Address of stack memory associated with temporary object of type 'int' returned}} expected-warning {{returning reference to local temporary}}
 
 const int &get_reference2() {
   const int &x = get_value(); // expected-note {{binding reference variable 'x' here}}
-  return x; // expected-warning{{Address of stack memory associated with temporary object of type 'const int' returned}} expected-warning {{returning reference to local temporary}}
+  return x; // expected-warning{{Address of stack memory associated with temporary object of type 'int' returned}} expected-warning {{returning reference to local temporary}}
 }
 
 const int &get_reference3() {
   const int &x1 = get_value(); // expected-note {{binding reference variable 'x1' here}}
   const int &x2 = x1; // expected-note {{binding reference variable 'x2' here}}
-  return x2; // expected-warning{{Address of stack memory associated with temporary object of type 'const int' returned}} expected-warning {{returning reference to local temporary}}
+  return x2; // expected-warning{{Address of stack memory associated with temporary object of type 'int' returned}} expected-warning {{returning reference to local temporary}}
 }
 
 int global_var;
@@ -56,7 +56,7 @@ int *f3() {
 const int *f4() {
   const int &x1 = get_value(); // expected-note {{binding reference variable 'x1' here}}
   const int &x2 = x1; // expected-note {{binding reference variable 'x2' here}}
-  return &x2; // expected-warning{{Address of stack memory associated with temporary object of type 'const int' returned}} expected-warning {{returning address of local temporary}}
+  return &x2; // expected-warning{{Address of stack memory associated with temporary object of type 'int' returned}} expected-warning {{returning address of local temporary}}
 }
 
 struct S {

diff --git a/test/Analysis/temporaries.cpp b/test/Analysis/temporaries.cpp
index 8d0dd6e04c9c2a33ba8d305b9dbdce82f9dbebeb..884a7e14e1688d852fd45358769eea0cafb87b57 100644 (file)
--- a/test/Analysis/temporaries.cpp
+++ b/test/Analysis/temporaries.cpp
@@ -18,7 +18,7 @@ Trivial getTrivial() {
 }
 
 const Trivial &getTrivialRef() {
-  return Trivial(42); // expected-warning {{Address of stack memory associated with temporary object of type 'const struct Trivial' returned to caller}}
+  return Trivial(42); // expected-warning {{Address of stack memory associated with temporary object of type 'struct Trivial' returned to caller}}
 }
 
 
@@ -27,7 +27,7 @@ NonTrivial getNonTrivial() {
 }
 
 const NonTrivial &getNonTrivialRef() {
-  return NonTrivial(42); // expected-warning {{Address of stack memory associated with temporary object of type 'const struct NonTrivial' returned to caller}}
+  return NonTrivial(42); // expected-warning {{Address of stack memory associated with temporary object of type 'struct NonTrivial' returned to caller}}
 }
 
 namespace rdar13265460 {
@@ -43,7 +43,7 @@ namespace rdar13265460 {
     return obj;
   }
 
-  void test() {
+  void testImmediate() {
     TrivialSubclass obj = getTrivialSub();
 
     clang_analyzer_eval(obj.value == 42); // expected-warning{{TRUE}}
@@ -52,5 +52,13 @@ namespace rdar13265460 {
     clang_analyzer_eval(getTrivialSub().value == 42); // expected-warning{{TRUE}}
     clang_analyzer_eval(getTrivialSub().anotherValue == -42); // expected-warning{{TRUE}}
   }
+
+  void testMaterializeTemporaryExpr() {
+    const TrivialSubclass &ref = getTrivialSub();
+    clang_analyzer_eval(ref.value == 42); // expected-warning{{TRUE}}
+
+    const Trivial &baseRef = getTrivialSub();
+    clang_analyzer_eval(baseRef.value == 42); // expected-warning{{TRUE}}
+  }
 }