QtQml: Mark values on the AOT-compiled stack during gc

Keep them in a special generated struct with virtual method that gets
called from the GC for each frame.

Pick-to: 6.10 6.9 6.8
Fixes: QTBUG-139059
Change-Id: I81bcbeab6531e174a5207d03f57d241461ae9ba3
Reviewed-by: Olivier De Cannière <olivier.decanniere@qt.io>

src/qml/jsruntime/qv4stackframe_p.h

@@ -56,6 +56,7 @@ struct Q_QML_EXPORT CppStackFrameBase
             ExecutionContext *context;
             QObject *thisObject;
             const QMetaType *metaTypes;
+            const QQmlPrivate::AOTTrackedLocalsStorage *locals;
             void **returnAndArgs;
             bool returnValueIsUndefined;
         };
@@ -70,12 +71,14 @@ struct Q_QML_EXPORT CppStackFrame : protected CppStackFrameBase
     // We want to have those public but we can't declare them as public without making the struct
     // non-standard layout. So we have this other struct with "using" in between.
     using CppStackFrameBase::instructionPointer;
+    using CppStackFrameBase::locals;
     using CppStackFrameBase::v4Function;
 
     void init(Function *v4Function, int argc, Kind kind) {
         this->v4Function = v4Function;
         originalArgumentsCount = argc;
         instructionPointer = 0;
+        locals = nullptr;
         this->kind = kind;
     }
 
@@ -131,9 +134,10 @@ struct Q_QML_EXPORT MetaTypesStackFrame : public CppStackFrame
               void **returnAndArgs, const QMetaType *metaTypes, int argc)
     {
         CppStackFrame::init(v4Function, argc, Kind::Meta);
-        CppStackFrameBase::thisObject = thisObject;
         CppStackFrameBase::context = context;
+        CppStackFrameBase::thisObject = thisObject;
         CppStackFrameBase::metaTypes = metaTypes;
+        CppStackFrameBase::locals = nullptr;
         CppStackFrameBase::returnAndArgs = returnAndArgs;
         CppStackFrameBase::returnValueIsUndefined = false;
     }
@@ -155,6 +159,12 @@ struct Q_QML_EXPORT MetaTypesStackFrame : public CppStackFrame
     ExecutionContext *context() const { return CppStackFrameBase::context; }
     void setContext(ExecutionContext *context) { CppStackFrameBase::context = context; }
 
+    const QQmlPrivate::AOTTrackedLocalsStorage *locals() const { return CppStackFrameBase::locals; }
+    void setLocals(const QQmlPrivate::AOTTrackedLocalsStorage *locals)
+    {
+        CppStackFrameBase::locals = locals;
+    }
+
     Heap::CallContext *callContext() const
     {
         return CppStackFrame::callContext(CppStackFrameBase::context->d());

src/qml/memory/qv4mm.cpp

@@ -1,30 +1,32 @@
 // Copyright (C) 2021 The Qt Company Ltd.
 // SPDX-License-Identifier: LicenseRef-Qt-Commercial OR LGPL-3.0-only OR GPL-2.0-only OR GPL-3.0-only
 
-#include "qv4engine_p.h"
-#include "qv4object_p.h"
-#include "qv4mm_p.h"
-#include "qv4qobjectwrapper_p.h"
-#include "qv4identifiertable_p.h"
-#include <QtCore/qalgorithms.h>
-#include <QtCore/private/qnumeric_p.h>
-#include <QtCore/qloggingcategory.h>
-#include <private/qv4alloca_p.h>
-#include <qqmlengine.h>
-#include "PageReservation.h"
 #include "PageAllocation.h"
+#include "PageReservation.h"
 
-#include <QElapsedTimer>
+#include <private/qnumeric_p.h>
-#include <QMap>
+#include <private/qv4alloca_p.h>
-#include <QScopedValueRollback>
+#include <private/qv4engine_p.h>
+#include <private/qv4identifiertable_p.h>
+#include <private/qv4mapobject_p.h>
+#include <private/qv4mm_p.h>
+#include <private/qv4object_p.h>
+#include <private/qv4profiling_p.h>
+#include <private/qv4qobjectwrapper_p.h>
+#include <private/qv4setobject_p.h>
+#include <private/qv4stackframe_p.h>
+
+#include <QtQml/qqmlengine.h>
+
+#include <QtCore/qalgorithms.h>
+#include <QtCore/qelapsedtimer.h>
+#include <QtCore/qloggingcategory.h>
+#include <QtCore/qmap.h>
+#include <QtCore/qscopedvaluerollback.h>
 
-#include <cstdlib>
 #include <algorithm>
-#include "qv4profiling_p.h"
-#include "qv4mapobject_p.h"
-#include "qv4setobject_p.h"
-
 #include <chrono>
+#include <cstdlib>
 
 //#define MM_STATS
 
@@ -1537,6 +1539,19 @@ void MemoryManager::collectFromJSStack(MarkStack *markStack) const
         }
         ++v;
     }
