Index: cfe/trunk/include/clang/StaticAnalyzer/Core/Checker.h =================================================================== --- cfe/trunk/include/clang/StaticAnalyzer/Core/Checker.h +++ cfe/trunk/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. Index: cfe/trunk/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp +++ cfe/trunk/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); } } Index: cfe/trunk/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp +++ cfe/trunk/lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp @@ -28,7 +28,7 @@ namespace { class NonNullParamChecker - : public Checker< check::PreCall > { + : public Checker< check::PreCall, EventDispatcher > { mutable std::unique_ptr BTAttrNonNull; mutable std::unique_ptr 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)) { - - std::unique_ptr R; - if (haveAttrNonNull) - R = genReportNullAttrNonNull(errorNode, ArgE); - else if (haveRefTypeParam) - R = genReportReferenceToNullPointer(errorNode, ArgE); + 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 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)); + + // Emit the bug report. + C.emitReport(std::move(R)); + } - // Highlight the range of the argument that was null. - R->addRange(Call.getArgSourceRange(idx)); - - // Emit the bug report. - C.emitReport(std::move(R)); + // 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); } - - // Always return. Either we cached out or we just emitted an error. - return; } // If a pointer value passed the check we should assume that it is Index: cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp =================================================================== --- cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp +++ cfe/trunk/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); } } Index: cfe/trunk/test/Analysis/nullability.mm =================================================================== --- cfe/trunk/test/Analysis/nullability.mm +++ cfe/trunk/test/Analysis/nullability.mm @@ -50,6 +50,8 @@ template 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);