From f020c4ba51e257915837f2672cc37b5ac3f51936 Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Thu, 18 Aug 2022 17:24:34 +0200 Subject: [PATCH] QIODevice: fix narrowing conversions in debugBinaryString MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The debugBinaryString() function is a hexedit-like pretty-printer which prints in increments of 16 octets, so it can go forever. The old code had an overload set (QByteArray) and (ptr, qint64 size), but made the (ptr, n) overload construct a QByteArray to call the QBA overload. That's a narrowing conversion right there, except it can't trigger because no machine can hold that much memory. But it may warn. The code inside went on with further narrowing conversions to int, this time, which does have an effect on 64-bit. Granted, you'd wait a long time for this inefficient pretty-printer to write out more than 16Mi lines of hex dump, but it's wrong nonetheless, because narrowing conversions work with modulo arithmetic and not saturation arithmetic, so passing a buffer of INT_MAX + 1 size would print nothing, which would probably cause some time lost hunting unrelated bugs. So, fix the whole thing as follows: - remove the QBA overload, it was never called - loop over the full range of up to LLONG_MAX characters; if developers pass too much data, that's SEP - remove the narrowing casts to qsizetype at the call sites (avoids modulo arithmetic) As a drive-by, make the function static, and to get compilers to actually see all this, make it [[maybe_unused]] instead of ifdef'ing out. Not picking back, as it's debug-only code. Task-number: QTBUG-103525 Change-Id: I8b06466365d8c57b14535d8752428a614f244297 Reviewed-by: Thiago Macieira Reviewed-by: Mårten Nordheim Reviewed-by: Qt CI Bot --- src/corelib/io/qiodevice.cpp | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/src/corelib/io/qiodevice.cpp b/src/corelib/io/qiodevice.cpp index 5630a8b226..cf430cf937 100644 --- a/src/corelib/io/qiodevice.cpp +++ b/src/corelib/io/qiodevice.cpp @@ -21,23 +21,23 @@ QT_BEGIN_NAMESPACE using namespace Qt::StringLiterals; -#ifdef QIODEVICE_DEBUG -void debugBinaryString(const QByteArray &input) +[[maybe_unused]] +static void debugBinaryString(const char *input, qint64 maxlen) { QByteArray tmp; - int startOffset = 0; - for (int i = 0; i < input.size(); ++i) { + qlonglong startOffset = 0; + for (qint64 i = 0; i < maxlen; ++i) { tmp += input[i]; - if ((i % 16) == 15 || i == (input.size() - 1)) { - printf("\n%15d:", startOffset); + if ((i % 16) == 15 || i == (maxlen - 1)) { + printf("\n%15lld:", startOffset); startOffset += tmp.size(); - for (int j = 0; j < tmp.size(); ++j) + for (qsizetype j = 0; j < tmp.size(); ++j) printf(" %02x", int(uchar(tmp[j]))); - for (int j = tmp.size(); j < 16 + 1; ++j) + for (qsizetype j = tmp.size(); j < 16 + 1; ++j) printf(" "); - for (int j = 0; j < tmp.size(); ++j) + for (qsizetype j = 0; j < tmp.size(); ++j) printf("%c", isprint(int(uchar(tmp[j]))) ? tmp[j] : '.'); tmp.clear(); } @@ -45,12 +45,6 @@ void debugBinaryString(const QByteArray &input) printf("\n\n"); } -void debugBinaryString(const char *data, qint64 maxlen) -{ - debugBinaryString(QByteArray(data, maxlen)); -} -#endif - #define Q_VOID static void checkWarnMessage(const QIODevice *device, const char *function, const char *what) @@ -1334,7 +1328,7 @@ qint64 QIODevice::readLine(char *data, qint64 maxSize) #if defined QIODEVICE_DEBUG printf("%p \treturning %lld, d->pos = %lld, d->buffer.size() = %lld, size() = %lld\n", this, readBytes, d->pos, d->buffer.size(), size()); - debugBinaryString(data, qsizetype(readBytes)); + debugBinaryString(data, readBytes); #endif return readBytes; @@ -1378,7 +1372,7 @@ qint64 QIODevicePrivate::readLine(char *data, qint64 maxSize) #if defined QIODEVICE_DEBUG printf("%p \tread from buffer: %lld bytes, last character read: %hhx\n", q, readSoFar, data[readSoFar - 1]); - debugBinaryString(data, qsizetype(readSoFar)); + debugBinaryString(data, readSoFar); #endif if (data[readSoFar - 1] == '\n') { if (openMode & QIODevice::Text) { @@ -1405,7 +1399,7 @@ qint64 QIODevicePrivate::readLine(char *data, qint64 maxSize) printf("%p \tread from readLineData: %lld bytes, readSoFar = %lld bytes\n", q, readBytes, readSoFar); if (readBytes > 0) { - debugBinaryString(data, qsizetype(readSoFar + readBytes)); + debugBinaryString(data, readSoFar + readBytes); } #endif if (readBytes < 0) {