[clang] [Clang] Detect bit copies that should be relocation. (PR #139104)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: cor3ntin (cor3ntin)

<details>
<summary>Changes</summary>

We already warn when memcpy/memmove involves a non-trivially-copyable type. Although we only warn when the target (not the source) is not trivially copyable.

As we moved to deprecate the old is __is_trivially_relocatable trait, it's useful to help users migrating away from memcpy.

Note that currently __builtin_trivially_relocate and memmove have the same effect, but that will change as ptrauth learn to handle relocatable types, we add preconditions (ubsan), etc.

---
Full diff: https://github.com/llvm/llvm-project/pull/139104.diff


3 Files Affected:

- (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+7) 
- (modified) clang/lib/Sema/SemaChecking.cpp (+36) 
- (modified) clang/test/SemaCXX/warn-memaccess.cpp (+36-1) 


``````````diff
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index e1b9ed0647bb9..65315fe0d38d7 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -853,6 +853,13 @@ def warn_cxxstruct_memaccess : Warning<
   "first argument in call to "
   "%0 is a pointer to non-trivially copyable type %1">,
   InGroup<NonTrivialMemcall>;
+
+def warn_cxxstruct_memaccess_relocatable
+    : Warning<"calling %0 with a pointer to a type %1 which is trivially "
+              "relocatable "
+              "but not trivially copyable; did you mean to call %2?">,
+      InGroup<NonTrivialMemcall>;
+
 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<
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 5a0cec3d112db..98aaa51dd3462 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -9659,6 +9659,42 @@ void Sema::CheckMemaccessArguments(const CallExpr *Call,
   if (BId == Builtin::BIbzero && !FirstArgTy->getAs<PointerType>())
     return;
 
+  // Try to detect a relocation operation
+  if (getLangOpts().CPlusPlus &&
+      (BId == Builtin::BImemmove || BId == Builtin::BImemcpy)) {
+    const Expr *Dest = Call->getArg(0)->IgnoreParenImpCasts();
+    const Expr *Src = Call->getArg(1)->IgnoreParenImpCasts();
+    QualType DestTy = Dest->getType();
+    QualType SrcTy = Src->getType();
+
+    QualType DestPointeeTy = DestTy->getPointeeType();
+    QualType SrcPointeeTy = SrcTy->getPointeeType();
+    bool HasSameTargetAndSource =
+        !DestPointeeTy.isNull() && !SrcPointeeTy.isNull() &&
+        Context.hasSameUnqualifiedType(DestPointeeTy, SrcPointeeTy);
+
+    if (HasSameTargetAndSource &&
+        !DestPointeeTy.getUnqualifiedType()->isIncompleteType() &&
+        !DestPointeeTy.isConstQualified() && !SrcPointeeTy.isConstQualified() &&
+        !DestPointeeTy.isTriviallyCopyableType(getASTContext()) &&
+        SemaRef.IsCXXTriviallyRelocatableType(DestPointeeTy)) {
+
+      bool SuggestStd = getLangOpts().CPlusPlus26 && getStdNamespace();
+      if (const Decl *D = Call->getReferencedDeclOfCallee();
+          D && !D->isInStdNamespace())
+        SuggestStd = false;
+
+      StringRef Replacement = SuggestStd ? "std::trivially_relocate"
+                                         : "__builtin_trivially_relocate";
+
+      DiagRuntimeBehavior(Dest->getExprLoc(), Dest,
+                          PDiag(diag::warn_cxxstruct_memaccess_relocatable)
+                              << Call->getCallee()->getSourceRange() << FnName
+                              << DestPointeeTy << Replacement);
+      return;
+    }
+  }
+
   for (unsigned ArgIdx = 0; ArgIdx != LastArg; ++ArgIdx) {
     const Expr *Dest = Call->getArg(ArgIdx)->IgnoreParenImpCasts();
     SourceRange ArgRange = Call->getArg(ArgIdx)->getSourceRange();
diff --git a/clang/test/SemaCXX/warn-memaccess.cpp b/clang/test/SemaCXX/warn-memaccess.cpp
index 2e60539b3e48e..20113170b9e93 100644
--- a/clang/test/SemaCXX/warn-memaccess.cpp
+++ b/clang/test/SemaCXX/warn-memaccess.cpp
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wnontrivial-memcall %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++11 -verify=expected,cxx11 -Wnontrivial-memcall %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++2c -verify=expected,cxx26 -Wnontrivial-memcall %s
 
 extern "C" void *bzero(void *, unsigned);
 extern "C" void *memset(void *, int, unsigned);
@@ -8,6 +9,9 @@ extern "C" void *memcpy(void *s1, const void *s2, unsigned n);
 class TriviallyCopyable {};
 class NonTriviallyCopyable { NonTriviallyCopyable(const NonTriviallyCopyable&);};
 struct Incomplete;
+class Relocatable {
+  virtual void f();
+};
 
 void test_bzero(TriviallyCopyable* tc,
                 NonTriviallyCopyable *ntc,
@@ -83,3 +87,34 @@ void test_memmove(TriviallyCopyable* tc0, TriviallyCopyable* tc1,
   // OK
   memmove((void*)ntc0, (void*)ntc1, sizeof(*ntc0));
 }
+
+
+void test_possible_relocation(Relocatable* r, const Relocatable* r2) {
+     // expected-warning at +1 {{calling 'memcpy' with a pointer to a type 'Relocatable' which is trivially relocatable but not trivially copyable; did you mean to call __builtin_trivially_relocate?}}
+     memcpy(r, r, sizeof(*r));
+     // expected-warning at +1 {{calling 'memmove' with a pointer to a type 'Relocatable' which is trivially relocatable but not trivially copyable; did you mean to call __builtin_trivially_relocate?}}
+     memmove(r, r, sizeof(*r));
+
+     // expected-warning at +2{{destination for this 'memcpy' call is a pointer to dynamic class 'Relocatable'}}
+     // expected-note at +1{{explicitly cast the pointer to silence this warning}}
+     memcpy(r, r2, sizeof(*r));
+
+     // expected-warning at +2{{destination for this 'memmove' call is a pointer to dynamic class 'Relocatable'}}
+     // expected-note at +1{{explicitly cast the pointer to silence this warning}}
+     memmove(r, r2, sizeof(*r));
+}
+
+namespace std {
+  using ::memcpy;
+  using ::memmove;
+};
+
+
+void test_possible_relocation_std(Relocatable* r, const Relocatable* r2) {
+     // cxx11-warning at +2 {{calling 'memcpy' with a pointer to a type 'Relocatable' which is trivially relocatable but not trivially copyable; did you mean to call __builtin_trivially_relocate?}}
+     // cxx26-warning at +1 {{calling 'memcpy' with a pointer to a type 'Relocatable' which is trivially relocatable but not trivially copyable; did you mean to call std::trivially_relocate?}}
+     std::memcpy(r, r, sizeof(*r));
+     // cxx11-warning at +2 {{calling 'memmove' with a pointer to a type 'Relocatable' which is trivially relocatable but not trivially copyable; did you mean to call __builtin_trivially_relocate?}}
+     // cxx26-warning at +1 {{calling 'memmove' with a pointer to a type 'Relocatable' which is trivially relocatable but not trivially copyable; did you mean to call std::trivially_relocate?}}
+     std::memmove(r, r, sizeof(*r));
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/139104