Hi, Am Freitag, 29. Dezember 2023, 10:53:45 CET schrieb Tommaso Massimi: > running plasma-systemmonitor with valgring a lot of problems are declared, > I'm trying to check them out. > > I'm not sure if this is the best way to communicate with the development > team, > so I'm writing this mail also to have some indication about that. Please cc > me, I'm not subscribed to the list
Ideally the bug/crash itself would have been filed as a report against kconfig or plasma-systemmonitor (bugs.kde,org). So that related discussion is stored at a central point with some scheme over it. As you even already came up with a patch based on own analysis, you might have created a MR on invent.kde.org. See details at https://community.kde.org/Get_Involved > part of valgrind output (neon unstable development 25-12-2023) > > ==70026== Invalid read of size 16 > ==70026== at 0x668FAF7: ??? (in > /usr/lib/x86_64-linux-gnu/libQt6Core.so.6.6.1) > ==70026== by 0x575CB05: calculateHash<QStringView> (qhash.h:57) > ==70026== by 0x575CB05: > QHashPrivate::Data<QHashPrivate::Node<QStringView, QHashDummyValue> > > >::findBucket(QStringView const&) const [clone .isra.0] (qhash.h:683) > > ==70026== by 0x575FF43: findOrInsert (qhash.h:718) > ==70026== by 0x575FF43: QHash<QStringView, QHashDummyValue>::iterator > QHash<QStringView, > QHashDummyValue>::emplace_helper<QHashDummyValue>(QStringView&&, > QHashDummyValue&&) [clone .isra.0] (qhash.h:1335) > ==70026== by 0x5761E89: emplace<QHashDummyValue> (qhash.h:1321) > ==70026== by 0x5761E89: insert (qset.h:158) > ==70026== by 0x5761E89: operator() (kconfig.cpp:325) > ==70026== by 0x5761E89: > forEachEntryWhoseGroupStartsWith<KConfigPrivate::groupList(const QString&) > const::<lambda(KEntryMapConstIterator)> > (kconfigdata_p.h:252) > ==70026== by 0x5761E89: KConfigPrivate::groupList(QString const&) const > (kconfig.cpp:320) > ==70026== by 0x5771089: KConfigGroup::groupList() const > (kconfiggroup.cpp:1147) > ==70026== by 0x1B94F929: PageDataObject::load(KConfigBase const&, > QString const&) (PageDataObject.cpp:235) > ==70026== by 0x1B95705E: PagesModel::componentComplete() > (PagesModel.cpp:99) > ==70026== by 0x53C1876: > QQmlObjectCreator::finalize(QQmlInstantiationInterrupt&) (in > /usr/lib/x86_64-linux-gnu/libQt6Qml.so.6.6.1) > ==70026== by 0x54489AC: > QQmlComponentPrivate::complete(QQmlEnginePrivate*, > QQmlComponentPrivate::ConstructionState*) (in > /usr/lib/x86_64-linux-gnu/libQt6Qml.so.6.6.1) > ==70026== by 0x5448CAB: QQmlComponentPrivate::completeCreate() (in > /usr/lib/x86_64-linux-gnu/libQt6Qml.so.6.6.1) > ==70026== by 0x544AC88: > QQmlComponentPrivate::createWithProperties(QObject*, QMap<QString, > QVariant> const&, QQmlContext*, QQmlComponentPrivate::CreateBehavior) (in > /usr/lib/x86_64-linux-gnu/libQt6Qml.so.6.6.1) > ==70026== by 0x54400DF: > QQmlApplicationEnginePrivate::finishLoad(QQmlComponent*) (in > /usr/lib/x86_64-linux-gnu/libQt6Qml.so.6.6.1) > ==70026== Address 0xcd3c40a is 26 bytes inside a block of size 38 alloc'd > ==70026== at 0x4848899: malloc (in > /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) > ==70026== by 0x65A5677: QArrayData::allocate(QArrayData**, long long, > long long, long long, QArrayData::AllocationOption) (in > /usr/lib/x86_64-linux-gnu/libQt6Core.so.6.6.1) > ==70026== by 0x657DCFE: QString::QString(long long, Qt::Initialization) > (in /usr/lib/x86_64-linux-gnu/libQt6Core.so.6.6.1) > ==70026== by 0x6589D97: QString::fromUtf8(QByteArrayView) (in > /usr/lib/x86_64-linux-gnu/libQt6Core.so.6.6.1) > ==70026== by 0x577DA4E: fromUtf8<> (qstring.h:588) > ==70026== by 0x577DA4E: KConfigIniBackend::parseConfig(QByteArray > const&, KEntryMap&, QFlags<KConfigBackend::ParseOption>, bool) > (kconfigini.cpp:157) > ==70026== by 0x5760C68: KConfigPrivate::parseConfigFiles() > (kconfig.cpp:798) > ==70026== by 0x5784E81: KSharedConfig::KSharedConfig(QString const&, > QFlags<KConfig::OpenFlag>, QStandardPaths::StandardLocation) > (ksharedconfig.cpp:123) > ==70026== by 0x57854E0: KSharedConfig::openConfig(QString const&, > QFlags<KConfig::OpenFlag>, QStandardPaths::StandardLocation) > (ksharedconfig.cpp:88) > ==70026== by 0x1B957006: PagesModel::componentComplete() > (PagesModel.cpp:96) > ==70026== by 0x53C1876: > QQmlObjectCreator::finalize(QQmlInstantiationInterrupt&) (in > /usr/lib/x86_64-linux-gnu/libQt6Qml.so.6.6.1) > ==70026== by 0x54489AC: > QQmlComponentPrivate::complete(QQmlEnginePrivate*, > QQmlComponentPrivate::ConstructionState*) (in > /usr/lib/x86_64-linux-gnu/libQt6Qml.so.6.6.1) > ==70026== by 0x5448CAB: QQmlComponentPrivate::completeCreate() (in > /usr/lib/x86_64-linux-gnu/libQt6Qml.so.6.6.1) > > > > this problem is generated in this function: > > > ==70026== by 0x5761E89: KConfigPrivate::groupList(QString const&) const > (kconfig.cpp:320) > > i.e. > > QStringList KConfigPrivate::groupList(const QString &groupName) const > { > const QString theGroup = groupName + QLatin1Char('\x1d'); > QSet<QStringView> groups; > > entryMap.forEachEntryWhoseGroupStartsWith(theGroup, [&theGroup, > &groups](KEntryMapConstIterator entryMapIt) { > if (isNonDeletedKey(entryMapIt)) { > const QString &entryGroup = entryMapIt->first.mGroup; > const auto subgroupStartPos = theGroup.size(); > const auto subgroupEndPos = findFirstGroupEndPos(entryGroup, > subgroupStartPos); > groups.insert(QStringView(entryGroup).mid(subgroupStartPos, > subgroupEndPos - subgroupStartPos)); > } > }); > > return stringListFromStringViewCollection(groups); > } > > > > in this line the function .mid (deprecated in QStringView) is creating a > temporary object which is inserted to groups, > > groups.insert(QStringView(entryGroup).mid(subgroupStartPos, > subgroupEndPos - subgroupStartPos)); > > > groups is declared as : > QSet<QStringView> groups; > > QStringView doesn't own data, it is like a wrapper/reference to a qstring. > so the value inserted on group is like a reference to a temporary qstring; > but the qstring will be deleted while the QStringView will remain in group > pointing to garbage After a more close look I would partially disagree and point to another possible cause: KConfigPrivate::groupList() is a const method. The member "entryMap" and all the QString instances it has in its data structure are therefore not changed by design (beware, KConfigBase API is not thread-safe, see also below). The whole method juggles with QStringViews on those QString instances, to avoid doing deep copies on all the string slices handling, unless finally needed when delivering the result to the caller. This includes QStringView::mid(), which does not create any temporary QString, instead returns another QStringView, using the own data pointers: QStringView(m_data + pos, n); See complete method e.g. at https://codebrowser.dev/qt6/qtbase/src/corelib/ text/qstringview.h.html#_ZNK11QStringView3midExx No temporary QString instances here by what I can see. Looking at the backtrace it seems the bad read indeed is in qHash(QStringView key, size_t seed) and so possibly due to a QStringView with invalid data. So the question is how with all the const QString instances and the views on them this can happen. Now, the backtrace you shared shows that there are more threads active at the same time. And in some other one KConfig structures are also manipulated, the one with KConfigIniBackend::parseConfig(...). The API docs of KConfig state that KSharedConfig::openConfig is thread-safe, but hints that one should do this per thread and have separate KConfig instances, see also https://api.kde.org/frameworks/kconfig/html/ classKSharedConfig.html. On a quick look it seems that though the separete KConfig instance creation happens for both threads, the failing one has code in PagesModel::componentComplete() to creat an own copy by auto config = KSharedConfig::openConfig(...), which then is passed on as the KConfigBase reference on which groupList() is called. Based on that I conclude for now the proposed patch might only shadow the actual cause by chance. Rather comes with a cost for everyone due to more deep QString instance creations. My gut instinct would have me look closer at https://invent.kde.org/ frameworks/kconfig/-/merge_requests/256 and see if the change from the implicit shared QMap to std::map did not miss out some special behaviour relied on before to share data between different thread instances, unless modified. Can you try to revert those changes locally and see if that resolves the bad read? Cheers Friedrich