[PATCH] D64270: [analyzer][NFC] Prepare visitors for different tracking kinds

[Kristóf Umann via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL368777: [analyzer][NFC] Prepare visitors for different 
tracking kinds (authored by Szelethus, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D64270?vs=211752=214995#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64270/new/

https://reviews.llvm.org/D64270

Files:
  cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
  cfe/trunk/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp
  cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

Index: cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
===
--- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
@@ -85,6 +85,40 @@
   const BugReport );
 };
 
+namespace bugreporter {
+
+/// Specifies the type of tracking for an expression.
+enum class TrackingKind {
+  /// Default tracking kind -- specifies that as much information should be
+  /// gathered about the tracked expression value as possible.
+  Thorough,
+  /// Specifies that a more moderate tracking should be used for the expression
+  /// value. This will essentially make sure that functions relevant to the it
+  /// aren't pruned, but otherwise relies on the user reading the code or
+  /// following the arrows.
+  Condition
+};
+
+/// Attempts to add visitors to track expression value back to its point of
+/// origin.
+///
+/// \param N A node "downstream" from the evaluation of the statement.
+/// \param E The expression value which we are tracking
+/// \param R The bug report to which visitors should be attached.
+/// \param EnableNullFPSuppression Whether we should employ false positive
+/// suppression (inlined defensive checks, returned null).
+///
+/// \return Whether or not the function was able to add visitors for this
+/// statement. Note that returning \c true does not actually imply
+/// that any visitors were added.
+bool trackExpressionValue(const ExplodedNode *N, const Expr *E, BugReport ,
+  TrackingKind TKind = TrackingKind::Thorough,
+  bool EnableNullFPSuppression = true);
+
+const Expr *getDerefExpr(const Stmt *S);
+
+} // namespace bugreporter
+
 /// Finds last store into the given region,
 /// which is different from a given symbolic value.
 class FindLastStoreBRVisitor final : public BugReporterVisitor {
@@ -96,15 +130,28 @@
   /// bug, we are going to employ false positive suppression.
   bool EnableNullFPSuppression;
 
+  using TrackingKind = bugreporter::TrackingKind;
+  TrackingKind TKind;
+
 public:
   /// Creates a visitor for every VarDecl inside a Stmt and registers it with
   /// the BugReport.
   static void registerStatementVarDecls(BugReport , const Stmt *S,
-bool EnableNullFPSuppression);
+bool EnableNullFPSuppression,
+TrackingKind TKind);
 
+  /// \param V We're searching for the store where \c R received this value.
+  /// \param R The region we're tracking.
+  /// \param EnableNullFPSuppression Whether we should employ false positive
+  /// suppression (inlined defensive checks, returned null).
+  /// \param TKind May limit the amount of notes added to the bug report.
   FindLastStoreBRVisitor(KnownSVal V, const MemRegion *R,
- bool InEnableNullFPSuppression)
-  : R(R), V(V), EnableNullFPSuppression(InEnableNullFPSuppression) {}
+ bool InEnableNullFPSuppression,
+ TrackingKind TKind)
+  : R(R), V(V), EnableNullFPSuppression(InEnableNullFPSuppression),
+TKind(TKind) {
+assert(R);
+  }
 
   void Profile(llvm::FoldingSetNodeID ) const override;
 
@@ -347,27 +394,6 @@
BugReport ) override;
 };
 
