[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.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@161288 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp b/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
index a94d7a7..e98c131 100644
--- a/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
@@ -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) {
diff --git a/test/Analysis/reference.cpp b/test/Analysis/reference.cpp
index c9bfadc..06e4a50 100644
--- a/test/Analysis/reference.cpp
+++ b/test/Analysis/reference.cpp
@@ -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
