Extend -Wnon-pod-memset to also encompass memcpy() and memmove(),

checking both the source and the destination operands, renaming the
warning group to -Wnon-pod-memaccess and tweaking the diagnostic text
in the process.

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

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 7056fbc9283275101d0a75bffe2aa210dfed69ea..7c4583f7dc306c058c9a917be579ffeb52716995 100644 (file)
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -261,13 +261,15 @@ def err_builtin_definition : Error<"definition of builtin function %0">;
 def err_types_compatible_p_in_cplusplus : Error<
   "__builtin_types_compatible_p is not valid in C++">;
 def warn_builtin_unknown : Warning<"use of unknown builtin %0">, DefaultError;
-def warn_dyn_class_memset : Warning<
-  "destination for this memset call is a pointer to a dynamic class %0">, 
-  InGroup<DiagGroup<"non-pod-memset">>;
-def warn_non_pod_memset : Warning<
-  "destination for this memset call is a pointer to a non-POD type %0">,
-  InGroup<DiagGroup<"non-pod-memset">>, DefaultIgnore;
-def note_non_pod_memset_silence : Note<
+def warn_dyn_class_memaccess : Warning<
+  "%select{destination for|source of}0 this %1 call is a pointer to dynamic "
+  "class %2; vtable pointer will be overwritten">, 
+  InGroup<DiagGroup<"non-pod-memaccess">>;
+def warn_non_pod_memaccess : Warning<
+  "%select{destination for|source of}0 this %1 call is a pointer to non-POD "
+  "type %2">,
+  InGroup<DiagGroup<"non-pod-memaccess">>, DefaultIgnore;
+def note_non_pod_memaccess_silence : Note<
   "explicitly cast the pointer to silence this warning">;
 
 /// main()

diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index 7d970a8695f8ee32004a5912dab3d4ca784f7e78..d70a67f9fef0c8d6965bf201786325947ec6623d 100644 (file)
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -5552,7 +5552,8 @@ private:
                                  unsigned format_idx, unsigned firstDataArg,
                                  bool isPrintf);
 
-  void CheckMemsetArguments(const CallExpr *Call);
+  void CheckMemsetcpymoveArguments(const CallExpr *Call,
+                                   const IdentifierInfo *FnName);
 
   void CheckReturnStackAddr(Expr *RetValExp, QualType lhsType,
                             SourceLocation ReturnLoc);

diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp
index 76f20ce530c28bece4d8b5ed32418f45f9e13ee7..10645fd23ecc8fa91a961a1025eeb9b96acc491e 100644 (file)
--- a/lib/Sema/SemaChecking.cpp
+++ b/lib/Sema/SemaChecking.cpp
@@ -318,11 +318,13 @@ bool Sema::CheckFunctionCall(FunctionDecl *FDecl, CallExpr *TheCall) {
                           TheCall->getCallee()->getLocStart());
   }
 
-  // Memset handling
-  if (FnInfo->isStr("memset") && 
-      FDecl->getLinkage() == ExternalLinkage &&
-      (!getLangOptions().CPlusPlus || FDecl->isExternC()))
-    CheckMemsetArguments(TheCall);
+  // Memset/memcpy/memmove handling
+  if (FDecl->getLinkage() == ExternalLinkage &&
+      (!getLangOptions().CPlusPlus || FDecl->isExternC())) {
+    if (FnInfo->isStr("memset") || FnInfo->isStr("memcpy") || 
+        FnInfo->isStr("memmove"))
+      CheckMemsetcpymoveArguments(TheCall, FnInfo);
+  }
 
   return false;
 }
