[clang] Warn about memset/memcpy to NonTriviallyCopyable types

This implements a warning that's similar to what GCC does in that
context: both memcpy and memset require their first and second operand
to be trivially copyable, let's warn if that's not the case.

Author: serge-sans-paille

clang/docs/ReleaseNotes.rst

@@ -266,6 +266,10 @@ Modified Compiler Flags
   the ``promoted`` algorithm for complex division when possible rather than the
   less basic (limited range) algorithm.
 
+- The ``-Wnontrivial-memaccess`` warning has been updated to also warn about
+  passing non-trivially-copyable parameter to ``memcpy``, ``memset`` and similar
+  functions for which it is a documented undefined behavior.
+
 Removed Compiler Flags
 -------------------------
 

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -790,6 +790,10 @@ def warn_cstruct_memaccess : Warning<
   "%1 call is a pointer to record %2 that is not trivial to "
   "%select{primitive-default-initialize|primitive-copy}3">,
   InGroup<NonTrivialMemaccess>;
+def warn_cxxstruct_memaccess : Warning<
+  "%select{destination for|source of|first operand of|second operand of}0 call to "
+  "%1 is a pointer to non-trivially copyable type %2">,
+  InGroup<NonTrivialMemaccess>;
 def note_nontrivial_field : Note<
   "field is non-trivial to %select{copy|default-initialize}0">;
 def err_non_trivial_c_union_in_invalid_context : Error<

clang/lib/Sema/SemaChecking.cpp

