r319834 - [analyzer] do not crash on cases where an array subscript is an rvalue

George Karpenkov via cfe-commits cfe-commits at lists.llvm.org
Tue Dec 5 13:19:59 PST 2017


Author: george.karpenkov
Date: Tue Dec  5 13:19:59 2017
New Revision: 319834

URL: http://llvm.org/viewvc/llvm-project?rev=319834&view=rev
Log:
[analyzer] do not crash on cases where an array subscript is an rvalue

Array subscript is almost always an lvalue, except for a few cases where
it is not, such as a subscript into an Objective-C property, or a
return from the function.
This commit prevents crashing in such cases.

Fixes rdar://34829842

Differential Revision: https://reviews.llvm.org/D40584

Added:
    cfe/trunk/test/Analysis/vector.m
      - copied, changed from r319697, cfe/trunk/test/Analysis/vector.c
Removed:
    cfe/trunk/test/Analysis/vector.c
Modified:
    cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
    cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.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=319834&r1=319833&r2=319834&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h Tue Dec  5 13:19:59 2017
@@ -332,9 +332,9 @@ public:
   void Visit(const Stmt *S, ExplodedNode *Pred, ExplodedNodeSet &Dst);
 
   /// VisitArraySubscriptExpr - Transfer function for array accesses.
