[clang][Analyzer] Add symbol uninterestingness to bug report.

`PathSensitiveBughReport` has a function to mark a symbol as interesting but
it was not possible to clear this flag. This can be useful in some cases,
so the functionality is added.

Reviewed By: NoQ

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

clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h

@@ -432,6 +432,8 @@ public:
   void markInteresting(SymbolRef sym, bugreporter::TrackingKind TKind =
                                           bugreporter::TrackingKind::Thorough);
 
+  void markNotInteresting(SymbolRef sym);
+
   /// Marks a region as interesting. Different kinds of interestingness will
   /// be processed differently by visitors (e.g. if the tracking kind is
   /// condition, will append "will be used as a condition" to the message).
@@ -439,6 +441,8 @@ public:
       const MemRegion *R,
       bugreporter::TrackingKind TKind = bugreporter::TrackingKind::Thorough);
 
+  void markNotInteresting(const MemRegion *R);
+
   /// Marks a symbolic value as interesting. Different kinds of interestingness
   /// will be processed differently by visitors (e.g. if the tracking kind is
   /// condition, will append "will be used as a condition" to the message).

clang/lib/StaticAnalyzer/Core/BugReporter.cpp

@@ -2249,10 +2249,22 @@ void PathSensitiveBugReport::markInteresting(SymbolRef sym,
 
   insertToInterestingnessMap(InterestingSymbols, sym, TKind);
 
+  // FIXME: No tests exist for this code and it is questionable:
+  // How to handle multiple metadata for the same region?
   if (const auto *meta = dyn_cast<SymbolMetadata>(sym))
     markInteresting(meta->getRegion(), TKind);
 }
 
