[analyzer] Allow null false positive suppression for conditions

[Szelethus]

Summary

If the a value has received its value through suspicious means, we suppress it. Tracked conditions are very much relevant to the occurrence of a bug, if their value is fishy, the entire bug report probably is as well.

[NoQ]

Aha! We need some positive results tho, given that these are heuristics.

clang/test/Analysis/inlining/inline-defensive-checks.m

113

This code definitely deserves a warning tho. Regardless of any path-sensitive reasoning, it's trivial to prove that either this line has a division by zero or it's outright dead code. I'd leave a FIXME here. This shouldn't be suppressed.

More importantly, we should have more tests to demonstrate how this patch is useful. I.e., some code snippets in which it's obvious that the suppressed warning is a false positive.

[Szelethus]

I'm too sure what the implications of such a change is, so I'll get some real-life results before even thinking of commiting.

clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

if (ControlDeps.isControlDependent(OriginB, NB)) {
     if (llvm::isa_and_nonnull<CXXForRangeStmt>(NB->getTerminatorStmt()))
       return nullptr;
 
     if (const Expr *Condition = NB->getLastCondition()) {
       // Keeping track of the already tracked conditions on a visitor level
       // isn't sufficient, because a new visitor is created for each tracked
       // expression, hence the BugReport level set.
       if (BR.addTrackedCondition(N)) {
         bugreporter::trackExpressionValue(
             N, Condition, BR, bugreporter::TrackingKind::Condition,
-            /*EnableNullFPSuppression=*/false);
+            /*EnableNullFPSuppression=*/true);
         return constructDebugPieceForTrackedCondition(Condition, N, BRC);
       }
     }
   }
 
   return nullptr;
 }
 

clang/test/Analysis/inlining/inline-defensive-checks.m

if (!mem)
     return 5/zero; // expected-warning {{Division by zero}}
   return 0;
 }
 
 int dontSuppressNilReceiverRetNull(Foo* fPtr) {
   unsigned zero = 0;
   fPtr = retNil();
   // On a path where fPtr is nil, mem should be nil.
-  // The warning is not suppressed because the receiver being nil is not
-  // directly related to the value that triggers the warning.
   Foo *mem = [fPtr getFooPtr];
   if (!mem)
-    return 5/zero; // expected-warning {{Division by zero}}
+    return 5/zero;
   return 0;
 }
 
 int dontSuppressNilReceiverIDC(Foo* fPtr) {
   unsigned zero = 0;
   idc(fPtr);
   // On a path where fPtr is nil, mem should be nil.
   // The warning is not suppressed because the receiver being nil is not
   // directly related to the value that triggers the warning.
   Foo *mem = [fPtr getFooPtr];
   if (!mem)
     return 5/zero; // expected-warning {{Division by zero}}
   return 0;
 }