[analyzer] Turn ReturnVisitor into a tracking visitor

[vsavchenko]

Summary

Whenever Tracker spawns a visitor that needs to call tracker
back, we have to use TrackingBugReporterVisitor in order to maintain
all the hooks that the checker might've used.

[vsavchenko]

Rebase

[NoQ]

Great!

[vsavchenko]

Rebase

clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

 
 /// Emits an extra note at the return statement of an interesting stack frame.
 ///
 /// The returned value is marked as an interesting value, and if it's null,
 /// adds a visitor to track where it became null.
 ///
 /// This visitor is intended to be used when another visitor discovers that an
 /// interesting value comes from an inlined function call.
-class ReturnVisitor : public BugReporterVisitor {
+class ReturnVisitor : public TrackingBugReporterVisitor {
   const StackFrameContext *CalleeSFC;
   enum {
     Initial,
     MaybeUnsuppress,
     Satisfied
   } Mode = Initial;
 
   bool EnableNullFPSuppression;
   bool ShouldInvalidate = true;
   AnalyzerOptions& Options;
   bugreporter::TrackingKind TKind;
 
 public:
-  ReturnVisitor(const StackFrameContext *Frame, bool Suppressed,
-                AnalyzerOptions &Options, bugreporter::TrackingKind TKind)
-      : CalleeSFC(Frame), EnableNullFPSuppression(Suppressed),
-        Options(Options), TKind(TKind) {}
+  ReturnVisitor(TrackerRef ParentTracker, const StackFrameContext *Frame,
+                bool Suppressed, AnalyzerOptions &Options,
+                bugreporter::TrackingKind TKind)
+      : TrackingBugReporterVisitor(ParentTracker), CalleeSFC(Frame),
+        EnableNullFPSuppression(Suppressed), Options(Options), TKind(TKind) {}
 
   static void *getTag() {
     static int Tag = 0;
     return static_cast<void *>(&Tag);
   }
 
   void Profile(llvm::FoldingSetNodeID &ID) const override {
     ID.AddPointer(ReturnVisitor::getTag());
     ID.AddPointer(CalleeSFC);
     ID.AddBoolean(EnableNullFPSuppression);
   }
 
   /// Adds a ReturnVisitor if the given statement represents a call that was
   /// inlined.
   ///
   /// This will search back through the ExplodedGraph, starting from the given
   /// node, looking for when the given statement was processed. If it turns out
   /// the statement is a call that was inlined, we add the visitor to the
   /// bug report, so it can print a note later.
-  static void addVisitorIfNecessary(const ExplodedNode *Node, const Stmt *S,
+  static void addVisitorIfNecessary(TrackerRef ParentTracker,
+                                    const ExplodedNode *Node, const Stmt *S,
                                     PathSensitiveBugReport &BR,
                                     bool InEnableNullFPSuppression,
                                     bugreporter::TrackingKind TKind) {
     if (!CallEvent::isCallStmt(S))
       return;
 
     // First, find when we processed the statement.
     // If we work with a 'CXXNewExpr' that is going to be purged away before
static void addVisitorIfNecessary(TrackerRef ParentTracker,
     // See if the return value is NULL. If so, suppress the report.
     AnalyzerOptions &Options = State->getAnalysisManager().options;
 
     bool EnableNullFPSuppression = false;
     if (InEnableNullFPSuppression && Options.ShouldSuppressNullReturnPaths)
       if (Optional<Loc> RetLoc = RetVal.getAs<Loc>())
         EnableNullFPSuppression = State->isNull(*RetLoc).isConstrainedTrue();
 
-    BR.addVisitor<ReturnVisitor>(CalleeContext, EnableNullFPSuppression,
-                                 Options, TKind);
+    BR.addVisitor<ReturnVisitor>(ParentTracker, CalleeContext,
+                                 EnableNullFPSuppression, Options, TKind);
   }
 
   PathDiagnosticPieceRef visitNodeInitial(const ExplodedNode *N,
                                           BugReporterContext &BRC,
                                           PathSensitiveBugReport &BR) {
     // Only print a message at the interesting return statement.
     if (N->getLocationContext() != CalleeSFC)
       return nullptr;
PathDiagnosticPieceRef visitNodeInitial(const ExplodedNode *N,
     // Ignore aggregate rvalues.
     if (V.getAs<nonloc::LazyCompoundVal>() ||
         V.getAs<nonloc::CompoundVal>())
       return nullptr;
 
     RetE = RetE->IgnoreParenCasts();
 
     // Let's track the return value.
-    bugreporter::trackExpressionValue(
-        N, RetE, BR, TKind, EnableNullFPSuppression);
+    getParentTracker().track(RetE, N, {TKind, EnableNullFPSuppression});
 
     // Build an appropriate message based on the return value.
     SmallString<64> Msg;
     llvm::raw_svector_ostream Out(Msg);
 
     bool WouldEventBeMeaningless = false;
 
     if (State->isNull(V).isConstrainedTrue()) {
for (unsigned I = 0, E = Call->getNumArgs(); I != E; ++I) {
       const Expr *ArgE = Call->getArgExpr(I);
       if (!ArgE)
         continue;
 
       // Is it possible for this argument to be non-null?
       if (!State->isNull(*ArgV).isConstrainedTrue())
         continue;
 
-      if (trackExpressionValue(N, ArgE, BR, TKind, EnableNullFPSuppression))
+      if (getParentTracker()
+              .track(ArgE, N, {TKind, EnableNullFPSuppression})
+              .FoundSomethingToTrack)
         ShouldInvalidate = false;
 
       // If we /can't/ track the null pointer, we should err on the side of
       // false negatives, and continue towards marking this report invalid.
       // (We will still look at the other arguments, though.)
     }
 
     return nullptr;
if (ExplodedGraph::isInterestingLValueExpr(Inner)) {
         return Result;
       }
     }
 
     // If the expression is not an "lvalue expression", we can still
     // track the constraints on its contents.
     SVal V = LVState->getSValAsScalarOrLoc(Inner, LVNode->getLocationContext());
 
-    ReturnVisitor::addVisitorIfNecessary(
-        LVNode, Inner, Report, Opts.EnableNullFPSuppression, Opts.Kind);
+    ReturnVisitor::addVisitorIfNecessary(&getParentTracker(), LVNode, Inner,
+                                         Report, Opts.EnableNullFPSuppression,
+                                         Opts.Kind);
 
     // Is it a symbolic value?
     if (auto L = V.getAs<loc::MemRegionVal>()) {
       // FIXME: this is a hack for fixing a later crash when attempting to
       // dereference a void* pointer.
       // We should not try to dereference pointers at all when we don't care
       // what is written inside the pointer.
       bool CanDereference = true;