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@+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@+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@+2{{destination for this 'memcpy' call is a pointer to dynamic class 'Relocatable'}} + // expected-note@+1{{explicitly cast the pointer to silence this warning}} + memcpy(r, r2, sizeof(*r)); + + // expected-warning@+2{{destination for this 'memmove' call is a pointer to dynamic class 'Relocatable'}} + // expected-note@+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@+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@+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@+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@+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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits