[clang] b0d38ad - [clang][Analyzer] Add symbol uninterestingness to bug report.
Balázs Kéri via cfe-commits
cfe-commits at lists.llvm.org
Thu Jul 15 00:50:35 PDT 2021
Author: Balázs Kéri
Date: 2021-07-15T10:02:18+02:00
New Revision: b0d38ad0bc254b887123cd063a5f0db30a80f938
URL: https://github.com/llvm/llvm-project/commit/b0d38ad0bc254b887123cd063a5f0db30a80f938
DIFF: https://github.com/llvm/llvm-project/commit/b0d38ad0bc254b887123cd063a5f0db30a80f938.diff
LOG: [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
Added:
clang/unittests/StaticAnalyzer/BugReportInterestingnessTest.cpp
Modified:
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
clang/lib/StaticAnalyzer/Core/BugReporter.cpp
clang/unittests/StaticAnalyzer/CMakeLists.txt
clang/unittests/StaticAnalyzer/Reusables.h
Removed:
################################################################################
diff --git a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
index 99cd24a52f2df..3c93ebeccde8d 100644
--- a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
+++ b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
@@ -432,6 +432,8 @@ class PathSensitiveBugReport : public BugReport {
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
diff erently 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 @@ class PathSensitiveBugReport : public BugReport {
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
diff erently by visitors (e.g. if the tracking kind is
/// condition, will append "will be used as a condition" to the message).
diff --git a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
index 6ace986b4e0df..cf4581e8f67b9 100644
--- a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ b/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);
diff --git a/clang/unittests/StaticAnalyzer/BugReportInterestingnessTest.cpp b/clang/unittests/StaticAnalyzer/BugReportInterestingnessTest.cpp
new file mode 100644
index 0000000000000..bc1b8edf05356
--- /dev/null
+++ b/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"));
+}
diff --git a/clang/unittests/StaticAnalyzer/CMakeLists.txt b/clang/unittests/StaticAnalyzer/CMakeLists.txt
index 4de6bec4d2167..d131e03057b5d 100644
--- a/clang/unittests/StaticAnalyzer/CMakeLists.txt
+++ b/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
diff --git a/clang/unittests/StaticAnalyzer/Reusables.h b/clang/unittests/StaticAnalyzer/Reusables.h
index 3f2fd61642008..609983e783ffd 100644
--- a/clang/unittests/StaticAnalyzer/Reusables.h
+++ b/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 @@ class ExprEngineConsumer : public ASTConsumer {
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
More information about the cfe-commits
mailing list