[analyzer] WIP: trackNullOrUndefValue: track last store to symbolic pointers.

[NoQ]

Summary

bugreporter::trackNullOrUndefValue() checker API function extends a bug report with a recursive family of bug report visitors (ReturnVisitor, FindLastStoreBRVisitor, etc.) that collectively try to figure out where the given value came from. In particular, a null or undefined value, which is useful for null dereferences, uninitialized value checks, or divisions by zero. This not only improves the bug report with additional helpful diagnostic pieces, but also helps us suppress bugs that come from inlined defensive checks (the more solid potential solution for at least some of these false positives would be to introduce a state merge at call exit, but state merge is a new operation over the program states, so it would require checker side support, which is heavy).

In this patch i attempt to add more logic into the tracking, namely to be able to track a value stored in an arbitrary memory region R back to the expression that caused this value to be stored there. Previously it only worked when R is coming from a ExplodedGraph::isInterestingLValueExpression() (the exact meaning of which is "the respective node would not be reclaimed during garbage collection" (!), of course it would indeed be a shame if the node we're looking for disappears). This leaves with plain variables, member variables, and ObjC instance variables.

FindLastStoreBRVisitor is already capable of doing this in the general case of an arbitrary memory region - not only it finds the node where getSVal(R) changed, but also it finds PostStore nodes to this region even if the newly written value is same as old value.

Note that visitors are deduplicated, so we're not afraid of adding too many identical visitors.

TODO: For now i just stuffed the visitor in there, and it seems to work. But i'd like to see how much the existing code for variables can (or even must) be reused before committing, hence WIP.

lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

if (Optional<loc::MemRegionVal> L = V.getAs<loc::MemRegionVal>()) {
     // However, if the rvalue is a symbolic region, we should track it as well.
     // Try to use the correct type when looking up the value.
     SVal RVal;
     if (const Expr *E = dyn_cast<Expr>(S))
       RVal = state->getRawSVal(L.getValue(), E->getType());
     else
       RVal = state->getSVal(L->getRegion());
 
-    const MemRegion *RegionRVal = RVal.getAsRegion();
     report.addVisitor(llvm::make_unique<UndefOrNullArgVisitor>(L->getRegion()));
+    if (Optional<KnownSVal> KV = RVal.getAs<KnownSVal>())
+      report.addVisitor(llvm::make_unique<FindLastStoreBRVisitor>(
+          *KV, L->getRegion(), EnableNullFPSuppression));
 
+    const MemRegion *RegionRVal = RVal.getAsRegion();
     if (RegionRVal && isa<SymbolicRegion>(RegionRVal)) {
       report.markInteresting(RegionRVal);
       report.addVisitor(llvm::make_unique<TrackConstraintBRVisitor>(
           loc::MemRegionVal(RegionRVal), false));
     }
   }
 
   return true;

test/Analysis/inlining/inline-defensive-checks.c

 struct S2 {
   int a[1];
 };
 
 void idcTrackZeroValueThroughUnaryPointerOperatorsWithArrayField(struct S2 *s) {
   idc(s);
   *(&(s->a[0])) = 7; // no-warning
 }
+
+void idcTrackConstraintThroughSymbolicRegion(int **x) {
+  idc(*x);
+  // FIXME: Should not warn.
+  **x = 7; // expected-warning{{Dereference of null pointer}}
+}
+
+int *idcPlainNull(int coin) {
+  if (coin)
+    return 0;
+  static int X;
+  return &X;
+}
+
+void idcTrackZeroValueThroughSymbolicRegion(int coin, int **x) {
+  *x = idcPlainNull(coin);
+  **x = 7; // no-warning
+}

test/Analysis/nullptr.cpp

void shouldNotCrash() {
   if (getSymbol()) // expected-note   {{Assuming the condition is false}}
                    // expected-note@-1{{Taking false branch}}
                    // expected-note@-2{{Assuming the condition is true}}
                    // expected-note@-3{{Taking true branch}}
     invokeF(nullptr); // expected-note   {{Calling 'invokeF'}}
                       // expected-note@-1{{Passing null pointer value via 1st parameter 'x'}}
   if (getSymbol()) {  // expected-note  {{Assuming the condition is true}}
                       // expected-note@-1{{Taking true branch}}
-    X *x = Type().x; // expected-note{{'x' initialized to a null pointer value}}
-    x->f(); // expected-warning{{Called C++ object pointer is null}}
+    X *xx = Type().x; // expected-note   {{Null pointer value stored to field 'x'}}
+                      // expected-note@-1{{'xx' initialized to a null pointer value}}
+    xx->f(); // expected-warning{{Called C++ object pointer is null}}
             // expected-note@-1{{Called C++ object pointer is null}}
   }
 }
 
 void f(decltype(nullptr) p) {
   int *q = nullptr;
   clang_analyzer_eval(p == 0); // expected-warning{{TRUE}}
                                // expected-note@-1{{TRUE}}