The gui will crash with clicking the button of reanalyze all files

[wy163]

Hi,
This issue is very easy to be reproduced, and it is 100% reproduced.

open the gui, and open a .cppcheck project file
when the analyze is done, click the button to reanalzye all files
Segmentation fault

[Oliver Stöneberg]

Unfortunately I am not able to reproduce it. If you could somehow provide a stacktrace that would be very helpful.

[wy163]

[wy163]

I think the problem is caused by the QStandardItem::setData.
For example:

QStandardItem *ResultsTree::ensureFileItem(const QString &fullpath, const QString &file0, bool hide)
{
    QString name = stripPath(fullpath, false);
    // Since item has path with native separators we must use path with
    // native separators to find it.
    QStandardItem *item = findFileItem(QDir::toNativeSeparators(name));

    if (item) {
        if (!hide)
            setRowHidden(item->row(), QModelIndex(), hide);
        return item;
    }

    // Ensure shown path is with native separators
    name = QDir::toNativeSeparators(name);
    item = createNormalItem(name);
    item->setIcon(QIcon(":images/text-x-generic.png"));

    //Add user data to that item
    QMap<QString, QVariant> itemdata;
    itemdata[FILENAME] = fullpath;
    itemdata[FILE0] = file0;
    item->setData(QVariant(itemdata));
    mModel.appendRow(item);

    setRowHidden(item->row(), QModelIndex(), hide);

    return item;
}

The reference of the data QVariant is set to the item, but the lifetime of the QVariant object is finished when the function ensureFileItem is exited.

[Oliver Stöneberg]

Would be great if I could reproduce it.

Seems related to https://trac.cppcheck.net/ticket/14011 and https://trac.cppcheck.net/ticket/12838.

[Oliver Stöneberg]

I did a short search on how to use this interface and the few useful results I got also had the variant go out of scope so it might be fine. Qt is weird with lifetimes and allocations.

[wy163]

I posted the bt trace in the previous post, don't why that can't be shown. I copied it again:

#0  0x00007a245d5c1e5e in QtPrivate::QMetaTypeForType<QMap<QString, QVariant> >::getDtor()::{lambda(QtPrivate::QMetaTypeInterface const*, void*)#1}::_FUN(QtPrivate::QMetaTypeInterface const*, void*) () from /home/w01/Qt/6.9.1/gcc_64/lib/libQt6Core.so.6
#1  0x00007a245d5febe8 in QVariant::~QVariant() () from /home/w01/Qt/6.9.1/gcc_64/lib/libQt6Core.so.6
#2  0x00007a245e1a9205 in QStandardItem::~QStandardItem() () from /home/w01/Qt/6.9.1/gcc_64/lib/libQt6Gui.so.6
#3  0x00007a245e1a9071 in QStandardItem::~QStandardItem() () from /home/w01/Qt/6.9.1/gcc_64/lib/libQt6Gui.so.6
#4  0x00007a245e1aa5e5 in QStandardItem::removeRows(int, int) () from /home/w01/Qt/6.9.1/gcc_64/lib/libQt6Gui.so.6

[wy163]

As you said, 'Qt is weird with lifetimes and allocations', so this is depends on the Qt how to handle this. Actually, we don't know what Qt does. And there is a possibility Qt will implement the QStandardItem in a different way some say.
If you can't reproduce this, I think it is a little difficult for us to track this issue. I am using the ubuntu and QT 6.9. And the test file for cppcheck:

#include <stdio.h>


int func(int a, int b = 100) {
    return a+b;
}

int call(int var) {
    return 100;
}

[Oliver Stöneberg]

We have a known (but unreliable) crash in a test involving the results which I assume is the samer issue - see https://trac.cppcheck.net/ticket/13223.

I have been trying to run the GUI tests with a sanitized build but that somehow fails to link. The work is included in https://github.com/danmar/cppcheck/pull/6820.

[wy163]

