[PATCH] D45682: [analyzer] Move `TaintBugVisitor` from `GenericTaintChecker.cpp` to `BugReporterVisitors.h`.
This revision was automatically updated to reflect the committed changes.
Closed by commit rC330596: [analyzer] Move `TaintBugVisitor` from
`GenericTaintChecker.cpp` to… (authored by henrywong, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D45682?vs=142621=143561#toc
Repository:
rC Clang
https://reviews.llvm.org/D45682
Files:
include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
Index: include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
===
--- include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
+++ include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
@@ -343,6 +343,22 @@
BugReport ) override;
};
+/// The bug visitor prints a diagnostic message at the location where a given
+/// variable was tainted.
+class TaintBugVisitor final : public BugReporterVisitorImpl {
+private:
+ const SVal V;
+
+public:
+ TaintBugVisitor(const SVal V) : V(V) {}
+ void Profile(llvm::FoldingSetNodeID ) const override { ID.Add(V); }
+
+ std::shared_ptr VisitNode(const ExplodedNode *N,
+ const ExplodedNode *PrevN,
+ BugReporterContext ,
+ BugReport ) override;
+};
+
namespace bugreporter {
/// Attempts to add visitors to trace a null or undefined value back to its
Index: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -2333,3 +2333,24 @@
return std::move(Piece);
}
+
+std::shared_ptr
+TaintBugVisitor::VisitNode(const ExplodedNode *N, const ExplodedNode *PrevN,
+ BugReporterContext , BugReport ) {
+
+ // Find the ExplodedNode where the taint was first introduced
+ if (!N->getState()->isTainted(V) || PrevN->getState()->isTainted(V))
+return nullptr;
+
+ const Stmt *S = PathDiagnosticLocation::getStmt(N);
+ if (!S)
+return nullptr;
+
+ const LocationContext *NCtx = N->getLocationContext();
+ PathDiagnosticLocation L =
+ PathDiagnosticLocation::createBegin(S, BRC.getSourceManager(), NCtx);
+ if (!L.isValid() || !L.asLocation().isValid())
+return nullptr;
+
+ return std::make_shared(L, "Taint originated here");
+}
Index: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -100,23 +100,6 @@
bool generateReportIfTainted(const Expr *E, const char Msg[],
CheckerContext ) const;
- /// The bug visitor prints a diagnostic message at the location where a given
- /// variable was tainted.
- class TaintBugVisitor
- : public BugReporterVisitorImpl {
- private:
-const SVal V;
-
- public:
-TaintBugVisitor(const SVal V) : V(V) {}
-void Profile(llvm::FoldingSetNodeID ) const override { ID.Add(V); }
-
-std::shared_ptr VisitNode(const ExplodedNode *N,
- const ExplodedNode *PrevN,
- BugReporterContext ,
- BugReport ) override;
- };
-
typedef SmallVector ArgVector;
/// \brief A struct used to specify taint propagation rules for a function.
@@ -214,28 +197,6 @@
/// points to data, which should be tainted on return.
REGISTER_SET_WITH_PROGRAMSTATE(TaintArgsOnPostVisit, unsigned)
-std::shared_ptr
-GenericTaintChecker::TaintBugVisitor::VisitNode(const ExplodedNode *N,
-const ExplodedNode *PrevN, BugReporterContext , BugReport ) {
-
- // Find the ExplodedNode where the taint was first introduced
- if (!N->getState()->isTainted(V) || PrevN->getState()->isTainted(V))
-return nullptr;
-
- const Stmt *S = PathDiagnosticLocation::getStmt(N);
- if (!S)
-return nullptr;
-
- const LocationContext *NCtx = N->getLocationContext();
- PathDiagnosticLocation L =
- PathDiagnosticLocation::createBegin(S, BRC.getSourceManager(), NCtx);
- if (!L.isValid() || !L.asLocation().isValid())
-return nullptr;
-
- return std::make_shared(
- L, "Taint originated here");
-}
-
GenericTaintChecker::TaintPropagationRule
GenericTaintChecker::TaintPropagationRule::getTaintPropagationRule(
const FunctionDecl *FDecl,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45682: [analyzer] Move `TaintBugVisitor` from `GenericTaintChecker.cpp` to `BugReporterVisitors.h`.
george.karpenkov accepted this revision. george.karpenkov added a comment. This revision is now accepted and ready to land. > can add extra notes like Ah right, than it's "can rely on" rather than "rely on". > it is necessary to explicitly include I've meant adding the include to `GenericTaintChecker.cpp`. LGTM otherwise. Repository: rC Clang https://reviews.llvm.org/D45682 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45682: [analyzer] Move `TaintBugVisitor` from `GenericTaintChecker.cpp` to `BugReporterVisitors.h`.
MTC added a comment. In https://reviews.llvm.org/D45682#1074407, @george.karpenkov wrote: > I'm new to the taint visitor, but I am quite confused by your change > description. > > > and many checkers rely on it > > How can other checkers rely on it if it's private to the taint checker? Thanks for your review, george! `TaintBugVisitor` is an utility to add extra information to illustrate where the taint information originated from. There are several checkers use taint information, e.g. `ArrayBoundCheckerV2.cpp`, in some cases it will report a warning, like `warning: Out of bound memory access (index is tainted)`. If `TaintBugVisitor` moves to `BugReporterVisitors.h`, `ArrayBoundCheckerV2` can add extra notes like `Taint originated here` to the report by adding `TaintBugVisitor`. > Also, it's probably to explicitly include BugReporterVisitors.h in the > checker file then. If these checkers want to add `Taint originated here` using `TaintBugVisitor`, it is necessary to explicitly include `BugReporterVisitors.h` in following patch. Repository: rC Clang https://reviews.llvm.org/D45682 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45682: [analyzer] Move `TaintBugVisitor` from `GenericTaintChecker.cpp` to `BugReporterVisitors.h`.
george.karpenkov added a comment. I'm new to the taint visitor, but I am quite confused by your change description. > and many checkers rely on it How can other checkers rely on it if it's private to the taint checker? Also, it's probably to explicitly include BugReporterVisitors.h in the checker file then. Repository: rC Clang https://reviews.llvm.org/D45682 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45682: [analyzer] Move `TaintBugVisitor` from `GenericTaintChecker.cpp` to `BugReporterVisitors.h`.
MTC created this revision.
MTC added reviewers: NoQ, george.karpenkov, xazax.hun.
Herald added subscribers: cfe-commits, a.sidorin, rnkovacs, szepet.
`TaintBugVisitor` is a universal visitor, and many checkers rely on it, such as
`ArrayBoundCheckerV2.cpp`, `DivZeroChecker.cpp` and `VLASizeChecker.cpp`.
Moving `TaintBugVisitor` to `BugReporterVisitors.h` enables other checker can
also track where `tainted` value came from.
Repository:
rC Clang
https://reviews.llvm.org/D45682
Files:
include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
Index: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -2333,3 +2333,24 @@
return std::move(Piece);
}
+
+std::shared_ptr
+TaintBugVisitor::VisitNode(const ExplodedNode *N, const ExplodedNode *PrevN,
+ BugReporterContext , BugReport ) {
+
+ // Find the ExplodedNode where the taint was first introduced
+ if (!N->getState()->isTainted(V) || PrevN->getState()->isTainted(V))
+return nullptr;
+
+ const Stmt *S = PathDiagnosticLocation::getStmt(N);
+ if (!S)
+return nullptr;
+
+ const LocationContext *NCtx = N->getLocationContext();
+ PathDiagnosticLocation L =
+ PathDiagnosticLocation::createBegin(S, BRC.getSourceManager(), NCtx);
+ if (!L.isValid() || !L.asLocation().isValid())
+return nullptr;
+
+ return std::make_shared(L, "Taint originated here");
+}
Index: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -100,23 +100,6 @@
bool generateReportIfTainted(const Expr *E, const char Msg[],
CheckerContext ) const;
- /// The bug visitor prints a diagnostic message at the location where a given
- /// variable was tainted.
- class TaintBugVisitor
- : public BugReporterVisitorImpl {
- private:
-const SVal V;
-
- public:
-TaintBugVisitor(const SVal V) : V(V) {}
-void Profile(llvm::FoldingSetNodeID ) const override { ID.Add(V); }
-
-std::shared_ptr VisitNode(const ExplodedNode *N,
- const ExplodedNode *PrevN,
- BugReporterContext ,
- BugReport ) override;
- };
-
typedef SmallVector ArgVector;
/// \brief A struct used to specify taint propagation rules for a function.
@@ -214,28 +197,6 @@
/// points to data, which should be tainted on return.
REGISTER_SET_WITH_PROGRAMSTATE(TaintArgsOnPostVisit, unsigned)
-std::shared_ptr
-GenericTaintChecker::TaintBugVisitor::VisitNode(const ExplodedNode *N,
-const ExplodedNode *PrevN, BugReporterContext , BugReport ) {
-
- // Find the ExplodedNode where the taint was first introduced
- if (!N->getState()->isTainted(V) || PrevN->getState()->isTainted(V))
-return nullptr;
-
- const Stmt *S = PathDiagnosticLocation::getStmt(N);
- if (!S)
-return nullptr;
-
- const LocationContext *NCtx = N->getLocationContext();
- PathDiagnosticLocation L =
- PathDiagnosticLocation::createBegin(S, BRC.getSourceManager(), NCtx);
- if (!L.isValid() || !L.asLocation().isValid())
-return nullptr;
-
- return std::make_shared(
- L, "Taint originated here");
-}
-
GenericTaintChecker::TaintPropagationRule
GenericTaintChecker::TaintPropagationRule::getTaintPropagationRule(
const FunctionDecl *FDecl,
Index: include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
===
--- include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
+++ include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
@@ -343,6 +343,22 @@
BugReport ) override;
};
+/// The bug visitor prints a diagnostic message at the location where a given
+/// variable was tainted.
+class TaintBugVisitor final : public BugReporterVisitorImpl {
+private:
+ const SVal V;
+
+public:
+ TaintBugVisitor(const SVal V) : V(V) {}
+ void Profile(llvm::FoldingSetNodeID ) const override { ID.Add(V); }
+
+ std::shared_ptr VisitNode(const ExplodedNode *N,
+ const ExplodedNode *PrevN,
+ BugReporterContext ,
+ BugReport ) override;
+};
+
namespace bugreporter {
/// Attempts to add visitors to trace a null or undefined value back to its
___
