[cfe-commits] r141902 - in /cfe/trunk: include/clang/Sema/Sema.h lib/Sema/SemaChecking.cpp test/SemaCXX/warn-memset-bad-sizeof.cpp

[Nico Weber]

Author: nico
Date: Thu Oct 13 17:30:23 2011
New Revision: 141902

URL: http://llvm.org/viewvc/llvm-project?rev=141902&view=rev
Log:
Extend -Wno-sizeof-array-argument to strncpy and friends.

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


Modified:
    cfe/trunk/include/clang/Sema/Sema.h
    cfe/trunk/lib/Sema/SemaChecking.cpp
    cfe/trunk/test/SemaCXX/warn-memset-bad-sizeof.cpp

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=141902&r1=141901&r2=141902&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Thu Oct 13 17:30:23 2011
@@ -6113,7 +6113,12 @@
     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,

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=141902&r1=141901&r2=141902&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Thu Oct 13 17:30:23 2011
@@ -390,6 +390,30 @@
     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 @@
         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 @@
                                    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 @@
         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 @@
           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;

Modified: cfe/trunk/test/SemaCXX/warn-memset-bad-sizeof.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-memset-bad-sizeof.cpp?rev=141902&r1=141901&r2=141902&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/warn-memset-bad-sizeof.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-memset-bad-sizeof.cpp Thu Oct 13 17:30:23 2011
@@ -112,3 +112,28 @@
   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}}
+}