@@ -1813,45 +1815,51 @@ static bool isDynamicClassType(QualType T) {
 /// \brief Check for dangerous or invalid arguments to memset().
 ///
 /// This issues warnings on known problematic or dangerous or unspecified
-/// arguments to the standard 'memset' function call.
+/// arguments to the standard 'memset', 'memcpy', and 'memmove' function calls.
 ///
 /// \param Call The call expression to diagnose.
-void Sema::CheckMemsetArguments(const CallExpr *Call) {
+void Sema::CheckMemsetcpymoveArguments(const CallExpr *Call,
+                                       const IdentifierInfo *FnName) {
   // It is possible to have a non-standard definition of memset.  Validate
   // we have the proper number of arguments, and if not, abort further
   // checking.
   if (Call->getNumArgs() != 3)
     return;
 
-  const Expr *Dest = Call->getArg(0)->IgnoreParenImpCasts();
-
-  QualType DestTy = Dest->getType();
-  if (const PointerType *DestPtrTy = DestTy->getAs<PointerType>()) {
-    QualType PointeeTy = DestPtrTy->getPointeeType();
-    if (PointeeTy->isVoidType())
-      return;
-
-    unsigned DiagID = 0;
-    // Always complain about dynamic classes.
-    if (isDynamicClassType(PointeeTy))
-      DiagID = diag::warn_dyn_class_memset;
-    // Check the C++11 POD definition regardless of language mode; it is more
-    // relaxed than earlier definitions and we don't want spurious warnings.
-    else if (!PointeeTy->isCXX11PODType())
-      DiagID = diag::warn_non_pod_memset;
-    else
-      return;
-
-    DiagRuntimeBehavior(
-      Dest->getExprLoc(), Dest,
-      PDiag(DiagID)
-        << PointeeTy << Call->getCallee()->getSourceRange());
-
-    SourceRange ArgRange = Call->getArg(0)->getSourceRange();
-    DiagRuntimeBehavior(
-      Dest->getExprLoc(), Dest,
-      PDiag(diag::note_non_pod_memset_silence)
-        << FixItHint::CreateInsertion(ArgRange.getBegin(), "(void*)"));
+  unsigned LastArg = FnName->isStr("memset")? 1 : 2;
+  for (unsigned ArgIdx = 0; ArgIdx != LastArg; ++ArgIdx) {
+    const Expr *Dest = Call->getArg(ArgIdx)->IgnoreParenImpCasts();
+
+    QualType DestTy = Dest->getType();
+    if (const PointerType *DestPtrTy = DestTy->getAs<PointerType>()) {
+      QualType PointeeTy = DestPtrTy->getPointeeType();
+      if (PointeeTy->isVoidType())
+        continue;
+
+      unsigned DiagID = 0;
+      // Always complain about dynamic classes.
+      if (isDynamicClassType(PointeeTy))
+        DiagID = diag::warn_dyn_class_memaccess;
+      // Check the C++11 POD definition regardless of language mode; it is more
+      // relaxed than earlier definitions and we don't want spurious warnings.
+      else if (!PointeeTy->isCXX11PODType())
+        DiagID = diag::warn_non_pod_memaccess;
+      else
+        continue;
+
+      DiagRuntimeBehavior(
+        Dest->getExprLoc(), Dest,
+        PDiag(DiagID)
+          << ArgIdx << FnName << PointeeTy 
+          << Call->getCallee()->getSourceRange());
+
+      SourceRange ArgRange = Call->getArg(0)->getSourceRange();
+      DiagRuntimeBehavior(
+        Dest->getExprLoc(), Dest,
+        PDiag(diag::note_non_pod_memaccess_silence)
+          << FixItHint::CreateInsertion(ArgRange.getBegin(), "(void*)"));
+      break;
+    }
   }
 }
 

diff --git a/test/SemaCXX/warn-non-pod-memset.cpp b/test/SemaCXX/warn-non-pod-memset.cpp
index 6b017f338b00826dc4b34a3fa4798378e2b7e534..e0abdbd2aff27c18e65832463cd93f6a660a9141 100644 (file)
--- a/test/SemaCXX/warn-non-pod-memset.cpp
+++ b/test/SemaCXX/warn-non-pod-memset.cpp
@@ -1,6 +1,8 @@
-// RUN: %clang_cc1 -fsyntax-only -Wnon-pod-memset -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wnon-pod-memaccess -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);
 
 // Several POD types that should not warn.
 struct S1 {} s1;
@@ -24,19 +26,32 @@ struct X5 : virtual S1 {} x5;
 
 void test_warn() {
   memset(&x1, 0, sizeof x1); // \
-      // expected-warning {{destination for this memset call is a pointer to a non-POD type}} \
+      // expected-warning {{destination for this 'memset' call is a pointer to non-POD type}} \
       // expected-note {{explicitly cast the pointer to silence this warning}}
   memset(&x2, 0, sizeof x2); // \
-      // expected-warning {{destination for this memset call is a pointer to a non-POD type}} \
+      // expected-warning {{destination for this 'memset' call is a pointer to non-POD type}} \
       // expected-note {{explicitly cast the pointer to silence this warning}}
   memset(&x3, 0, sizeof x3); // \
-      // expected-warning {{destination for this memset call is a pointer to a dynamic class}} \
+      // expected-warning {{destination for this 'memset' call is a pointer to dynamic class}} \
       // expected-note {{explicitly cast the pointer to silence this warning}}
   memset(&x4, 0, sizeof x4); // \
-      // expected-warning {{destination for this memset call is a pointer to a non-POD type}} \
+      // expected-warning {{destination for this 'memset' call is a pointer to non-POD type}} \
       // expected-note {{explicitly cast the pointer to silence this warning}}
   memset(&x5, 0, sizeof x5); // \
-      // expected-warning {{destination for this memset call is a pointer to a dynamic class}} \
+      // expected-warning {{destination for this 'memset' call is a pointer to dynamic class}} \
+      // expected-note {{explicitly cast the pointer to silence this warning}}
+
+  memmove(&x1, 0, sizeof x1); // \
+      // expected-warning{{destination for this 'memmove' call is a pointer to non-POD type 'struct X1'}} \
+      // expected-note {{explicitly cast the pointer to silence this warning}}
+  memmove(0, &x1, sizeof x1); // \
+      // expected-warning{{source of this 'memmove' call is a pointer to non-POD type 'struct X1'}} \
+      // expected-note {{explicitly cast the pointer to silence this warning}}
+  memcpy(&x1, 0, sizeof x1); // \
+      // expected-warning{{destination for this 'memcpy' call is a pointer to non-POD type 'struct X1'}} \
+      // expected-note {{explicitly cast the pointer to silence this warning}}
+  memcpy(0, &x1, sizeof x1); // \
+      // expected-warning{{source of this 'memcpy' call is a pointer to non-POD type 'struct X1'}} \
       // expected-note {{explicitly cast the pointer to silence this warning}}
 }