[Static Analyzer] Make NonNullParamChecker emit implicit null dereference events.

[xazax.hun]

Summary

For most of the implicit null pointer dereferences (when a pointer is dereferenced and the static analyzer can not prove that the pointer is non null, and can not prove that the pointer is null either) the DereferenceChecker will emit an event. However in some cases (e.g. binding such pointers to references in function parameters) no event will be emitted. This patch addresses this problem by extending NonNullParamChecker to emit an event in such cases. Future checkers (listening to these events) will profit from this change. For example I am working on a checker for nullability qualifiers that depend on this patch.

[zaks.anna]

I believe we saw a regression in diagnostics with this modification. Gabor, do you recall what it was?

[xazax.hun]

The problem here is that, this checker can emit a warning for two cases:
1, Null was bound to a reference which should be reported as a dereference
2, Null was passed to parameter that should be non-null (marked by the attribute, not the qualifier) which should be reported appropriately

Right now exactly the same event will be triggered for both cases, so the checkers that process this event has no information whether it is a dereference or a value passed to a nonnull parameter, so it can not provide appropriate diagnostic for both cases.

One possibility would be to have two separate events, the other is to have an argument to an event that determines its origin. Probably it would be better to have two separate events, since it might be confusing to have dereference in the name of the event in the second case.

What do you think?

[xazax.hun]

Only send implicit dereference events, when the null pointer was bound to a reference.

[zaks.anna]

Please, commit this after committing the nullability checker so that this could have tests. Two tests need to be added:

1. the reference case
2. the attribute nonnull case

Also, I'd suggest adding a field to ImplicitNullDerefEvent instead of creating a new event.

[xazax.hun]

• Updated to latest trunk.
• Added a new field to the event with documentation.
• Rebased the patch on the top of nullability checker.
• Added covered cases to the nullability checker tests.

[zaks.anna]

Thanks!

lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp

 
 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;
 

     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);
 
           // 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() };
+        dispatchEvent(event);
+      }
     }
 
     // If a pointer value passed the check we should assume that it is