Hi Oliver Stöneberg, this is 100% reproduced on my side. Have you tested it with the Linux version? I think it is also possible to be related with the Qt version. I am using the 6.9.
As my testing, this is involved by the commit 2e9b67c903322d2393f3d09435bedd186e178c32.
(fixed #13133 - removed deprecated Qt5 support)

[Oliver Stöneberg]

Yes. I experienced the issue myself multiple times and 100% reproducible but unfortunately I never got anything useful out of tools like ASAN or valgrind. And the stacktrace was detailed but I did not understand how it can be working fine on other platforms.

And after updates to packages by the distro it usually went away for a while.

Your observation appears to make sense but I checked the actual implementation of QStandardItem::setData() (see https://github.com/qt/qtbase/blob/d812e32bcba17f4be5a54c48651c3068487478f7/src/gui/itemmodels/qstandarditemmodel.cpp#L883) and it will store the value as a QStandardItemData object which makes a copy of the given value (see https://github.com/qt/qtbase/blob/dev/src/gui/itemmodels/qstandarditemmodel_p.h#L36). That checks out fine in my eyes.

So maybe something is wrong with QStandardItem the data is being stored in. Although new is used across the Qt code you only actually need to explicitly free that in a few cases as it is handled via the object ownership (and there is also ref-counting going on internally).

[Oliver Stöneberg]

While debugging something I suddenly experienced the issue again.

[Oliver Stöneberg]

I have a minimal reproducer and I think I found the issue.

In ResultsTree we pass the address of QStandardItemModel to QTreeView. Based on other pattern in Qt I assume that the view will take ownership of the model and thus try to delete it. So that needs to be allocated instead. That at least fixes my reproducer.

[Oliver Stöneberg]

Unfortunately this only fixes the known failing test. The fix has been published as https://github.com/danmar/cppcheck/pull/7798.

[Oliver Stöneberg]

Unfortunately it is not fixing anything since now it leaks memory.

[Oliver Stöneberg]

The view does not take ownership according to the documentation: https://doc.qt.io/qt-6/qabstractitemview.html#setModel

[Oliver Stöneberg]

If I make itemdata (which is being passed to setData()) static it works. So it really appears to take a reference and that needs to be stored outside of the model. But changes to the object from the outside are not being propagated. It really looks to me like Qt is broken here.

[Oliver Stöneberg]

Here's a guide on how to use a model: https://runebook.dev/en/articles/qt/qtreeview/setModel

Unfortunately it does not set data on the items.

[wy163]

Hi Oliver Stöneberg, I made a test on my side. I just modified the gui/main.cpp to show a simple tree view. Below is the code change.

diff --git a/gui/main.cpp b/gui/main.cpp
index 22e1bc8cb..288dcdc3d 100644
--- a/gui/main.cpp
+++ b/gui/main.cpp
@@ -39,44 +39,43 @@
 #include <QString>
 #include <QStringList>
 #include <QVariant>
-
+#include <QStandardItemModel>
+#include <QTreeView>
+#include <QScreen>

 static void ShowUsage();
 static void ShowVersion();
 static bool CheckArgs(const QStringList &args);

+void setdata(QStandardItemModel * model) {
+    auto *item = new QStandardItem("item two");
+    QMap<QString, QVariant> itemdata;
+    itemdata["FILENAME"] = "12345";
+    item->setData(QVariant(itemdata));
+    model->appendRow(item);
+}
+
 int main(int argc, char *argv[])
 {
     QApplication app(argc, argv);
+    QTreeView * tree_view = new QTreeView();
+    QPalette palette = tree_view->palette();
+    palette.setColor(QPalette::Window, Qt::darkCyan);
+    tree_view->setPalette(palette);
+    tree_view->setAutoFillBackground(true);
+    tree_view->setWindowTitle("Simple Tree");
+    const auto screenSize = tree_view->screen()->availableSize();
+    tree_view->resize({screenSize.width() / 2, screenSize.height() * 2 / 3});
+    QStandardItemModel * model = new QStandardItemModel();
+    auto *item = new QStandardItem("item one");
+    item->setEditable(false);
+    model->appendRow(item);
+    tree_view->setModel(model);
+    setdata(model);
+    tree_view->show();
+
+    model->removeRows(0, model->rowCount());

-    QCoreApplication::setOrganizationName("Cppcheck");
-    QCoreApplication::setApplicationName("Cppcheck-GUI");
-
-    auto* settings = new QSettings("Cppcheck", "Cppcheck-GUI", &app);
-
-    // Set data dir..
-    const QStringList args = QApplication::arguments();
-    auto it = std::find_if(args.cbegin(), args.cend(), [](const QString& arg) {
-        return arg.startsWith("--data-dir=");
-    });
-    if (it != args.end()) {
-        settings->setValue("DATADIR", it->mid(11));
-        return 0;
-    }
-
-    auto* th = new TranslationHandler(&app);
-    th->setLanguage(settings->value(SETTINGS_LANGUAGE, th->suggestLanguage()).toString());
-
-    if (!CheckArgs(QApplication::arguments()))
-        return 0;
-
-    QApplication::setWindowIcon(QIcon(":cppcheck-gui.png"));
-
-    // Register this metatype that is used to transfer error info
-    qRegisterMetaType<ErrorItem>("ErrorItem");
-
-    MainWindow window(th, settings);
-    window.show();
     return QApplication::exec();
 }

While launching the cppcheck-gui, the crash still happen. And the crash point is still the deconstructor of QVariant. But I didn't gdb the details of the core. I think the crash is the same.
I don't know if you tested the commit 2e9b67c903322d2393f3d09435bedd186e178c32. Not sure if you have the same scenery with me.

Another suspect is that this is related with the QMap. With below code, there is no crash.

void setdata(QStandardItemModel * model) {
    auto *item = new QStandardItem("item two");
    //QMap<QString, QVariant> itemdata;
    //itemdata["FILENAME"] = "12345";
    QDateTime d;
    item->setData(QVariant(d));
    model->appendRow(item);
}

Use QDateTime instead of Qmap.

[wy163]

As my testing, this is really strange. I also made a simple demo to try the TreeView outside the cppcheck project. There is no problem with the usage of QStandardItem::setData when not new the data.
If we can find out what triggers the crash, maybe we can post a ticket to QT.
I checked a sample under the QT SDK:
/home/w01/Qt/Examples/Qt-6.9.1/widgets/itemviews/simpletreemodel
In the demo, the model uses the smart pointer to hold the data. If we can't get the root cause of the crash, maybe that is the way we need to follow to fix it completely.

[Oliver Stöneberg]

Thanks for also looking into it. I did the same and got it down even further (see also in https://github.com/danmar/cppcheck/pull/7798).

void f() {
    auto* model = new QStandardItemModel;
    auto *item = new QCustomItem("0");
    {
        QMap<QString, QVariant> itemdata;
        itemdata["file"] = "file1";
        item->setData(itemdata);
        itemdata["file"] = "file2";
        item->setData(itemdata); // crash
    }
    model/*->invisibleRootItem()*/->appendRow(item);
    model->clear();
    delete model;
}

That produces thefollowing with GCC only

==26739== Use of uninitialised value of size 8
==26739== at 0x5B4F527: QtPrivate::QMetaTypeForType<qmap\<qstring, qvariant=""> >::getDtor()::{lambda(QtPrivate::QMetaTypeInterface const, void)#1}::_FUN(QtPrivate::QMetaTypeInterface const, void) (in /usr/lib/libQt6Core.so.6.9.1)
==26739== by 0x5B8E595: QVariant::operator=(QVariant const&) (in /usr/lib/libQt6Core.so.6.9.1)
==26739== by 0x55D67B4: QStandardItem::setData(QVariant const&, int) (in /usr/lib/libQt6Gui.so.6.9.1)
==26739== by 0x4007422: TestResultsTree::test1() const (testresultstree.cpp:30)
==26739== by 0x40046FF: TestResultsTree::qt_static_metacall(QObject, QMetaObject::Call, int, void) (moc_testresultstree.cpp:72)
==26739== by 0x5B2E19A: QMetaMethodInvoker::invokeImpl(QMetaMethod, void, Qt::ConnectionType, long long, void const const, char const const, QtPrivate::QMetaTypeInterface const const) (in /usr/lib/libQt6Core.so.6.9.1)
==26739== by 0x5B2EA3E: QMetaMethod::invokeImpl(QMetaMethod, void, Qt::ConnectionType, long long, void const const, char const const, QtPrivate::QMetaTypeInterface const const) (in /usr/lib/libQt6Core.so.6.9.1)
==26739== by 0x50187AE: ??? (in /usr/lib/libQt6Test.so.6.9.1)
==26739== by 0x501BA81: ??? (in /usr/lib/libQt6Test.so.6.9.1)
==26739== by 0x502916F: QTest::qRun() (in /usr/lib/libQt6Test.so.6.9.1)
==26739== by 0x502A0B5: QTest::qExec(QObject, int, char) (in /usr/lib/libQt6Test.so.6.9.1)
==26739== by 0x4007636: main (testresultstree.cpp:37)
==26739== Uninitialised value was created by a heap allocation
==26739== at 0x4861F93: operator new(unsigned long) (vg_replace_malloc.c:487)
==26739== by 0x400A9F8: QMap<qstring, qvariant="">::detach() (qmap.h:279)
==26739== by 0x4009DDF: QMap<qstring, qvariant="">::operator (qmap.h:379)
==26739== by 0x400730C: TestResultsTree::test1() const (testresultstree.cpp:27)
==26739== by 0x40046FF: TestResultsTree::qt_static_metacall(QObject*, QMetaObject::Call, int, void</qstring,></qstring,>) (moc_testresultstree.cpp:72)
==26739== by 0x5B2E19A: QMetaMethodInvoker::invokeImpl(QMetaMethod, void, Qt::ConnectionType, long long, void const const, char const const, QtPrivate::QMetaTypeInterface const const) (in /usr/lib/libQt6Core.so.6.9.1)
==26739== by 0x5B2EA3E: QMetaMethod::invokeImpl(QMetaMethod, void, Qt::ConnectionType, long long, void const const, char const const, QtPrivate::QMetaTypeInterface const const) (in /usr/lib/libQt6Core.so.6.9.1)
==26739== by 0x50187AE: ??? (in /usr/lib/libQt6Test.so.6.9.1)
==26739== by 0x501BA81: ??? (in /usr/lib/libQt6Test.so.6.9.1)
==26739== by 0x502916F: QTest::qRun() (in /usr/lib/libQt6Test.so.6.9.1)
==26739== by 0x502A0B5: QTest::qExec(QObject, int, char*) (in /usr/lib/libQt6Test.so.6.9.1)
==26739== by 0x4007636: main (testresultstree.cpp:37)</qmap\<qstring,>

So this very much looks like a bug in Qt or the compiler.

[Bassam Abdul-Baki]

Not to hijack this discussion, but the GUI crashes for me as soon as I open a project. I started a new one several times and the GUI is fine for editing. When I click analyze, it crashes. When I reopen it and attempt to reopen the project, it crashes. If I don't reopen the project and select edit instead, it opens the file for editing and I can make and save modifications. However, if I click open or analyze, it crashes every single time. Deleting everything and starting new does nothing.

I have yet to test this with a much more restrictive number of files.

[Oliver Stöneberg]

Not a hijack. It's all related and has been around for quite a while and just got much worse. I need to report this upstream because I see nothing what we can do at our end.

If possible, please use Clang as compiler for now since that does not seem to experience the issue.