Hi again,
So you see how we can't agree... ;)
Absence of proof is not proof of absence.
We don't know what users do out there in the field. All we know is a) the
behaviour of the function as of now and b) that if you pass objects originally
marked as const, that's UB, according to the standard.
We specifically do not know, whether passing const objects actually does
"something", we also do not know whether users can upgrade their Qt to receive
any of the two other proposed fixes (mutable or not nulling d_ptr). Finally, we
do not know whether re-adding the "soft-leak" that nulling the d_ptr plugs is
going to hurt anyone out there. We can speculate, e.g., that, since
disconnect() already cleaned up, users may just free() a buffer of Connection
objects instead of running their destructors, turning a "soft" into a "hard"
leak.
But all that are speculations. Users to this, compilers don't do that. After 14
years in the wild, at the intersection of Hyrum's and Murphy's Laws, everything
we can think of happening probably does happen out there. I also wouldn't've
imagined to see a QHash<const char*> ever in my life. I found several just the
other week. We also thought file-static functions do not have an effect outside
their TU. And then came Unity Builds (and it's not that we couldn't know - KDE
has had that feature since automake times, IIRC).
"Sì! la speranza che delude sempre!" — Turandot
That is why I prefer to stick with what we know: That not passing const objects
is always fine, always was, always will be. It's guaranteed by the standard. If
you follow that, you fall into the Pit Of Success. It's a trivial change, and
you fix it for all the past and all the future.
As such, that patch in itself is not the fix. It's a tool for users to fix
their code.
I am not saying we shouldn't add the mutable. In fact, I have a patch up, and
think it's a viable mitigation strategy. But it's just another assumption: that
users can upgrade their Qt. I would say that the only safe assumption to make
is that _if_ users can update _anything_ in their product, that their own code
is always one of these things.
Though, come to think of it, even users that cannot update anything, can at
least apply the mutable patch, recompile, and see whether their product is
byte-for-byte identical. If it is, then there's nothing to do, either. If there
are differences, return to the start. Assumes reproducible builds, of course.
Thanks,
Marc
On 27.03.26 02:16, Giuseppe D'Angelo via Development wrote:
Hi,
Il 26/03/26 19:41, Marc Mutz via Development ha scritto:
Hi,
We Qt developers cannot figure out how to proceed from here, so let me give you
a heads-up here, so you can take matters into your own hands.
TL;DR: never pass Connection objects originally declared const to
QObject::disconnect(Connection).
The function const_cast<>s away the const of the argument to reset the d_ptr to
nullptr. It doesn't matter why, suffice to say that it's not trivial to fix
either way on the Qt side.
To be honest, I don't understand the alarmism of this email. There's a bug in
Qt; and we fix bugs all the time. The impact of this particular bug is between
miniscule and non-existent.
And I disagree, the bug *is* completely trivial to fix: we can make the
affected member as `mutable`. The only reason the patch wasn't +2 already on
Gerrit was for the commit message that talked about impending deprecations
(when there's strong disagreement around it).
If we follow what I consider an extremely speculative line of reasoning, and
conclude that the bug might be manifesting itself in inline code, then there's
no solution: people have to update Qt _and_ recompile, which is always the case
when there's a bug in inline code.
If we reject that line of reasoning (and I do, see below), then it's sufficient
that people update Qt (UB happens in out-of-line code) and they'll get the fix.
In any case the "avoid passing const objects to disconnect()" is there as a
workaround for those who can recompile their code, but can't upgrade Qt. Maybe
it can become a gated Clazy check.
But I doubt anyone will listen; their code works completely fine today as their
compiler doesn't do anything wrong, and they'll never upgrade their toolchain
without also upgrading Qt.
I don't get why we disagree on such a simple fix that works, can be 100% safely
cherrypicked in all the open branches, and completely preserves behavior (incl.
no soft-leaks); and instead we are now researching extremely more invasive
solutions, including possible deprecations of a 15-20 years old API for a
purely theoretical problem, deprecation which requires fixes in all our
repositories and will just annoy the heck out of our users.
--
Now, the gritty details:
The bug is: there's UB happening in an out of line function. We const_cast away
an object and mutate it; if the object was originally marked as const, that's
formally UB.
As many people associate "UB" with "segfault", I need to clarify: NO, the
program does not crash; it does not write into arbitrary memory / readonly
memory; it does not trigger sanitizers or anything of the sorts.
In fact, programs work just fine, as demonstrated by the fact that this issue
is some ~14 years old and we noticed only now (and not because we got bug
reports: we detected the issue by reading the sources).
I'll state it again:
* the UB happens in out of line code (inside QObject::disconnect(const
QMetaObject::Connection &)).
* the declaration of a QMetaObject::Connection object as `const` happens in
client code;
* therefore, compilers do NOT normally see both the const_cast mutating the
object and original object declared as const, detect UB, and do evil things
based on the presence of UB. LTO or similar technology would be necessary for
the compiler to even be able to do this kind of reasoning. This firewalling
alone reduces the possibility of things going horribly wrong to infinitesimally
small quantities.
* but even without firewalling, no optimizing compiler we support today detects
or exploits UBs in const_cast; even when they're "blatant". If the Big Three
don't optimize on this I reject the notion that "some other compiler out there
might do it". "Some other compiler we don't know about (and therefore DO NOT
support)" isn't a starting point.
* I also reject the notion of "what about compilers of tomorrow". People who
upgrade compilers also upgrade Qt.
* Finally, what *might* be happening (which makes this an issue in "inline
code", see the discussion above) is that a compiler doesn't reload
QMetaObject::Connection's data members because the object was originally
declared `const`.
A sequence like:
const QMetaObject::Connection c = QObject::connect(~~~);
assert(c); // #1
disconnect(c); // const_cast in here
assert(c); // #2
in #1 and #2 touches an inline code path (operator bool()) which loads c.d_ptr.
An optimizing compiler might read c.d_ptr on #1, and not re-read it on #2 (that
is, re-use the value read on #1), despite the fact that disconnect() call in
principle has changed it.
And again major compiler actually does this today:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86318
https://github.com/llvm/llvm-project/issues/160441
So, all in all: this a problem that has zero impact, and we're creating a
meteor-size blast radius (deprecation, users annoyed, code to be ported,
documentation, best practices) ... for what gain exactly?
In anticipation of a deprecation of the const overload.
I still reject this idea. It *works* but it's just not worth the hassle.
My 2 c,
--
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, Jouni Lintunen
Sitz der Gesellschaft: Berlin,
Registergericht: Amtsgericht Charlottenburg,
HRB 144331 B
Public
--
Development mailing list
[email protected]
https://lists.qt-project.org/listinfo/development
