QML Debugger: Don't crash when looking up values from imported modules

We cannot look up the imports from other modules because those are
stored in the CU. But we can avoid the crash.

Pick-to: 6.6 6.5 6.2 5.15
Fixes: QTBUG-117479
Change-Id: Ib5660c94dfb7ed20baedf7f71b2f175e6be042b1
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>

13 files changed, 100 insertions, 23 deletions

diff --git a/src/plugins/qmltooling/qmldbg_debugger/qv4datacollector.cpp b/src/plugins/qmltooling/qmldbg_debugger/qv4datacollector.cpp
index 6aa44dda0d..90edf86acb 100644
--- a/src/plugins/qmltooling/qmldbg_debugger/qv4datacollector.cpp
+++ b/src/plugins/qmltooling/qmldbg_debugger/qv4datacollector.cpp
@@ -188,9 +188,10 @@ bool QV4DataCollector::collectScope(QJsonObject *dict, int frameNr, int scopeNr)
         QV4::ScopedValue v(scope);
         QV4::Heap::InternalClass *ic = ctxt->internalClass();
         for (uint i = 0; i < ic->size; ++i) {
-            QString name = ic->keyAt(i);
-            names.append(name);
-            v = static_cast<QV4::Heap::CallContext *>(ctxt->d())->locals[i];
+            QV4::ScopedValue stringOrSymbol(scope, ic->keyAt(i));
+            QV4::ScopedString propName(scope, stringOrSymbol->toString(scope.engine));
+            names.append(propName->toQString());
+            v = ctxt->getProperty(propName);
             collectedRefs.append(addValueRef(v));
         }
 
diff --git a/src/plugins/qmltooling/qmldbg_nativedebugger/qqmlnativedebugservice.cpp b/src/plugins/qmltooling/qmldbg_nativedebugger/qqmlnativedebugservice.cpp
index 10d52c75ee..84cd1236d9 100644
--- a/src/plugins/qmltooling/qmldbg_nativedebugger/qqmlnativedebugservice.cpp
+++ b/src/plugins/qmltooling/qmldbg_nativedebugger/qqmlnativedebugservice.cpp
@@ -457,9 +457,10 @@ void NativeDebugger::handleVariables(QJsonObject *response, const QJsonObject &a
         QV4::Heap::InternalClass *ic = callContext->internalClass();
         QV4::ScopedValue v(scope);
         for (uint i = 0; i < ic->size; ++i) {
-            QString name = ic->keyAt(i);
-            v = callContext->d()->locals[i];
-            collector.collect(&output, QString(), name, v);
+            QV4::ScopedValue stringOrSymbol(scope, ic->keyAt(i));
+            QV4::ScopedString propName(scope, stringOrSymbol->toString(scope.engine));
+            v = callContext->getProperty(propName);
+            collector.collect(&output, QString(), propName->toQString(), v);
         }
     }
 
diff --git a/src/qml/jsapi/qjsmanagedvalue.cpp b/src/qml/jsapi/qjsmanagedvalue.cpp
index eeadf875de..a4f2f97432 100644
--- a/src/qml/jsapi/qjsmanagedvalue.cpp
+++ b/src/qml/jsapi/qjsmanagedvalue.cpp
@@ -1086,8 +1086,11 @@ QStringList QJSManagedValue::jsMetaMembers() const
         const int size = heapClass->size;
         QStringList result;
         result.reserve(size);
-        for (int i = 0; i < size; ++i)
-            result.append(heapClass->keyAt(i));
+        QV4::Scope scope(c->engine());
+        for (int i = 0; i < size; ++i) {
+            QV4::ScopedValue key(scope, heapClass->keyAt(i));
+            result.append(key->toQString());
+        }
         return result;
     }
 
diff --git a/src/qml/jsruntime/qv4context.cpp b/src/qml/jsruntime/qv4context.cpp
index b702dbe2db..3ae3e5d24c 100644
--- a/src/qml/jsruntime/qv4context.cpp
+++ b/src/qml/jsruntime/qv4context.cpp
@@ -299,9 +299,14 @@ ReturnedValue ExecutionContext::getProperty(String *name)
         case Heap::ExecutionContext::Type_CallContext: {
             Heap::CallContext *c = static_cast<Heap::CallContext *>(ctx);
 
-            uint index = c->internalClass->indexOfValueOrGetter(id);
-            if (index < UINT_MAX)
+            const uint index = c->internalClass->indexOfValueOrGetter(id);
+            if (index < c->locals.alloc)
                 return c->locals[index].asReturnedValue();
+
+            // TODO: We should look up the module imports here, but those are part of the CU:
+            //       imports[index - c->locals.size];
+            //       See QTBUG-118478
+
             Q_FALLTHROUGH();
         }
         case Heap::ExecutionContext::Type_WithContext:
