whytofly
/
qt6-bb10
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

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<qint64>::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<qint64>::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<qint64>::min().

Amends 9d23aebb27.

Pick-to: 6.5 6.2 5.15
Coverity-Id: 474294
Change-Id: I9a5a5acbdcf881a624bb827ca1dd970771f1bb6e
Reviewed-by: Shawn Rutledge <shawn.rutledge@qt.io>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
(cherry picked from commit b93575de01760ff2ab0d817557a642c71cdb4414)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
(cherry picked from commit 6cefb8be965759cc022ce3ef1ae7a722f0566ce7)
bb10
Marc Mutz 2025-02-17 10:22:41 +01:00 committed by Qt Cherry-pick Bot
parent 4d9f2a85d5
commit 73c44c8381
2 changed files with 2 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
src/corelib/text/qlocale.cpp
View File

@ -4798,9 +4798,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:

6
tests/auto/corelib/text/qlocale/tst_qlocale.cpp
View File

@ -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