[PATCH] D59637: [analyzer] Use the custom propagation rules and sinks in GenericTaintChecker
Artem Dergachev via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Aug 30 15:00:08 PDT 2019
NoQ added a comment.
I have a few immediate style issues but otherwise it looks good :)
================
Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:237
-const char GenericTaintChecker::MsgUncontrolledFormatString[] =
+const llvm::StringLiteral GenericTaintChecker::MsgUncontrolledFormatString =
"Untrusted data is used as a format string "
----------------
`StringLiteral`s need to always be declared as `constexpr`.
================
Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:251
+const llvm::StringLiteral GenericTaintChecker::MsgCustomSink =
+ "Untrusted data is passed to a user defined sink";
+;
----------------
It should be "user-defined" because it's a single adjective.
I recommend against using the word "sink" in user-facing messages because it's too jargony. Do we have a better word for this?
================
Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:252
+ "Untrusted data is passed to a user defined sink";
+;
} // end of anonymous namespace
----------------
This semicolon looks unnecessary.
================
Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:595-596
+ StringRef Name = C.getCalleeName(C.getCalleeDecl(CE));
+ llvm::errs() << "Skip out of bound SrcArg: " << Name << ":" << ArgNum
+ << '\n';
+ continue;
----------------
These debug prints should not land.
================
Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:840
+
+ const GenericTaintChecker::ArgVector &Args = It->getValue();
+ for (unsigned ArgNum : Args) {
----------------
The `GenericTaintChecker::` qualifier is unnecessary.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59637/new/
https://reviews.llvm.org/D59637
More information about the cfe-commits
mailing list