From a4b4932efb631a3c467c9bb4b3e4f99ca70a066d Mon Sep 17 00:00:00 2001 From: Chris Adams Date: Thu, 2 Feb 2012 14:51:17 +1000 Subject: Check dynamic slot function for nullness before evaluation Previously, we didn't check whether the function object handle associated with a dynamic slot's method index was null before attempting to evaluate it, which could cause a crash in some circumstances. This change also adds better error reporting during function compilation. Task-number: QTBUG-24064 Task-number: QTBUG-24037 Task-number: QTBUG-23387 Change-Id: I3c5e35e8c16187870125736013a5935fcc5cb1f2 Reviewed-by: Aaron Kennedy --- src/declarative/qml/qdeclarativeexpression.cpp | 26 +++++++++++++++++++--- src/declarative/qml/qdeclarativevmemetaobject.cpp | 11 +++++++++ .../data/v8functionException.qml | 15 +++++++++++++ .../tst_qdeclarativeecmascript.cpp | 14 ++++++++++++ 4 files changed, 63 insertions(+), 3 deletions(-) create mode 100644 tests/auto/declarative/qdeclarativeecmascript/data/v8functionException.qml diff --git a/src/declarative/qml/qdeclarativeexpression.cpp b/src/declarative/qml/qdeclarativeexpression.cpp index 94ed7b7296..97afaa8281 100644 --- a/src/declarative/qml/qdeclarativeexpression.cpp +++ b/src/declarative/qml/qdeclarativeexpression.cpp @@ -148,13 +148,33 @@ QDeclarativeExpressionPrivate::evalFunction(QDeclarativeContextData *ctxt, QObje // QtScript days v8::HandleScope handle_scope; v8::Context::Scope ctxtscope(ep->v8engine()->context()); - + v8::TryCatch tc; v8::Local scopeobject = ep->v8engine()->qmlScope(ctxt, scope); v8::Local script = ep->v8engine()->qmlModeCompile(code, filename, line); - if (tc.HasCaught()) return v8::Persistent(); + if (tc.HasCaught()) { + QDeclarativeError error; + error.setDescription(QLatin1String("Exception occurred during function compilation")); + error.setLine(line); + error.setUrl(QUrl::fromLocalFile(filename)); + v8::Local message = tc.Message(); + if (!message.IsEmpty()) + QDeclarativeExpressionPrivate::exceptionToError(message, error); + ep->warning(error); + return v8::Persistent(); + } v8::Local result = script->Run(scopeobject); - if (tc.HasCaught()) return v8::Persistent(); + if (tc.HasCaught()) { + QDeclarativeError error; + error.setDescription(QLatin1String("Exception occurred during function evaluation")); + error.setLine(line); + error.setUrl(QUrl::fromLocalFile(filename)); + v8::Local message = tc.Message(); + if (!message.IsEmpty()) + QDeclarativeExpressionPrivate::exceptionToError(message, error); + ep->warning(error); + return v8::Persistent(); + } if (qmlscope) *qmlscope = qPersistentNew(scopeobject); return qPersistentNew(v8::Local::Cast(result)); } diff --git a/src/declarative/qml/qdeclarativevmemetaobject.cpp b/src/declarative/qml/qdeclarativevmemetaobject.cpp index e5c798e354..18b29f3411 100644 --- a/src/declarative/qml/qdeclarativevmemetaobject.cpp +++ b/src/declarative/qml/qdeclarativevmemetaobject.cpp @@ -730,6 +730,17 @@ int QDeclarativeVMEMetaObject::metaCall(QMetaObject::Call c, int _id, void **a) ep->referenceScarceResources(); // "hold" scarce resources in memory during evaluation. v8::Handle function = method(id); + if (function.IsEmpty()) { + // The function was not compiled. There are some exceptional cases which the + // expression rewriter does not rewrite properly (e.g., \r-terminated lines + // are not rewritten correctly but this bug is deemed out-of-scope to fix for + // performance reasons; see QTBUG-24064) and thus compilation will have failed. + QDeclarativeError e; + e.setDescription(QString(QLatin1String("Exception occurred during compilation of function: %1")).arg(QMetaObject::method(_id).signature())); + ep->warning(e); + return -1; // The dynamic method with that id is not available. + } + QDeclarativeVMEMetaData::MethodData *data = metaData->methodData() + id; v8::HandleScope handle_scope; diff --git a/tests/auto/declarative/qdeclarativeecmascript/data/v8functionException.qml b/tests/auto/declarative/qdeclarativeecmascript/data/v8functionException.qml new file mode 100644 index 0000000000..51df1c65d8 --- /dev/null +++ b/tests/auto/declarative/qdeclarativeecmascript/data/v8functionException.qml @@ -0,0 +1,15 @@ +import QtQuick 2.0 + +// This test uses a multi-line string which has \r-terminated +// string fragments. The expression rewriter deliberately doesn't +// handle \r-terminated string fragments (see QTBUG-24064) and thus +// this test ensures that we don't crash when the client attempts +// to invoke a non-compiled dynamic slot. + +Item { + id: root + + function dynamicSlot() { + var someString = "Hello, this is a multiline string"; + } +} diff --git a/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp b/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp index 1c42d9d425..02f79d2dd7 100644 --- a/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp +++ b/tests/auto/declarative/qdeclarativeecmascript/tst_qdeclarativeecmascript.cpp @@ -196,6 +196,7 @@ private slots: void functionAssignmentfromJS_invalid(); void eval(); void function(); + void functionException(); void qtbug_10696(); void qtbug_11606(); void qtbug_11600(); @@ -5071,6 +5072,19 @@ void tst_qdeclarativeecmascript::function() delete o; } +void tst_qdeclarativeecmascript::functionException() +{ + // QTBUG-24037 - shouldn't crash. + QString errstr = testFileUrl("v8functionException.qml").toString() + QLatin1String(":13: SyntaxError: Unexpected token ILLEGAL"); + QTest::ignoreMessage(QtWarningMsg, qPrintable(errstr)); + QTest::ignoreMessage(QtWarningMsg, ": Exception occurred during compilation of function: dynamicSlot()"); + QDeclarativeComponent component(&engine, testFileUrl("v8functionException.qml")); + QObject *o = component.create(); + QVERIFY(o != 0); + QMetaObject::invokeMethod(o, "dynamicSlot"); + delete o; +} + // Test the "Qt.include" method void tst_qdeclarativeecmascript::include() { -- cgit v1.2.3