[clang] [libcxx] [clang] Warn about memset/memcpy to NonTriviallyCopyable types (PR #111434)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Oct 24 06:52:32 PDT 2024
https://github.com/serge-sans-paille updated https://github.com/llvm/llvm-project/pull/111434
>From c941368f21a769b4903e0d2aac9f8fa732a1e644 Mon Sep 17 00:00:00 2001
From: serge-sans-paille <sguelton at mozilla.com>
Date: Mon, 7 Oct 2024 15:30:24 +0200
Subject: [PATCH 1/2] [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.
---
clang/docs/ReleaseNotes.rst | 4 ++
.../clang/Basic/DiagnosticSemaKinds.td | 4 ++
clang/lib/Sema/SemaChecking.cpp | 18 +++++
clang/test/SemaCXX/constexpr-string.cpp | 2 +
clang/test/SemaCXX/warn-memaccess.cpp | 68 +++++++++++++++++++
5 files changed, 96 insertions(+)
create mode 100644 clang/test/SemaCXX/warn-memaccess.cpp
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 46021c9c17feac..d6de1759b47535 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -302,6 +302,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
-------------------------
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 8e4718008ece72..a7d6123d5c7e19 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -795,6 +795,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<
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 27b274d74ce716..b64ac3cc676bc3 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -8899,18 +8899,36 @@ 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 && ArgIdx == 0) {
+ // FIXME: Limiting this warning to dest argument until we decide
+ // whether it's valid for source argument too.
+ 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 && ArgIdx == 0) {
+ // FIXME: Limiting this warning to dest argument until we decide
+ // whether it's valid for source argument too.
+ DiagRuntimeBehavior(Dest->getExprLoc(), Dest,
+ PDiag(diag::warn_cxxstruct_memaccess)
+ << ArgIdx << FnName << PointeeTy);
} else {
continue;
}
diff --git a/clang/test/SemaCXX/constexpr-string.cpp b/clang/test/SemaCXX/constexpr-string.cpp
index c456740ef7551f..41305e3f8bcba5 100644
--- a/clang/test/SemaCXX/constexpr-string.cpp
+++ b/clang/test/SemaCXX/constexpr-string.cpp
@@ -670,6 +670,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 at +2 {{destination for call to '__builtin_memcpy' is a pointer to non-trivially copyable type 'Incomplete'}}
+ // expected-note at +1 {{explicitly cast the pointer to silence this warning}}
__builtin_memcpy(&x, &x, 4);
// expected-note at -1 2{{cannot constant evaluate 'memcpy' between objects of incomplete type 'Incomplete'}}
return true;
diff --git a/clang/test/SemaCXX/warn-memaccess.cpp b/clang/test/SemaCXX/warn-memaccess.cpp
new file mode 100644
index 00000000000000..35c90e4f606608
--- /dev/null
+++ b/clang/test/SemaCXX/warn-memaccess.cpp
@@ -0,0 +1,68 @@
+// 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 at +2{{destination for call to 'bzero' is a pointer to non-trivially copyable type 'NonTriviallyCopyable'}}
+ // expected-note at +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 at +2{{destination for call to 'memset' is a pointer to non-trivially copyable type 'NonTriviallyCopyable'}}
+ // expected-note at +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 at +2{{destination for call to 'memcpy' is a pointer to non-trivially copyable type 'NonTriviallyCopyable'}}
+ // expected-note at +1{{explicitly cast the pointer to silence this warning}}
+ memcpy(ntc0, ntc1, sizeof(*ntc0));
+
+ // ~ OK
+ 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 at +2{{destination for call to 'memmove' is a pointer to non-trivially copyable type 'NonTriviallyCopyable'}}
+ // expected-note at +1{{explicitly cast the pointer to silence this warning}}
+ memmove(ntc0, ntc1, sizeof(*ntc0));
+
+ // ~ OK
+ memmove((void*)ntc0, ntc1, sizeof(*ntc0));
+
+ // OK
+ memmove((void*)ntc0, (void*)ntc1, sizeof(*ntc0));
+}
>From 3d183560b8292b89f57aca8a563f19b0476ed643 Mon Sep 17 00:00:00 2001
From: serge-sans-paille <sguelton at mozilla.com>
Date: Tue, 8 Oct 2024 14:17:22 +0200
Subject: [PATCH 2/2] [libc++] Silent warning related to memcpy of non
trivially-copyable data
---
libcxx/include/__memory/uninitialized_algorithms.h | 2 +-
libcxx/test/std/utilities/expected/types.h | 6 +++---
libcxx/test/support/min_allocator.h | 4 ++--
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/libcxx/include/__memory/uninitialized_algorithms.h b/libcxx/include/__memory/uninitialized_algorithms.h
index 54af1fa1a1cc55..2649712b8c96c8 100644
--- a/libcxx/include/__memory/uninitialized_algorithms.h
+++ b/libcxx/include/__memory/uninitialized_algorithms.h
@@ -638,7 +638,7 @@ __uninitialized_allocator_relocate(_Alloc& __alloc, _Tp* __first, _Tp* __last, _
__guard.__complete();
std::__allocator_destroy(__alloc, __first, __last);
} else {
- __builtin_memcpy(__result, __first, sizeof(_Tp) * (__last - __first));
+ __builtin_memcpy((void*)__result, __first, sizeof(_Tp) * (__last - __first));
}
}
diff --git a/libcxx/test/std/utilities/expected/types.h b/libcxx/test/std/utilities/expected/types.h
index 2b6983fb399c67..7690165de6fe35 100644
--- a/libcxx/test/std/utilities/expected/types.h
+++ b/libcxx/test/std/utilities/expected/types.h
@@ -162,7 +162,7 @@ template <int Constant>
struct TailClobberer {
constexpr TailClobberer() noexcept {
if (!std::is_constant_evaluated()) {
- std::memset(this, Constant, sizeof(*this));
+ std::memset((void*)this, Constant, sizeof(*this));
}
// Always set `b` itself to `false` so that the comparison works.
b = false;
@@ -245,7 +245,7 @@ struct BoolWithPadding {
constexpr explicit BoolWithPadding() noexcept : BoolWithPadding(false) {}
constexpr BoolWithPadding(bool val) noexcept {
if (!std::is_constant_evaluated()) {
- std::memset(this, 0, sizeof(*this));
+ std::memset((void*)this, 0, sizeof(*this));
}
val_ = val;
}
@@ -268,7 +268,7 @@ struct IntWithoutPadding {
constexpr explicit IntWithoutPadding() noexcept : IntWithoutPadding(0) {}
constexpr IntWithoutPadding(int val) noexcept {
if (!std::is_constant_evaluated()) {
- std::memset(this, 0, sizeof(*this));
+ std::memset((void*)this, 0, sizeof(*this));
}
val_ = val;
}
diff --git a/libcxx/test/support/min_allocator.h b/libcxx/test/support/min_allocator.h
index 13ee98289c36b7..c804242233b144 100644
--- a/libcxx/test/support/min_allocator.h
+++ b/libcxx/test/support/min_allocator.h
@@ -465,14 +465,14 @@ class safe_allocator {
TEST_CONSTEXPR_CXX20 T* allocate(std::size_t n) {
T* memory = std::allocator<T>().allocate(n);
if (!TEST_IS_CONSTANT_EVALUATED)
- std::memset(memory, 0, sizeof(T) * n);
+ std::memset((void*)memory, 0, sizeof(T) * n);
return memory;
}
TEST_CONSTEXPR_CXX20 void deallocate(T* p, std::size_t n) {
if (!TEST_IS_CONSTANT_EVALUATED)
- DoNotOptimize(std::memset(p, 0, sizeof(T) * n));
+ DoNotOptimize(std::memset((void*)p, 0, sizeof(T) * n));
std::allocator<T>().deallocate(p, n);
}
More information about the cfe-commits
mailing list