Tweak null dereference checker to give better diagnostics for null dereferences resulting from array accesses.

llvm-svn: 117334

3 files changed, 55 insertions, 27 deletions

diff --git a/clang/lib/Checker/DereferenceChecker.cpp b/clang/lib/Checker/DereferenceChecker.cpp
index 50392b28cd2..bb40a84cb00 100644
--- a/clang/lib/Checker/DereferenceChecker.cpp
+++ b/clang/lib/Checker/DereferenceChecker.cpp
@@ -36,6 +36,9 @@ public:
                           ImplicitNullDerefNodes.data() +
                           ImplicitNullDerefNodes.size());
   }
+  void AddDerefSource(llvm::raw_ostream &os,
+                      llvm::SmallVectorImpl<SourceRange> &Ranges,
+                      const Expr *Ex, bool loadedFrom = false);
 };
 } // end anonymous namespace
 
@@ -52,6 +55,33 @@ clang::GetImplicitNullDereferences(GRExprEngine &Eng) {
   return checker->getImplicitNodes();
 }
 
+void DereferenceChecker::AddDerefSource(llvm::raw_ostream &os,
+                                     llvm::SmallVectorImpl<SourceRange> &Ranges,
+                                        const Expr *Ex,
+                                        bool loadedFrom) {
+  switch (Ex->getStmtClass()) {
+    default:
+      return;
+    case Stmt::DeclRefExprClass: {
+      const DeclRefExpr *DR = cast<DeclRefExpr>(Ex);
+      if (const VarDecl *VD = dyn_cast<VarDecl>(DR->getDecl())) {
+        os << " (" << (loadedFrom ? "loaded from" : "from")
+           << " variable '" <<  VD->getName() << "')";
+        Ranges.push_back(DR->getSourceRange());
+      }
+      return;
+    }
+    case Stmt::MemberExprClass: {
+      const MemberExpr *ME = cast<MemberExpr>(Ex);
+      os << " (" << (loadedFrom ? "loaded from" : "via")
+         << " field '" << ME->getMemberNameInfo() << "')";
+      SourceLocation L = ME->getMemberLoc();
+      Ranges.push_back(SourceRange(L, L));
+      break;
+    }
+  }
+}
+
 void DereferenceChecker::VisitLocation(CheckerContext &C, const Stmt *S,
                                        SVal l) {
   // Check for dereference of an undefined value.
@@ -96,31 +126,29 @@ void DereferenceChecker::VisitLocation(CheckerContext &C, const Stmt *S,
       llvm::SmallVector<SourceRange, 2> Ranges;
 
       switch (S->getStmtClass()) {
+        case Stmt::ArraySubscriptExprClass: {
+          llvm::raw_svector_ostream os(buf);
+          os << "Array access";
+          const ArraySubscriptExpr *AE = cast<ArraySubscriptExpr>(S);
+          AddDerefSource(os, Ranges, AE->getBase()->IgnoreParenCasts());
+          os << " results in a null pointer dereference";
+          break;
+        }
         case Stmt::UnaryOperatorClass: {
+          llvm::raw_svector_ostream os(buf);
+          os << "Dereference of null pointer";
           const UnaryOperator *U = cast<UnaryOperator>(S);
-          const Expr *SU = U->getSubExpr()->IgnoreParens();
-          if (const DeclRefExpr *DR = dyn_cast<DeclRefExpr>(SU)) {
-            if (const VarDecl *VD = dyn_cast<VarDecl>(DR->getDecl())) {
-              llvm::raw_svector_ostream os(buf);
-              os << "Dereference of null pointer (loaded from variable '"
-                 << VD->getName() << "')";
-              Ranges.push_back(DR->getSourceRange());
-            }
-          }
+          AddDerefSource(os, Ranges, U->getSubExpr()->IgnoreParens(), true);
           break;
         }
         case Stmt::MemberExprClass: {
           const MemberExpr *M = cast<MemberExpr>(S);
-          if (M->isArrow())
-            if (DeclRefExpr *DR =
-                dyn_cast<DeclRefExpr>(M->getBase()->IgnoreParenCasts())) {
-              if (const VarDecl *VD = dyn_cast<VarDecl>(DR->getDecl())) {
-                llvm::raw_svector_ostream os(buf);
-                os << "Field access results in a dereference of a null pointer "
-                      "(loaded from variable '" << VD->getName() << "')";
-                Ranges.push_back(M->getBase()->getSourceRange());
-              }
-            }
+          if (M->isArrow()) {
+            llvm::raw_svector_ostream os(buf);
+            os << "Access to field '" << M->getMemberNameInfo()
+               << "' results in a dereference of a null pointer";
+            AddDerefSource(os, Ranges, M->getBase()->IgnoreParenCasts(), true);
+          }
           break;
         }
         case Stmt::ObjCIvarRefExprClass: {
diff --git a/clang/test/Analysis/null-deref-ps.c b/clang/test/Analysis/null-deref-ps.c
index 58f5d30c093..1e3f2db34b2 100644
--- a/clang/test/Analysis/null-deref-ps.c
+++ b/clang/test/Analysis/null-deref-ps.c
@@ -26,7 +26,7 @@ int f2(struct foo_struct* p) {
   if (p)
     p->x = 1;
     
-  return p->x++; // expected-warning{{Field access results in a dereference of a null pointer (loaded from variable 'p')}}
+  return p->x++; // expected-warning{{Access to field 'x' results in a dereference of a null pointer (loaded from variable 'p')}}
 }
 
 int f3(char* x) {
@@ -36,7 +36,7 @@ int f3(char* x) {
   if (x)
     return x[i - 1];
   
-  return x[i+1]; // expected-warning{{Dereference of null pointer}}
+  return x[i+1]; // expected-warning{{Array access (from variable 'x') results in a null pointer dereference}}
 }
 
 int f3_b(char* x) {
@@ -46,7 +46,7 @@ int f3_b(char* x) {
   if (x)
     return x[i - 1];
   
-  return x[i+1]++; // expected-warning{{Dereference of null pointer}}
+  return x[i+1]++; // expected-warning{{Array access (from variable 'x') results in a null pointer dereference}}
 }
 
 int f4(int *p) {
diff --git a/clang/test/Analysis/plist-output-alternate.m b/clang/test/Analysis/plist-output-alternate.m
index d063348a116..12697b4c6c8 100644
--- a/clang/test/Analysis/plist-output-alternate.m
+++ b/clang/test/Analysis/plist-output-alternate.m
@@ -743,23 +743,23 @@ void rdar8331641(int x) {
 // CHECK:        <array>
 // CHECK:         <dict>
 // CHECK:          <key>line</key><integer>37</integer>
-// CHECK:          <key>col</key><integer>3</integer>
+// CHECK:          <key>col</key><integer>7</integer>
 // CHECK:          <key>file</key><integer>0</integer>
 // CHECK:         </dict>
 // CHECK:         <dict>
 // CHECK:          <key>line</key><integer>37</integer>
-// CHECK:          <key>col</key><integer>8</integer>
+// CHECK:          <key>col</key><integer>7</integer>
 // CHECK:          <key>file</key><integer>0</integer>
 // CHECK:         </dict>
 // CHECK:        </array>
 // CHECK:      </array>
 // CHECK:      <key>extended_message</key>
-// CHECK:      <string>Dereference of null pointer</string>
+// CHECK:      <string>Dereference of null pointer (loaded from field &apos;p&apos;)</string>
 // CHECK:      <key>message</key>
-// CHECK: <string>Dereference of null pointer</string>
+// CHECK: <string>Dereference of null pointer (loaded from field &apos;p&apos;)</string>
 // CHECK:     </dict>
 // CHECK:    </array>
-// CHECK:    <key>description</key><string>Dereference of null pointer</string>
+// CHECK:    <key>description</key><string>Dereference of null pointer (loaded from field &apos;p&apos;)</string>
 // CHECK:    <key>category</key><string>Logic error</string>
 // CHECK:    <key>type</key><string>Dereference of null pointer</string>
 // CHECK:   <key>location</key>