QString -> QStringLiteral conversions might make applications crash on exit

[Frank Reininghaus]

Hi everyone,

sorry if most of you know about this already, but since it seems that
QStringLiterals are being introduced everywhere right now, I think
that it is important to raise awareness about the fact that this might
be more dangerous that it seems at first sight.

QStringLiteral has the nice property that it stores the string data in
read-only memory and avoids heap allocations when it is used to
construct a QString. The QString, and any copies that are made, just
contain a pointer to read-only data. There is a very nice overview by
Olivier at

https://woboq.com/blog/qstringliteral.html

However, QString is still a non-POD type, even if it has been
constructed with QStringLiteral (or copied from such a QString), so
its destructor is being run, which accesses the read-only data, then
finds out that it's been made with QStringLiteral, such that no
refcount updates and deallocations are needed.

This becomes a problem if the read-only data that the QString refers
to are not there any more, which can happen if the QString was stored
in a global static object from one library, and the QStringLiteral is
from another library, which might have been unloaded before the global
static object was destroyed.

I believe that this is just what happens right now with a global
static KIconLoader from kicontnkhemes and a QStringLiteral from
frameworkintegration:

https://bugs.kde.org/show_bug.cgi?id=359758

If my analysis is wrong, please let me know!

If this crash is really caused by the QStringLiteral, then we should
think about how we want to treat QStringLiteral in the future. The
current approach seems to be "use it everywhere", but this might cause
more crashes in the future.

Possible solutions could be

a) Everyone who makes QString -> QStringLiteral replacements should be
extremely careful (which is very difficult, since it is not always
obvious if passing a QString to a function will result in the string
being stored in a global static object). Automated tools like clazy
should then not recommend to use QStringLiteral any more.

b) Classes like KIconLoader, which are used as global static objects,
should copy all strings that they get to the heap in order to prevent
such crashes (which might also be quite difficult to do consistently).

Accidental mistakes are very easy to make either way, so I'm wondering
if the use of QStringLiteral should be restricted to situations where
it's 100% clear that there will be no risk (this will often not be the
case, I think), and it also seems likely that using it will have an
impact on the performance and/or memory usage (because it's a string
that will be copied a lot, or because it's in a hot code path).

Opinions?

Regards,
Frank