Improve the correctness and accuracy of the message for -Wdynamic-class-memaccess

llvm-svn: 138074

3 files changed, 27 insertions, 18 deletions

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 93521ed596d..c76dca0f7ba 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -264,8 +264,9 @@ 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_memaccess : Warning<
-  "%select{destination for|source of}0 this %1 call is a pointer to dynamic "
-  "class %2; vtable pointer will be overwritten">, 
+  "%select{destination for|source of|first operand of|second operand of}0 this "
+  "%1 call is a pointer to dynamic class %2; vtable pointer will be "
+  "%select{overwritten|copied|moved|compared}3">,
   InGroup<DiagGroup<"dynamic-class-memaccess">>;
 def note_bad_memaccess_silence : Note<
   "explicitly cast the pointer to silence this warning">;
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 82c97a63cdb..530f81289a9 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -1971,24 +1971,27 @@ void Sema::CheckMemaccessArguments(const CallExpr *Call,
         }
       }
 
-      unsigned DiagID;
-
       // Always complain about dynamic classes.
       if (isDynamicClassType(PointeeTy))
-        DiagID = diag::warn_dyn_class_memaccess;
+        DiagRuntimeBehavior(
+          Dest->getExprLoc(), Dest,
+          PDiag(diag::warn_dyn_class_memaccess)
+            << (CMF == CMF_Memcmp ? ArgIdx + 2 : ArgIdx) << FnName << PointeeTy
+            // "overwritten" if we're warning about the destination for any call
+            // but memcmp; otherwise a verb appropriate to the call.
+            << (ArgIdx == 0 && CMF != CMF_Memcmp ? 0 : (unsigned)CMF)
+            << Call->getCallee()->getSourceRange());
       else if (PointeeTy.hasNonTrivialObjCLifetime() && CMF != CMF_Memset)
-        DiagID = diag::warn_arc_object_memaccess;
+        DiagRuntimeBehavior(
+          Dest->getExprLoc(), Dest,
+          PDiag(diag::warn_arc_object_memaccess)
+            << ArgIdx << FnName << PointeeTy
+            << Call->getCallee()->getSourceRange());
       else
         continue;
 
       DiagRuntimeBehavior(
         Dest->getExprLoc(), Dest,
-        PDiag(DiagID)
-          << ArgIdx << FnName << PointeeTy 
-          << Call->getCallee()->getSourceRange());
-
-      DiagRuntimeBehavior(
-        Dest->getExprLoc(), Dest,
         PDiag(diag::note_bad_memaccess_silence)
           << FixItHint::CreateInsertion(ArgRange.getBegin(), "(void*)"));
       break;
diff --git a/clang/test/SemaCXX/warn-bad-memaccess.cpp b/clang/test/SemaCXX/warn-bad-memaccess.cpp
index 9a998f020cb..3a02c84e9fc 100644
--- a/clang/test/SemaCXX/warn-bad-memaccess.cpp
+++ b/clang/test/SemaCXX/warn-bad-memaccess.cpp
@@ -3,6 +3,7 @@
 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);
+extern "C" void *memcmp(void *s1, const void *s2, unsigned n);
 
 // Several types that should not warn.
 struct S1 {} s1;
@@ -27,16 +28,22 @@ void test_warn() {
       // 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 dynamic class}} \
+      // expected-warning{{destination for this 'memmove' call is a pointer to dynamic class 'struct X1'; vtable pointer will be overwritten}} \
       // 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 dynamic class}} \
+      // expected-warning{{source of this 'memmove' call is a pointer to dynamic class 'struct X1'; vtable pointer will be moved}} \
       // 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 dynamic class}} \
+      // expected-warning{{destination for this 'memcpy' call is a pointer to dynamic class 'struct X1'; vtable pointer will be overwritten}} \
       // 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 dynamic class}} \
+      // expected-warning{{source of this 'memcpy' call is a pointer to dynamic class 'struct X1'; vtable pointer will be copied}} \
+      // expected-note {{explicitly cast the pointer to silence this warning}}
+  memcmp(&x1, 0, sizeof x1); // \
+      // expected-warning{{first operand of this 'memcmp' call is a pointer to dynamic class 'struct X1'; vtable pointer will be compared}} \
+      // expected-note {{explicitly cast the pointer to silence this warning}}
+  memcmp(0, &x1, sizeof x1); // \
+      // expected-warning{{second operand of this 'memcmp' call is a pointer to dynamic class 'struct X1'; vtable pointer will be compared}} \
       // expected-note {{explicitly cast the pointer to silence this warning}}
 
   __builtin_memset(&x1, 0, sizeof x1); // \
@@ -108,5 +115,3 @@ namespace N {
     N::memset(&x1, 0, sizeof x1);
   }
 }
-
-