From 6490b17e6f61dd38f2c71e5f28a51274b055dc64 Mon Sep 17 00:00:00 2001 From: Rasmus Munk Larsen <4643818-rmlarsen1@users.noreply.gitlab.com> Date: Sun, 22 Mar 2026 09:10:16 -0700 Subject: [PATCH] Fix sanitizer regressions in sparse serializer and packet tests libeigen/eigen!2319 Co-authored-by: Rasmus Munk Larsen --- Eigen/src/Core/GenericPacketMath.h | 18 ++++- Eigen/src/Core/arch/clang/PacketMath.h | 6 +- Eigen/src/SparseCore/SparseMatrix.h | 16 +++- Eigen/src/SparseCore/SparseVector.h | 16 +++- test/array_cwise.cpp | 14 +++- test/packetmath.cpp | 107 +++++++++++++++++-------- test/threads_runqueue.cpp | 4 +- 7 files changed, 129 insertions(+), 52 deletions(-) diff --git a/Eigen/src/Core/GenericPacketMath.h b/Eigen/src/Core/GenericPacketMath.h index 10c798aaf..64427e700 100644 --- a/Eigen/src/Core/GenericPacketMath.h +++ b/Eigen/src/Core/GenericPacketMath.h @@ -821,10 +821,24 @@ EIGEN_DEVICE_FUNC inline Packet pset1(const typename unpacket_traits::ty template EIGEN_DEVICE_FUNC inline Packet pset1frombits(BitsType a); +template ::value, int> = 0> +EIGEN_DEVICE_FUNC inline Scalar pload1_scalar(const Scalar* a) { + Scalar scalar; + EIGEN_USING_STD(memcpy) + memcpy(&scalar, a, sizeof(Scalar)); + return scalar; +} + +template ::value, int> = 0> +EIGEN_DEVICE_FUNC inline Scalar pload1_scalar(const Scalar* a) { + return Scalar(*a); +} + /** \internal \returns a packet with constant coefficients \a a[0], e.g.: (a[0],a[0],a[0],a[0]) */ template EIGEN_DEVICE_FUNC inline Packet pload1(const typename unpacket_traits::type* a) { - return pset1(*a); + using Scalar = typename unpacket_traits::type; + return pset1(pload1_scalar(a)); } /** \internal \returns a packet with elements of \a *from duplicated. @@ -834,7 +848,7 @@ EIGEN_DEVICE_FUNC inline Packet pload1(const typename unpacket_traits::t */ template EIGEN_DEVICE_FUNC EIGEN_STRONG_INLINE Packet ploaddup(const typename unpacket_traits::type* from) { - return *from; + return pload1(from); } /** \internal \returns a packet with elements of \a *from quadrupled. diff --git a/Eigen/src/Core/arch/clang/PacketMath.h b/Eigen/src/Core/arch/clang/PacketMath.h index 84126944f..491c0e298 100644 --- a/Eigen/src/Core/arch/clang/PacketMath.h +++ b/Eigen/src/Core/arch/clang/PacketMath.h @@ -245,7 +245,8 @@ EIGEN_STRONG_INLINE VectorT load_vector_unaligned(const scalar_type_of_vector_t< template EIGEN_STRONG_INLINE VectorT load_vector_aligned(const scalar_type_of_vector_t* from) { - return *reinterpret_cast(assume_aligned(from)); + eigen_assert((std::uintptr_t(from) % alignof(VectorT) == 0) && "load_vector_aligned"); + return *reinterpret_cast(assume_aligned(from)); } template @@ -255,7 +256,8 @@ EIGEN_STRONG_INLINE void store_vector_unaligned(scalar_type_of_vector_t template EIGEN_STRONG_INLINE void store_vector_aligned(scalar_type_of_vector_t* to, const VectorT& from) { - *reinterpret_cast(assume_aligned(to)) = from; + eigen_assert((std::uintptr_t(to) % alignof(VectorT) == 0) && "store_vector_aligned"); + *reinterpret_cast(assume_aligned(to)) = from; } } // namespace detail diff --git a/Eigen/src/SparseCore/SparseMatrix.h b/Eigen/src/SparseCore/SparseMatrix.h index 36646c8ab..451c7b089 100644 --- a/Eigen/src/SparseCore/SparseMatrix.h +++ b/Eigen/src/SparseCore/SparseMatrix.h @@ -1846,26 +1846,34 @@ class Serializer, void> { // Inner non-zero counts. std::size_t data_bytes = sizeof(StorageIndex) * header.outer_size; if (EIGEN_PREDICT_FALSE(src + data_bytes > end)) return nullptr; - memcpy(value.innerNonZeroPtr(), src, data_bytes); + if (data_bytes != 0) { + memcpy(value.innerNonZeroPtr(), src, data_bytes); + } src += data_bytes; } // Outer indices. std::size_t data_bytes = sizeof(StorageIndex) * (header.outer_size + 1); if (EIGEN_PREDICT_FALSE(src + data_bytes > end)) return nullptr; - memcpy(value.outerIndexPtr(), src, data_bytes); + if (data_bytes != 0) { + memcpy(value.outerIndexPtr(), src, data_bytes); + } src += data_bytes; // Inner indices. data_bytes = sizeof(StorageIndex) * header.inner_buffer_size; if (EIGEN_PREDICT_FALSE(src + data_bytes > end)) return nullptr; - memcpy(value.innerIndexPtr(), src, data_bytes); + if (data_bytes != 0) { + memcpy(value.innerIndexPtr(), src, data_bytes); + } src += data_bytes; // Values. data_bytes = sizeof(Scalar) * header.inner_buffer_size; if (EIGEN_PREDICT_FALSE(src + data_bytes > end)) return nullptr; - memcpy(value.valuePtr(), src, data_bytes); + if (data_bytes != 0) { + memcpy(value.valuePtr(), src, data_bytes); + } src += data_bytes; return src; } diff --git a/Eigen/src/SparseCore/SparseVector.h b/Eigen/src/SparseCore/SparseVector.h index 66fbb07e5..a1b89afdf 100644 --- a/Eigen/src/SparseCore/SparseVector.h +++ b/Eigen/src/SparseCore/SparseVector.h @@ -487,12 +487,16 @@ class Serializer, void> { // Inner indices. std::size_t data_bytes = sizeof(StorageIndex) * header.num_non_zeros; - memcpy(dest, value.innerIndexPtr(), data_bytes); + if (data_bytes != 0) { + memcpy(dest, value.innerIndexPtr(), data_bytes); + } dest += data_bytes; // Values. data_bytes = sizeof(Scalar) * header.num_non_zeros; - memcpy(dest, value.valuePtr(), data_bytes); + if (data_bytes != 0) { + memcpy(dest, value.valuePtr(), data_bytes); + } dest += data_bytes; return dest; @@ -515,13 +519,17 @@ class Serializer, void> { // Inner indices. std::size_t data_bytes = sizeof(StorageIndex) * header.num_non_zeros; if (EIGEN_PREDICT_FALSE(src + data_bytes > end)) return nullptr; - memcpy(value.innerIndexPtr(), src, data_bytes); + if (data_bytes != 0) { + memcpy(value.innerIndexPtr(), src, data_bytes); + } src += data_bytes; // Values. data_bytes = sizeof(Scalar) * header.num_non_zeros; if (EIGEN_PREDICT_FALSE(src + data_bytes > end)) return nullptr; - memcpy(value.valuePtr(), src, data_bytes); + if (data_bytes != 0) { + memcpy(value.valuePtr(), src, data_bytes); + } src += data_bytes; return src; } diff --git a/test/array_cwise.cpp b/test/array_cwise.cpp index f73a3bf85..dd39cf165 100644 --- a/test/array_cwise.cpp +++ b/test/array_cwise.cpp @@ -12,13 +12,21 @@ #include "random_without_cast_overflow.h" // suppress annoying unsigned integer warnings -template ::IsSigned> +template ::IsSigned && NumTraits::IsInteger, + bool IsSigned = NumTraits::IsSigned> struct negative_or_zero_impl { + static Scalar run(const Scalar& a) { + using UnsignedScalar = std::make_unsigned_t; + return static_cast(UnsignedScalar(0) - static_cast(a)); + } +}; +template +struct negative_or_zero_impl { static Scalar run(const Scalar& a) { return -a; } }; template -struct negative_or_zero_impl { - static Scalar run(const Scalar&) { return 0; } +struct negative_or_zero_impl { + static Scalar run(const Scalar&) { return Scalar(0); } }; template Scalar negative_or_zero(const Scalar& a) { diff --git a/test/packetmath.cpp b/test/packetmath.cpp index b4300b316..6620096d7 100644 --- a/test/packetmath.cpp +++ b/test/packetmath.cpp @@ -11,18 +11,33 @@ #include "packetmath_test_shared.h" #include "random_without_cast_overflow.h" -template +template ::IsInteger || !NumTraits::IsSigned, int> = 0> inline T REF_ADD(const T& a, const T& b) { return a + b; } -template +template ::IsInteger && NumTraits::IsSigned, int> = 0> +inline T REF_ADD(const T& a, const T& b) { + using UnsignedT = std::make_unsigned_t; + return static_cast(static_cast(a) + static_cast(b)); +} +template ::IsInteger || !NumTraits::IsSigned, int> = 0> inline T REF_SUB(const T& a, const T& b) { return a - b; } -template +template ::IsInteger && NumTraits::IsSigned, int> = 0> +inline T REF_SUB(const T& a, const T& b) { + using UnsignedT = std::make_unsigned_t; + return static_cast(static_cast(a) - static_cast(b)); +} +template ::IsInteger || !NumTraits::IsSigned, int> = 0> inline T REF_MUL(const T& a, const T& b) { return a * b; } +template ::IsInteger && NumTraits::IsSigned, int> = 0> +inline T REF_MUL(const T& a, const T& b) { + using UnsignedT = std::make_unsigned_t; + return static_cast(static_cast(a) * static_cast(b)); +} template struct madd_impl { @@ -41,8 +56,30 @@ struct madd_impl { }; template -struct madd_impl::value && Eigen::NumTraits::IsSigned>> { +struct madd_impl::IsInteger && NumTraits::IsSigned>> { + using UnsignedScalar = std::make_unsigned_t; + + static EIGEN_DEVICE_FUNC EIGEN_ALWAYS_INLINE Scalar madd(const Scalar& a, const Scalar& b, const Scalar& c) { + return static_cast(static_cast(a) * static_cast(b) + + static_cast(c)); + } + static EIGEN_DEVICE_FUNC EIGEN_ALWAYS_INLINE Scalar msub(const Scalar& a, const Scalar& b, const Scalar& c) { + return static_cast(static_cast(a) * static_cast(b) - + static_cast(c)); + } + static EIGEN_DEVICE_FUNC EIGEN_ALWAYS_INLINE Scalar nmadd(const Scalar& a, const Scalar& b, const Scalar& c) { + return static_cast(static_cast(c) - + static_cast(a) * static_cast(b)); + } + static EIGEN_DEVICE_FUNC EIGEN_ALWAYS_INLINE Scalar nmsub(const Scalar& a, const Scalar& b, const Scalar& c) { + return static_cast(UnsignedScalar(0) - (static_cast(a) * static_cast(b) + + static_cast(c))); + } +}; + +template +struct madd_impl::value && + Eigen::NumTraits::IsSigned && !NumTraits::IsInteger>> { static EIGEN_DEVICE_FUNC EIGEN_ALWAYS_INLINE Scalar madd(const Scalar& a, const Scalar& b, const Scalar& c) { return numext::madd(a, b, c); } @@ -354,9 +391,9 @@ void packetmath_boolean_mask_ops() { using RealScalar = typename NumTraits::Real; const int PacketSize = internal::unpacket_traits::size; const int size = 2 * PacketSize; - EIGEN_ALIGN_MAX Scalar data1[size]; - EIGEN_ALIGN_MAX Scalar data2[size]; - EIGEN_ALIGN_MAX Scalar ref[size]; + EIGEN_ALIGN_TO_BOUNDARY(sizeof(Packet)) Scalar data1[size]; + EIGEN_ALIGN_TO_BOUNDARY(sizeof(Packet)) Scalar data2[size]; + EIGEN_ALIGN_TO_BOUNDARY(sizeof(Packet)) Scalar ref[size]; for (int i = 0; i < size; ++i) { data1[i] = internal::random(); @@ -389,8 +426,8 @@ template void packetmath_boolean_mask_ops_real() { const int PacketSize = internal::unpacket_traits::size; const int size = 2 * PacketSize; - EIGEN_ALIGN_MAX Scalar data1[size]; - EIGEN_ALIGN_MAX Scalar data2[size]; + EIGEN_ALIGN_TO_BOUNDARY(sizeof(Packet)) Scalar data1[size]; + EIGEN_ALIGN_TO_BOUNDARY(sizeof(Packet)) Scalar data2[size]; for (int i = 0; i < PacketSize; ++i) { data1[i] = internal::random(); @@ -426,8 +463,8 @@ struct packetmath_boolean_mask_ops_notcomplex_test< static void run() { const int PacketSize = internal::unpacket_traits::size; const int size = 2 * PacketSize; - EIGEN_ALIGN_MAX Scalar data1[size]; - EIGEN_ALIGN_MAX Scalar data2[size]; + EIGEN_ALIGN_TO_BOUNDARY(sizeof(Packet)) Scalar data1[size]; + EIGEN_ALIGN_TO_BOUNDARY(sizeof(Packet)) Scalar data2[size]; for (int i = 0; i < PacketSize; ++i) { data1[i] = internal::random(); @@ -465,9 +502,9 @@ struct packetmath_minus_zero_add_test::size; const int size = 2 * PacketSize; - EIGEN_ALIGN_MAX Scalar data1[size] = {}; - EIGEN_ALIGN_MAX Scalar data2[size] = {}; - EIGEN_ALIGN_MAX Scalar ref[size] = {}; + EIGEN_ALIGN_TO_BOUNDARY(sizeof(Packet)) Scalar data1[size] = {}; + EIGEN_ALIGN_TO_BOUNDARY(sizeof(Packet)) Scalar data2[size] = {}; + EIGEN_ALIGN_TO_BOUNDARY(sizeof(Packet)) Scalar ref[size] = {}; for (int i = 0; i < PacketSize; ++i) { data1[i] = Scalar(-0.0); @@ -537,10 +574,10 @@ void packetmath() { const int max_size = PacketSize > 4 ? PacketSize : 4; const int size = PacketSize * max_size; - EIGEN_ALIGN_MAX Scalar data1[size]; - EIGEN_ALIGN_MAX Scalar data2[size]; - EIGEN_ALIGN_MAX Scalar data3[size]; - EIGEN_ALIGN_MAX Scalar ref[size]; + EIGEN_ALIGN_TO_BOUNDARY(sizeof(Packet)) Scalar data1[size]; + EIGEN_ALIGN_TO_BOUNDARY(sizeof(Packet)) Scalar data2[size]; + EIGEN_ALIGN_TO_BOUNDARY(sizeof(Packet)) Scalar data3[size]; + EIGEN_ALIGN_TO_BOUNDARY(sizeof(Packet)) Scalar ref[size]; RealScalar refvalue = RealScalar(0); eigen_optimization_barrier_test::run(); @@ -818,9 +855,9 @@ void packetmath_real() { const int PacketSize = internal::unpacket_traits::size; const int size = PacketSize * 4; - EIGEN_ALIGN_MAX Scalar data1[PacketSize * 4] = {}; - EIGEN_ALIGN_MAX Scalar data2[PacketSize * 4] = {}; - EIGEN_ALIGN_MAX Scalar ref[PacketSize * 4] = {}; + EIGEN_ALIGN_TO_BOUNDARY(sizeof(Packet)) Scalar data1[PacketSize * 4] = {}; + EIGEN_ALIGN_TO_BOUNDARY(sizeof(Packet)) Scalar data2[PacketSize * 4] = {}; + EIGEN_ALIGN_TO_BOUNDARY(sizeof(Packet)) Scalar ref[PacketSize * 4] = {}; // Negate with -0. if (PacketTraits::HasNegate) { @@ -1234,9 +1271,9 @@ std::enable_if_t run_ieee_cases(const FunctorT& fun) { } constexpr int size = PacketSize * 2; - EIGEN_ALIGN_MAX Scalar data1[size]; - EIGEN_ALIGN_MAX Scalar data2[size]; - EIGEN_ALIGN_MAX Scalar ref[size]; + EIGEN_ALIGN_TO_BOUNDARY(sizeof(Packet)) Scalar data1[size]; + EIGEN_ALIGN_TO_BOUNDARY(sizeof(Packet)) Scalar data2[size]; + EIGEN_ALIGN_TO_BOUNDARY(sizeof(Packet)) Scalar ref[size]; for (int i = 0; i < size; ++i) { data1[i] = data2[i] = ref[i] = Scalar(0); } @@ -1316,9 +1353,9 @@ void packetmath_notcomplex() { typedef internal::packet_traits PacketTraits; const int PacketSize = internal::unpacket_traits::size; - EIGEN_ALIGN_MAX Scalar data1[PacketSize * 4]; - EIGEN_ALIGN_MAX Scalar data2[PacketSize * 4]; - EIGEN_ALIGN_MAX Scalar ref[PacketSize * 4]; + EIGEN_ALIGN_TO_BOUNDARY(sizeof(Packet)) Scalar data1[PacketSize * 4]; + EIGEN_ALIGN_TO_BOUNDARY(sizeof(Packet)) Scalar data2[PacketSize * 4]; + EIGEN_ALIGN_TO_BOUNDARY(sizeof(Packet)) Scalar ref[PacketSize * 4]; Array::Map(data1, PacketSize * 4).setRandom(); @@ -1600,12 +1637,12 @@ void packetmath_complex() { const int PacketSize = internal::unpacket_traits::size; const int size = PacketSize * 4; - EIGEN_ALIGN_MAX Scalar data1[PacketSize * 4]; - EIGEN_ALIGN_MAX Scalar data2[PacketSize * 4]; - EIGEN_ALIGN_MAX Scalar ref[PacketSize * 4]; - EIGEN_ALIGN_MAX Scalar pval[PacketSize * 4]; - EIGEN_ALIGN_MAX RealScalar realdata[PacketSize * 4]; - EIGEN_ALIGN_MAX RealScalar realref[PacketSize * 4]; + EIGEN_ALIGN_TO_BOUNDARY(sizeof(Packet)) Scalar data1[PacketSize * 4]; + EIGEN_ALIGN_TO_BOUNDARY(sizeof(Packet)) Scalar data2[PacketSize * 4]; + EIGEN_ALIGN_TO_BOUNDARY(sizeof(Packet)) Scalar ref[PacketSize * 4]; + EIGEN_ALIGN_TO_BOUNDARY(sizeof(Packet)) Scalar pval[PacketSize * 4]; + EIGEN_ALIGN_TO_BOUNDARY(sizeof(Packet)) RealScalar realdata[PacketSize * 4]; + EIGEN_ALIGN_TO_BOUNDARY(sizeof(Packet)) RealScalar realref[PacketSize * 4]; for (int i = 0; i < size; ++i) { data1[i] = internal::random() * Scalar(1e2); @@ -1745,7 +1782,7 @@ template void packetmath_scatter_gather() { typedef typename NumTraits::Real RealScalar; const int PacketSize = internal::unpacket_traits::size; - EIGEN_ALIGN_MAX Scalar data1[PacketSize]; + EIGEN_ALIGN_TO_BOUNDARY(sizeof(Packet)) Scalar data1[PacketSize]; RealScalar refvalue = RealScalar(0); for (int i = 0; i < PacketSize; ++i) { data1[i] = internal::random(); diff --git a/test/threads_runqueue.cpp b/test/threads_runqueue.cpp index 4847eaeba..4eaaaefa5 100644 --- a/test/threads_runqueue.cpp +++ b/test/threads_runqueue.cpp @@ -182,7 +182,7 @@ void test_stress_runqueue() { })); for (int i = 0; i < 2; i++) { threads.emplace_back(new std::thread([&q, &total]() { - int sum = 0; + int64_t sum = 0; for (int j = 1; j < kEvents; j++) { if (q.PushBack(j) == 0) { sum += j; @@ -194,7 +194,7 @@ void test_stress_runqueue() { total += sum; })); threads.emplace_back(new std::thread([&q, &total]() { - int sum = 0; + int64_t sum = 0; std::vector stolen; for (int j = 1; j < kEvents;) { if (q.PopBackHalf(&stolen) == 0) {