Warn on memset(ptr, 0, sizeof(ptr)). Diagnostic wording by Jordy Rose.

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

3 files changed, 127 insertions, 1 deletions

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index d7c352e570..6e851091d4 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -269,6 +269,9 @@ def warn_dyn_class_memaccess : Warning<
   InGroup<DiagGroup<"dynamic-class-memaccess">>;
 def note_bad_memaccess_silence : Note<
   "explicitly cast the pointer to silence this warning">;
+def warn_sizeof_pointer : Warning<
+  "the argument to sizeof is pointer type %0, expected %1 to match "
+  "%select{first|second}2 argument to %3">;
 
 /// main()
 // static/inline main() are not errors in C, just in C++.
diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp
index 9c47e1e1e2..389dd91f32 100644
--- a/lib/Sema/SemaChecking.cpp
+++ b/lib/Sema/SemaChecking.cpp
@@ -1812,6 +1812,20 @@ static bool isDynamicClassType(QualType T) {
   return false;
 }
 
+/// \brief If E is a sizeof expression, returns the expression's type in
+/// OutType.
+static bool sizeofExprType(const Expr* E, QualType *OutType) {
+  if (const UnaryExprOrTypeTraitExpr *SizeOf =
+      dyn_cast<UnaryExprOrTypeTraitExpr>(E)) {
+    if (SizeOf->getKind() != clang::UETT_SizeOf)
+      return false;
+
+    *OutType = SizeOf->getTypeOfArgument();
+    return true;
+  }
+  return false;
+}
+
 /// \brief Check for dangerous or invalid arguments to memset().
 ///
 /// This issues warnings on known problematic, dangerous or unspecified
