diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index df165b91252505..85a79218e03054 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/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 ------------------------- diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index f4a2d4a3f0656a..e822b989ab6994 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; +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; 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()) { + + 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)); +} 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 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++; } 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(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)); } } 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 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().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().deallocate(p, n); } 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 && std::is_trivially_copyable_v) { - memcpy(reinterpret_cast(Buckets), OtherBuckets, + memcpy(reinterpret_cast(Buckets), + reinterpret_cast(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(&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 : public SmallVectorTemplateCommon { /// starting with "Dest", constructing elements into it as needed. template static void uninitialized_copy( - T1 *I, T1 *E, T2 *Dest, - std::enable_if_t, T2>::value> * = - nullptr) { + const T1 *I, const T1 *E, T2 *Dest, + std::enable_if_t::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(Dest), I, (E - I) * sizeof(T)); + memcpy(reinterpret_cast(Dest), reinterpret_cast(I), + (E - I) * sizeof(T)); } /// Double the size of the allocated memory, guaranteeing space for at @@ -560,7 +560,8 @@ class SmallVectorTemplateBase : public SmallVectorTemplateCommon { public: void push_back(ValueParamT Elt) { const T *EltPtr = reserveForParamAndGetAddress(Elt); - memcpy(reinterpret_cast(this->end()), EltPtr, sizeof(T)); + memcpy(reinterpret_cast(this->end()), + reinterpret_cast(EltPtr), sizeof(T)); this->set_size(this->size() + 1); }