A review of endianness bugs in Qt, and how they were fixed

As you may know, I am Qt 5 maintainer in Debian. Maintaning Qt means not only bumping the version each time a new version is released, but also making sure Qt builds successfully on all architectures that are supported in Debian (and for some submodules, the automatic tests pass).

An important sort of build failures are endianness specific failures. Most widely used architectures (x86_64, aarch64) are little endian. However, Debian officially supports one big endian architecture (s390x), and unofficially a few more ports are provided, such as ppc64 and sparc64.

Unfortunately, Qt upstream does not have any big endian machine in their CI system, so endianness issues get noticed only when the packages fail to build on our build daemons. In the last years I have discovered and fixed some such issues in various parts of Qt, so I decided to write a post to illustrate how to write really cross-platform C/C++ code.

Issue 1: the WebP image format handler (code review)

The relevant code snippet is:

if (srcImage.format() != QImage::Format_ARGB32)
    srcImage = srcImage.convertToFormat(QImage::Format_ARGB32);
// ...
if (!WebPPictureImportBGRA(&picture, srcImage.bits(), srcImage.bytesPerLine())) {
    // ...
}

The code here is serializing the images into QImage::Format_ARGB32 format, and then passing the bytes into WebP’s import function. With this format, the image is stored using a 32-bit ARGB format (0xAARRGGBB). This means that the bytes will be 0xBB, 0xGG, 0xRR, 0xAA or little endian and 0xAA, 0xRR, 0xGG, 0xBB on big endian. However, WebPPictureImportBGRA expects the first format on all architectures.

The fix was to use QImage::Format_RGBA8888. As the QImage documentation says, with this format the order of the colors is the same on any architecture if read as bytes 0xRR, 0xGG, 0xBB, 0xAA.

Issue 2: qimage_converter_map structure (code review)

The code seems to already support big endian. But maybe you can spot the error?

#if Q_BYTE_ORDER == Q_LITTLE_ENDIAN
        0,
        convert_ARGB_to_ARGB_PM,
#else
        0,
        0
#endif

It is the missing comma! It is present in the little endian block, but not in the big endian one. This was fixed trivially.

Issue 3: QHandle, part of Qt 3D module (code review)

QHandle class uses a union that is declared as follows:

struct Data {
    quint32 m_index : IndexBits;
    quint32 m_counter : CounterBits;
    quint32 m_unused : 2;
};
union {
    Data d;
    quint32 m_handle;
};

The sizes are declared such as IndexBits + CounterBits + 2 is always equal to 32 (four bytes).

Then we have a constructor that sets the values of Data struct:

QHandle(quint32 i, quint32 count)
{
    d.m_index = i;
    d.m_counter = count;
    d.m_unused = 0;
}

The value of m_handle will be different depending on endianness! So the test that was expecting a particular value with given constructor arguments was failing. I fixed it by using the following macro:

#if Q_BYTE_ORDER == Q_BIG_ENDIAN
#define GET_EXPECTED_HANDLE(qHandle) ((qHandle.index() << (qHandle.CounterBits + 2)) + (qHandle.counter() << 2))
#else /* Q_LITTLE_ENDIAN */
#define GET_EXPECTED_HANDLE(qHandle) (qHandle.index() + (qHandle.counter() << qHandle.IndexBits))
#endif

Issue 4: QML compiler (code review)

The QML compiler used a helper class named LEUInt32 (based on QLEInteger) that always stored the numbers in little endian internally. This class can be safely mixed with native quint32 on little endian systems, but not on big endian.

Usually the compiler would warn about type mismatch, but here the code used reinterpret_cast, such as:

quint32 *objectTable = reinterpret_cast<quint32*>(data + qmlUnit->offsetToObjects);

So this was not noticed on build time, but the compiler was crashing. The fix was trivial again, replacing quint32 with QLEUInt32.

Issue 5: QModbusPdu, part of Qt Serial Bus module (code review)

The code snippet is simple:

QModbusPdu::FunctionCode code = QModbusPdu::Invalid;
if (stream.readRawData((char *) (&code), sizeof(quint8)) != sizeof(quint8))
    return stream;

QModbusPdu::FunctionCode is an enum, so code is a multi-byte value (even if only one byte is significant). However, (char *) (&code) returns a pointer to the first byte of it. It is the needed byte on little endian systems, but it is the wrong byte on big endian ones!

The correct fix was using a temporary one-byte variable:

quint8 codeByte = 0;
if (stream.readRawData((char *) (&codeByte), sizeof(quint8)) != sizeof(quint8))
    return stream;
QModbusPdu::FunctionCode code = (QModbusPdu::FunctionCode) codeByte;

Issue 6: qt_is_ascii (code review)

This function, as the name says, checks whether a string is ASCII. It does that by splitting the string into 4-byte chunks:

while (ptr + 4 <= end) {
    quint32 data = qFromUnaligned<quint32>(ptr);
    if (data &= 0x80808080U) {
        uint idx = qCountTrailingZeroBits(data);
        ptr += idx / 8;
        return false;
    }
    ptr += 4;
}

idx / 8 is the number of trailing zero bytes. However, the bytes which are trailing on little endian are actually leading on big endian! So we can use qCountLeadingZeroBits there.

Issue 7: the bundled copy of tinycbor (upstream pull request)

Similar to issue 5, the code was reading into the wrong byte:

if (bytesNeeded <= 2) {
    read_bytes_unchecked(it, &it->extra, 1, bytesNeeded);
    if (bytesNeeded == 2)
        it->extra = cbor_ntohs(it->extra);
}

extra has type uint16_t, so it has two bytes. When we need only one byte, we read into the wrong byte, so the resulting number is 256 times higher on big endian than it should be. Adding a temporary one-byte variable fixed it.

Issue 8: perfparser, part of Qt Creator (code review)

Here it is not trivial to find the issue just looking at the code:

qint32 dataStreamVersion = qToLittleEndian(QDataStream::Qt_DefaultCompiledVersion);

However the linker was producing an error:

undefined reference to `QDataStream::Version qbswap(QDataStream::Version)'

On little endian systems, qToLittleEndian is a no-op, but on big endian systems, it is a template function defined for some known types. But it turns out we need to explicitly convert enum values to a simple type, so the fix was passing qint32(QDataStream::Qt_DefaultCompiledVersion) to that function.

Issue 9: Qt Personal Information Management (code review)

The code in test was trying to represent a number as a sequence of bytes, using reinterpret_cast:

static inline QContactId makeId(const QString &managerName, uint id)
{
    return QContactId(QStringLiteral("qtcontacts:basic%1:").arg(managerName), QByteArray(reinterpret_cast<const char *>(&id), sizeof(uint)));
}

The order of bytes will be different on little endian and big endian systems! The fix was adding this line to the beginning of the function:

id = qToLittleEndian(id);

This will cause the bytes to be reversed on big endian systems.

What remains unfixed

There are still some bugs, which require deeper investigation, for example:

P.S. We are looking for new people to help with maintaining Qt 6. Join our team if you want to do some fun work like described above!