-  void VisitLvalArraySubscriptExpr(const ArraySubscriptExpr *Ex,
-                                   ExplodedNode *Pred,
-                                   ExplodedNodeSet &Dst);
+  void VisitArraySubscriptExpr(const ArraySubscriptExpr *Ex,
+                               ExplodedNode *Pred,
+                               ExplodedNodeSet &Dst);
 
   /// VisitGCCAsmStmt - Transfer function logic for inline asm.
   void VisitGCCAsmStmt(const GCCAsmStmt *A, ExplodedNode *Pred,

Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp?rev=319834&r1=319833&r2=319834&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp Tue Dec  5 13:19:59 2017
@@ -1150,7 +1150,7 @@ void ExprEngine::Visit(const Stmt *S, Ex
 
     case Stmt::ArraySubscriptExprClass:
       Bldr.takeNodes(Pred);
-      VisitLvalArraySubscriptExpr(cast<ArraySubscriptExpr>(S), Pred, Dst);
+      VisitArraySubscriptExpr(cast<ArraySubscriptExpr>(S), Pred, Dst);
       Bldr.addNodes(Dst);
       break;
 
@@ -2126,10 +2126,9 @@ void ExprEngine::VisitCommonDeclRefExpr(
 }
 
 /// VisitArraySubscriptExpr - Transfer function for array accesses
-void ExprEngine::VisitLvalArraySubscriptExpr(const ArraySubscriptExpr *A,
+void ExprEngine::VisitArraySubscriptExpr(const ArraySubscriptExpr *A,
                                              ExplodedNode *Pred,
                                              ExplodedNodeSet &Dst){
-
   const Expr *Base = A->getBase()->IgnoreParens();
   const Expr *Idx  = A->getIdx()->IgnoreParens();
 
@@ -2138,18 +2137,32 @@ void ExprEngine::VisitLvalArraySubscript
 
   ExplodedNodeSet EvalSet;
   StmtNodeBuilder Bldr(CheckerPreStmt, EvalSet, *currBldrCtx);
-  assert(A->isGLValue() ||
-          (!AMgr.getLangOpts().CPlusPlus &&
-           A->getType().isCForbiddenLValueType()));
+
+  bool IsVectorType = A->getBase()->getType()->isVectorType();
+
+  // The "like" case is for situations where C standard prohibits the type to
+  // be an lvalue, e.g. taking the address of a subscript of an expression of
+  // type "void *".
+  bool IsGLValueLike = A->isGLValue() ||
+    (A->getType().isCForbiddenLValueType() && !AMgr.getLangOpts().CPlusPlus);
 
   for (auto *Node : CheckerPreStmt) {
     const LocationContext *LCtx = Node->getLocationContext();
     ProgramStateRef state = Node->getState();
-    SVal V = state->getLValue(A->getType(),
-                              state->getSVal(Idx, LCtx),
-                              state->getSVal(Base, LCtx));
-    Bldr.generateNode(A, Node, state->BindExpr(A, LCtx, V), nullptr,
-                      ProgramPoint::PostLValueKind);
+
+    if (IsGLValueLike) {
+      SVal V = state->getLValue(A->getType(),
+          state->getSVal(Idx, LCtx),
+          state->getSVal(Base, LCtx));
+      Bldr.generateNode(A, Node, state->BindExpr(A, LCtx, V), nullptr,
+          ProgramPoint::PostLValueKind);
+    } else if (IsVectorType) {
+      // FIXME: non-glvalue vector reads are not modelled.
+      Bldr.generateNode(A, Node, state, nullptr);
+    } else {
+      llvm_unreachable("Array subscript should be an lValue when not \
+a vector and not a forbidden lvalue type");
+    }
   }
 
   getCheckerManager().runCheckersForPostStmt(Dst, EvalSet, A, *this);

Removed: cfe/trunk/test/Analysis/vector.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/vector.c?rev=319833&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/vector.c (original)
+++ cfe/trunk/test/Analysis/vector.c (removed)
@@ -1,28 +0,0 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify %s
-
-typedef int __attribute__((ext_vector_type(2))) V;
-
-void clang_analyzer_numTimesReached();
-void clang_analyzer_eval(int);
-
-int flag;
-
-V pass_through_and_set_flag(V v) {
-  flag = 1;
-  return v;
-}
-
-V dont_crash_and_dont_split_state(V x, V y) {
-  flag = 0;
-  V z = x && pass_through_and_set_flag(y);
-  clang_analyzer_eval(flag); // expected-warning{{TRUE}}
-  // FIXME: For now we treat vector operator && as short-circuit,
-  // but in fact it is not. It should always evaluate
-  // pass_through_and_set_flag(). It should not split state.
-  // Now we also get FALSE on the other path.
-  // expected-warning at -5{{FALSE}}
-
-  // FIXME: Should be 1 since we should not split state.
-  clang_analyzer_numTimesReached(); // expected-warning{{2}}
-  return z;
-}

Copied: cfe/trunk/test/Analysis/vector.m (from r319697, cfe/trunk/test/Analysis/vector.c)
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/vector.m?p2=cfe/trunk/test/Analysis/vector.m&p1=cfe/trunk/test/Analysis/vector.c&r1=319697&r2=319834&rev=319834&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/vector.c (original)
+++ cfe/trunk/test/Analysis/vector.m Tue Dec  5 13:19:59 2017
@@ -2,6 +2,7 @@
 
 typedef int __attribute__((ext_vector_type(2))) V;
 
+void clang_analyzer_warnIfReached();
 void clang_analyzer_numTimesReached();
 void clang_analyzer_eval(int);
 
@@ -26,3 +27,35 @@ V dont_crash_and_dont_split_state(V x, V
   clang_analyzer_numTimesReached(); // expected-warning{{2}}
   return z;
 }
+
+void test_read() {
+  V x;
+  x[0] = 0;
+  x[1] = 1;
+
+  clang_analyzer_eval(x[0] == 0); // expected-warning{{TRUE}}
+}
+
+V return_vector() {
+  V z;
+  z[0] = 0;
+  z[1] = 0;
+  return z;
+}
+
+int test_vector_access() {
+  return return_vector()[0]; // no-crash no-warning
+}
+
+ at interface I
+ at property V v;
+ at end
+
+// Do not crash on subscript operations into ObjC properties.
+int myfunc(I *i2) {
+  int out = i2.v[0]; // no-crash no-warning
+
+  // Check that the analysis continues.
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  return out;
+}




More information about the cfe-commits mailing list