Extend -Wno-sizeof-array-argument to strncpy and friends.

This finds 2 bugs in chromium and 1 in hunspell, with 0 false positives.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@141902 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index f4b4c70a4d957263c002b9cb5b4d623ca51c4503..dabbcb4734205f9f4b5c05a5e56feb0d63ff7e4b 100644 (file)
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -6113,7 +6113,12 @@ private:
     CMF_Memset,
     CMF_Memcpy,
     CMF_Memmove,
-    CMF_Memcmp
+    CMF_Memcmp,
+    CMF_Strncpy,
+    CMF_Strncmp,
+    CMF_Strncasecmp,
+    CMF_Strncat,
+    CMF_Strndup
   };
   
   void CheckMemaccessArguments(const CallExpr *Call, CheckedMemoryFunction CMF,

diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp
index a9d79bb70a0254505bf876b68f5f9ca0c81e029a..e95feacbab5da0ab65d1b30ea0217748d374d111 100644 (file)
--- a/lib/Sema/SemaChecking.cpp
+++ b/lib/Sema/SemaChecking.cpp
@@ -390,6 +390,30 @@ bool Sema::CheckFunctionCall(FunctionDecl *FDecl, CallExpr *TheCall) {
     CMF = CMF_Memcmp;
     break;
     
+  case Builtin::BI__builtin_strncpy:
+  case Builtin::BI__builtin___strncpy_chk:
+  case Builtin::BIstrncpy:
+    CMF = CMF_Strncpy;
+    break;
+
+  case Builtin::BI__builtin_strncmp:
+    CMF = CMF_Strncmp;
+    break;
+
+  case Builtin::BI__builtin_strncasecmp:
+    CMF = CMF_Strncasecmp;
+    break;
+
+  case Builtin::BI__builtin_strncat:
+  case Builtin::BIstrncat:
+    CMF = CMF_Strncat;
+    break;
+
+  case Builtin::BI__builtin_strndup:
+  case Builtin::BIstrndup:
+    CMF = CMF_Strndup;
+    break;
+
   default:
     if (FDecl->getLinkage() == ExternalLinkage &&
         (!getLangOptions().CPlusPlus || FDecl->isExternC())) {
@@ -401,6 +425,16 @@ bool Sema::CheckFunctionCall(FunctionDecl *FDecl, CallExpr *TheCall) {
         CMF = CMF_Memmove;
       else if (FnInfo->isStr("memcmp"))
         CMF = CMF_Memcmp;
+      else if (FnInfo->isStr("strncpy"))
+        CMF = CMF_Strncpy;
+      else if (FnInfo->isStr("strncmp"))
+        CMF = CMF_Strncmp;
+      else if (FnInfo->isStr("strncasecmp"))
+        CMF = CMF_Strncasecmp;
+      else if (FnInfo->isStr("strncat"))
+        CMF = CMF_Strncat;
+      else if (FnInfo->isStr("strndup"))
+        CMF = CMF_Strndup;
     }
     break;
   }
@@ -2124,11 +2158,13 @@ void Sema::CheckMemaccessArguments(const CallExpr *Call,
                                    IdentifierInfo *FnName) {
   // It is possible to have a non-standard definition of memset.  Validate
   // we have enough arguments, and if not, abort further checking.
-  if (Call->getNumArgs() < 3)
+  unsigned ExpectedNumArgs = (CMF == CMF_Strndup ? 2 : 3);
+  if (Call->getNumArgs() < ExpectedNumArgs)
     return;
 
-  unsigned LastArg = (CMF == CMF_Memset? 1 : 2);
-  const Expr *LenExpr = Call->getArg(2)->IgnoreParenImpCasts();
+  unsigned LastArg = (CMF == CMF_Memset || CMF == CMF_Strndup ? 1 : 2);
+  unsigned LenArg = (CMF == CMF_Strndup ? 1 : 2);
+  const Expr *LenExpr = Call->getArg(LenArg)->IgnoreParenImpCasts();
 
   // We have special checking when the length is a sizeof expression.
   QualType SizeOfArgTy = getSizeOfArgType(LenExpr);
@@ -2162,6 +2198,8 @@ void Sema::CheckMemaccessArguments(const CallExpr *Call,
         llvm::FoldingSetNodeID DestID;
         Dest->Profile(DestID, Context, true);
         if (DestID == SizeOfArgID) {
+          // TODO: For strncpy() and friends, this could suggest sizeof(dst)
+          //       over sizeof(src) as well.
           unsigned ActionIdx = 0; // Default is to suggest dereferencing.
           if (const UnaryOperator *UnaryOp = dyn_cast<UnaryOperator>(Dest))
             if (UnaryOp->getOpcode() == UO_AddrOf)
@@ -2169,9 +2207,10 @@ void Sema::CheckMemaccessArguments(const CallExpr *Call,
           if (Context.getTypeSize(PointeeTy) == Context.getCharWidth())
             ActionIdx = 2; // If the pointee's size is sizeof(char),
                            // suggest an explicit length.
+          unsigned DestSrcSelect = (CMF == CMF_Strndup ? 1 : ArgIdx);
           DiagRuntimeBehavior(SizeOfArg->getExprLoc(), Dest,
                               PDiag(diag::warn_sizeof_pointer_expr_memaccess)
-                                << FnName << ArgIdx << ActionIdx
+                                << FnName << DestSrcSelect << ActionIdx
                                 << Dest->getSourceRange()
                                 << SizeOfArg->getSourceRange());
           break;

diff --git a/test/SemaCXX/warn-memset-bad-sizeof.cpp b/test/SemaCXX/warn-memset-bad-sizeof.cpp
index f4876ab442fc5488072002fe2b1c3f079b7df11b..a018223cbdae312d5ce59b73da633de1b45f40de 100644 (file)
--- a/test/SemaCXX/warn-memset-bad-sizeof.cpp
+++ b/test/SemaCXX/warn-memset-bad-sizeof.cpp
@@ -112,3 +112,28 @@ void f(int* i) {
   memset(i, 0, sizeof(i));
 }
 }
+
+extern "C" int strncmp(const char *s1, const char *s2, unsigned n);
+extern "C" int strncasecmp(const char *s1, const char *s2, unsigned n);
+extern "C" char *strncpy(char *det, const char *src, unsigned n);
+extern "C" char *strncat(char *dst, const char *src, unsigned n);
+extern "C" char *strndup(const  char *src, unsigned n);
+
+void strcpy_and_friends() {
+  const char* FOO = "<- should be an array instead";
+  const char* BAR = "<- this, too";
+
+  strncmp(FOO, BAR, sizeof(FOO)); // \
+      // expected-warning {{argument to 'sizeof' in 'strncmp' call is the same expression as the destination}}
+  strncasecmp(FOO, BAR, sizeof(FOO));  // \
+      // expected-warning {{argument to 'sizeof' in 'strncasecmp' call is the same expression as the destination}}
+
+  char buff[80];
+
+  strncpy(buff, BAR, sizeof(BAR)); // \
+      // expected-warning {{argument to 'sizeof' in 'strncpy' call is the same expression as the source}}
+  strncat(buff, BAR, sizeof(BAR)); // \
+      // expected-warning {{argument to 'sizeof' in 'strncat' call is the same expression as the source}}
+  strndup(FOO, sizeof(FOO)); // \
+      // expected-warning {{argument to 'sizeof' in 'strndup' call is the same expression as the source}}
+}