[Static Analyzer] Make NonNullParamChecker emit implicit null dereference events.
Differential Revision: http://reviews.llvm.org/D11433
llvm-svn: 246182
diff --git a/clang/include/clang/StaticAnalyzer/Core/Checker.h b/clang/include/clang/StaticAnalyzer/Core/Checker.h
index 099d763..6468553 100644
--- a/clang/include/clang/StaticAnalyzer/Core/Checker.h
+++ b/clang/include/clang/StaticAnalyzer/Core/Checker.h
@@ -514,6 +514,10 @@
bool IsLoad;
ExplodedNode *SinkNode;
BugReporter *BR;
+ // When true, the dereference is in the source code directly. When false, the
+ // dereference might happen later (for example pointer passed to a parameter
+ // that is marked with nonnull attribute.)
+ bool IsDirectDereference;
};
/// \brief A helper class which wraps a boolean value set to false by default.
diff --git a/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
index cf9d9f7..07be5a5 100644
--- a/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp
@@ -220,7 +220,8 @@
// null or not-null. Record the error node as an "implicit" null
// dereference.
if (ExplodedNode *N = C.generateSink(nullState)) {
- ImplicitNullDerefEvent event = { l, isLoad, N, &C.getBugReporter() };
+ ImplicitNullDerefEvent event = {l, isLoad, N, &C.getBugReporter(),
+ /*IsDirectDereference=*/false};
dispatchEvent(event);
}
}
@@ -257,8 +258,9 @@
// At this point the value could be either null or non-null.
// Record this as an "implicit" null dereference.
if (ExplodedNode *N = C.generateSink(StNull)) {
- ImplicitNullDerefEvent event = { V, /*isLoad=*/true, N,
- &C.getBugReporter() };
+ ImplicitNullDerefEvent event = {V, /*isLoad=*/true, N,
+ &C.getBugReporter(),
+ /*IsDirectDereference=*/false};
dispatchEvent(event);
}
}
diff --git a/clang/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp
index 73f8087..a7586c4 100644
--- a/clang/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp
@@ -28,7 +28,7 @@
namespace {
class NonNullParamChecker
- : public Checker< check::PreCall > {
+ : public Checker< check::PreCall, EventDispatcher<ImplicitNullDerefEvent> > {
mutable std::unique_ptr<BugType> BTAttrNonNull;
mutable std::unique_ptr<BugType> BTNullRefArg;
@@ -139,26 +139,34 @@
ProgramStateRef stateNotNull, stateNull;
std::tie(stateNotNull, stateNull) = CM.assumeDual(state, *DV);
- if (stateNull && !stateNotNull) {
- // Generate an error node. Check for a null node in case
- // we cache out.
- if (ExplodedNode *errorNode = C.generateSink(stateNull)) {
+ if (stateNull) {
+ if (!stateNotNull) {
+ // Generate an error node. Check for a null node in case
+ // we cache out.
+ if (ExplodedNode *errorNode = C.generateSink(stateNull)) {
- std::unique_ptr<BugReport> R;
- if (haveAttrNonNull)
- R = genReportNullAttrNonNull(errorNode, ArgE);
- else if (haveRefTypeParam)
- R = genReportReferenceToNullPointer(errorNode, ArgE);
+ std::unique_ptr<BugReport> R;
+ if (haveAttrNonNull)
+ R = genReportNullAttrNonNull(errorNode, ArgE);
+ else if (haveRefTypeParam)
+ R = genReportReferenceToNullPointer(errorNode, ArgE);
- // Highlight the range of the argument that was null.
- R->addRange(Call.getArgSourceRange(idx));
+ // Highlight the range of the argument that was null.
+ R->addRange(Call.getArgSourceRange(idx));
- // Emit the bug report.
- C.emitReport(std::move(R));
+ // Emit the bug report.
+ C.emitReport(std::move(R));
+ }
+
+ // Always return. Either we cached out or we just emitted an error.
+ return;
}
-
- // Always return. Either we cached out or we just emitted an error.
- return;
+ if (ExplodedNode *N = C.generateSink(stateNull)) {
+ ImplicitNullDerefEvent event = {
+ V, false, N, &C.getBugReporter(),
+ /*IsDirectDereference=*/haveRefTypeParam};
+ dispatchEvent(event);
+ }
}
// If a pointer value passed the check we should assume that it is
diff --git a/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
index f488b5f..ceb437d 100644
--- a/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
@@ -335,7 +335,10 @@
if (Filter.CheckNullableDereferenced &&
TrackedNullability->getValue() == Nullability::Nullable) {
BugReporter &BR = *Event.BR;
- reportBug(ErrorKind::NullableDereferenced, Event.SinkNode, Region, BR);
+ if (Event.IsDirectDereference)
+ reportBug(ErrorKind::NullableDereferenced, Event.SinkNode, Region, BR);
+ else
+ reportBug(ErrorKind::NullablePassedToNonnull, Event.SinkNode, Region, BR);
}
}
diff --git a/clang/test/Analysis/nullability.mm b/clang/test/Analysis/nullability.mm
index 4b68e3d..ec7ea3a 100644
--- a/clang/test/Analysis/nullability.mm
+++ b/clang/test/Analysis/nullability.mm
@@ -50,6 +50,8 @@
template <typename T> T *eraseNullab(T *p) { return p; }
+void takesAttrNonnull(Dummy *p) __attribute((nonnull(1)));
+
void testBasicRules() {
Dummy *p = returnsNullable();
int *ptr = returnsNullableInt();
@@ -73,10 +75,8 @@
Dummy dd(d);
break;
}
- // Here the copy constructor is called, so a reference is initialized with the
- // value of p. No ImplicitNullDereference event will be dispatched for this
- // case. A followup patch is expected to fix this in NonNullParamChecker.
- default: { Dummy d = *p; } break; // No warning.
+ case 5: takesAttrNonnull(p); break; // expected-warning {{Nullable pointer is passed to}}
+ default: { Dummy d = *p; } break; // expected-warning {{Nullable pointer is dereferenced}}
}
if (p) {
takesNonnull(p);