@@ -8899,18 +8899,32 @@ void Sema::CheckMemaccessArguments(const CallExpr *Call,
           << ArgIdx << FnName << PointeeTy
           << Call->getCallee()->getSourceRange());
     else if (const auto *RT = PointeeTy->getAs<RecordType>()) {
+
+      bool IsTriviallyCopyableCXXRecord =
+          RT->desugar().isTriviallyCopyableType(Context);
+
       if ((BId == Builtin::BImemset || BId == Builtin::BIbzero) &&
           RT->getDecl()->isNonTrivialToPrimitiveDefaultInitialize()) {
         DiagRuntimeBehavior(Dest->getExprLoc(), Dest,
                             PDiag(diag::warn_cstruct_memaccess)
                                 << ArgIdx << FnName << PointeeTy << 0);
         SearchNonTrivialToInitializeField::diag(PointeeTy, Dest, *this);
+      } else if ((BId == Builtin::BImemset || BId == Builtin::BIbzero) &&
+                 !IsTriviallyCopyableCXXRecord) {
+        DiagRuntimeBehavior(Dest->getExprLoc(), Dest,
+                            PDiag(diag::warn_cxxstruct_memaccess)
+                                << ArgIdx << FnName << PointeeTy);
       } else if ((BId == Builtin::BImemcpy || BId == Builtin::BImemmove) &&
                  RT->getDecl()->isNonTrivialToPrimitiveCopy()) {
         DiagRuntimeBehavior(Dest->getExprLoc(), Dest,
                             PDiag(diag::warn_cstruct_memaccess)
                                 << ArgIdx << FnName << PointeeTy << 1);
         SearchNonTrivialToCopyField::diag(PointeeTy, Dest, *this);
+      } else if ((BId == Builtin::BImemcpy || BId == Builtin::BImemmove) &&
+                 !IsTriviallyCopyableCXXRecord) {
+        DiagRuntimeBehavior(Dest->getExprLoc(), Dest,
+                            PDiag(diag::warn_cxxstruct_memaccess)
+                                << ArgIdx << FnName << PointeeTy);
       } else {
         continue;
       }

clang/test/SemaCXX/constexpr-string.cpp

@@ -603,12 +603,16 @@ namespace MemcpyEtc {
   };
   constexpr bool test_nontrivial_memcpy() { // expected-error {{never produces a constant}}
     NonTrivial arr[3] = {};
+    // expected-warning@+2 {{source of call to '__builtin_memcpy' is a pointer to non-trivially copyable type 'NonTrivial'}}
+    // expected-note@+1 {{explicitly cast the pointer to silence this warning}}
     __builtin_memcpy(arr, arr + 1, sizeof(NonTrivial)); // expected-note 2{{non-trivially-copyable}}
     return true;
   }
   static_assert(test_nontrivial_memcpy()); // expected-error {{constant}} expected-note {{in call}}
   constexpr bool test_nontrivial_memmove() { // expected-error {{never produces a constant}}
     NonTrivial arr[3] = {};
+    // expected-warning@+2 {{source of call to '__builtin_memcpy' is a pointer to non-trivially copyable type 'NonTrivial'}}
+    // expected-note@+1 {{explicitly cast the pointer to silence this warning}}
     __builtin_memcpy(arr, arr + 1, sizeof(NonTrivial)); // expected-note 2{{non-trivially-copyable}}
     return true;
   }
@@ -670,6 +674,8 @@ namespace MemcpyEtc {
   constexpr bool test_address_of_incomplete_struct_type() { // expected-error {{never produces a constant}}
     struct Incomplete;
     extern Incomplete x, y;
+    // expected-warning@+2 {{destination for call to '__builtin_memcpy' is a pointer to non-trivially copyable type 'Incomplete'}}
+    // expected-note@+1 {{explicitly cast the pointer to silence this warning}}
     __builtin_memcpy(&x, &x, 4);
     // expected-note@-1 2{{cannot constant evaluate 'memcpy' between objects of incomplete type 'Incomplete'}}
     return true;

clang/test/SemaCXX/warn-memaccess.cpp

@@ -0,0 +1,70 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wnontrivial-memaccess %s
+
+extern "C" void *bzero(void *, unsigned);
+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);
+
+class TriviallyCopyable {};
+class NonTriviallyCopyable { NonTriviallyCopyable(const NonTriviallyCopyable&);};
+
+void test_bzero(TriviallyCopyable* tc,
+                 NonTriviallyCopyable *ntc) {
+  // OK
+  bzero(tc, sizeof(*tc));
+
+  // expected-warning@+2{{destination for call to 'bzero' is a pointer to non-trivially copyable type 'NonTriviallyCopyable'}}
+  // expected-note@+1{{explicitly cast the pointer to silence this warning}}
+  bzero(ntc, sizeof(*ntc));
+
+  // OK
+  bzero((void*)ntc, sizeof(*ntc));
+}
+
+void test_memset(TriviallyCopyable* tc,
+                 NonTriviallyCopyable *ntc) {
+  // OK
+  memset(tc, 0, sizeof(*tc));
+
+  // expected-warning@+2{{destination for call to 'memset' is a pointer to non-trivially copyable type 'NonTriviallyCopyable'}}
+  // expected-note@+1{{explicitly cast the pointer to silence this warning}}
+  memset(ntc, 0, sizeof(*ntc));
+
+  // OK
+  memset((void*)ntc, 0, sizeof(*ntc));
+}
+
+
+void test_memcpy(TriviallyCopyable* tc0, TriviallyCopyable* tc1,
+                 NonTriviallyCopyable *ntc0, NonTriviallyCopyable *ntc1) {
+  // OK
+  memcpy(tc0, tc1, sizeof(*tc0));
+
+  // expected-warning@+2{{destination for call to 'memcpy' is a pointer to non-trivially copyable type 'NonTriviallyCopyable'}}
+  // expected-note@+1{{explicitly cast the pointer to silence this warning}}
+  memcpy(ntc0, ntc1, sizeof(*ntc0));
+
+  // expected-warning@+2{{source of call to 'memcpy' is a pointer to non-trivially copyable type 'NonTriviallyCopyable'}}
+  // expected-note@+1{{explicitly cast the pointer to silence this warning}}
+  memcpy((void*)ntc0, ntc1, sizeof(*ntc0));
+
+  // OK
+  memcpy((void*)ntc0, (void*)ntc1, sizeof(*ntc0));
+}
+
+void test_memmove(TriviallyCopyable* tc0, TriviallyCopyable* tc1,
+                 NonTriviallyCopyable *ntc0, NonTriviallyCopyable *ntc1) {
+  // OK
+  memmove(tc0, tc1, sizeof(*tc0));
+
+  // expected-warning@+2{{destination for call to 'memmove' is a pointer to non-trivially copyable type 'NonTriviallyCopyable'}}
+  // expected-note@+1{{explicitly cast the pointer to silence this warning}}
+  memmove(ntc0, ntc1, sizeof(*ntc0));
+
+  // expected-warning@+2{{source of call to 'memmove' is a pointer to non-trivially copyable type 'NonTriviallyCopyable'}}
+  // expected-note@+1{{explicitly cast the pointer to silence this warning}}
+  memmove((void*)ntc0, ntc1, sizeof(*ntc0));
+
+  // OK
+  memmove((void*)ntc0, (void*)ntc1, sizeof(*ntc0));
+}