[PATCH] D59637: [analyzer] Use the custom propagation rules and sinks in GenericTaintChecker

[Artem Dergachev via Phabricator via cfe-commits]

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