From 6cefb8be965759cc022ce3ef1ae7a722f0566ce7 Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Mon, 17 Feb 2025 10:22:41 +0100 Subject: [PATCH] QLocale: fix UB (signed overflow) in formattedDataSize() Coverity complains that qCountLeadingZeroBits() may return 64 and then the RHS will be out-of-range for int. Of course, 64 leading zero bits means that the argument is 0, which we excluded in the first `if` of the if-else chain. I can only guess (because it doesn't tell me which value of `bytes` it is using for the analysis) that Coverity assumes bytes == numeric_limits::min() and considers the overflow in qAbs() to yield a 0, because, it being UB, it may yield anything. Be that as it may, the fact remains that formattedDataSize() invokes UB when passed numeric_limits::min(), as confirmed by ubsan: global/qnumeric.h:479:26: runtime error: negation of -9223372036854775808 cannot be represented in type 'long long int'; cast to an unsigned type to negate this value to itself text/qlocale.cpp:5062:82: runtime error: signed integer overflow: -2147483648 * 3 cannot be represented in type 'int' text/qlocale.cpp:5062:26: runtime error: division by zero FAIL! : tst_QLocale::formattedDataSize(English-Decimal-min) Compared values are not the same Actual (QLocale(language).formattedDataSize(bytes, decimalPlaces, units)): "-inf bytes" Expected ("output") : "-9.22 EB" So fix the overflow by using the new QtPrivate::qUnsignedAbs(). Then sit back and await Coverity's verdict on the next run. [ChangeLog][QtCore][QLocale] Fix issue when calling formattedDataSize() with numeric_limits::min(). Amends 9d23aebb271ea534a66cb3aceb2e63d9a1c870d6. Pick-to: 6.8 6.5 6.2 5.15 Coverity-Id: 474294 Change-Id: I9a5a5acbdcf881a624bb827ca1dd970771f1bb6e Reviewed-by: Shawn Rutledge Reviewed-by: Thiago Macieira Reviewed-by: Edward Welbourne (cherry picked from commit b93575de01760ff2ab0d817557a642c71cdb4414) Reviewed-by: Qt Cherry-pick Bot --- src/corelib/text/qlocale.cpp | 4 ++-- tests/auto/corelib/text/qlocale/tst_qlocale.cpp | 6 ------ 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/src/corelib/text/qlocale.cpp b/src/corelib/text/qlocale.cpp index f318ec8316a..0d46506c764 100644 --- a/src/corelib/text/qlocale.cpp +++ b/src/corelib/text/qlocale.cpp @@ -4933,9 +4933,9 @@ QString QLocale::formattedDataSize(qint64 bytes, int precision, DataSizeFormats if (!bytes) { power = 0; } else if (format & DataSizeBase1000) { - power = int(std::log10(qAbs(bytes)) / 3); + power = int(std::log10(QtPrivate::qUnsignedAbs(bytes)) / 3); } else { // Compute log2(bytes) / 10: - power = int((63 - qCountLeadingZeroBits(quint64(qAbs(bytes)))) / 10); + power = int((63 - qCountLeadingZeroBits(QtPrivate::qUnsignedAbs(bytes))) / 10); base = 1024; } // Only go to doubles if we'll be using a quantifier: diff --git a/tests/auto/corelib/text/qlocale/tst_qlocale.cpp b/tests/auto/corelib/text/qlocale/tst_qlocale.cpp index 4c0dfe322f3..b8cacda937f 100644 --- a/tests/auto/corelib/text/qlocale/tst_qlocale.cpp +++ b/tests/auto/corelib/text/qlocale/tst_qlocale.cpp @@ -3970,9 +3970,7 @@ void tst_QLocale::formattedDataSize_data() ROWQ("IEC-1235k", 2, 1234567, "1", "18 Mi"); ROWQ("IEC-1374k", 2, 1374744, "1", "31 Mi"); ROWQ("IEC-1234M", 2, 1234567890, "1", "15 Gi"); - if (false) { // hits UB ROWQ("IEC-min", 2, min64, "-8", "00 Ei"); - } ROWQ("IEC-max", 2, max64, "8", "00 Ei"); } { @@ -3986,9 +3984,7 @@ void tst_QLocale::formattedDataSize_data() ROWQ("Trad--1235k", 2, -1234567, "-1", "18 M"); ROWQ("Trad-1374k", 2, 1374744, "1", "31 M"); ROWQ("Trad-1234M", 2, 1234567890, "1", "15 G"); - if (false) { // hits UB ROWQ("Trad-min", 2, min64, "-8", "00 E"); - } ROWQ("Trad-max", 2, max64, "8", "00 E"); } { @@ -4001,9 +3997,7 @@ void tst_QLocale::formattedDataSize_data() ROWQ("Decimal-1374k", 2, 1374744, "1", "37 M"); ROWQ("Decimal-1234M", 2, 1234567890, "1", "23 G"); ROWQ("Decimal--1234M", 2, -1234567890, "-1", "23 G"); - if (false) { // hits UB ROWQ("Decimal-min", 2, min64, "-9", "22 E"); - } ROWQ("Decimal-max", 2, max64, "9", "22 E"); } #undef ROWQ