+void PathSensitiveBugReport::markNotInteresting(SymbolRef sym) {
+  if (!sym)
+    return;
+  InterestingSymbols.erase(sym);
+
+  // The metadata part of markInteresting is not reversed here.
+  // Just making the same region not interesting is incorrect
+  // in specific cases.
+}
+
 void PathSensitiveBugReport::markInteresting(const MemRegion *R,
                                              bugreporter::TrackingKind TKind) {
   if (!R)
@@ -2265,6 +2277,17 @@ void PathSensitiveBugReport::markInteresting(const MemRegion *R,
     markInteresting(SR->getSymbol(), TKind);
 }
 
+void PathSensitiveBugReport::markNotInteresting(const MemRegion *R) {
+  if (!R)
+    return;
+
+  R = R->getBaseRegion();
+  InterestingRegions.erase(R);
+
+  if (const auto *SR = dyn_cast<SymbolicRegion>(R))
+    markNotInteresting(SR->getSymbol());
+}
+
 void PathSensitiveBugReport::markInteresting(SVal V,
                                              bugreporter::TrackingKind TKind) {
   markInteresting(V.getAsRegion(), TKind);

clang/unittests/StaticAnalyzer/BugReportInterestingnessTest.cpp

@@ -0,0 +1,162 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "Reusables.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
+#include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Frontend/AnalysisConsumer.h"
+#include "clang/StaticAnalyzer/Frontend/CheckerRegistry.h"
+#include "clang/Tooling/Tooling.h"
+#include "gtest/gtest.h"
+
+using namespace clang;
+using namespace ento;
+using namespace llvm;
+
+namespace {
+
+class InterestingnessTestChecker : public Checker<check::PreCall> {
+  BugType BT_TestBug;
+
+  using HandlerFn = std::function<void(const InterestingnessTestChecker *,
+                                       const CallEvent &, CheckerContext &)>;
+
+  CallDescriptionMap<HandlerFn> Handlers = {
+      {{"setInteresting", 1}, &InterestingnessTestChecker::handleInteresting},
+      {{"setNotInteresting", 1},
+       &InterestingnessTestChecker::handleNotInteresting},
+      {{"check", 1}, &InterestingnessTestChecker::handleCheck},
+      {{"bug", 1}, &InterestingnessTestChecker::handleBug},
+  };
+
+  void handleInteresting(const CallEvent &Call, CheckerContext &C) const;
+  void handleNotInteresting(const CallEvent &Call, CheckerContext &C) const;
+  void handleCheck(const CallEvent &Call, CheckerContext &C) const;
+  void handleBug(const CallEvent &Call, CheckerContext &C) const;
+
+public:
+  InterestingnessTestChecker()
+      : BT_TestBug(this, "InterestingnessTestBug", "Test") {}
+
+  void checkPreCall(const CallEvent &Call, CheckerContext &C) const {
+    const HandlerFn *Handler = Handlers.lookup(Call);
+    if (!Handler)
+      return;
+
+    (*Handler)(this, Call, C);
+  }
+};
+
+} // namespace
+
+void InterestingnessTestChecker::handleInteresting(const CallEvent &Call,
+                                                   CheckerContext &C) const {
+  SymbolRef Sym = Call.getArgSVal(0).getAsSymbol();
+  assert(Sym);
+  C.addTransition(nullptr, C.getNoteTag([Sym](PathSensitiveBugReport &BR) {
+    BR.markInteresting(Sym);
+    return "";
+  }));
+}
+
+void InterestingnessTestChecker::handleNotInteresting(const CallEvent &Call,
+                                                      CheckerContext &C) const {
+  SymbolRef Sym = Call.getArgSVal(0).getAsSymbol();
+  assert(Sym);
+  C.addTransition(nullptr, C.getNoteTag([Sym](PathSensitiveBugReport &BR) {
+    BR.markNotInteresting(Sym);
+    return "";
+  }));
+}
+
+void InterestingnessTestChecker::handleCheck(const CallEvent &Call,
+                                             CheckerContext &C) const {
+  SymbolRef Sym = Call.getArgSVal(0).getAsSymbol();
+  assert(Sym);
+  C.addTransition(nullptr, C.getNoteTag([Sym](PathSensitiveBugReport &BR) {
+    if (BR.isInteresting(Sym))
+      return "Interesting";
+    else
+      return "NotInteresting";
+  }));
+}
+
+void InterestingnessTestChecker::handleBug(const CallEvent &Call,
+                                           CheckerContext &C) const {
+  ExplodedNode *N = C.generateErrorNode();
+  C.emitReport(
+      std::make_unique<PathSensitiveBugReport>(BT_TestBug, "test bug", N));
+}
+
+namespace {
+
+class TestAction : public ASTFrontendAction {
+  ExpectedDiagsTy ExpectedDiags;
+
+public:
+  TestAction(ExpectedDiagsTy &&ExpectedDiags)
+      : ExpectedDiags(std::move(ExpectedDiags)) {}
+
+  std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &Compiler,
+                                                 StringRef File) override {
+    std::unique_ptr<AnalysisASTConsumer> AnalysisConsumer =
+        CreateAnalysisConsumer(Compiler);
+    AnalysisConsumer->AddDiagnosticConsumer(new VerifyPathDiagnosticConsumer(
+        std::move(ExpectedDiags), Compiler.getSourceManager()));
+    AnalysisConsumer->AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
+      Registry.addChecker<InterestingnessTestChecker>("test.Interestingness",
+                                                      "Description", "");
+    });
+    Compiler.getAnalyzerOpts()->CheckersAndPackages = {
+        {"test.Interestingness", true}};
+    return std::move(AnalysisConsumer);
+  }
+};
+
+} // namespace
+
+TEST(BugReportInterestingness, Symbols) {
+  EXPECT_TRUE(tooling::runToolOnCode(
+      std::make_unique<TestAction>(ExpectedDiagsTy{
+          {{15, 7},
+           "test bug",
+           "test bug",
+           "test.Interestingness",
+           "InterestingnessTestBug",
+           "Test",
+           {
+               {{8, 7}, "Interesting", {{{8, 7}, {8, 14}}}},
+               {{10, 7}, "NotInteresting", {{{10, 7}, {10, 14}}}},
+               {{12, 7}, "Interesting", {{{12, 7}, {12, 14}}}},
+               {{14, 7}, "NotInteresting", {{{14, 7}, {14, 14}}}},
+               {{15, 7}, "test bug", {{{15, 7}, {15, 12}}}},
+           }}}),
+      R"(
+    void setInteresting(int);
+    void setNotInteresting(int);
+    void check(int);
+    void bug(int);
+
+    void f(int A) {
+      check(A);
+      setInteresting(A);
+      check(A);
+      setNotInteresting(A);
+      check(A);
+      setInteresting(A);
+      check(A);
+      bug(A);
+    }
+  )",
+      "input.cpp"));
+}

clang/unittests/StaticAnalyzer/CMakeLists.txt

