Details
-
Bug
-
Resolution: Unresolved
-
P1: Critical
-
None
-
5.15.18, 6.5.8, 6.8.3, 6.9.0
-
5
-
51cd57116 (dev), 6a8b3344c (6.9), e3a80c2dd (6.8), e5f9eb504 (tqtc/lts-6.5)
Description
QPointer is used to track whether a QObject is still alive, and so it's unsurprising that checking whether the pointer is null, or equal to some other pointer (raw or Q) is sometimes done in situations where the QObject is already in the process of being destroyed and therefore already demoted from T, the QPointer template argument, to some base class.
Because of the way we made QPointer SCARY¹, calling data() will cast from QObject* to T*, which is UB if the T was already demoted. A comparison on the raw pointer level would not cast, so would not invoke UB, so it's (our implementation of) QPointer which is introducing this problem.
It's hard to fix this in the general case, but the various ways to spell isNull() should all be equivalent to isNull() (which does not cause UB, because it doesn't cast), and when we compare against some base X of T, we should do the comparison at the common_type<X*,T*> level, to allow users in those situation to avoid the cast<T> by comparing with a lesser-derived other.
¹ Originally 3f7741fbe7ab4140f8f971c0cf88bb04e7feea6b, then changed in
cc7239da8d1ab95e68e12a64df3ca3051419cb34 and
351c738fc4586bf354c9363fb78e190bdfca4617.
Eventually, we may need to rethink how this works, and store an actual T*, and share code by casting that down to QObject* where needed, and not the other ways around.
But that's Qt 7 talk. We should fix what we can in Qt 6 and make a follow-up task for Qt 7.
Attachments
Gerrit Reviews
For Gerrit Dashboard: QTBUG-135626 | ||||||
---|---|---|---|---|---|---|
# | Subject | Branch | Project | Status | CR | V |
637591,1 | QPointer: don't cause UB when comparing for (in)equality | dev | qt/qtbase | Status: NEW | -1 | 0 |
637590,2 | QPointer: don't cause UB when checking for nullptr | dev | qt/qtbase | Status: MERGED | +2 | 0 |
638025,2 | QPointer: don't cause UB when checking for nullptr | 6.9 | qt/qtbase | Status: MERGED | +2 | 0 |
639295,2 | QPointer: don't cause UB when checking for nullptr | 6.8 | qt/qtbase | Status: MERGED | +2 | 0 |
639310,4 | QPointer: don't cause UB when checking for nullptr | tqtc/lts-6.5 | qt/tqtc-qtbase | Status: MERGED | +2 | 0 |