[Libreoffice-commits] core.git: include/o3tl o3tl/qa

[Mike Kaganski (via logerrit)]

 include/o3tl/safeint.hxx         |    4 ++--
 include/o3tl/unit_conversion.hxx |   17 +++++++++++++----
 o3tl/qa/test-unit_conversion.cxx |    2 ++
 3 files changed, 17 insertions(+), 6 deletions(-)

New commits:
commit 635a0b426e0ab47ec136af7d110354db862c59fa
Author:     Mike Kaganski <mike.kagan...@collabora.com>
AuthorDate: Mon Oct 24 09:37:37 2022 +0200
Commit:     Mike Kaganski <mike.kagan...@collabora.com>
CommitDate: Mon Oct 24 14:07:31 2022 +0200

    Fix MulDivSaturate to avoid overflow
    
    Thanks to Caolan
    https://gerrit.libreoffice.org/c/core/+/110839/comments/5c47cab2_b0523546
    
    > oss-fuzz has come up with a case of n 9223372036854775807 m 72 d 127
    >
    > include/o3tl/unit_conversion.hxx:105:28: runtime error: signed integer
    > overflow: 9223372036854775807 + 63 cannot be represented in type 'long'
    
    Change-Id: I0fe26f6c854a90cf577a60528d15f3da5471b914
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/141711
    Tested-by: Jenkins
    Reviewed-by: Mike Kaganski <mike.kagan...@collabora.com>

diff --git a/include/o3tl/safeint.hxx b/include/o3tl/safeint.hxx
index c697a93164aa..a32c6beea142 100644
--- a/include/o3tl/safeint.hxx
+++ b/include/o3tl/safeint.hxx
@@ -27,7 +27,7 @@
 namespace o3tl
 {
 
-template <typename T> inline T saturating_add(T a, T b)
+template <typename T> inline constexpr T saturating_add(T a, T b)
 {
     if (b >= 0) {
         if (a <= std::numeric_limits<T>::max() - b) {
@@ -44,7 +44,7 @@ template <typename T> inline T saturating_add(T a, T b)
     }
 }
 
-template <typename T> inline T saturating_sub(T a, T b)
+template <typename T> inline constexpr T saturating_sub(T a, T b)
 {
     if (b >= 0) {
         if (a >= std::numeric_limits<T>::min() + b) {
diff --git a/include/o3tl/unit_conversion.hxx b/include/o3tl/unit_conversion.hxx
index 67830f0d16d7..7f0053627f50 100644
--- a/include/o3tl/unit_conversion.hxx
+++ b/include/o3tl/unit_conversion.hxx
@@ -98,11 +98,20 @@ constexpr sal_Int64 MulDiv(I n, sal_Int64 m, sal_Int64 d, 
bool& bOverflow, sal_I
 template <typename I, std::enable_if_t<std::is_integral_v<I>, int> = 0>
 constexpr sal_Int64 MulDivSaturate(I n, sal_Int64 m, sal_Int64 d)
 {
-    if (!isBetween(n, (SAL_MIN_INT64 + d / 2) / m, (SAL_MAX_INT64 - d / 2) / 
m))
+    if (sal_Int64 d_2 = d / 2; !isBetween(n, (SAL_MIN_INT64 + d_2) / m, 
(SAL_MAX_INT64 - d_2) / m))
     {
-        if (m > d && !isBetween(n, SAL_MIN_INT64 / m * d + d / 2, 
SAL_MAX_INT64 / m * d - d / 2))
-            return n > 0 ? SAL_MAX_INT64 : SAL_MIN_INT64; // saturate
-        return (n >= 0 ? n + d / 2 : n - d / 2) / d * m; // divide before 
multiplication
+        if (n >= 0)
+        {
+            if (m > d && std::make_unsigned_t<I>(n) > sal_uInt64(SAL_MAX_INT64 
/ m * d - d_2))
+                return SAL_MAX_INT64; // saturate
+            return saturating_add<sal_uInt64>(n, d_2) / d * m; // divide 
before multiplication
+        }
+        else if constexpr (std::is_signed_v<I>) // n < 0; don't compile for 
unsigned n
+        {
+            if (m > d && n < SAL_MIN_INT64 / m * d + d_2)
+                return SAL_MIN_INT64; // saturate
+            return saturating_sub<sal_Int64>(n, d_2) / d * m; // divide before 
multiplication
+        }
     }
     return MulDiv(n, m, d);
 }
diff --git a/o3tl/qa/test-unit_conversion.cxx b/o3tl/qa/test-unit_conversion.cxx
index 8b2c05c54549..d140f1da7ec6 100644
--- a/o3tl/qa/test-unit_conversion.cxx
+++ b/o3tl/qa/test-unit_conversion.cxx
@@ -878,4 +878,6 @@ static_assert(o3tl::toTwips(20, o3tl::Length::mm) == 1134);
 // 847 100thmm used to represent 24pt
 static_assert(o3tl::convert(24, o3tl::Length::pt, o3tl::Length::mm100) == 847);
 
+static_assert(o3tl::convertSaturate(SAL_MAX_INT64, 72, 127) == 
5228998320106644552); // no overflow
+
 /* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s 
cinkeys+=0=break: */