[analyzer] Create a temporary region when accessing a struct rvalue.

In C++, rvalues that need to have their address taken (for example, to be
passed to a function by const reference) will be wrapped in a
MaterializeTemporaryExpr, which lets CodeGen know to create a temporary
region to store this value. However, MaterializeTemporaryExprs are /not/
created when a method is called on an rvalue struct, even though the 'this'
pointer needs a valid value. CodeGen works around this by creating a
temporary region anyway; now, so does the analyzer.

The analyzer also does this when accessing a field of a struct rvalue.
This is a little unfortunate, since the rest of the struct will soon be
thrown away, but it does make things consistent with the rest of the
analyzer.

This allows us to bring back the assumption that all known 'this' values
are Locs. This is a revised version of r164828-9, reverted in r164876-7.

<rdar://problem/12137950>

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

diff --git a/lib/StaticAnalyzer/Core/CallEvent.cpp b/lib/StaticAnalyzer/Core/CallEvent.cpp
index 3b4c13471b10bcf22af08ccb0593e0944e8222c8..31e4cf05d992fbdb55d0385ac9732cd9c4f3c04e 100644 (file)
--- a/lib/StaticAnalyzer/Core/CallEvent.cpp
+++ b/lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -401,15 +401,7 @@ SVal CXXInstanceCall::getCXXThisVal() const {
     return UnknownVal();
 
   SVal ThisVal = getSVal(Base);
-
-  // FIXME: This is only necessary because we can call member functions on
-  // struct rvalues, which do not have regions we can use for a 'this' pointer.
-  // Ideally this should eventually be changed to an assert, i.e. all
-  // non-Unknown, non-null 'this' values should be loc::MemRegionVals.
-  if (isa<DefinedSVal>(ThisVal))
-    if (!ThisVal.getAsRegion() && !ThisVal.isConstant())
-      return UnknownVal();
-
+  assert(ThisVal.isUnknownOrUndef() || isa<Loc>(ThisVal));
   return ThisVal;
 }
 

diff --git a/lib/StaticAnalyzer/Core/ExprEngine.cpp b/lib/StaticAnalyzer/Core/ExprEngine.cpp
index 007bcf52085e7ab69802a0d3dbb941325e39cc3d..213ab7ad5cc869cdcc40f7b18b4a4887029df31f 100644 (file)
--- a/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -163,6 +163,23 @@ 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 *E) {
+  SVal V = State->getSVal(E, LC);
+
+  if (isa<NonLoc>(V)) {
+    MemRegionManager &MRMgr = State->getStateManager().getRegionManager();
+    const MemRegion *R  = MRMgr.getCXXTempObjectRegion(E, LC);
+    State = State->bindLoc(loc::MemRegionVal(R), V);
+    State = State->BindExpr(E, LC, loc::MemRegionVal(R));
+  }
+
+  return State;
+}
+
 //===----------------------------------------------------------------------===//
 // Top-level transfer function logic (Dispatcher).
 //===----------------------------------------------------------------------===//
@@ -717,8 +734,26 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
       break;
     }
 
+    case Stmt::CXXOperatorCallExprClass: {
+      const CXXOperatorCallExpr *OCE = cast<CXXOperatorCallExpr>(S);
+
+      // For instance method operators, make sure the 'this' argument has a
+      // valid region.
+      const Decl *Callee = OCE->getCalleeDecl();
+      if (const CXXMethodDecl *MD = dyn_cast_or_null<CXXMethodDecl>(Callee)) {
+        if (MD->isInstance()) {
+          ProgramStateRef State = Pred->getState();
+          const LocationContext *LCtx = Pred->getLocationContext();
+          ProgramStateRef NewState =
+            createTemporaryRegionIfNeeded(State, LCtx, OCE->getArg(0));
+          if (NewState != State)
+            Pred = Bldr.generateNode(OCE, Pred, NewState, /*Tag=*/0,
+                                     ProgramPoint::PreStmtKind);
+        }
+      }
+      // FALLTHROUGH
+    }
     case Stmt::CallExprClass:
