https://github.com/serge-sans-paille updated
https://github.com/llvm/llvm-project/pull/111434
>From 894cc0a4624fdf9409b9f448276d6c70a365c046 Mon Sep 17 00:00:00 2001
From: serge-sans-paille <sguel...@mozilla.com>
Date: Mon, 7 Oct 2024 15:30:24 +0200
Subject: [PATCH 1/4] [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 | 14 ++++
clang/test/SemaCXX/constexpr-string.cpp | 6 ++
clang/test/SemaCXX/warn-memaccess.cpp | 70 +++++++++++++++++++
5 files changed, 98 insertions(+)
create mode 100644 clang/test/SemaCXX/warn-memaccess.cpp
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 583c1e6b4215c5..6b5b2d980fbcb5 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -258,6 +258,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 536211a6da335b..2b5251bc6685e7 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/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<
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 2bcb930acdcb57..38cd16e3e81466 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/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;
}
diff --git a/clang/test/SemaCXX/constexpr-string.cpp
b/clang/test/SemaCXX/constexpr-string.cpp
index c456740ef7551f..31fa98f8a972a7 100644
--- a/clang/test/SemaCXX/constexpr-string.cpp
+++ b/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;
diff --git a/clang/test/SemaCXX/warn-memaccess.cpp
b/clang/test/SemaCXX/warn-memaccess.cpp
new file mode 100644
index 00000000000000..95131dc55cfa3c
--- /dev/null
+++ b/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));
+}
>From 9b01d53535ef7612d6a90ff2abea677644c4f0e4 Mon Sep 17 00:00:00 2001
From: serge-sans-paille <sguel...@mozilla.com>
Date: Tue, 8 Oct 2024 14:15:23 +0200
Subject: [PATCH 2/4] [compiler-rt] Silent warning related to memcpy of non
trivially-copiable data
---
compiler-rt/lib/memprof/memprof_rawprofile.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/compiler-rt/lib/memprof/memprof_rawprofile.cpp
b/compiler-rt/lib/memprof/memprof_rawprofile.cpp
index a897648584828b..58e87fd833db7f 100644
--- a/compiler-rt/lib/memprof/memprof_rawprofile.cpp
+++ b/compiler-rt/lib/memprof/memprof_rawprofile.cpp
@@ -73,7 +73,7 @@ void SerializeSegmentsToBuffer(ArrayRef<LoadedModule> Modules,
CHECK(Module.uuid_size() <= MEMPROF_BUILDID_MAX_SIZE);
Entry.BuildIdSize = Module.uuid_size();
memcpy(Entry.BuildId, Module.uuid(), Module.uuid_size());
- memcpy(Ptr, &Entry, sizeof(SegmentEntry));
+ memcpy(Ptr, (void *)&Entry, sizeof(SegmentEntry));
Ptr += sizeof(SegmentEntry);
NumSegmentsRecorded++;
}
>From c0c2f025815ff19810c46d8d358e2fecfa9466d4 Mon Sep 17 00:00:00 2001
From: serge-sans-paille <sguel...@mozilla.com>
Date: Tue, 8 Oct 2024 14:17:22 +0200
Subject: [PATCH 3/4] [libc++] Silent warning related to memcpy of non
trivially-copyable data
---
libcxx/include/__iterator/aliasing_iterator.h | 2 +-
libcxx/include/__memory/uninitialized_algorithms.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/libcxx/include/__iterator/aliasing_iterator.h
b/libcxx/include/__iterator/aliasing_iterator.h
index 94ba577078b5e8..48e121eaf0452d 100644
--- a/libcxx/include/__iterator/aliasing_iterator.h
+++ b/libcxx/include/__iterator/aliasing_iterator.h
@@ -102,7 +102,7 @@ struct __aliasing_iterator_wrapper {
_LIBCPP_HIDE_FROM_ABI _Alias operator*() const _NOEXCEPT {
_Alias __val;
- __builtin_memcpy(&__val, std::__to_address(__base_), sizeof(value_type));
+ __builtin_memcpy(&__val, static_cast<const
void*>(std::__to_address(__base_)), sizeof(value_type));
return __val;
}
diff --git a/libcxx/include/__memory/uninitialized_algorithms.h
b/libcxx/include/__memory/uninitialized_algorithms.h
index dd72f3c10cf15a..e87cd5045c17f0 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, (void*)__first, sizeof(_Tp) * (__last -
__first));
}
}
>From 9cd993dbc2be6fe9c520b4421e8c182832423a79 Mon Sep 17 00:00:00 2001
From: serge-sans-paille <sguel...@mozilla.com>
Date: Tue, 8 Oct 2024 14:19:16 +0200
Subject: [PATCH 4/4] [ADT] Silent warning related to memcpy of non
trivially-copyable data
---
llvm/include/llvm/ADT/DenseMap.h | 3 ++-
llvm/include/llvm/ADT/Hashing.h | 2 +-
llvm/include/llvm/ADT/SmallVector.h | 11 ++++++-----
3 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/llvm/include/llvm/ADT/DenseMap.h b/llvm/include/llvm/ADT/DenseMap.h
index f0f992f8eac389..5d5002dec16caf 100644
--- a/llvm/include/llvm/ADT/DenseMap.h
+++ b/llvm/include/llvm/ADT/DenseMap.h
@@ -442,7 +442,8 @@ class DenseMapBase : public DebugEpochBase {
const size_t NumBuckets = getNumBuckets();
if constexpr (std::is_trivially_copyable_v<KeyT> &&
std::is_trivially_copyable_v<ValueT>) {
- memcpy(reinterpret_cast<void *>(Buckets), OtherBuckets,
+ memcpy(reinterpret_cast<void *>(Buckets),
+ reinterpret_cast<const void *>(OtherBuckets),
NumBuckets * sizeof(BucketT));
} else {
const KeyT EmptyKey = getEmptyKey();
diff --git a/llvm/include/llvm/ADT/Hashing.h b/llvm/include/llvm/ADT/Hashing.h
index 17dcf31c616aa7..c3e9f80bb204e9 100644
--- a/llvm/include/llvm/ADT/Hashing.h
+++ b/llvm/include/llvm/ADT/Hashing.h
@@ -508,7 +508,7 @@ struct hash_combine_recursive_helper {
// with the variadic combine because that formation can have varying
// argument types.
size_t partial_store_size = buffer_end - buffer_ptr;
- memcpy(buffer_ptr, &data, partial_store_size);
+ memcpy(buffer_ptr, reinterpret_cast<void *>(&data), partial_store_size);
// If the store fails, our buffer is full and ready to hash. We have to
// either initialize the hash state (on the first full buffer) or mix
diff --git a/llvm/include/llvm/ADT/SmallVector.h
b/llvm/include/llvm/ADT/SmallVector.h
index bd3e887e36bce4..efeadeb60acfcd 100644
--- a/llvm/include/llvm/ADT/SmallVector.h
+++ b/llvm/include/llvm/ADT/SmallVector.h
@@ -509,15 +509,15 @@ class SmallVectorTemplateBase<T, true> : public
SmallVectorTemplateCommon<T> {
/// starting with "Dest", constructing elements into it as needed.
template <typename T1, typename T2>
static void uninitialized_copy(
- T1 *I, T1 *E, T2 *Dest,
- std::enable_if_t<std::is_same<std::remove_const_t<T1>, T2>::value> * =
- nullptr) {
+ const T1 *I, const T1 *E, T2 *Dest,
+ std::enable_if_t<std::is_same<T1, T2>::value> * = nullptr) {
// Use memcpy for PODs iterated by pointers (which includes SmallVector
// iterators): std::uninitialized_copy optimizes to memmove, but we can
// use memcpy here. Note that I and E are iterators and thus might be
// invalid for memcpy if they are equal.
if (I != E)
- memcpy(reinterpret_cast<void *>(Dest), I, (E - I) * sizeof(T));
+ memcpy(reinterpret_cast<void *>(Dest), reinterpret_cast<const void *>(I),
+ (E - I) * sizeof(T));
}
/// Double the size of the allocated memory, guaranteeing space for at
@@ -560,7 +560,8 @@ class SmallVectorTemplateBase<T, true> : public
SmallVectorTemplateCommon<T> {
public:
void push_back(ValueParamT Elt) {
const T *EltPtr = reserveForParamAndGetAddress(Elt);
- memcpy(reinterpret_cast<void *>(this->end()), EltPtr, sizeof(T));
+ memcpy(reinterpret_cast<void *>(this->end()),
+ reinterpret_cast<const void *>(EltPtr), sizeof(T));
this->set_size(this->size() + 1);
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
