From 83c812e1322a2b004bc604be3e6f61fb83246eb0 Mon Sep 17 00:00:00 2001 From: Giuseppe D'Angelo Date: Tue, 28 Jan 2025 02:33:43 +0100 Subject: [PATCH] QNumeric: add a private qUnsignedAbs Checking qAbs preconditions reveals that several places into Qt are calling it with minimal values, causing UB. Those places do not actually need that the absolute value is of the same type as the parameter. Therefore, introduce qUnsignedAbs, which is like qAbs() but whose return type is always unsigned, and therefore can accept everything. This function is private; I don't want to encourage users to rely on our extension. Aggressively cherry picking because this will be needed in subsequent bugfixes. Change-Id: I2047768a5fd35f12bf898ca8c2008813434edd8d Pick-to: 6.9 6.8 6.5 6.2 Reviewed-by: Thiago Macieira --- src/corelib/global/qnumeric.h | 10 ++++ .../corelib/global/qnumeric/tst_qnumeric.cpp | 46 +++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/src/corelib/global/qnumeric.h b/src/corelib/global/qnumeric.h index 5a25b1c2392..2e13da4b850 100644 --- a/src/corelib/global/qnumeric.h +++ b/src/corelib/global/qnumeric.h @@ -336,6 +336,16 @@ template bool qMulOverflow(T v1, T *r) template constexpr inline T qAbs(const T &t) { return t >= 0 ? t : -t; } +namespace QtPrivate { +template , bool> = true> +constexpr inline auto qUnsignedAbs(T t) +{ + using U = std::make_unsigned_t; + return (t >= 0) ? U(t) : U(~U(t) + U(1)); +} +} // namespace QtPrivate + // gcc < 10 doesn't have __has_builtin #if defined(Q_PROCESSOR_ARM_64) && (__has_builtin(__builtin_round) || defined(Q_CC_GNU)) && !defined(Q_CC_CLANG) // ARM64 has a single instruction that can do C++ rounding with conversion to integer. diff --git a/tests/auto/corelib/global/qnumeric/tst_qnumeric.cpp b/tests/auto/corelib/global/qnumeric/tst_qnumeric.cpp index 37127b4ce09..ec7896ca4fc 100644 --- a/tests/auto/corelib/global/qnumeric/tst_qnumeric.cpp +++ b/tests/auto/corelib/global/qnumeric/tst_qnumeric.cpp @@ -9,6 +9,7 @@ #include #include +#include #include @@ -789,5 +790,50 @@ static_assert(q26::saturate_cast(min) == 0); } // namespace SaturateCastTest +namespace UnsignedAbsTest { +using QtPrivate::qUnsignedAbs; + +static_assert(std::is_same_v); +static_assert(std::is_same_v); +static_assert(std::is_same_v); +static_assert(std::is_same_v); +static_assert(std::is_same_v); +static_assert(std::is_same_v); +static_assert(std::is_same_v); +static_assert(std::is_same_v); +static_assert(std::is_same_v); +static_assert(std::is_same_v); +static_assert(std::is_same_v); + +template constexpr bool isEqual(T a, T b) { return a == b; } + +#define TEST_TYPE(type, utype, minimal, maximal) \ + static_assert(isEqual(qUnsignedAbs(type(minimal)), (utype)((utype)(maximal) + (utype)1))); \ + static_assert(isEqual(qUnsignedAbs(type(0)), (utype)(0))); \ + static_assert(isEqual(qUnsignedAbs(type(1)), (utype)(1))); \ + static_assert(isEqual(qUnsignedAbs(type(-1)), (utype)(1))); \ + static_assert(isEqual(qUnsignedAbs(type(10)), (utype)(10))); \ + static_assert(isEqual(qUnsignedAbs(type(-10)), (utype)(10))); \ + static_assert(isEqual(qUnsignedAbs(type(maximal)), (utype)(maximal))); \ + +using schar = signed char; +using uchar = unsigned char; +using ushort = unsigned short; +using uint = unsigned int; +using ulong = unsigned long; + +#if CHAR_MAX == 127 +// char is signed +TEST_TYPE(char, uchar, CHAR_MIN, CHAR_MAX) +#endif +TEST_TYPE(schar, uchar, SCHAR_MIN, SCHAR_MAX) +TEST_TYPE(short, ushort, SHRT_MIN, SHRT_MAX) +TEST_TYPE(int, uint, INT_MIN, INT_MAX) +TEST_TYPE(long, ulong, LONG_MIN, LONG_MAX) +TEST_TYPE(qlonglong, qulonglong, LLONG_MIN, LLONG_MAX) + +#undef TEST_TYPE +} // namespace UnsignedAbsTest + QTEST_APPLESS_MAIN(tst_QNumeric) #include "tst_qnumeric.moc"