-    case Stmt::CXXOperatorCallExprClass:
     case Stmt::CXXMemberCallExprClass:
     case Stmt::UserDefinedLiteralClass: {
       Bldr.takeNodes(Pred);
@@ -1478,6 +1513,7 @@ void ExprEngine::VisitMemberExpr(const MemberExpr *M, ExplodedNode *Pred,
   ExplodedNodeSet Dst;
   Decl *member = M->getMemberDecl();
 
+  // Handle static member variables accessed via member syntax.
   if (VarDecl *VD = dyn_cast<VarDecl>(member)) {
     assert(M->isGLValue());
     Bldr.takeNodes(Pred);
@@ -1486,40 +1522,27 @@ void ExprEngine::VisitMemberExpr(const MemberExpr *M, ExplodedNode *Pred,
     return;
   }
 
+  ProgramStateRef state = Pred->getState();
+  const LocationContext *LCtx = Pred->getLocationContext();
+  Expr *BaseExpr = M->getBase();
+
   // Handle C++ method calls.
   if (const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(member)) {
-    Bldr.takeNodes(Pred);
-    SVal MDVal = svalBuilder.getFunctionPointer(MD);
-    ProgramStateRef state =
-      Pred->getState()->BindExpr(M, Pred->getLocationContext(), MDVal);
-    Bldr.generateNode(M, Pred, state);
-    return;
-  }
-
+    if (MD->isInstance())
+      state = createTemporaryRegionIfNeeded(state, LCtx, BaseExpr);
 
-  FieldDecl *field = dyn_cast<FieldDecl>(member);
-  if (!field) // FIXME: skipping member expressions for non-fields
-    return;
+    SVal MDVal = svalBuilder.getFunctionPointer(MD);
+    state = state->BindExpr(M, LCtx, MDVal);
 
-  Expr *baseExpr = M->getBase()->IgnoreParens();
-  ProgramStateRef state = Pred->getState();
-  const LocationContext *LCtx = Pred->getLocationContext();
-  SVal baseExprVal = state->getSVal(baseExpr, Pred->getLocationContext());
-  if (isa<nonloc::LazyCompoundVal>(baseExprVal) ||
-      isa<nonloc::CompoundVal>(baseExprVal) ||
-      // FIXME: This can originate by conjuring a symbol for an unknown
-      // temporary struct object, see test/Analysis/fields.c:
-      // (p = getit()).x
-      isa<nonloc::SymbolVal>(baseExprVal)) {
-    Bldr.generateNode(M, Pred, state->BindExpr(M, LCtx, UnknownVal()));
+    Bldr.generateNode(M, Pred, state);
     return;
   }
 
-  // FIXME: Should we insert some assumption logic in here to determine
-  // if "Base" is a valid piece of memory?  Before we put this assumption
-  // later when using FieldOffset lvals (which we no longer have).
+  // Handle regular struct fields / member variables.
+  state = createTemporaryRegionIfNeeded(state, LCtx, BaseExpr);
+  SVal baseExprVal = state->getSVal(BaseExpr, LCtx);
 
-  // For all other cases, compute an lvalue.    
+  FieldDecl *field = cast<FieldDecl>(member);
   SVal L = state->getLValue(field, baseExprVal);
   if (M->isGLValue()) {
     if (field->getType()->isReferenceType()) {

diff --git a/test/Analysis/array-struct-region.cpp b/test/Analysis/array-struct-region.cpp
index 3581566bdccff84d72a78390bcc63f99b7aa1dc2..cffa64d21ab5d834c2359800172e3776112e19c3 100644 (file)
--- a/test/Analysis/array-struct-region.cpp
+++ b/test/Analysis/array-struct-region.cpp
@@ -1,5 +1,7 @@
 // RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.core,debug.ExprInspection -verify -x c %s
 // RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.core,debug.ExprInspection -verify -x c++ -analyzer-config c++-inlining=constructors %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.core,debug.ExprInspection -DINLINE -verify -x c %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.core,debug.ExprInspection -DINLINE -verify -x c++ -analyzer-config c++-inlining=constructors %s
 
 void clang_analyzer_eval(int);
 
@@ -8,10 +10,29 @@ struct S {
 
 #if __cplusplus
   const struct S *getThis() const { return this; }
+  const struct S *operator +() const { return this; }
+
+  bool check() const { return this == this; }
+  bool operator !() const { return this != this; }
+
+  int operator *() const { return field; }
 #endif
 };
 
+#if __cplusplus
+const struct S *operator -(const struct S &s) { return &s; }
+bool operator ~(const struct S &s) { return &s != &s; }
+#endif
+
+
+#ifdef INLINE
+struct S getS() {
+  struct S s = { 42 };
+  return s;
+}
+#else
 struct S getS();
+#endif
 
 
 void testAssignment() {
@@ -25,6 +46,14 @@ void testAssignment() {
 
 #if __cplusplus
   clang_analyzer_eval(s.getThis() == &s); // expected-warning{{TRUE}}
+  clang_analyzer_eval(+s == &s); // expected-warning{{TRUE}}
+  clang_analyzer_eval(-s == &s); // expected-warning{{TRUE}}
+
+  clang_analyzer_eval(s.check()); // expected-warning{{TRUE}}
+  clang_analyzer_eval(!s); // expected-warning{{FALSE}}
+  clang_analyzer_eval(~s); // expected-warning{{FALSE}}
+
+  clang_analyzer_eval(*s == 0); // expected-warning{{TRUE}}
 #endif
 }
 
@@ -37,6 +66,12 @@ void testImmediateUse() {
 
 #if __cplusplus
   clang_analyzer_eval((void *)getS().getThis() == (void *)&x); // expected-warning{{FALSE}}
+  clang_analyzer_eval((void *)+getS() == (void *)&x); // expected-warning{{FALSE}}
+  clang_analyzer_eval((void *)-getS() == (void *)&x); // expected-warning{{FALSE}}
+
+  clang_analyzer_eval(getS().check()); // expected-warning{{TRUE}}
+  clang_analyzer_eval(!getS()); // expected-warning{{FALSE}}
+  clang_analyzer_eval(~getS()); // expected-warning{{FALSE}}
 #endif
 }
 
@@ -68,6 +103,13 @@ void testReferenceAssignment() {
   clang_analyzer_eval(s.field == 42); // expected-warning{{TRUE}}
 
   clang_analyzer_eval(s.getThis() == &s); // expected-warning{{TRUE}}
+  clang_analyzer_eval(+s == &s); // expected-warning{{TRUE}}
+
+  clang_analyzer_eval(s.check()); // expected-warning{{TRUE}}
+  clang_analyzer_eval(!s); // expected-warning{{FALSE}}
+  clang_analyzer_eval(~s); // expected-warning{{FALSE}}
+
+  clang_analyzer_eval(*s == 42); // expected-warning{{TRUE}}
 }
 
 
@@ -80,8 +122,39 @@ bool checkThis(const S &s) {
   return s.getThis() == &s;
 }
 
+bool checkThisOp(const S &s) {
+  return +s == &s;
+}
+
+bool checkThisStaticOp(const S &s) {
+  return -s == &s;
+}
+
 void testReferenceArgument() {
   clang_analyzer_eval(getConstrainedFieldRef(getS()) == 42); // expected-warning{{TRUE}}
   clang_analyzer_eval(checkThis(getS())); // expected-warning{{TRUE}}
+  clang_analyzer_eval(checkThisOp(getS())); // expected-warning{{TRUE}}
+  clang_analyzer_eval(checkThisStaticOp(getS())); // expected-warning{{TRUE}}
+}
+
+
+int getConstrainedFieldOp(S s) {
+  if (*s != 42) return 42;
+  return *s;
+}
+
+int getConstrainedFieldRefOp(const S &s) {
+  if (*s != 42) return 42;
+  return *s;
 }
+
+void testImmediateUseOp() {
+  int x = *getS();
+  if (x != 42) return;
+  clang_analyzer_eval(x == 42); // expected-warning{{TRUE}}
+
+  clang_analyzer_eval(getConstrainedFieldOp(getS()) == 42); // expected-warning{{TRUE}}
+  clang_analyzer_eval(getConstrainedFieldRefOp(getS()) == 42); // expected-warning{{TRUE}}
+}
+
 #endif

diff --git a/test/Analysis/fields.c b/test/Analysis/fields.c
index da0847a5607464a53f5c6bfd54edc959e6317eb1..12e8bbf3671c1e9639fb49056f22b0aff91d9130 100644 (file)
--- a/test/Analysis/fields.c
+++ b/test/Analysis/fields.c
@@ -1,4 +1,6 @@
-// RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.core %s -analyzer-store=region -verify
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,alpha.core,debug.ExprInspection %s -analyzer-store=region -verify
+
+void clang_analyzer_eval(int);
 
 unsigned foo();
 typedef struct bf { unsigned x:2; } bf;
@@ -26,3 +28,11 @@ void test() {
   Point p;
   (void)(p = getit()).x;
 }
+
+
+void testLazyCompoundVal() {
+  Point p = {42, 0};
+  Point q;
+  clang_analyzer_eval((q = p).x == 42); // expected-warning{{TRUE}}
+  clang_analyzer_eval(q.x == 42); // expected-warning{{TRUE}}
+}

diff --git a/test/Analysis/reference.cpp b/test/Analysis/reference.cpp
index 374f3f7261c76f47fe716e209a91c60872931043..ce0ee8ed57d07139335319c299e0f927e8cec154 100644 (file)
--- a/test/Analysis/reference.cpp
+++ b/test/Analysis/reference.cpp
@@ -116,10 +116,8 @@ void testReferenceAddress(int &x) {
 
   struct S { int &x; };
 
-  // FIXME: Should be TRUE. Fields of return-by-value structs are not yet
-  // symbolicated. Tracked by <rdar://problem/12137950>.
   extern S getS();
-  clang_analyzer_eval(&getS().x != 0); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(&getS().x != 0); // expected-warning{{TRUE}}
 
   extern S *getSP();
   clang_analyzer_eval(&getSP()->x != 0); // expected-warning{{TRUE}}