[analyzer] strlcat() syntax check: Fix an off-by-one error.

[NoQ]

Summary

That's a fix on top of D49722.

Both strlcat and strlcpy cut off their safe bound argument value at sizeof(destination). There's no need to subtract 1 in one of these cases.

[dcoughlin]

Looks good to me.

lib/StaticAnalyzer/Checkers/CStringSyntaxChecker.cpp

bool WalkAST::containsBadStrncatPattern(const CallExpr *CE) {
   if (isSizeof(LenArg, SrcArg))
     return true;
   return false;
 }
 
 bool WalkAST::containsBadStrlcpyStrlcatPattern(const CallExpr *CE) {
   if (CE->getNumArgs() != 3)
     return false;
-  const FunctionDecl *FD = CE->getDirectCallee();
-  bool Append = CheckerContext::isCLibraryFunction(FD, "strlcat");
   const Expr *DstArg = CE->getArg(0);
   const Expr *LenArg = CE->getArg(2);
 
   const auto *DstArgDecl = dyn_cast<DeclRefExpr>(DstArg->IgnoreParenImpCasts());
   const auto *LenArgDecl = dyn_cast<DeclRefExpr>(LenArg->IgnoreParenLValueCasts());
   uint64_t DstOff = 0;
   if (isSizeof(LenArg, DstArg))
     return false;
if (!DstArgDecl) {
         }
       }
     }
     if (DstArgDecl) {
       if (const auto *Buffer = dyn_cast<ConstantArrayType>(DstArgDecl->getType())) {
         ASTContext &C = BR.getContext();
         uint64_t BufferLen = C.getTypeSize(Buffer) / 8;
         auto RemainingBufferLen = BufferLen - DstOff;
-        if (Append) {
-          if (RemainingBufferLen <= ILRawVal)
-            return true;
-        } else {
-          if (RemainingBufferLen < ILRawVal)
-            return true;
-        }
-      }
-    }
-  }
 
   return false;
 }
 
 void WalkAST::VisitCallExpr(CallExpr *CE) {
   const FunctionDecl *FD = CE->getDirectCallee();
   if (!FD)
     return;

test/Analysis/cstring-syntax.c

void testStrlcpy(const char *src) {
   strlcpy(dest, src, sizeof(dest));
   strlcpy(dest, src, destlen);
   strlcpy(dest, src, 10);
   strlcpy(dest, src, 20); // expected-warning {{The third argument allows to potentially copy more bytes than it should. Replace with the value sizeof(dest) or lower}}
   strlcpy(dest, src, badlen); // expected-warning {{The third argument allows to potentially copy more bytes than it should. Replace with the value sizeof(dest) or lower}}
   strlcpy(dest, src, ulen);
   strlcpy(dest + 5, src, 5);
   strlcpy(dest + 5, src, 10); // expected-warning {{The third argument allows to potentially copy more bytes than it should. Replace with the value sizeof(<destination buffer>) or lower}}
+  strlcpy(dest, "aaaaaaaaaaaaaaa", 10); // no-warning
 }
 
 void testStrlcat(const char *src) {
   char dest[10];
   size_t badlen = 20;
   size_t ulen;
   strlcpy(dest, "aaaaa", sizeof("aaaaa") - 1);
   strlcat(dest, "bbbb", (sizeof("bbbb") - 1) - sizeof(dest) - 1);
   strlcpy(dest, "012345678", sizeof(dest));
   strlcat(dest, "910", sizeof(dest));
   strlcpy(dest, "0123456789", sizeof(dest));
   strlcpy(dest, "0123456789", sizeof(dest));
   strlcat(dest, "0123456789", badlen / 2);
   strlcat(dest, "0123456789", badlen); // expected-warning {{The third argument allows to potentially copy more bytes than it should. Replace with the value sizeof(dest) or lower}}
   strlcat(dest, "0123456789", badlen - strlen(dest) - 1);
   strlcat(dest, src, ulen);
   strlcpy(dest, src, 5);
   strlcat(dest + 5, src, badlen); // expected-warning {{The third argument allows to potentially copy more bytes than it should. Replace with the value sizeof(<destination buffer>) or lower}}
+  strlcat(dest, "aaaaaaaaaaaaaaa", 10); // no-warning
 }