+
+    for (auto *frame = engine->currentStackFrame; frame; frame = frame->parentFrame()) {
+        if (!frame->isMetaTypesFrame())
+            continue;
+
+        const QQmlPrivate::AOTTrackedLocalsStorage *locals
+                = static_cast<const MetaTypesStackFrame *>(frame)->locals();
+
+        // locals have to be initialized first thing when calling the function
+        Q_ASSERT(locals);
+
+        locals->markObjects(markStack);
+    }
 }
 
 GCStateMachine::GCStateMachine()

src/qml/qml/qqml.cpp

@@ -3,8 +3,6 @@
 
 #include "qqml.h"
 
-#include <QtQml/qqmlprivate.h>
-
 #include <private/qjsvalue_p.h>
 #include <private/qqmlbuiltinfunctions_p.h>
 #include <private/qqmlcomponent_p.h>
@@ -24,7 +22,10 @@
 #include <private/qv4lookup_p.h>
 #include <private/qv4qobjectwrapper_p.h>
 
+#include <QtQml/qqmlprivate.h>
+
 #include <QtCore/qmutex.h>
+#include <QtCore/qsequentialiterable.h>
 
 QT_BEGIN_NAMESPACE
 
@@ -1183,6 +1184,12 @@ void AOTCompiledContext::setInstructionPointer(int offset) const
         frame->instructionPointer = offset;
 }
 
+void AOTCompiledContext::setLocals(const AOTTrackedLocalsStorage *locals) const
+{
+    if (auto *frame = engine->handle()->currentStackFrame)
+        frame->locals = locals;
+}
+
 void AOTCompiledContext::setReturnValueUndefined() const
 {
     if (auto *frame = engine->handle()->currentStackFrame) {
@@ -1191,6 +1198,89 @@ void AOTCompiledContext::setReturnValueUndefined() const
     }
 }
 
