From 604da0a395840d7370cde1a55db460156c3a3e8c Mon Sep 17 00:00:00 2001 From: Ulf Hermann Date: Wed, 18 Oct 2023 11:37:37 +0200 Subject: 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 --- .../qmldbg_debugger/qv4datacollector.cpp | 7 ++-- .../qqmlnativedebugservice.cpp | 7 ++-- src/qml/jsapi/qjsmanagedvalue.cpp | 7 +++- src/qml/jsruntime/qv4context.cpp | 18 +++++++-- src/qml/jsruntime/qv4internalclass.cpp | 12 ++++-- src/qml/jsruntime/qv4internalclass_p.h | 2 +- src/qml/jsruntime/qv4module.cpp | 9 +++-- .../qv4debugger/data/breakPointInJSModule.qml | 4 ++ .../auto/qml/debugger/qv4debugger/data/module1.js | 5 +++ .../auto/qml/debugger/qv4debugger/data/module2.mjs | 7 ++++ .../auto/qml/debugger/qv4debugger/data/module3.mjs | 0 .../auto/qml/debugger/qv4debugger/data/module4.mjs | 0 .../qml/debugger/qv4debugger/tst_qv4debugger.cpp | 45 ++++++++++++++++++++-- 13 files changed, 100 insertions(+), 23 deletions(-) create mode 100644 tests/auto/qml/debugger/qv4debugger/data/breakPointInJSModule.qml create mode 100644 tests/auto/qml/debugger/qv4debugger/data/module1.js create mode 100644 tests/auto/qml/debugger/qv4debugger/data/module2.mjs create mode 100644 tests/auto/qml/debugger/qv4debugger/data/module3.mjs create mode 100644 tests/auto/qml/debugger/qv4debugger/data/module4.mjs 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(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(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(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 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 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 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 m_statesWhenPaused; QList m_breakPointsToAddWhenPaused; QVector 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 v4Debugger = new QV4Debugger(v4); + v4->setDebugger(v4Debugger.data()); + + QScopedPointer debugThread(new QThread); + debugThread->start(); + QScopedPointer 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 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) -- cgit v1.2.3