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

Jordan Rose jordan_rose at apple.com
Thu Feb 21 17:51:16 PST 2013


Author: jrose
Date: Thu Feb 21 19:51:15 2013
New Revision: 175854

URL: http://llvm.org/viewvc/llvm-project?rev=175854&view=rev
Log:
[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)

Modified:
    cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
    cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
    cfe/trunk/test/Analysis/stack-addr-ps.cpp
    cfe/trunk/test/Analysis/temporaries.cpp

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h?rev=175854&r1=175853&r2=175854&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h Thu Feb 21 19:51:15 2013
@@ -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.

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp?rev=175854&r1=175853&r2=175854&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp Thu Feb 21 19:51:15 2013
@@ -165,47 +165,46 @@ ProgramStateRef ExprEngine::getInitialSt
   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();
-    }
-
-    // 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);
-
-    // 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);
-    }
+  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;
 
-    State = State->BindExpr(Ex, LC, Reg);
+    Inner = CE->getSubExpr()->IgnoreParens();
   }
 
+  // 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);
+
+  // 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;
 }
 

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp?rev=175854&r1=175853&r2=175854&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp Thu Feb 21 19:51:15 2013
@@ -30,23 +30,17 @@ void ExprEngine::CreateCXXTemporaryObjec
   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,

Modified: cfe/trunk/test/Analysis/stack-addr-ps.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/stack-addr-ps.cpp?rev=175854&r1=175853&r2=175854&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/stack-addr-ps.cpp (original)
+++ cfe/trunk/test/Analysis/stack-addr-ps.cpp Thu Feb 21 19:51:15 2013
@@ -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 {

Modified: cfe/trunk/test/Analysis/temporaries.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/temporaries.cpp?rev=175854&r1=175853&r2=175854&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/temporaries.cpp (original)
+++ cfe/trunk/test/Analysis/temporaries.cpp Thu Feb 21 19:51:15 2013
@@ -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}}
+  }
 }
 





More information about the cfe-commits mailing list