+void AOTCompiledContext::mark(QObject *object, QV4::MarkStack *markStack)
+{
+    QV4::QObjectWrapper::markWrapper(object, markStack);
+}
+
+static bool markPointer(const QVariant &element, QV4::MarkStack *markStack)
+{
+    if (!element.metaType().flags().testFlag(QMetaType::PointerToQObject))
+        return false;
+
+    QV4::QObjectWrapper::markWrapper(
+            *static_cast<QObject *const *>(element.constData()), markStack);
+    return true;
+}
+
+static void iterateVariant(const QVariant &element, std::vector<QVariant> *elements)
+{
+#define ADD_CASE(Type, id, T) \
+    case QMetaType::Type:
+
+    switch (element.metaType().id()) {
+    case QMetaType::QVariantMap:
+        for (const QVariant &variant : *static_cast<const QVariantMap *>(element.constData()))
+            elements->push_back(variant);
+        return;
+    case QMetaType::QVariantHash:
+        for (const QVariant &variant : *static_cast<const QVariantHash *>(element.constData()))
+            elements->push_back(variant);
+        return;
+    case QMetaType::QVariantList:
+        for (const QVariant &variant : *static_cast<const QVariantList *>(element.constData()))
+            elements->push_back(variant);
+        return;
+    QT_FOR_EACH_STATIC_PRIMITIVE_TYPE(ADD_CASE)
+    QT_FOR_EACH_STATIC_CORE_CLASS(ADD_CASE)
+    QT_FOR_EACH_STATIC_GUI_CLASS(ADD_CASE)
+    case QMetaType::QStringList:
+    case QMetaType::QByteArrayList:
+        return;
+    default:
+        break;
+    }
+
+    QSequentialIterable iterable;
+    if (!QMetaType::convert(
+                element.metaType(), element.constData(),
+                QMetaType::fromType<QSequentialIterable>(), &iterable)) {
+        return;
+    }
+
+    switch (iterable.valueMetaType().id()) {
+    QT_FOR_EACH_STATIC_PRIMITIVE_TYPE(ADD_CASE)
+    QT_FOR_EACH_STATIC_CORE_CLASS(ADD_CASE)
+    QT_FOR_EACH_STATIC_GUI_CLASS(ADD_CASE)
+    case QMetaType::QStringList:
+    case QMetaType::QByteArrayList:
+        return;
+    default:
+        break;
+    }
+
+    for (auto it = iterable.constBegin(), end = iterable.constEnd(); it != end; ++it)
+        elements->push_back(*it);
+
+#undef ADD_CASE
+}
+
+void AOTCompiledContext::mark(const QVariant &variant, QV4::MarkStack *markStack)
+{
+    if (markPointer(variant, markStack))
+        return;
+
+    std::vector<QVariant> stack;
+    iterateVariant(variant, &stack);
+
+    while (!stack.empty()) {
+        const QVariant &element = std::as_const(stack).back();
+        if (!markPointer(element, markStack))
+            iterateVariant(element, &stack);
+        stack.pop_back();
+    }
+}
+
 static void captureFallbackProperty(
         QObject *object, int coreIndex, int notifyIndex, bool isConstant,
         const AOTCompiledContext *aotContext)

src/qml/qml/qqmlprivate.h

@@ -50,6 +50,7 @@ using QQmlAttachedPropertiesFunc = A *(*)(QObject *);
 
 namespace QV4 {
 struct ExecutionEngine;
+struct MarkStack;
 class ExecutableCompilationUnit;
 namespace CompiledData {
 struct Unit;
@@ -617,6 +618,12 @@ namespace QQmlPrivate
         QVector<int> *qmlTypeIds;
     };
 
+    struct AOTTrackedLocalsStorage
+    {
+        virtual ~AOTTrackedLocalsStorage() = default;
+        virtual void markObjects(QV4::MarkStack *markStack) const = 0;
+    };
+
     struct Q_QML_EXPORT AOTCompiledContext {
         enum: uint { InvalidStringId = (std::numeric_limits<uint>::max)() };
 
@@ -633,8 +640,14 @@ namespace QQmlPrivate
 
         QJSValue jsMetaType(int index) const;
         void setInstructionPointer(int offset) const;
+        void setLocals(const AOTTrackedLocalsStorage *locals) const;
         void setReturnValueUndefined() const;
 
+        static void mark(QObject *object, QV4::MarkStack *markStack);
+        static void mark(const QVariant &variant, QV4::MarkStack *markStack);
+        template<typename T>
+        static void mark(T, QV4::MarkStack *) {}
+
         // Run QQmlPropertyCapture::captureProperty() without retrieving the value.
         bool captureLookup(uint index, QObject *object) const;
         bool captureQmlContextPropertyLookup(uint index) const;

src/qmlcompiler/qqmljscodegenerator.cpp

@@ -139,10 +139,10 @@ static QString registerName(int registerIndex, int offset)
     // That's why we need both 'v' and 'c'.
 
     if (offset < 0)
-        return u"a%1"_s.arg(registerIndex - QQmlJSCompilePass::Argc);
+        return u"s.a%1"_s.arg(registerIndex - QQmlJSCompilePass::Argc);
     if (registerIndex < 0)
-        return u"c%1_%2"_s.arg(-registerIndex).arg(offset);
+        return u"s.c%1_%2"_s.arg(-registerIndex).arg(offset);
-    return u"v%1_%2"_s.arg(registerIndex).arg(offset);
+    return u"s.v%1_%2"_s.arg(registerIndex).arg(offset);
 }
 
 QQmlJSAotFunction QQmlJSCodeGenerator::run(const Function *function, bool basicBlocksValidationFailed)
@@ -217,9 +217,13 @@ QT_WARNING_POP
             .arg(m_context->name).arg(m_context->line).arg(m_context->column);
 
     QStringList initializations;
+    QStringList markings;
     for (auto registerIt = m_registerVariables.cbegin(), registerEnd = m_registerVariables.cend();
          registerIt != registerEnd; ++registerIt) {
 
+        // Remove the "s.". Inside the struct we need the plain name.
+        QString declarationName = registerIt->variableName.mid(2);
+
         const int registerIndex = registerIt->initialRegisterIndex;
         const bool registerIsArgument = isArgument(registerIndex);
 
@@ -238,7 +242,7 @@ QT_WARNING_POP
                 && registerIndex != This
                 && !function->registerTypes[registerIndex - firstRegisterIndex()].contains(
                     m_typeResolver->voidType())) {
-            code += registerIt->variableName + u" = "_s;
+            code += declarationName + u" = "_s;
             code += convertStored(m_typeResolver->voidType(), storedType, QString());
         } else if (registerIsArgument && argumentType(registerIndex).isStoredIn(storedType)) {
             const int argumentIndex = registerIndex - FirstArgument;
@@ -257,7 +261,7 @@ QT_WARNING_POP
                 code += u'&';
             }
 
-            code += registerIt->variableName + u" = "_s;
+            code += declarationName + u" = "_s;
 
             const auto originalContained = m_typeResolver->originalContainedType(argument);
             QString originalValue;
@@ -278,19 +282,51 @@ QT_WARNING_POP
                 code += conversion(originalArgument, argument, originalValue);
             else
                 code += originalValue;
+        } else if (isPointer) {
+            code += declarationName + u" = nullptr"_s;
         } else {
-            code += registerIt->variableName;
+            code += declarationName;
         }
         code += u";\n"_s;
 
         initializations.push_back(std::move(code));