@@ -1827,8 +1841,10 @@ void Sema::CheckMemsetcpymoveArguments(const CallExpr *Call,
     return;
 
   unsigned LastArg = FnName->isStr("memset")? 1 : 2;
+  const Expr *LenExpr = Call->getArg(2)->IgnoreParenImpCasts();
   for (unsigned ArgIdx = 0; ArgIdx != LastArg; ++ArgIdx) {
     const Expr *Dest = Call->getArg(ArgIdx)->IgnoreParenImpCasts();
+    SourceRange ArgRange = Call->getArg(ArgIdx)->getSourceRange();
 
     QualType DestTy = Dest->getType();
     if (const PointerType *DestPtrTy = DestTy->getAs<PointerType>()) {
@@ -1836,6 +1852,17 @@ void Sema::CheckMemsetcpymoveArguments(const CallExpr *Call,
       if (PointeeTy->isVoidType())
         continue;
 
+      // Catch "memset(p, 0, sizeof(p))" -- needs to be sizeof(*p).
+      QualType SizeofTy;
+      if (sizeofExprType(LenExpr, &SizeofTy) &&
+          Context.typesAreCompatible(SizeofTy, DestTy)) {
+        // Note: This complains about sizeof(typeof(p)) as well.
+        SourceLocation loc = LenExpr->getSourceRange().getBegin();
+        Diag(loc, diag::warn_sizeof_pointer)
+            << SizeofTy <<  PointeeTy << ArgIdx << FnName;
+        break;
+      }
+
       // Always complain about dynamic classes.
       if (isDynamicClassType(PointeeTy)) {
         DiagRuntimeBehavior(
@@ -1847,7 +1874,6 @@ void Sema::CheckMemsetcpymoveArguments(const CallExpr *Call,
         continue;
       }
 
-      SourceRange ArgRange = Call->getArg(ArgIdx)->getSourceRange();
       DiagRuntimeBehavior(
         Dest->getExprLoc(), Dest,
         PDiag(diag::note_bad_memaccess_silence)
diff --git a/test/SemaCXX/warn-memset-bad-sizeof.cpp b/test/SemaCXX/warn-memset-bad-sizeof.cpp
new file mode 100644
index 0000000000..c000b44773
--- /dev/null
+++ b/test/SemaCXX/warn-memset-bad-sizeof.cpp
@@ -0,0 +1,97 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+//
+extern "C" void *memset(void *, int, unsigned);
+extern "C" void *memmove(void *s1, const void *s2, unsigned n);
+extern "C" void *memcpy(void *s1, const void *s2, unsigned n);
+
+struct S {int a, b, c, d;};
+typedef S* PS;
+
+struct Foo {};
+typedef const Foo& CFooRef;
+typedef const Foo CFoo;
+typedef volatile Foo VFoo;
+typedef const volatile Foo CVFoo;
+
+typedef double Mat[4][4];
+
+template <class Dest, class Source>
+inline Dest bit_cast(const Source& source) {
+  Dest dest;
+  memcpy(&dest, &source, sizeof(dest));
+  return dest;
+}
+
+// http://www.lysator.liu.se/c/c-faq/c-2.html#2-6
+void f(char fake_array[8], Mat m, const Foo& const_foo) {
+  S s;
+  S* ps = &s;
+  PS ps2 = &s;
+  char arr[5];
+  char* parr[5];
+  Foo foo;
+
+  /* Should warn */
+  memset(&s, 0, sizeof(&s));  // \
+      // expected-warning {{the argument to sizeof is pointer type 'S *', expected 'S' to match first argument to 'memset'}}
+  memset(ps, 0, sizeof(ps));  // \
+      // expected-warning {{the argument to sizeof is pointer type 'S *', expected 'S' to match first argument to 'memset'}}
+  memset(ps2, 0, sizeof(ps2));  // \
+      // expected-warning {{the argument to sizeof is pointer type 'PS' (aka 'S *'), expected 'S' to match first argument to 'memset'}}
+  memset(ps2, 0, sizeof(typeof(ps2)));  // \
+      // expected-warning {{the argument to sizeof is pointer type 'typeof (ps2)' (aka 'S *'), expected 'S' to match first argument to 'memset'}}
+  memset(ps2, 0, sizeof(PS));  // \
+      // expected-warning {{the argument to sizeof is pointer type 'PS' (aka 'S *'), expected 'S' to match first argument to 'memset'}}
+  memset(fake_array, 0, sizeof(fake_array));  // \
+      // expected-warning {{the argument to sizeof is pointer type 'char *', expected 'char' to match first argument to 'memset'}}
+
+  memcpy(&s, 0, sizeof(&s));  // \
+      // expected-warning {{the argument to sizeof is pointer type 'S *', expected 'S' to match first argument to 'memcpy'}}
+  memcpy(0, &s, sizeof(&s));  // \
+      // expected-warning {{the argument to sizeof is pointer type 'S *', expected 'S' to match second argument to 'memcpy'}}
+
+  /* Shouldn't warn */
+  memset((void*)&s, 0, sizeof(&s));
+  memset(&s, 0, sizeof(s));
+  memset(&s, 0, sizeof(S));
+  memset(&s, 0, sizeof(const S));
+  memset(&s, 0, sizeof(volatile S));
+  memset(&s, 0, sizeof(volatile const S));
+  memset(&foo, 0, sizeof(CFoo));
+  memset(&foo, 0, sizeof(VFoo));
+  memset(&foo, 0, sizeof(CVFoo));
+  memset(ps, 0, sizeof(*ps));
+  memset(ps2, 0, sizeof(*ps2));
+  memset(ps2, 0, sizeof(typeof(*ps2)));
+  memset(arr, 0, sizeof(arr));
+  memset(parr, 0, sizeof(parr));
+
+  memcpy(&foo, &const_foo, sizeof(Foo));
+  memcpy((void*)&s, 0, sizeof(&s));
+  memcpy(0, (void*)&s, sizeof(&s));
+
+  CFooRef cfoo = foo;
+  memcpy(&foo, &cfoo, sizeof(Foo));
+
+  memcpy(0, &arr, sizeof(arr));
+  typedef char Buff[8];
+  memcpy(0, &arr, sizeof(Buff));
+
+  unsigned char* puc;
+  bit_cast<char*>(puc);
+
+  float* pf;
+  bit_cast<int*>(pf);
+
+  int iarr[14];
+  memset(&iarr[0], 0, sizeof iarr);
+
+  int* iparr[14];
+  memset(&iparr[0], 0, sizeof iparr);
+
+  memset(m, 0, sizeof(Mat));
+
+  // Copy to raw buffer shouldn't warn either
+  memcpy(&foo, &arr, sizeof(Foo));
+  memcpy(&arr, &foo, sizeof(Foo));
+}