[cfe-commits] r161288 - in /cfe/trunk: lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp test/Analysis/reference.cpp

[Jordan Rose]

Author: jrose
Date: Fri Aug  3 19:25:30 2012
New Revision: 161288

URL: http://llvm.org/viewvc/llvm-project?rev=161288&view=rev
Log:
[analyzer] Don't assume values bound to references are automatically non-null.

While there is no such thing as a "null reference" in the C++ standard,
many implementations of references (including Clang's) do not actually
check that the location bound to them is non-null. Thus unlike a regular
null dereference, this will not cause a problem at runtime until the
reference is actually used. In order to catch these cases, we need to not
prune out paths on which the input pointer is null.

Modified:
    cfe/trunk/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
    cfe/trunk/test/Analysis/reference.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp?rev=161288&r1=161287&r2=161288&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp Fri Aug  3 19:25:30 2012
@@ -233,7 +233,7 @@
 
 void DereferenceChecker::checkBind(SVal L, SVal V, const Stmt *S,
                                    CheckerContext &C) const {
-  // If we're binding to a reference, check if the value is potentially null.
+  // If we're binding to a reference, check if the value is known to be null.
   if (V.isUndef())
     return;
 
@@ -265,8 +265,23 @@
     }
   }
 
-  // From here on out, assume the value is non-null.
-  C.addTransition(StNonNull);
+  // Unlike a regular null dereference, initializing a reference with a
+  // dereferenced null pointer does not actually cause a runtime exception in
+  // Clang's implementation of references.
+  //
+  //   int &r = *p; // safe??
+  //   if (p != NULL) return; // uh-oh
+  //   r = 5; // trap here
+  //
+  // The standard says this is invalid as soon as we try to create a "null
+  // reference" (there is no such thing), but turning this into an assumption
+  // that 'p' is never null will not match our actual runtime behavior.
+  // So we do not record this assumption, allowing us to warn on the last line
+  // of this example.
+  //
+  // We do need to add a transition because we may have generated a sink for
+  // the "implicit" null dereference.
+  C.addTransition(State, this);
 }
 
 void ento::registerDereferenceChecker(CheckerManager &mgr) {

Modified: cfe/trunk/test/Analysis/reference.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/reference.cpp?rev=161288&r1=161287&r2=161288&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/reference.cpp (original)
+++ cfe/trunk/test/Analysis/reference.cpp Fri Aug  3 19:25:30 2012
@@ -91,12 +91,25 @@
   }
 }
 
-void testRef() {
+void testNullReference() {
   int *x = 0;
   int &y = *x; // expected-warning{{Dereference of null pointer}}
   y = 5;
 }
 
+void testRetroactiveNullReference(int *x) {
+  // According to the C++ standard, there is no such thing as a
+  // "null reference". So the 'if' statement ought to be dead code.
+  // However, Clang (and other compilers) don't actually check that a pointer
+  // value is non-null in the implementation of references, so it is possible
+  // to produce a supposed "null reference" at runtime. The analyzer shoeuld
+  // still warn when it can prove such errors.
+  int &y = *x;
+  if (x != 0)
+    return;
+  y = 5; // expected-warning{{Dereference of null pointer}}
+}
+
 
 // ------------------------------------
 // False negatives