@@ -5,6 +5,7 @@ set(LLVM_LINK_COMPONENTS
 
 add_clang_unittest(StaticAnalysisTests
   AnalyzerOptionsTest.cpp
+  BugReportInterestingnessTest.cpp
   CallDescriptionTest.cpp
   CallEventTest.cpp
   FalsePositiveRefutationBRVisitorTest.cpp

clang/unittests/StaticAnalyzer/Reusables.h

@@ -10,9 +10,10 @@
 #define LLVM_CLANG_UNITTESTS_STATICANALYZER_REUSABLES_H
 
 #include "clang/ASTMatchers/ASTMatchFinder.h"
-#include "clang/Frontend/CompilerInstance.h"
 #include "clang/CrossTU/CrossTranslationUnit.h"
+#include "clang/Frontend/CompilerInstance.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
+#include "gtest/gtest.h"
 
 namespace clang {
 namespace ento {
@@ -66,6 +67,88 @@ public:
         Eng(CTU, AMgr, &VisitedCallees, &FS, ExprEngine::Inline_Regular) {}
 };
 
+struct ExpectedLocationTy {
+  unsigned Line;
+  unsigned Column;
+
+  void testEquality(SourceLocation L, SourceManager &SM) const {
+    EXPECT_EQ(SM.getSpellingLineNumber(L), Line);
+    EXPECT_EQ(SM.getSpellingColumnNumber(L), Column);
+  }
+};
+
+struct ExpectedRangeTy {
+  ExpectedLocationTy Begin;
+  ExpectedLocationTy End;
+
+  void testEquality(SourceRange R, SourceManager &SM) const {
+    Begin.testEquality(R.getBegin(), SM);
+    End.testEquality(R.getEnd(), SM);
+  }
+};
+
+struct ExpectedPieceTy {
+  ExpectedLocationTy Loc;
+  std::string Text;
+  std::vector<ExpectedRangeTy> Ranges;
+
+  void testEquality(const PathDiagnosticPiece &Piece, SourceManager &SM) {
+    Loc.testEquality(Piece.getLocation().asLocation(), SM);
+    EXPECT_EQ(Piece.getString(), Text);
+    EXPECT_EQ(Ranges.size(), Piece.getRanges().size());
+    for (const auto &RangeItem : llvm::enumerate(Piece.getRanges()))
+      Ranges[RangeItem.index()].testEquality(RangeItem.value(), SM);
+  }
+};
+
+struct ExpectedDiagTy {
+  ExpectedLocationTy Loc;
+  std::string VerboseDescription;
+  std::string ShortDescription;
+  std::string CheckerName;
+  std::string BugType;
+  std::string Category;
+  std::vector<ExpectedPieceTy> Path;
+
+  void testEquality(const PathDiagnostic &Diag, SourceManager &SM) {
+    Loc.testEquality(Diag.getLocation().asLocation(), SM);
+    EXPECT_EQ(Diag.getVerboseDescription(), VerboseDescription);
+    EXPECT_EQ(Diag.getShortDescription(), ShortDescription);
+    EXPECT_EQ(Diag.getCheckerName(), CheckerName);
+    EXPECT_EQ(Diag.getBugType(), BugType);
+    EXPECT_EQ(Diag.getCategory(), Category);
+
+    EXPECT_EQ(Path.size(), Diag.path.size());
+    for (const auto &PieceItem : llvm::enumerate(Diag.path)) {
+      if (PieceItem.index() < Path.size())
+        Path[PieceItem.index()].testEquality(*PieceItem.value(), SM);
+    }
+  }
+};
+
+using ExpectedDiagsTy = std::vector<ExpectedDiagTy>;
+
+// A consumer to verify the generated diagnostics.
+class VerifyPathDiagnosticConsumer : public PathDiagnosticConsumer {
+  ExpectedDiagsTy ExpectedDiags;
+  SourceManager &SM;
+
+public:
+  VerifyPathDiagnosticConsumer(ExpectedDiagsTy &&ExpectedDiags,
+                               SourceManager &SM)
+      : ExpectedDiags(ExpectedDiags), SM(SM) {}
+
+  StringRef getName() const override { return "verify test diagnostics"; }
+
+  void FlushDiagnosticsImpl(std::vector<const PathDiagnostic *> &Diags,
+                            FilesMade *filesMade) override {
+    EXPECT_EQ(Diags.size(), ExpectedDiags.size());
+    for (const auto &Item : llvm::enumerate(Diags))
+      if (Item.index() < ExpectedDiags.size())
+        ExpectedDiags[Item.index()].testEquality(*Item.value(), SM);
+  }
+};
+
 } // namespace ento
 } // namespace clang