[clang] b16a593 - [analyzer][NFC] Pass the diagnostic message to the TrackConstraintBRVisitor

[Donát Nagy via cfe-commits]

Author: Endre Fulop
Date: 2023-06-06T16:28:31+02:00
New Revision: b16a59328fc5120006aeac501637229cc7e30357

URL: https://github.com/llvm/llvm-project/commit/b16a59328fc5120006aeac501637229cc7e30357
DIFF: https://github.com/llvm/llvm-project/commit/b16a59328fc5120006aeac501637229cc7e30357.diff

LOG: [analyzer][NFC] Pass the diagnostic message to the TrackConstraintBRVisitor

The `TrackConstraintBRVisitor` should accept a message for the note
instead of creating one. It would let us inject domain-specific
knowledge in a non-intrusive way, leading to a more generic visitor.

Differential Revision: https://reviews.llvm.org/D152255

Added: 
    

Modified: 
    clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
    clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
index 9c81e02245ecd..d9b3d9352d322 100644
--- a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
+++ b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
@@ -392,19 +392,19 @@ const Expr *getDerefExpr(const Stmt *S);
 } // namespace bugreporter
 
 class TrackConstraintBRVisitor final : public BugReporterVisitor {
-  DefinedSVal Constraint;
-  bool Assumption;
+  const SmallString<64> Message;
+  const DefinedSVal Constraint;
+  const bool Assumption;
   bool IsSatisfied = false;
-  bool IsZeroCheck;
 
   /// We should start tracking from the last node along the path in which the
   /// value is constrained.
   bool IsTrackingTurnedOn = false;
 
 public:
-  TrackConstraintBRVisitor(DefinedSVal constraint, bool assumption)
-      : Constraint(constraint), Assumption(assumption),
-        IsZeroCheck(!Assumption && isa<Loc>(Constraint)) {}
+  TrackConstraintBRVisitor(DefinedSVal constraint, bool assumption,
+                           StringRef Message)
+      : Message(Message), Constraint(constraint), Assumption(assumption) {}
 
   void Profile(llvm::FoldingSetNodeID &ID) const override;
 
@@ -417,6 +417,9 @@ class TrackConstraintBRVisitor final : public BugReporterVisitor {
                                    PathSensitiveBugReport &BR) override;
 
 private:
+  /// Checks if the constraint refers to a null-location.
+  bool isZeroCheck() const;
+
   /// Checks if the constraint is valid in the current state.
   bool isUnderconstrained(const ExplodedNode *N) const;
 };

diff  --git a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
index 2b461acf9a731..42d03f67510cf 100644
--- a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -1786,6 +1786,7 @@ PathDiagnosticPieceRef StoreSiteFinder::VisitNode(const ExplodedNode *Succ,
 void TrackConstraintBRVisitor::Profile(llvm::FoldingSetNodeID &ID) const {
   static int tag = 0;
   ID.AddPointer(&tag);
+  ID.AddString(Message);
   ID.AddBoolean(Assumption);
   ID.Add(Constraint);
 }
@@ -1796,8 +1797,12 @@ const char *TrackConstraintBRVisitor::getTag() {
   return "TrackConstraintBRVisitor";
 }
 
+bool TrackConstraintBRVisitor::isZeroCheck() const {
+  return !Assumption && Constraint.getAs<Loc>();
+}
+
 bool TrackConstraintBRVisitor::isUnderconstrained(const ExplodedNode *N) const {
-  if (IsZeroCheck)
+  if (isZeroCheck())
     return N->getState()->isNull(Constraint).isUnderconstrained();
   return (bool)N->getState()->assume(Constraint, !Assumption);
 }
@@ -1827,19 +1832,6 @@ PathDiagnosticPieceRef TrackConstraintBRVisitor::VisitNode(
     // the transition point.
     assert(!isUnderconstrained(N));
 
-    // We found the transition point for the constraint.  We now need to
-    // pretty-print the constraint. (work-in-progress)
-    SmallString<64> sbuf;
-    llvm::raw_svector_ostream os(sbuf);
-
-    if (isa<Loc>(Constraint)) {
-      os << "Assuming pointer value is ";
-      os << (Assumption ? "non-null" : "null");
-    }
-
-    if (os.str().empty())
-      return nullptr;
-
     // Construct a new PathDiagnosticPiece.
     ProgramPoint P = N->getLocation();
 
@@ -1854,7 +1846,7 @@ PathDiagnosticPieceRef TrackConstraintBRVisitor::VisitNode(
     if (!L.isValid())
       return nullptr;
 
-    auto X = std::make_shared<PathDiagnosticEventPiece>(L, os.str());
+    auto X = std::make_shared<PathDiagnosticEventPiece>(L, Message);
     X->setTag(getTag());
     return std::move(X);
   }
@@ -2366,8 +2358,9 @@ class InterestingLValueHandler final : public ExpressionHandler {
         // null.
         if (V.getAsLocSymbol(/*IncludeBaseRegions=*/true))
           if (LVState->isNull(V).isConstrainedTrue())
-            Report.addVisitor<TrackConstraintBRVisitor>(V.castAs<DefinedSVal>(),
-                                                        false);
+            Report.addVisitor<TrackConstraintBRVisitor>(
+                V.castAs<DefinedSVal>(),
+                /*Assumption=*/false, "Assuming pointer value is null");
 
         // Add visitor, which will suppress inline defensive checks.
         if (auto DV = V.getAs<DefinedSVal>())
@@ -2531,7 +2524,7 @@ class DefaultExpressionHandler final : public ExpressionHandler {
         Report.markInteresting(RegionRVal, Opts.Kind);
         Report.addVisitor<TrackConstraintBRVisitor>(
             loc::MemRegionVal(RegionRVal),
-            /*assumption=*/false);
+            /*Assumption=*/false, "Assuming pointer value is null");
         Result.FoundSomethingToTrack = true;
       }
     }