+
+        if (isPointer) {
+            markings.append(u"    aotContext->mark("_s + declarationName + u", markStack);\n");
+        } else if (storedType == m_typeResolver->varType()) {
+            markings.append(u"    aotContext->mark("_s + declarationName + u", markStack);\n");
+        } else if (storedType == m_typeResolver->listPropertyType()) {
+            // No need to mark that since it's always backed by a property
+        } else if (storedType == m_typeResolver->variantMapType()
+                   || storedType->accessSemantics() == QQmlJSScope::AccessSemantics::Sequence) {
+            QString marking = u"    for (const auto &v : std::as_const(" + declarationName + u"))\n"
+                    + u"        aotContext->mark(v, markStack);\n";
+            markings.append(std::move(marking));
+        }
     }
 
+    result.code += u"struct Storage : QQmlPrivate::AOTTrackedLocalsStorage {\n"_s;
+    result.code += u"Storage(const QQmlPrivate::AOTCompiledContext *ctxt, void **a)"_s;
+    result.code += u"   : aotContext(ctxt), argv(a) {}\n"_s;
+    result.code += u"void markObjects(QV4::MarkStack *markStack) const final {"_s;
+    result.code += u"    Q_UNUSED(markStack);\n"_s;
+
+    markings.sort();
+    for (const QString &marking : std::as_const(markings))
+        result.code += marking;
+
+    result.code += u"}\n"_s;
+    result.code += u"const QQmlPrivate::AOTCompiledContext *aotContext;\n"_s;
+    result.code += u"void **argv;\n"_s;
+
     // Sort them to obtain stable output.
     initializations.sort();
     for (const QString &initialization : std::as_const(initializations))
         result.code += initialization;
 
+    result.code += u"};\nStorage s(aotContext, argv);\n"_s;
+    result.code += u"aotContext->setLocals(&s);\n"_s;
+
     result.code += m_body;
 
 
@@ -2692,8 +2728,9 @@ void QQmlJSCodeGenerator::generate_GetIterator(int iterator)
         REJECT(u"using non-iterator as iterator"_s);
 
     const QString identifier = QString::number(iteratorType.baseLookupIndex());
-    const QString iteratorName = m_state.accumulatorVariableOut + u"Iterator" + identifier;
+    QString baseName = m_state.accumulatorVariableOut.mid(2); // remove "s."
-    const QString listName = m_state.accumulatorVariableOut + u"List" + identifier;
+    const QString iteratorName = baseName + u"Iterator" + identifier;
+    const QString listName = baseName + u"List" + identifier;
 
     m_body += u"QJSListFor"_s
             + (iterator == int(QQmlJS::AST::ForEachType::In) ? u"In"_s : u"Of"_s)