-namespace bugreporter {
-
-/// Attempts to add visitors to track expression value back to its point of
-/// origin.
-///
-/// \param N A node "downstream" from the evaluation of the statement.
-/// \param E The expression value which we are tracking
-/// \param R The bug report to which visitors should be attached.
-/// \param EnableNullFPSuppression Whether we should employ false positive
-/// suppression (inlined defensive checks, returned null).
-///
-/// \return Whether or not the function was able to add visitors for this
-/// statement. Note that returning \c true does not actually imply
-/// that any 

[PATCH] D64270: [analyzer][NFC] Prepare visitors for different tracking kinds

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus updated this revision to Diff 211752.
Szelethus added a comment.

- Make sure that the tracking kind is forwarded for each invocation of 
`trackExpressionValue` in `BugReporterVisitor.cpp`
- Expose the tracking kind to all users
- Add some more docs


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64270/new/

https://reviews.llvm.org/D64270

Files:
  clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
  clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/ObjCContainersChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -851,13 +851,13 @@
   bool EnableNullFPSuppression;
   bool ShouldInvalidate = true;
   AnalyzerOptions& Options;
+  bugreporter::TrackingKind TKind;
 
 public:
-  ReturnVisitor(const StackFrameContext *Frame,
-bool Suppressed,
-AnalyzerOptions )
+  ReturnVisitor(const StackFrameContext *Frame, bool Suppressed,
+AnalyzerOptions , bugreporter::TrackingKind TKind)
   : CalleeSFC(Frame), EnableNullFPSuppression(Suppressed),
-Options(Options) {}
+Options(Options), TKind(TKind) {}
 
   static void *getTag() {
 static int Tag = 0;
@@ -879,7 +879,8 @@
   /// bug report, so it can print a note later.
   static void addVisitorIfNecessary(const ExplodedNode *Node, const Stmt *S,
 BugReport ,
-bool InEnableNullFPSuppression) {
+bool InEnableNullFPSuppression,
+bugreporter::TrackingKind TKind) {
 if (!CallEvent::isCallStmt(S))
   return;
 
@@ -952,7 +953,7 @@
 
 BR.addVisitor(llvm::make_unique(CalleeContext,
EnableNullFPSuppression,
-   Options));
+   Options, TKind));
   }
 
   std::shared_ptr
@@ -1000,8 +1001,9 @@
 
 RetE = RetE->IgnoreParenCasts();
 
-// If we're returning 0, we should track where that 0 came from.
-bugreporter::trackExpressionValue(N, RetE, BR, EnableNullFPSuppression);
+// Let's track the return value.
+bugreporter::trackExpressionValue(
+N, RetE, BR, TKind, EnableNullFPSuppression);
 
 // Build an appropriate message based on the return value.
 SmallString<64> Msg;
@@ -1114,7 +1116,7 @@
   if (!State->isNull(*ArgV).isConstrainedTrue())
 continue;
 
-  if (bugreporter::trackExpressionValue(N, ArgE, BR, EnableNullFPSuppression))
+  if (trackExpressionValue(N, ArgE, BR, TKind, EnableNullFPSuppression))
 ShouldInvalidate = false;
 
   // If we /can't/ track the null pointer, we should err on the side of
@@ -1158,9 +1160,45 @@
   ID.AddPointer();
   ID.AddPointer(R);
   ID.Add(V);
+  ID.AddInteger(static_cast(TKind));
   ID.AddBoolean(EnableNullFPSuppression);
 }
 