@@ -349,9 +354,14 @@ ReturnedValue ExecutionContext::getPropertyAndBase(String *name, Value *base)
         case Heap::ExecutionContext::Type_CallContext: {
             Heap::CallContext *c = static_cast<Heap::CallContext *>(ctx);
 
-            uint index = c->internalClass->indexOfValueOrGetter(id);
-            if (index < UINT_MAX)
+            const uint index = c->internalClass->indexOfValueOrGetter(id);
+            if (index < c->locals.alloc)
                 return c->locals[index].asReturnedValue();
+
+            // TODO: We should look up the module imports here, but those are part of the CU:
+            //       imports[index - c->locals.size];
+            //       See QTBUG-118478
+
             Q_FALLTHROUGH();
         }
         case Heap::ExecutionContext::Type_GlobalContext: {
diff --git a/src/qml/jsruntime/qv4internalclass.cpp b/src/qml/jsruntime/qv4internalclass.cpp
index c72effd406..bf19b806e4 100644
--- a/src/qml/jsruntime/qv4internalclass.cpp
+++ b/src/qml/jsruntime/qv4internalclass.cpp
@@ -280,9 +280,15 @@ void InternalClass::destroy()
     Base::destroy();
 }
 
-QString InternalClass::keyAt(uint index) const
-{
-    return nameMap.at(index).toQString();
+ReturnedValue InternalClass::keyAt(uint index) const
+{
+    PropertyKey key = nameMap.at(index);
+    if (!key.isValid())
+        return Encode::undefined();
+    if (key.isArrayIndex())
+        return Encode(key.asArrayIndex());
+    Q_ASSERT(key.isStringOrSymbol());
+    return key.asStringOrSymbol()->asReturnedValue();
 }
 
 void InternalClass::changeMember(QV4::Object *object, PropertyKey id, PropertyAttributes data, InternalClassEntry *entry)
diff --git a/src/qml/jsruntime/qv4internalclass_p.h b/src/qml/jsruntime/qv4internalclass_p.h
index df73ee6ccc..4180402c92 100644
--- a/src/qml/jsruntime/qv4internalclass_p.h
+++ b/src/qml/jsruntime/qv4internalclass_p.h
@@ -332,7 +332,7 @@ struct InternalClass : Base {
     void init(InternalClass *other);
     void destroy();
 
-    Q_QML_PRIVATE_EXPORT QString keyAt(uint index) const;
+    Q_QML_PRIVATE_EXPORT ReturnedValue keyAt(uint index) const;
     Q_REQUIRED_RESULT InternalClass *nonExtensible();
     Q_REQUIRED_RESULT InternalClass *locked();
 
diff --git a/src/qml/jsruntime/qv4module.cpp b/src/qml/jsruntime/qv4module.cpp
index 1a289f25b8..e044d285ee 100644
--- a/src/qml/jsruntime/qv4module.cpp
+++ b/src/qml/jsruntime/qv4module.cpp
@@ -223,9 +223,12 @@ OwnPropertyKeyIterator *Module::virtualOwnPropertyKeys(const Object *o, Value *t
     if (module->d()->unit->isESModule()) {
         names = module->d()->unit->exportedNames();
     } else {
-        Heap::InternalClass *scopeClass = module->d()->scope->internalClass;
-        for (uint i = 0; i < scopeClass->size; ++i)
-            names << scopeClass->keyAt(i);
+        QV4::Scope scope(module->engine());
+        QV4::Scoped<InternalClass> scopeClass(scope, module->d()->scope->internalClass);
+        for (uint i = 0, end = scopeClass->d()->size; i < end; ++i) {
+            QV4::ScopedValue key(scope, scopeClass->d()->keyAt(i));
+            names << key->toQString();
+        }
     }
 
     return new ModuleNamespaceIterator(names);
diff --git a/tests/auto/qml/debugger/qv4debugger/data/breakPointInJSModule.qml b/tests/auto/qml/debugger/qv4debugger/data/breakPointInJSModule.qml
new file mode 100644
index 0000000000..2582a23ec5
--- /dev/null
+++ b/tests/auto/qml/debugger/qv4debugger/data/breakPointInJSModule.qml
@@ -0,0 +1,4 @@
+import QtQml 2.15
+import "module1.js" as Module1
+
+QtObject {}
diff --git a/tests/auto/qml/debugger/qv4debugger/data/module1.js b/tests/auto/qml/debugger/qv4debugger/data/module1.js
new file mode 100644
index 0000000000..9ce1f1e6b1
--- /dev/null
+++ b/tests/auto/qml/debugger/qv4debugger/data/module1.js
@@ -0,0 +1,5 @@
+.pragma library
+
+.import "module2.mjs" as Module2
+
+Module2.crashMe();
diff --git a/tests/auto/qml/debugger/qv4debugger/data/module2.mjs b/tests/auto/qml/debugger/qv4debugger/data/module2.mjs
new file mode 100644
index 0000000000..80f82af953
--- /dev/null
+++ b/tests/auto/qml/debugger/qv4debugger/data/module2.mjs
@@ -0,0 +1,7 @@
+import * as Module3 from "module3.mjs"
+import * as Module4 from "module4.mjs"
+
+export function crashMe()
+{
+    console.log("Hello world!");
+}
diff --git a/tests/auto/qml/debugger/qv4debugger/data/module3.mjs b/tests/auto/qml/debugger/qv4debugger/data/module3.mjs
new file mode 100644
index 0000000000..e69de29bb2
--- /dev/null
+++ b/tests/auto/qml/debugger/qv4debugger/data/module3.mjs
diff --git a/tests/auto/qml/debugger/qv4debugger/data/module4.mjs b/tests/auto/qml/debugger/qv4debugger/data/module4.mjs
new file mode 100644
index 0000000000..e69de29bb2
--- /dev/null
+++ b/tests/auto/qml/debugger/qv4debugger/data/module4.mjs
diff --git a/tests/auto/qml/debugger/qv4debugger/tst_qv4debugger.cpp b/tests/auto/qml/debugger/qv4debugger/tst_qv4debugger.cpp
index b0cc837a7a..1aefa9e511 100644
--- a/tests/auto/qml/debugger/qv4debugger/tst_qv4debugger.cpp
+++ b/tests/auto/qml/debugger/qv4debugger/tst_qv4debugger.cpp
@@ -217,10 +217,14 @@ public:
             QJsonArray scopes = frameObj.value(QLatin1String("scopes")).toArray();
             int nscopes = scopes.size();
             int s = 0;
-            for (s = 0; s < nscopes; ++s) {
-                QJsonObject o = scopes.at(s).toObject();
-                if (o.value(QLatin1String("type")).toInt(-2) == 1) // CallContext
-                    break;
+            if (m_targetScope != -1) {
+                s = m_targetScope;
+            } else {
+                for (s = 0; s < nscopes; ++s) {
+                    QJsonObject o = scopes.at(s).toObject();
+                    if (o.value(QLatin1String("type")).toInt(-2) == 1) // CallContext
+                        break;
+                }
             }
             if (s == nscopes)
                 return;
@@ -250,6 +254,7 @@ public:
     bool m_wasPaused;
     QV4Debugger::PauseReason m_pauseReason;
     bool m_captureContextInfo;
+    int m_targetScope = -1;
     QList<QV4Debugger::ExecutionState> m_statesWhenPaused;
     QList<TestBreakPoint> m_breakPointsToAddWhenPaused;
     QVector<QV4::StackFrame> m_stackTrace;
@@ -324,6 +329,9 @@ private slots:
     void readThis();
     void signalParameters();
     void debuggerNoCrash();
+
+    void breakPointInJSModule();
+
 private:
     QV4Debugger *debugger() const
     {
@@ -969,6 +977,35 @@ void tst_qv4debugger::debuggerNoCrash()
     debugThread->wait();
 }
 
+void tst_qv4debugger::breakPointInJSModule()
+{
+    QQmlEngine engine;
+    QV4::ExecutionEngine *v4 = engine.handle();
+    QPointer<QV4Debugger> v4Debugger = new QV4Debugger(v4);
+    v4->setDebugger(v4Debugger.data());
+
+    QScopedPointer<QThread> debugThread(new QThread);
+    debugThread->start();
+    QScopedPointer<TestAgent> debuggerAgent(new TestAgent(v4));
+    debuggerAgent->addDebugger(v4Debugger);
+    debuggerAgent->moveToThread(debugThread.data());
+
+    QQmlComponent component(&engine, testFileUrl("breakPointInJSModule.qml"));
+    QVERIFY2(component.isReady(), qPrintable(component.errorString()));
+
+    debuggerAgent->m_captureContextInfo = true;
+    debuggerAgent->m_targetScope = 1;
+    v4Debugger->addBreakPoint("module2.mjs", 6);
+
+    QScopedPointer<QObject> obj(component.create());
+    QVERIFY(!obj.isNull());
+
+    QVERIFY(!debuggerAgent->m_capturedScope.isEmpty());
+
+    debugThread->quit();
+    debugThread->wait();
+}
+
 tst_qv4debugger::tst_qv4debugger() : QQmlDataTest(QT_QMLTEST_DATADIR) { }
 
 QTEST_MAIN(tst_qv4debugger)