tests/auto/qml/qmlcppcodegen/data/CMakeLists.txt

@@ -7,6 +7,7 @@ add_subdirectory(WithSubDir)
 set(cpp_sources
     ambiguous.h
     birthdayparty.cpp birthdayparty.h
+    collector.h
     convertQJSPrimitiveValueToIntegral.h
     cppbaseclass.h
     detachedreferences.h
@@ -118,6 +119,7 @@ set(qml_files
     callObjectLookupOnNull.qml
     callWithSpread.qml
     childobject.qml
+    collector.qml
     colorAsVariant.qml
     colorString.qml
     compareOriginals.qml

tests/auto/qml/qmlcppcodegen/data/collector.h

@@ -0,0 +1,25 @@
+// Copyright (C) 2025 The Qt Company Ltd.
+// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only
+
+#ifndef COLLECTOR_H
+#define COLLECTOR_H
+
+#include <QtCore/qobject.h>
+#include <QtQmlIntegration/qqmlintegration.h>
+#include <QtQml/qjsengine.h>
+
+class Collector : public QObject
+{
+    Q_OBJECT
+    QML_ELEMENT
+    Q_PROPERTY(int gc READ gc CONSTANT)
+
+public:
+    int gc() const
+    {
+        qjsEngine(this)->collectGarbage();
+        return 1;
+    }
+};
+
+#endif // COLLECTOR_H

tests/auto/qml/qmlcppcodegen/data/collector.qml

@@ -0,0 +1,41 @@
+// Copyright (C) 2025 The Qt Company Ltd.
+// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only
+
+pragma Strict
+import QtQml
+import TestTypes
+
+Collector {
+    id: self
+    property Component c: QtObject { objectName: "dynamic" }
+    property QtObject o: c.createObject()
+    property QtObject o2
+    property int gcRun: 0
+
+    // Pass the object through a property once so that we know
+    // it has a wrapper (missing wrapper will be fixed independently)
+    property QtObject hidden: c.createObject()
+    function os() : list<var> {
+        let result = [hidden]
+        hidden = null
+        return result
+    }
+
+    Component.onCompleted: {
+        var oo = o
+        o = null
+        var oso = os()
+
+        // Hide this behind a property so that the side effect
+        // can't be detected.
+        var one = self.gc
+
+        // Don't rely on QV4::Sequence to check its members, yet
+        o2 = oso[0]
+        o = oo
+
+        // Actually use the value retrieved to trigger the gc
+        // Otherwise the access is optimized out.
+        gcRun = one
+    }
+}

tests/auto/qml/qmlcppcodegen/tst_qmlcppcodegen.cpp

@@ -72,6 +72,7 @@ private slots:
     void callContextPropertyLookupResult();
     void callObjectLookupOnNull();
     void callWithSpread();
+    void collectGarbageDuringAotCode();
     void colorAsVariant();
     void colorString();
     void compareOriginals();
@@ -1100,6 +1101,25 @@ void tst_QmlCppCodegen::callWithSpread()
     QVERIFY(!o.isNull());
 }
 
+void tst_QmlCppCodegen::collectGarbageDuringAotCode()
+{
+    QQmlEngine engine;
+    QQmlComponent c(&engine, QUrl(u"qrc:/qt/qml/TestTypes/collector.qml"_s));
+    QVERIFY2(c.isReady(), qPrintable(c.errorString()));
+    QScopedPointer<QObject> o(c.create());
+    QVERIFY(!o.isNull());
+
+    QObject *inner = o->property("o").value<QObject *>();
+    QVERIFY(inner);
+    QCOMPARE(inner->objectName(), u"dynamic"_s);
+
+    inner = o->property("o2").value<QObject *>();
+    QVERIFY(inner);
+    QCOMPARE(inner->objectName(), u"dynamic"_s);
+
+    QCOMPARE(o->property("gcRun").toInt(), 1);
+}
+
 void tst_QmlCppCodegen::colorAsVariant()
 {
     QQmlEngine engine;