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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
