[PATCH] D49722: [CStringSyntaxChecker] Check strlcat sizeof check

George Karpenkov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jul 30 11:54:24 PDT 2018


george.karpenkov requested changes to this revision.
george.karpenkov added inline comments.
This revision now requires changes to proceed.


================
Comment at: lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp:154
 
-bool WalkAST::containsBadStrlcpyPattern(const CallExpr *CE) {
+bool WalkAST::containsBadStrlcpyStrlcatPattern(const CallExpr *CE, bool Append) {
   if (CE->getNumArgs() != 3)
----------------
`Append` can be deduced from a call expression; I think it would be better to drop it from the parameter list and calculate again in the function body.


================
Comment at: lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp:164
+  // - sizeof(dst)
+  if (Append && isSizeof(LenArg, DstArg))
+    return true;
----------------
I am confused on what is happening in this branch and why is it bad. Could we add a commend?


================
Comment at: lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp:196
         uint64_t BufferLen = C.getTypeSize(Buffer) / 8;
-        if ((BufferLen - DstOff) < ILRawVal)
-          return true;
+        BufferLen -= DstOff;
+        if (Append) {
----------------
Better to introduce a new variable, e.g. `RemainingBufferLen`


https://reviews.llvm.org/D49722





More information about the cfe-commits mailing list