Hi,
No TL;DR: You're supposed to read and understand all of it.
As I write this, the UB fix for QArrayDataObs/Pointer¹ that held up² adding
RHEL10 to the CI has merged to all branches, except the ESM ones (6.5 and
5.15), but it's just a keystroke of Jani's away there, too.
If the following reads like "I told you so", I'm sorry, but ... I did ;)
Act 1: Phantom Menace
When we were trying to enable RHEL 10 (using GCC 14.3) in the CI, we hit a FAIL
in tst_qarraydata. This is _good_. It means it didn't blow up at some customer
first (for all we know). Several highly-esteemed developers and two chat bots
concluded that this is a compiler bug. The problem is that the compiler isn't
very exotic, so what does it bring us if we know it's a compiler bug? Nothing.
We'd still need to work around it.
Act 2: Deus Ex Machina
It just so happens that this developer ran into a very similar issue last year,
and found, with the help of the GCC devs³, that the compiler is completely
correct, and it was my fault. I was casting a char*& to a reference to a
pointer to enum qchar8 : uchar {}, incremented it, and expected the original
pointer object to have been incremented. But this is not guaranteed by the
standard. Only char etc is allowed to alias other objects, _not_ char*, and
certainly not enum*. So the compiler was right in caching the old value of the
object and continue using it.
Side note: if this sounds familiar to you, it's because we've just talked about
that this is exactly how the UB in QObject::disconnect() would likely manifest.⁴
The analysis of the QArrayData assembly sounded eerily similar: one of the
three fields that QArrayDataPointer::swap() swaps wasn't swapped. Thiago found
the culprit⁵, I merely connected the dots: we were inheriting QArrayDataOps :
QArrayDataPointer, and static_cast<Ops*>(this) from QArrayDataPointer::op->().
The cast is UB, because *this isn't actually an object of type Ops, but only
QArrayDataPointer. Period. This code has been stitting there for 15 years
(since Qt 5.0, but unused until Qt 6.0)⁵, and not making any problems. Until an
innocent compiler upgrade.
Side note: Casting a Base to a Derived : Base is UB if the object isn't
actually of dynamic type Derived. It doesn't matter whether sizeof(Derived) ==
sizeof(Base). It _is_ always UB. I have lost count of the number of code
snippets I have fixed in tests, mostly, where ubsan complained about invalid
casts where it turned out that this "technique" was used. It was _very_
prevalent in Qt, as if somone somewhere put up the rule that, regardless of
what the std _actually_ says, casting to a non-actual derived type is ok as
long as the sizeof's are the same. Well, it's not.
Once the bug was identified, the fix was lengthy, but pretty straight-forward.
But since this was "actively exploited" UB in one of Qt's most central classes
(QList, QString, QByteArray depend on this, so I'd be hard-pressed to find
anything in Qt that does _not_ depend on QArrayData), we decided that we need
to pick this all the way back, incl. into ESM branches.
Act 3: Odyssey
This is where the vast amount of my work was sunk: cherry-picking through a
forest of cleanup and rewrite commits.
I have long said that, from a software-engineering POV, you do _not_ want your
active branches to diverge very much. If you find a security issue, speed is of
the essence, and you a) don't want to slow down by resolving conflicts (you
want to stage the change in dev and wake up the next morning with the change
merged to all branches by the bots) and b) don't want to introduce a lot of
risk by combining commit sets in a unique combination found nowhere else.
Well, this bug is as close to a security issue as you can have without an
actual security issue. And it ran full steam into one the worst violations of
that principle, which, I hasten to add, is _not_ standing Qt policy.
The end effect is that, the issue took a full week to make it to the ESM
branches, instead of a few hours. It didn't help that the cherry-picking has
its own weird ideas about where to pick to⁶, but that only manifests in
multi-commit fixing scenarios, so doesn't affect your typical security fix
(which tend to be one-liners more often than not).
Epilogue
To me, this issue reinforces what I have known to be true for years, to wit:
- the compiler is more clever than you, you MUST NOT fall into UB, regardless
of whether _you_ think it's benign (even if you think you can prove it)
- only features shouldn't be picked back, in particular
- test changes should always be picked back, with a XFAIL, if necessary
- refactorings should always be picked back (or not done at all)
- bugfixes should always be picked back
We have known issues in both categories. I'm pretty sure we have more classes
in Qt that are never instantiated, only cast to, and we have at least one
module (you know who you are), which is security-critical, and where a high
code turnover is paired with lack of cherry-picking. I have experienced the
same frustrating conflicts there than I did in shoveling QArrayData patches
though. That's not to single out said module or it's authors and maintainers.
They work by the book. It's the book that's wrong here, and I would hope we
have a discussion about changing it to aggressive picking going forward, for
security-critical components. We have the CRA markings now. I would say that
one goal should be that all security-critical components in Qt are at the same
level in all active branches. This is what we do with 3rd-party components
already. It seems odd to exclude Qt's own security-critical components, where
we have the added benefit of being able to exclude feature commits from
picking. In 3rd-party components, we always get both.
Hope this provides some food for thought (and change).
Thanks for reading this far,
Marc
¹ https://codereview.qt-project.org/c/qt/qtbase/+/736274 /
https://codereview.qt-project.org/q/topic:QArrayDataOps-2026
² https://qt-project.atlassian.net/browse/QTBUG-146652 /
https://qt-project.atlassian.net/browse/QTQAINFRA-7202
³ https://gcc.gnu.org/bugzilla/show_bug.cgi?id=121593
⁴ https://lists.qt-project.org/pipermail/development/2026-March/047026.html
⁵ introduced, but not necessarily taken into use, by
https://codereview.qt-project.org/c/qt/qtbase/+/9847
⁶ https://qt-project.atlassian.net/browse/QTQAINFRA-7900 /
https://qt-project.atlassian.net/browse/QTQAINFRA-7903
--
Marc Mutz <[email protected]><mailto:[email protected]> (he/his)
Principal Software Engineer
The Qt Company
Erich-Thilo-Str. 10 12489
Berlin, Germany
www.qt.io<http://www.qt.io>
Geschäftsführer: Mika Pälsi, Juha Varelius, Juha Puputti
Sitz der Gesellschaft: Berlin,
Registergericht: Amtsgericht Charlottenburg,
HRB 144331 B
Public
--
Development mailing list
[email protected]
https://lists.qt-project.org/listinfo/development
