r301251 - [analyzer] Improve subscripting null arrays for catching null dereferences.

[Artem Dergachev via cfe-commits]

Author: dergachev
Date: Mon Apr 24 15:55:07 2017
New Revision: 301251

URL: http://llvm.org/viewvc/llvm-project?rev=301251&view=rev
Log:
[analyzer] Improve subscripting null arrays for catching null dereferences.

Array-to-pointer cast now works correctly when the pointer to the array
is concrete, eg. null, which allows further symbolic calculations involving
such values.

Inlined defensive checks are now detected correctly when the resulting null
symbol is being array-subscripted before dereference.

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

Modified:
    cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
    cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
    cfe/trunk/test/Analysis/null-deref-offsets.c

Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp?rev=301251&r1=301250&r2=301251&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Mon Apr 24 15:55:07 2017
@@ -61,7 +61,9 @@ const Expr *bugreporter::getDerefExpr(co
         return U->getSubExpr()->IgnoreParenCasts();
     }
     else if (const MemberExpr *ME = dyn_cast<MemberExpr>(E)) {
-      if (ME->isArrow() || isDeclRefExprToReference(ME->getBase())) {
+      if (ME->isImplicitAccess()) {
+        return ME;
+      } else if (ME->isArrow() || isDeclRefExprToReference(ME->getBase())) {
         return ME->getBase()->IgnoreParenCasts();
       } else {
         // If we have a member expr with a dot, the base must have been
@@ -73,9 +75,9 @@ const Expr *bugreporter::getDerefExpr(co
       return IvarRef->getBase()->IgnoreParenCasts();
     }
     else if (const ArraySubscriptExpr *AE = dyn_cast<ArraySubscriptExpr>(E)) {
-      return AE->getBase();
+      return getDerefExpr(AE->getBase());
     }
-    else if (isDeclRefExprToReference(E)) {
+    else if (isa<DeclRefExpr>(E)) {
       return E;
     }
     break;
@@ -974,14 +976,11 @@ bool bugreporter::trackNullOrUndefValue(
     // This code interacts heavily with this hack; otherwise the value
     // would not be null at all for most fields, so we'd be unable to track it.
     if (const auto *Op = dyn_cast<UnaryOperator>(Ex))
-      if (Op->getOpcode() == UO_AddrOf && Op->getSubExpr()->isLValue()) {
-        Ex = Op->getSubExpr()->IgnoreParenCasts();
-        while (const auto *ME = dyn_cast<MemberExpr>(Ex)) {
-          Ex = ME->getBase()->IgnoreParenCasts();
-        }
-      }
+      if (Op->getOpcode() == UO_AddrOf && Op->getSubExpr()->isLValue())
+        if (const Expr *DerefEx = getDerefExpr(Op->getSubExpr()))
+          Ex = DerefEx;
 
-    if (ExplodedGraph::isInterestingLValueExpr(Ex) || CallEvent::isCallStmt(Ex))
+    if (Ex && (ExplodedGraph::isInterestingLValueExpr(Ex) || CallEvent::isCallStmt(Ex)))
       Inner = Ex;
   }
 

Modified: cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp?rev=301251&r1=301250&r2=301251&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp Mon Apr 24 15:55:07 2017
@@ -1338,6 +1338,9 @@ RegionStoreManager::getSizeInElements(Pr
 ///  the array).  This is called by ExprEngine when evaluating casts
 ///  from arrays to pointers.
 SVal RegionStoreManager::ArrayToPointer(Loc Array, QualType T) {
+  if (Array.getAs<loc::ConcreteInt>())
+    return Array;
+
   if (!Array.getAs<loc::MemRegionVal>())
     return UnknownVal();
 

Modified: cfe/trunk/test/Analysis/null-deref-offsets.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/null-deref-offsets.c?rev=301251&r1=301250&r2=301251&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/null-deref-offsets.c (original)
+++ cfe/trunk/test/Analysis/null-deref-offsets.c Mon Apr 24 15:55:07 2017
@@ -7,7 +7,7 @@ struct S {
   int z[2];
 };
 
-void testOffsets(struct S *s) {
+void testOffsets(struct S *s, int coin) {
   if (s != 0)
     return;
 
@@ -21,14 +21,17 @@ void testOffsets(struct S *s) {
 
   // FIXME: These should ideally be true.
   clang_analyzer_eval(&(s->y) == 4); // expected-warning{{FALSE}}
-  clang_analyzer_eval(&(s->z[0]) == 8); // expected-warning{{UNKNOWN}}
-  clang_analyzer_eval(&(s->z[1]) == 12); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(&(s->z[0]) == 8); // expected-warning{{FALSE}}
+  clang_analyzer_eval(&(s->z[1]) == 12); // expected-warning{{FALSE}}
 
   // FIXME: These should ideally be false.
   clang_analyzer_eval(&(s->y) == 0); // expected-warning{{TRUE}}
-  clang_analyzer_eval(&(s->z[0]) == 0); // expected-warning{{UNKNOWN}}
-  clang_analyzer_eval(&(s->z[1]) == 0); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(&(s->z[0]) == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(&(s->z[1]) == 0); // expected-warning{{TRUE}}
 
-  // But this should still be a null dereference.
-  s->y = 5; // expected-warning{{Access to field 'y' results in a dereference of a null pointer (loaded from variable 's')}}
+  // But these should still be reported as null dereferences.
+  if (coin)
+    s->y = 5; // expected-warning{{Access to field 'y' results in a dereference of a null pointer (loaded from variable 's')}}
+  else
+    s->z[1] = 6; // expected-warning{{Array access (via field 'z') results in a null pointer dereference}}
 }