+void FindLastStoreBRVisitor::registerStatementVarDecls(
+BugReport , const Stmt *S, bool EnableNullFPSuppression,
+TrackingKind TKind) {
+
+  const ExplodedNode *N = BR.getErrorNode();
+  std::deque WorkList;
+  WorkList.push_back(S);
+
+  while (!WorkList.empty()) {
+const Stmt *Head = WorkList.front();
+WorkList.pop_front();
+
+ProgramStateManager  = N->getState()->getStateManager();
+
+if (const auto *DR = dyn_cast(Head)) {
+  if (const auto *VD = dyn_cast(DR->getDecl())) {
+const VarRegion *R =
+StateMgr.getRegionManager().getVarRegion(VD, N->getLocationContext());
+
+// What did we load?
+SVal V = N->getSVal(S);
+
+if (V.getAs() || V.getAs()) {
+  // Register a new visitor with the BugReport.
+  BR.addVisitor(llvm::make_unique(
+  V.castAs(), R, EnableNullFPSuppression, TKind));
+}
+  }
+}
+
+for (const Stmt *SubStmt : Head->children())
+  WorkList.push_back(SubStmt);
+  }
+}
+
 /// Returns true if \p N represents the DeclStmt declaring and initializing
 /// \p VR.
 static bool isInitializationOfVar(const ExplodedNode *N, const VarRegion *VR) {
@@ -1331,7 +1369,7 @@
   // should track the initializer expression.
   if (Optional PIP = Pred->getLocationAs()) {
 const MemRegion *FieldReg = (const MemRegion *)PIP->getLocationValue();
-if (FieldReg && FieldReg == R) {
+if (FieldReg == R) {
   StoreSite = Pred;
   InitE = PIP->getInitializer()->getInit();
 }
@@ -1396,8 +1434,8 @@
 if (!IsParam)
   InitE = InitE->IgnoreParenCasts();
 
-bugreporter::trackExpressionValue(StoreSite, InitE, BR,
-  

[PATCH] D64270: [analyzer][NFC] Prepare visitors for different tracking kinds

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:81
+
+bool bugreporter::trackExpressionValue(const ExplodedNode *InputNode,
+   const Expr *E, BugReport ,

Szelethus wrote:
> xazax.hun wrote:
> > Do we need this overload? What about a default argument?
> I don't want to expose this functionality outside of this file. I don't 
> insist though!
I think we'll have to expose it because different checkers would want different 
tracking kinds for their values. For instance, null dereference checker could 
really use condition tracking as its default tracking mode.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64270/new/

https://reviews.llvm.org/D64270



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64270: [analyzer][NFC] Prepare visitors for different tracking kinds

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus marked an inline comment as done.
Szelethus added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:110
+  ConditionTracking
+};
+

Charusso wrote:
> What about the following?
> ```
> enum class TrackKind {
>   Full, // comment what it does
>   Condition // comment what it does
> };
> ```
> 
> Please consider the following example by unspoken naming conventions:
> ```
> enum class ColoringKind {
>   RedColoring,
>   BlueColoring
> }
> ```
> would be
> ```
> enum class Color { Red, Blue }
> ```
> where each trivial what is does, by name.
> 
> Please also note that, the thoroughness not working with redecls, 
> assumptions, loop conditions, anything we failed to inline, etc... so it is 
> not really that full tracking. But in our capabilities, it is the full what 
> we could do.
There's a good reasoning behind this -- if you take a look at this, and 
followup patches, the behavior the different `TrackingKind`s cause may differ 
from visitor to visitor, so I decided to document each behavior there. That 
way, you can't avoid the documents even if you didn't glance over this enum.

> Please also note that, the thoroughness not working with redecls, 
> assumptions, loop conditions, anything we failed to inline, etc... so it is 
> not really that full tracking. But in our capabilities, it is the full what 
> we could do.

I don't see what you mean here?



Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:81
+
+bool bugreporter::trackExpressionValue(const ExplodedNode *InputNode,
+   const Expr *E, BugReport ,

xazax.hun wrote:
> Do we need this overload? What about a default argument?
I don't want to expose this functionality outside of this file. I don't insist 
though!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64270/new/

https://reviews.llvm.org/D64270



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64270: [analyzer][NFC] Prepare visitors for different tracking kinds

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:81
+
+bool bugreporter::trackExpressionValue(const ExplodedNode *InputNode,
+   const Expr *E, BugReport ,

Do we need this overload? What about a default argument?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64270/new/

https://reviews.llvm.org/D64270



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64270: [analyzer][NFC] Prepare visitors for different tracking kinds

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus updated this revision to Diff 210337.
Szelethus marked an inline comment as done.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64270/new/

https://reviews.llvm.org/D64270

Files:
  clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
  clang/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -71,6 +71,20 @@
 // Utility functions.
 //===--===//
 
+/// Implementation function for trackExpressionValue().
+static bool trackExpressionValue(
+const ExplodedNode *InputNode,
+const Expr *E, BugReport ,
+bool EnableNullFPSuppression,
+bugreporter::TrackingKind TKind);
+
+bool bugreporter::trackExpressionValue(const ExplodedNode *InputNode,
+   const Expr *E, BugReport ,
+   bool EnableNullFPSuppression) {
+  return ::trackExpressionValue(InputNode, E, report, EnableNullFPSuppression,
+TrackingKind::ThoroughTracking);
+}
+
 static const Expr *peelOffPointerArithmetic(const BinaryOperator *B) {
   if (B->isAdditiveOp() && B->getType()->isPointerType()) {
 if (B->getLHS()->getType()->isPointerType()) {
@@ -1162,9 +1176,45 @@
   ID.AddPointer();
   ID.AddPointer(R);
   ID.Add(V);
+  ID.AddInteger(static_cast(TKind));
   ID.AddBoolean(EnableNullFPSuppression);
 }
 
+void FindLastStoreBRVisitor::registerStatementVarDecls(
+BugReport , const Stmt *S, bool EnableNullFPSuppression,
+TrackingKind TKind) {
+
+  const ExplodedNode *N = BR.getErrorNode();
+  std::deque WorkList;
+  WorkList.push_back(S);
+
+  while (!WorkList.empty()) {
+const Stmt *Head = WorkList.front();
+WorkList.pop_front();
+
+ProgramStateManager  = N->getState()->getStateManager();
+
+if (const auto *DR = dyn_cast(Head)) {
+  if (const auto *VD = dyn_cast(DR->getDecl())) {
+const VarRegion *R =
+StateMgr.getRegionManager().getVarRegion(VD, N->getLocationContext());
+
+// What did we load?
+SVal V = N->getSVal(S);
+
+if (V.getAs() || V.getAs()) {
+  // Register a new visitor with the BugReport.
+  BR.addVisitor(llvm::make_unique(
+  V.castAs(), R, EnableNullFPSuppression, TKind));
+}
+  }
+}
+
+for (const Stmt *SubStmt : Head->children())
+  WorkList.push_back(SubStmt);
+  }
+}
+
 /// Returns true if \p N represents the DeclStmt declaring and initializing
 /// \p VR.
 static bool isInitializationOfVar(const ExplodedNode *N, const VarRegion *VR) {
@@ -1335,7 +1385,7 @@
   // should track the initializer expression.
   if (Optional PIP = Pred->getLocationAs()) {
 const MemRegion *FieldReg = (const MemRegion *)PIP->getLocationValue();
-if (FieldReg && FieldReg == R) {
+if (FieldReg == R) {
   StoreSite = Pred;
   InitE = PIP->getInitializer()->getInit();
 }
@@ -1400,8 +1450,7 @@
 if (!IsParam)
   InitE = InitE->IgnoreParenCasts();
 
-bugreporter::trackExpressionValue(StoreSite, InitE, BR,
-  EnableNullFPSuppression);
+trackExpressionValue(StoreSite, InitE, BR, EnableNullFPSuppression, TKind);
   }
 
   // Okay, we've found the binding. Emit an appropriate message.
@@ -1429,7 +1478,7 @@
   if (const VarRegion *OriginalR = BDR->getOriginalRegion(VR)) {
 if (auto KV = State->getSVal(OriginalR).getAs())
   BR.addVisitor(llvm::make_unique(
-  *KV, OriginalR, EnableNullFPSuppression));
+  *KV, OriginalR, EnableNullFPSuppression, TKind));
   }
 }
   }
@@ -1730,8 +1779,9 @@
   // 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, /*EnableNullFPSuppression=*/false);
+trackExpressionValue(
+N, Condition, BR, /*EnableNullFPSuppression=*/false,
+bugreporter::TrackingKind::ConditionTracking);
 return constructDebugPieceForTrackedCondition(Condition, N, BRC);
   }
 }
@@ -1843,9 +1893,14 @@
   return N;
 }
 
-bool bugreporter::trackExpressionValue(const ExplodedNode *InputNode,
-   const Expr *E, BugReport ,
-   bool EnableNullFPSuppression) {
+static bool trackExpressionValue(
+const ExplodedNode *InputNode,
+const Expr *E, BugReport ,
+bool EnableNullFPSuppression,
+bugreporter::TrackingKind TKind /* = TrackingKind::ThoroughTracking*/) {
+

[PATCH] D64270: [analyzer][NFC] Prepare visitors for different tracking kinds

[Csaba Dabis via Phabricator via cfe-commits]

Charusso accepted this revision.
Charusso added a comment.

Document!!4!44! It is great you have started to limit the notes, thanks!




Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:110
+  ConditionTracking
+};
+

What about the following?
```
enum class TrackKind {
  Full, // comment what it does
  Condition // comment what it does
};
```

Please consider the following example by unspoken naming conventions:
```
enum class ColoringKind {
  RedColoring,
  BlueColoring
}
```
would be
```
enum class Color { Red, Blue }
```
where each trivial what is does, by name.

Please also note that, the thoroughness not working with redecls, assumptions, 
loop conditions, anything we failed to inline, etc... so it is not really that 
full tracking. But in our capabilities, it is the full what we could do.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64270/new/

https://reviews.llvm.org/D64270



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64270: [analyzer][NFC] Prepare visitors for different tracking kinds

[Artem Dergachev via Phabricator via cfe-commits]

NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.

Approve!

> null pointer dereference

...should in my opinion be tracked in a mild manner, as if it's a condition, 
because D62978 . I think the definition we're 
looking for is "when we care only about why it's null, regardless of what it is 
or where it comes from".

Say, in case of MallocChecker the malloc() call is the "origin" of the tracked 
pointer; we need to track it back to its original malloc. Similarly, in case of 
null dereference, the collapse point should be treated as an "origin" of the 
null pointer. The same for condition.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64270/new/

https://reviews.llvm.org/D64270



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64270: [analyzer][NFC] Prepare visitors for different tracking kinds

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus created this revision.
Szelethus added reviewers: NoQ, xazax.hun, rnkovacs, Charusso, 
baloghadamsoftware, dcoughlin.
Szelethus added a project: clang.
Herald added subscribers: cfe-commits, gamesh411, dkrupp, donat.nagy, 
mikhail.ramalho, a.sidorin, szepet, whisperity.

When we're tracking a variable that is responsible for a null pointer 
dereference or some other sinister programming error, we of course would like 
to gather as much information why we think that the variable has that specific 
value as possible. However, the newly introduced condition tracking shows that 
tracking all values this thoroughly could easily cause an intolerable growth in 
the bug report's length.

Take, for instance the following report as an example: BEFORE condition 
tracking 
,
 AFTER condition tracking 
.
 Having an already lengthy, bug report with 18 events grew to 45 is inexcusable.

There are a variety of heuristics we discussed on the mailing list 
 to combat this, 
all of them requiring to differentiate in between tracking a "regular value" 
and a "condition".

This patch introduces the new `bugreporter::TrackingKind` enum, adds it to 
`FindLastStoreBRVisitor` as a non-optional argument, and moves some functions 
around to make the code a little more coherent.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D64270

Files:
  clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
  clang/lib/StaticAnalyzer/Checkers/UndefCapturedBlockVarChecker.cpp
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -71,6 +71,20 @@
 // Utility functions.
 //===--===//
 
+/// Implementation function for trackExpressionValue().
+static bool trackExpressionValue(
+const ExplodedNode *InputNode,
+const Expr *E, BugReport ,
+bool EnableNullFPSuppression,
+bugreporter::TrackingKind TKind);
+
+bool bugreporter::trackExpressionValue(const ExplodedNode *InputNode,
+   const Expr *E, BugReport ,
+   bool EnableNullFPSuppression) {
+  return ::trackExpressionValue(InputNode, E, report, EnableNullFPSuppression,
+TrackingKind::ThoroughTracking);
+}
+
 static const Expr *peelOffPointerArithmetic(const BinaryOperator *B) {
   if (B->isAdditiveOp() && B->getType()->isPointerType()) {
 if (B->getLHS()->getType()->isPointerType()) {
@@ -1141,9 +1155,45 @@
   ID.AddPointer();
   ID.AddPointer(R);
   ID.Add(V);
+  ID.AddInteger(static_cast(TKind));
   ID.AddBoolean(EnableNullFPSuppression);
 }
 
+void FindLastStoreBRVisitor::registerStatementVarDecls(
+BugReport , const Stmt *S, bool EnableNullFPSuppression,
+TrackingKind TKind) {
+
+  const ExplodedNode *N = BR.getErrorNode();
+  std::deque WorkList;
+  WorkList.push_back(S);
+
+  while (!WorkList.empty()) {
+const Stmt *Head = WorkList.front();
+WorkList.pop_front();
+
+ProgramStateManager  = N->getState()->getStateManager();
+
+if (const auto *DR = dyn_cast(Head)) {
+  if (const auto *VD = dyn_cast(DR->getDecl())) {
+const VarRegion *R =
+StateMgr.getRegionManager().getVarRegion(VD, N->getLocationContext());
+
+// What did we load?
+SVal V = N->getSVal(S);
+
+if (V.getAs() || V.getAs()) {
+  // Register a new visitor with the BugReport.
+  BR.addVisitor(llvm::make_unique(
+  V.castAs(), R, EnableNullFPSuppression, TKind));
+}
+  }
+}
+
+for (const Stmt *SubStmt : Head->children())
+  WorkList.push_back(SubStmt);
+  }
+}
+
 /// Returns true if \p N represents the DeclStmt declaring and initializing
 /// \p VR.
 static bool isInitializationOfVar(const ExplodedNode *N, const VarRegion *VR) {
@@ -1314,7 +1364,7 @@
   // should track the initializer expression.
   if (Optional PIP = Pred->getLocationAs()) {
 const MemRegion *FieldReg = (const MemRegion *)PIP->getLocationValue();
-if (FieldReg && FieldReg == R) {
+if