[analyzer] When binding to a ParenExpr, bind to its inner expression instead.

This actually looks through several kinds of expression, such as
OpaqueValueExpr and ExprWithCleanups. The idea is that binding and lookup
should be consistent, and so if the environment needs to be modified later,
the code doing the modification will not have to manually look through these
"transparent" expressions to find the real binding to change.

This is necessary for proper updating of struct rvalues as described in
the previous commit.

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

diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/Environment.h b/include/clang/StaticAnalyzer/Core/PathSensitive/Environment.h
index 54fce6530b735da783aa514f01d2ce4c323a0a7e..eb9bd85fe64d7253758e4b4bea6d5b2219222425 100644 (file)
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/Environment.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/Environment.h
@@ -33,10 +33,11 @@ class SValBuilder;
 /// other things.
 class EnvironmentEntry : public std::pair<const Stmt*,
                                           const StackFrameContext *> {
+  friend class EnvironmentManager;
+  EnvironmentEntry makeLocation() const;
+
 public:
-  EnvironmentEntry(const Stmt *s, const LocationContext *L)
-    : std::pair<const Stmt*,
-                const StackFrameContext*>(s, L ? L->getCurrentStackFrame():0) {}
+  EnvironmentEntry(const Stmt *s, const LocationContext *L);
 
   const Stmt *getStmt() const { return first; }
   const LocationContext *getLocationContext() const { return second; }

diff --git a/lib/StaticAnalyzer/Core/Environment.cpp b/lib/StaticAnalyzer/Core/Environment.cpp
index 07659cb601cf6a63adf9b3f316c1d096a68bb779..bab89c545c343f79ec93e8c6b359102e2fd086bd 100644 (file)
--- a/lib/StaticAnalyzer/Core/Environment.cpp
+++ b/lib/StaticAnalyzer/Core/Environment.cpp
@@ -20,6 +20,44 @@
 using namespace clang;
 using namespace ento;
 
+static const Expr *ignoreTransparentExprs(const Expr *E) {
+  E = E->IgnoreParens();
+
+  switch (E->getStmtClass()) {
+  case Stmt::OpaqueValueExprClass:
+    E = cast<OpaqueValueExpr>(E)->getSourceExpr();
+    break;
+  case Stmt::ExprWithCleanupsClass:
+    E = cast<ExprWithCleanups>(E)->getSubExpr();
+    break;
+  case Stmt::CXXBindTemporaryExprClass:
+    E = cast<CXXBindTemporaryExpr>(E)->getSubExpr();
+    break;
+  case Stmt::SubstNonTypeTemplateParmExprClass:
+    E = cast<SubstNonTypeTemplateParmExpr>(E)->getReplacement();
+    break;
+  case Stmt::CXXDefaultArgExprClass:
+    E = cast<CXXDefaultArgExpr>(E)->getExpr();
+    break;
+  default:
+    // This is the base case: we can't look through more than we already have.
+    return E;
+  }
+
+  return ignoreTransparentExprs(E);
+}
+
+static const Stmt *ignoreTransparentExprs(const Stmt *S) {
+  if (const Expr *E = dyn_cast<Expr>(S))
+    return ignoreTransparentExprs(E);
+  return S;
+}
+
+EnvironmentEntry::EnvironmentEntry(const Stmt *S, const LocationContext *L)
+  : std::pair<const Stmt *,
+              const StackFrameContext *>(ignoreTransparentExprs(S),
+                                         L ? L->getCurrentStackFrame() : 0) {}
+
 SVal Environment::lookupExpr(const EnvironmentEntry &E) const {
   const SVal* X = ExprBindings.lookup(E);
   if (X) {
@@ -31,93 +69,71 @@ SVal Environment::lookupExpr(const EnvironmentEntry &E) const {
 
 SVal Environment::getSVal(const EnvironmentEntry &Entry,
                           SValBuilder& svalBuilder) const {
-  const Stmt *E = Entry.getStmt();
+  const Stmt *S = Entry.getStmt();
   const LocationContext *LCtx = Entry.getLocationContext();
-  
-  for (;;) {
-    if (const Expr *Ex = dyn_cast<Expr>(E))
-      E = Ex->IgnoreParens();
-
-    switch (E->getStmtClass()) {
-      case Stmt::AddrLabelExprClass:
-        return svalBuilder.makeLoc(cast<AddrLabelExpr>(E));
-      case Stmt::OpaqueValueExprClass: {
-        const OpaqueValueExpr *ope = cast<OpaqueValueExpr>(E);
-        E = ope->getSourceExpr();
-        continue;        
-      }        
-      case Stmt::ParenExprClass:
-      case Stmt::GenericSelectionExprClass:
-        llvm_unreachable("ParenExprs and GenericSelectionExprs should "
-                         "have been handled by IgnoreParens()");
-      case Stmt::CharacterLiteralClass: {
-        const CharacterLiteral* C = cast<CharacterLiteral>(E);
-        return svalBuilder.makeIntVal(C->getValue(), C->getType());
-      }
-      case Stmt::CXXBoolLiteralExprClass: {
-        const SVal *X = ExprBindings.lookup(EnvironmentEntry(E, LCtx));
-        if (X) 
-          return *X;
-        else 
-          return svalBuilder.makeBoolVal(cast<CXXBoolLiteralExpr>(E));
-      }
-      case Stmt::CXXScalarValueInitExprClass:
-      case Stmt::ImplicitValueInitExprClass: {
-        QualType Ty = cast<Expr>(E)->getType();
-        return svalBuilder.makeZeroVal(Ty);
-      }
-      case Stmt::IntegerLiteralClass: {
-        // In C++, this expression may have been bound to a temporary object.
-        SVal const *X = ExprBindings.lookup(EnvironmentEntry(E, LCtx));
-        if (X)
-          return *X;
-        else
-          return svalBuilder.makeIntVal(cast<IntegerLiteral>(E));
-      }
-      case Stmt::ObjCBoolLiteralExprClass:
-        return svalBuilder.makeBoolVal(cast<ObjCBoolLiteralExpr>(E));
-
-      // For special C0xx nullptr case, make a null pointer SVal.
-      case Stmt::CXXNullPtrLiteralExprClass:
-        return svalBuilder.makeNull();
-      case Stmt::ExprWithCleanupsClass:
-        E = cast<ExprWithCleanups>(E)->getSubExpr();
-        continue;
-      case Stmt::CXXBindTemporaryExprClass:
-        E = cast<CXXBindTemporaryExpr>(E)->getSubExpr();
-        continue;
-      case Stmt::SubstNonTypeTemplateParmExprClass:
-        E = cast<SubstNonTypeTemplateParmExpr>(E)->getReplacement();
-        continue;
-      case Stmt::CXXDefaultArgExprClass:
-        E = cast<CXXDefaultArgExpr>(E)->getExpr();
-        continue;
-      case Stmt::ObjCStringLiteralClass: {
-        MemRegionManager &MRMgr = svalBuilder.getRegionManager();
-        const ObjCStringLiteral *SL = cast<ObjCStringLiteral>(E);
-        return svalBuilder.makeLoc(MRMgr.getObjCStringRegion(SL));
-      }
-      case Stmt::StringLiteralClass: {
-        MemRegionManager &MRMgr = svalBuilder.getRegionManager();
-        const StringLiteral *SL = cast<StringLiteral>(E);
-        return svalBuilder.makeLoc(MRMgr.getStringRegion(SL));
-      }
-      case Stmt::ReturnStmtClass: {
-        const ReturnStmt *RS = cast<ReturnStmt>(E);
-        if (const Expr *RE = RS->getRetValue()) {
-          E = RE;
-          continue;
-        }
-        return UndefinedVal();        
-      }
-        
-      // Handle all other Stmt* using a lookup.
-      default:
-        break;
-    };
+
+  switch (S->getStmtClass()) {
+  case Stmt::CXXBindTemporaryExprClass:
+  case Stmt::CXXDefaultArgExprClass:
+  case Stmt::ExprWithCleanupsClass:
+  case Stmt::GenericSelectionExprClass:
+  case Stmt::OpaqueValueExprClass:
+  case Stmt::ParenExprClass:
+  case Stmt::SubstNonTypeTemplateParmExprClass:
+    llvm_unreachable("Should have been handled by ignoreTransparentExprs");
+
+  case Stmt::AddrLabelExprClass:
+    return svalBuilder.makeLoc(cast<AddrLabelExpr>(S));
+
+  case Stmt::CharacterLiteralClass: {
+    const CharacterLiteral *C = cast<CharacterLiteral>(S);
+    return svalBuilder.makeIntVal(C->getValue(), C->getType());
+  }
+
+  case Stmt::CXXBoolLiteralExprClass:
+    return svalBuilder.makeBoolVal(cast<CXXBoolLiteralExpr>(S));
+
+  case Stmt::CXXScalarValueInitExprClass:
+  case Stmt::ImplicitValueInitExprClass: {
+    QualType Ty = cast<Expr>(S)->getType();
+    return svalBuilder.makeZeroVal(Ty);
+  }
+
+  case Stmt::IntegerLiteralClass:
+    return svalBuilder.makeIntVal(cast<IntegerLiteral>(S));
+
+  case Stmt::ObjCBoolLiteralExprClass:
+    return svalBuilder.makeBoolVal(cast<ObjCBoolLiteralExpr>(S));
+
+  // For special C0xx nullptr case, make a null pointer SVal.
+  case Stmt::CXXNullPtrLiteralExprClass:
+    return svalBuilder.makeNull();
+
+  case Stmt::ObjCStringLiteralClass: {
+    MemRegionManager &MRMgr = svalBuilder.getRegionManager();
+    const ObjCStringLiteral *SL = cast<ObjCStringLiteral>(S);
+    return svalBuilder.makeLoc(MRMgr.getObjCStringRegion(SL));
+  }
+
+  case Stmt::StringLiteralClass: {
+    MemRegionManager &MRMgr = svalBuilder.getRegionManager();
+    const StringLiteral *SL = cast<StringLiteral>(S);
+    return svalBuilder.makeLoc(MRMgr.getStringRegion(SL));
+  }
+
+  case Stmt::ReturnStmtClass: {
+    const ReturnStmt *RS = cast<ReturnStmt>(S);
+    if (const Expr *RE = RS->getRetValue())
+      return getSVal(EnvironmentEntry(RE, LCtx), svalBuilder);
+    return UndefinedVal();        
+  }
+    
+  // Handle all other Stmt* using a lookup.
+  default:
     break;
   }
-  return lookupExpr(EnvironmentEntry(E, LCtx));
+  
+  return lookupExpr(EnvironmentEntry(S, LCtx));
 }
 
 Environment EnvironmentManager::bindExpr(Environment Env,
@@ -133,16 +149,16 @@ Environment EnvironmentManager::bindExpr(Environment Env,
   return Environment(F.add(Env.ExprBindings, E, V));
 }
 
-static inline EnvironmentEntry MakeLocation(const EnvironmentEntry &E) {
-  const Stmt *S = E.getStmt();
-  S = (const Stmt*) (((uintptr_t) S) | 0x1);
-  return EnvironmentEntry(S, E.getLocationContext());
+EnvironmentEntry EnvironmentEntry::makeLocation() const {
+  EnvironmentEntry Result = *this;
+  reinterpret_cast<uintptr_t &>(Result.first) |= 0x1;
+  return Result;
 }
 
 Environment EnvironmentManager::bindExprAndLocation(Environment Env,
                                                     const EnvironmentEntry &E,
                                                     SVal location, SVal V) {
-  return Environment(F.add(F.add(Env.ExprBindings, MakeLocation(E), location),
+  return Environment(F.add(F.add(Env.ExprBindings, E.makeLocation(), location),
                            E, V));
 }
 

diff --git a/test/Analysis/array-struct-region.cpp b/test/Analysis/array-struct-region.cpp
index cffa64d21ab5d834c2359800172e3776112e19c3..12ae5d3eba6ec26168463ddcc9a95bb1349cd1f7 100644 (file)
--- a/test/Analysis/array-struct-region.cpp
+++ b/test/Analysis/array-struct-region.cpp
@@ -90,6 +90,22 @@ void testArgument() {
   clang_analyzer_eval(getAssignedField(getS()) == 42); // expected-warning{{TRUE}}
 }
 
+void testImmediateUseParens() {
+  int x = ((getS())).field;
+
+  if (x != 42) return;
+  clang_analyzer_eval(x == 42); // expected-warning{{TRUE}}
+
+  clang_analyzer_eval(getConstrainedField(((getS()))) == 42); // expected-warning{{TRUE}}
+  clang_analyzer_eval(getAssignedField(((getS()))) == 42); // expected-warning{{TRUE}}
+
+#if __cplusplus
+  clang_analyzer_eval(((getS())).check()); // expected-warning{{TRUE}}
+  clang_analyzer_eval(!((getS()))); // expected-warning{{FALSE}}
+  clang_analyzer_eval(~((getS()))); // expected-warning{{FALSE}}
+#endif
+}
+
 
 //--------------------
 // C++-only tests