[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

It'd look good in clang-tidy, but if Daniel is interested in having this feature in the analyzer (and picked by clang-tidy from there), i wouldn't mind.

I wonder how noisy this check is - did you test it on large codebases? Because these functions are popular, and in many cases it'd be fine to use insecure functions, i wonder if it's worth it to have this check on by default. Like, if it's relatively quiet - it's fine, but if it'd constitute 90% of the analyzer's warnings on popular projects, that'd probably not be fine.



================
Comment at: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp:597-598
+
+  if(!BR.getContext().getLangOpts().C11)
+    return;
+
----------------
Note that you cannot easily figure out if the code is intended to get compiled only under C11 and above - maybe it's accidentally compiled under C11 for this user, but is otherwise intended to keep working under older standards.


================
Comment at: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp:632
+
+void WalkAST::checkUnsafeBufferHandling(const CallExpr *CE, const FunctionDecl *FD) { //TODO:TESTS
+  if (!filter.check_UnsafeBufferHandling)
----------------
Because it also checks deprecated buffer handling, i'd rename this function to `checkDeprecatedOrUnsafeBufferHandling`.


================
Comment at: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp:670-675
+  auto FormatString =
+    dyn_cast<StringLiteral>(CE->getArg(ArgIndex)->IgnoreParenImpCasts());
+  if(FormatString &&
+     FormatString->getString().find("%s") == StringRef::npos &&
+     FormatString->getString().find("%[") == StringRef::npos)
+    return;
----------------
You'd probably also want to quit early if the format string is not a literal.


Repository:
  rL LLVM

https://reviews.llvm.org/D35068