From 8ac57ff6bc778519bb1edc4036ce79ab8f688e27 Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Sat, 15 Jun 2024 09:17:25 -0700 Subject: [PATCH] QBitArray: fix read of uninitialized terminating null Commit 54c373faa4f9582fd09a802727821fd544a7b2c5 updated the bitwise operations to be more efficient, bypassing QByteArray and going straight to QByteArrayData (a.k.a. QArrayDataPointer). This meant we also bypassed the initialization of the null terminator. This wasn't caught in our unit testing and with some runtimes because the memory we allocated happened to be zero or contain the information we wanted. But with Visual Studio, the debug-mode runtime initializes all newly allocated memory with pattern 0xcd, which showed up as a problem. [ChangeLog][QtCore][QBitArray] Fixed a regression introduced in 6.7.0 that could cause QBitArray to report wrong bit counts after a bitwise operation. Pick-to: 6.7 6.8 Fixes: QTBUG-126343 Change-Id: Icdc467f26dea4b05ad90fffd17d939c3b416adca Reviewed-by: Fabian Kosmale --- src/corelib/tools/qbitarray.cpp | 11 +- .../corelib/tools/qbitarray/tst_qbitarray.cpp | 111 ++++++++++-------- 2 files changed, 67 insertions(+), 55 deletions(-) diff --git a/src/corelib/tools/qbitarray.cpp b/src/corelib/tools/qbitarray.cpp index e4276d383d1..d5643df0250 100644 --- a/src/corelib/tools/qbitarray.cpp +++ b/src/corelib/tools/qbitarray.cpp @@ -531,12 +531,17 @@ static QBitArray sizedForOverwrite(const QBitArray &a1, const QBitArray &a2) QByteArrayData bytes(n, n); // initialize the count of bits in the last byte (see construction note) - if (n1 > n2) + // and the QByteArray null termination (some of our algorithms read it) + if (n1 > n2) { *bytes.ptr = *d1.ptr; - else if (n2 > n1) + bytes.ptr[n1] = 0; + } else if (n2 > n1) { *bytes.ptr = *d2.ptr; - else if (n1) // n1 == n2 + bytes.ptr[n2] = 0; + } else if (n1) { // n1 == n2 *bytes.ptr = qMin(*d1.ptr, *d2.ptr); + bytes.ptr[n1] = 0; + } result.data_ptr() = std::move(bytes); return result; diff --git a/tests/auto/corelib/tools/qbitarray/tst_qbitarray.cpp b/tests/auto/corelib/tools/qbitarray/tst_qbitarray.cpp index f52a368aa9c..4281d931799 100644 --- a/tests/auto/corelib/tools/qbitarray/tst_qbitarray.cpp +++ b/tests/auto/corelib/tools/qbitarray/tst_qbitarray.cpp @@ -375,6 +375,13 @@ void tst_QBitArray::operator_andeq_data() << QStringToQBitArray(QString()); } +#define COMPARE_BITARRAY_EQ(actual, expected) do { \ + QT_TEST_EQUALITY_OPS(actual, expected, true); \ + QCOMPARE(actual.count(), expected.count()); \ + QCOMPARE(actual.count(true), expected.count(true)); \ + QCOMPARE(actual.count(false), expected.count(false)); \ + } while (false) + void tst_QBitArray::operator_andeq() { QFETCH(QBitArray, input1); @@ -383,32 +390,32 @@ void tst_QBitArray::operator_andeq() QBitArray result = input1; result &= input2; - QT_TEST_EQUALITY_OPS(result, res, true); + COMPARE_BITARRAY_EQ(result, res); result = input1; result &= std::move(input2); - QT_TEST_EQUALITY_OPS(result, res, true); + COMPARE_BITARRAY_EQ(result, res); result = input1; result &= detached(input2); - QT_TEST_EQUALITY_OPS(result, res, true); + COMPARE_BITARRAY_EQ(result, res); // operation is commutative result = input2; result &= input1; - QT_TEST_EQUALITY_OPS(result, res, true); + COMPARE_BITARRAY_EQ(result, res); result = input2; result &= std::move(input1); - QT_TEST_EQUALITY_OPS(result, res, true); + COMPARE_BITARRAY_EQ(result, res); result = input2; result &= detached(input1); - QT_TEST_EQUALITY_OPS(result, res, true); + COMPARE_BITARRAY_EQ(result, res); // operation is idempotent result &= result; - QT_TEST_EQUALITY_OPS(result, res, true); + COMPARE_BITARRAY_EQ(result, res); result &= std::move(result); - QT_TEST_EQUALITY_OPS(result, res, true); + COMPARE_BITARRAY_EQ(result, res); result &= detached(result); - QT_TEST_EQUALITY_OPS(result, res, true); + COMPARE_BITARRAY_EQ(result, res); } void tst_QBitArray::operator_and() @@ -418,27 +425,27 @@ void tst_QBitArray::operator_and() QFETCH(QBitArray, res); QBitArray result = input1 & input2; - QT_TEST_EQUALITY_OPS(result, res, true); + COMPARE_BITARRAY_EQ(result, res); result = input1 & QBitArray(input2); - QT_TEST_EQUALITY_OPS(result, res, true); + COMPARE_BITARRAY_EQ(result, res); result = input1 & detached(input2); - QT_TEST_EQUALITY_OPS(result, res, true); + COMPARE_BITARRAY_EQ(result, res); // operation is commutative result = input2 & input1; - QT_TEST_EQUALITY_OPS(result, res, true); + COMPARE_BITARRAY_EQ(result, res); result = input2 & QBitArray(input1); - QT_TEST_EQUALITY_OPS(result, res, true); + COMPARE_BITARRAY_EQ(result, res); result = input2 & detached(input1); - QT_TEST_EQUALITY_OPS(result, res, true); + COMPARE_BITARRAY_EQ(result, res); // operation is idempotent result = result & result; - QT_TEST_EQUALITY_OPS(result, res, true); + COMPARE_BITARRAY_EQ(result, res); result = result & QBitArray(result); - QT_TEST_EQUALITY_OPS(result, res, true); + COMPARE_BITARRAY_EQ(result, res); result = result & detached(result); - QT_TEST_EQUALITY_OPS(result, res, true); + COMPARE_BITARRAY_EQ(result, res); } void tst_QBitArray::operator_oreq_data() @@ -489,32 +496,32 @@ void tst_QBitArray::operator_oreq() QBitArray result = input1; result |= input2; - QT_TEST_EQUALITY_OPS(result, res, true); + COMPARE_BITARRAY_EQ(result, res); result = input1; result |= QBitArray(input2); - QT_TEST_EQUALITY_OPS(result, res, true); + COMPARE_BITARRAY_EQ(result, res); result = input1; result |= detached(input2); - QT_TEST_EQUALITY_OPS(result, res, true); + COMPARE_BITARRAY_EQ(result, res); // operation is commutative result = input2; result |= input1; - QT_TEST_EQUALITY_OPS(result, res, true); + COMPARE_BITARRAY_EQ(result, res); result = input2; result |= QBitArray(input1); - QT_TEST_EQUALITY_OPS(result, res, true); + COMPARE_BITARRAY_EQ(result, res); result = input2; result |= detached(input1); - QT_TEST_EQUALITY_OPS(result, res, true); + COMPARE_BITARRAY_EQ(result, res); // operation is idempotent result |= result; - QT_TEST_EQUALITY_OPS(result, res, true); + COMPARE_BITARRAY_EQ(result, res); result |= QBitArray(result); - QT_TEST_EQUALITY_OPS(result, res, true); + COMPARE_BITARRAY_EQ(result, res); result |= detached(result); - QT_TEST_EQUALITY_OPS(result, res, true); + COMPARE_BITARRAY_EQ(result, res); } void tst_QBitArray::operator_or() @@ -524,27 +531,27 @@ void tst_QBitArray::operator_or() QFETCH(QBitArray, res); QBitArray result = input1 | input2; - QT_TEST_EQUALITY_OPS(result, res, true); + COMPARE_BITARRAY_EQ(result, res); result = input1 | QBitArray(input2); - QT_TEST_EQUALITY_OPS(result, res, true); + COMPARE_BITARRAY_EQ(result, res); result = input1 | detached(input2); - QT_TEST_EQUALITY_OPS(result, res, true); + COMPARE_BITARRAY_EQ(result, res); // operation is commutative result = input2 | input1; - QT_TEST_EQUALITY_OPS(result, res, true); + COMPARE_BITARRAY_EQ(result, res); result = input2 | QBitArray(input1); - QT_TEST_EQUALITY_OPS(result, res, true); + COMPARE_BITARRAY_EQ(result, res); result = input2 | detached(input1); - QT_TEST_EQUALITY_OPS(result, res, true); + COMPARE_BITARRAY_EQ(result, res); // operation is idempotent result = result | result; - QT_TEST_EQUALITY_OPS(result, res, true); + COMPARE_BITARRAY_EQ(result, res); result = result | QBitArray(result); - QT_TEST_EQUALITY_OPS(result, res, true); + COMPARE_BITARRAY_EQ(result, res); result = result | detached(result); - QT_TEST_EQUALITY_OPS(result, res, true); + COMPARE_BITARRAY_EQ(result, res); } void tst_QBitArray::operator_xoreq_data() @@ -593,24 +600,24 @@ void tst_QBitArray::operator_xoreq() QBitArray result = input1; result ^= input2; - QT_TEST_EQUALITY_OPS(result, res, true); + COMPARE_BITARRAY_EQ(result, res); result = input1; result ^= QBitArray(input2); - QT_TEST_EQUALITY_OPS(result, res, true); + COMPARE_BITARRAY_EQ(result, res); result = input1; result ^= detached(input2); - QT_TEST_EQUALITY_OPS(result, res, true); + COMPARE_BITARRAY_EQ(result, res); // operation is commutative result = input2; result ^= input1; - QT_TEST_EQUALITY_OPS(result, res, true); + COMPARE_BITARRAY_EQ(result, res); result = input2; result ^= QBitArray(input1); - QT_TEST_EQUALITY_OPS(result, res, true); + COMPARE_BITARRAY_EQ(result, res); result = input2; result ^= detached(input1); - QT_TEST_EQUALITY_OPS(result, res, true); + COMPARE_BITARRAY_EQ(result, res); // XORing with oneself is nilpotent result = input1; @@ -651,19 +658,19 @@ void tst_QBitArray::operator_xor() QFETCH(QBitArray, res); QBitArray result = input1 ^ input2; - QT_TEST_EQUALITY_OPS(result, res, true); + COMPARE_BITARRAY_EQ(result, res); result = input1 ^ QBitArray(input2); - QT_TEST_EQUALITY_OPS(result, res, true); + COMPARE_BITARRAY_EQ(result, res); result = input1 ^ detached(input2); - QT_TEST_EQUALITY_OPS(result, res, true); + COMPARE_BITARRAY_EQ(result, res); // operation is commutative result = input2 ^ input1; - QT_TEST_EQUALITY_OPS(result, res, true); + COMPARE_BITARRAY_EQ(result, res); result = input2 ^ QBitArray(input1); - QT_TEST_EQUALITY_OPS(result, res, true); + COMPARE_BITARRAY_EQ(result, res); result = input2 ^ detached(input1); - QT_TEST_EQUALITY_OPS(result, res, true); + COMPARE_BITARRAY_EQ(result, res); // XORing with oneself is nilpotent result = input1 ^ input1; @@ -734,7 +741,7 @@ void tst_QBitArray::operator_neg() input = ~input; - QT_TEST_EQUALITY_OPS(input, res, true); + COMPARE_BITARRAY_EQ(input, res); QT_TEST_EQUALITY_OPS(~~input, res, true); // performs two in-place negations } @@ -795,9 +802,9 @@ void tst_QBitArray::datastream() QCOMPARE(array1.count(true), onBits); QCOMPARE(array1.count(false), numBits - onBits); - QT_TEST_EQUALITY_OPS(array1, bits, true); - QT_TEST_EQUALITY_OPS(array2, bits, true); - QT_TEST_EQUALITY_OPS(array3, bits, true); + COMPARE_BITARRAY_EQ(array1, bits); + COMPARE_BITARRAY_EQ(array2, bits); + COMPARE_BITARRAY_EQ(array3, bits); } void tst_QBitArray::invertOnNull() const