From e0284ab41f7a1889f28e719212df66e942959f4c Mon Sep 17 00:00:00 2001 From: Lars Knoll Date: Mon, 21 Oct 2013 09:50:27 +0200 Subject: [PATCH] Properly propagate parse errors Replace all try/catch statements used when parsing with checks for engine->hasException. Change-Id: I4493cb600d5a3eb095c2003bb88bd031403e47c9 Reviewed-by: Simon Hausmann --- src/qml/jsapi/qjsengine.cpp | 14 ++--- src/qml/jsruntime/qv4engine.cpp | 2 +- src/qml/jsruntime/qv4engine_p.h | 2 +- src/qml/jsruntime/qv4globalobject.cpp | 2 + src/qml/jsruntime/qv4include.cpp | 18 +++--- src/qml/jsruntime/qv4qobjectwrapper.cpp | 2 +- src/qml/jsruntime/qv4script.cpp | 16 +++-- src/qml/qml/qqmlcomponent.cpp | 2 +- src/qml/qml/qqmljavascriptexpression.cpp | 18 +++--- src/qml/qml/qqmltypeloader.cpp | 9 ++- src/qml/qml/qqmlvmemetaobject.cpp | 2 +- src/qml/qml/qqmlxmlhttprequest.cpp | 2 +- src/qml/types/qquickworkerscript.cpp | 12 ++-- .../auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp | 73 ++++++++++++---------- tools/qmljs/main.cpp | 22 +++---- 15 files changed, 108 insertions(+), 88 deletions(-) diff --git a/src/qml/jsapi/qjsengine.cpp b/src/qml/jsapi/qjsengine.cpp index d1bd493..5d8a020 100644 --- a/src/qml/jsapi/qjsengine.cpp +++ b/src/qml/jsapi/qjsengine.cpp @@ -262,15 +262,15 @@ QJSValue QJSEngine::evaluate(const QString& program, const QString& fileName, in QV4::Scope scope(d->m_v4Engine); QV4::ExecutionContext *ctx = d->m_v4Engine->current; QV4::ScopedValue result(scope); - try { - QV4::Script script(ctx, program, fileName, lineNumber); - script.strictMode = ctx->strictMode; - script.inheritContext = true; - script.parse(); + + QV4::Script script(ctx, program, fileName, lineNumber); + script.strictMode = ctx->strictMode; + script.inheritContext = true; + script.parse(); + if (!scope.engine->hasException) result = script.run(); - } catch (...) { + if (scope.engine->hasException) result = ctx->catchException(); - } return new QJSValuePrivate(d->m_v4Engine, result); } diff --git a/src/qml/jsruntime/qv4engine.cpp b/src/qml/jsruntime/qv4engine.cpp index 73db541..b0ede6f 100644 --- a/src/qml/jsruntime/qv4engine.cpp +++ b/src/qml/jsruntime/qv4engine.cpp @@ -838,7 +838,7 @@ ReturnedValue ExecutionEngine::catchException(ExecutionContext *catchingContext, return res; } -QQmlError ExecutionEngine::convertJavaScriptException(ExecutionContext *context) +QQmlError ExecutionEngine::catchExceptionAsQmlError(ExecutionContext *context) { QV4::StackTrace trace; QV4::Scope scope(context); diff --git a/src/qml/jsruntime/qv4engine_p.h b/src/qml/jsruntime/qv4engine_p.h index 3eaf08d..94a4dff 100644 --- a/src/qml/jsruntime/qv4engine_p.h +++ b/src/qml/jsruntime/qv4engine_p.h @@ -330,7 +330,7 @@ struct Q_QML_EXPORT ExecutionEngine ReturnedValue catchException(ExecutionContext *catchingContext, StackTrace *trace); // Use only inside catch(...) -- will re-throw if no JS exception - static QQmlError convertJavaScriptException(QV4::ExecutionContext *context); + static QQmlError catchExceptionAsQmlError(QV4::ExecutionContext *context); void Q_NORETURN throwInternal(); // ---- diff --git a/src/qml/jsruntime/qv4globalobject.cpp b/src/qml/jsruntime/qv4globalobject.cpp index 4dcdb08..d95ab97 100644 --- a/src/qml/jsruntime/qv4globalobject.cpp +++ b/src/qml/jsruntime/qv4globalobject.cpp @@ -400,6 +400,8 @@ ReturnedValue EvalFunction::evalCall(CallData *callData, bool directCall) script.strictMode = (directCall && parentContext->strictMode); script.inheritContext = inheritContext; script.parse(); + if (scope.engine->hasException) + return Encode::undefined(); Function *function = script.function(); if (!function) diff --git a/src/qml/jsruntime/qv4include.cpp b/src/qml/jsruntime/qv4include.cpp index 47d2832..355817e 100644 --- a/src/qml/jsruntime/qv4include.cpp +++ b/src/qml/jsruntime/qv4include.cpp @@ -157,14 +157,15 @@ void QV4Include::finished() QV4::ExecutionContext *ctx = v4->current; QV4::ScopedString status(scope, v4->newString("status")); - try { - script.parse(); + script.parse(); + if (!scope.engine->hasException) script.run(); - resultObj->put(status, QV4::ScopedValue(scope, QV4::Primitive::fromInt32(Ok))); - } catch (...) { + if (scope.engine->hasException) { QV4::ScopedValue ex(scope, ctx->catchException()); resultObj->put(status, QV4::ScopedValue(scope, QV4::Primitive::fromInt32(Exception))); resultObj->put(QV4::ScopedString(scope, v4->newString("exception")), ex); + } else { + resultObj->put(status, QV4::ScopedValue(scope, QV4::Primitive::fromInt32(Ok))); } } else { resultObj->put(QV4::ScopedString(scope, v4->newString("status")), QV4::ScopedValue(scope, QV4::Primitive::fromInt32(NetworkError))); @@ -222,14 +223,15 @@ QV4::ReturnedValue QV4Include::method_include(QV4::SimpleCallContext *ctx) QV4::Script script(v4, qmlcontextobject, code, url.toString()); QV4::ExecutionContext *ctx = v4->current; - try { - script.parse(); + script.parse(); + if (!v4->hasException) script.run(); - result = resultValue(v4, Ok); - } catch (...) { + if (v4->hasException) { QV4::ScopedValue ex(scope, ctx->catchException()); result = resultValue(v4, Exception); result->asObject()->put(QV4::ScopedString(scope, v4->newString("exception")), ex); + } else { + result = resultValue(v4, Ok); } } else { result = resultValue(v4, NetworkError); diff --git a/src/qml/jsruntime/qv4qobjectwrapper.cpp b/src/qml/jsruntime/qv4qobjectwrapper.cpp index 1fd47c7..05d5532 100644 --- a/src/qml/jsruntime/qv4qobjectwrapper.cpp +++ b/src/qml/jsruntime/qv4qobjectwrapper.cpp @@ -727,7 +727,7 @@ struct QObjectSlotDispatcher : public QtPrivate::QSlotObjectBase try { f->call(callData); } catch (...) { - QQmlError error = QV4::ExecutionEngine::convertJavaScriptException(ctx); + QQmlError error = QV4::ExecutionEngine::catchExceptionAsQmlError(ctx); if (error.description().isEmpty()) error.setDescription(QString(QLatin1String("Unknown exception occurred during evaluation of connected function: %1")).arg(f->name->toQString())); QQmlEnginePrivate::get(v4->v8Engine->engine())->warning(error); diff --git a/src/qml/jsruntime/qv4script.cpp b/src/qml/jsruntime/qv4script.cpp index daca700..a2d89a3 100644 --- a/src/qml/jsruntime/qv4script.cpp +++ b/src/qml/jsruntime/qv4script.cpp @@ -185,6 +185,7 @@ void Script::parse() foreach (const QQmlJS::DiagnosticMessage &m, parser.diagnosticMessages()) { if (m.isError()) { scope->throwSyntaxError(m.message, sourceFile, m.loc.startLine, m.loc.startColumn); + return; } else { qWarning() << sourceFile << ':' << m.loc.startLine << ':' << m.loc.startColumn << ": warning: " << m.message; @@ -208,6 +209,9 @@ void Script::parse() RuntimeCodegen cg(scope, strictMode); cg.generateFromProgram(sourceFile, sourceCode, program, &module, parseAsBinding ? QQmlJS::Codegen::QmlBinding : QQmlJS::Codegen::EvalCode, inheritedLocals); + if (v4->hasException) + return; + QV4::Compiler::JSUnitGenerator jsGenerator(&module); QScopedPointer isel(v4->iselFactory->create(v4->executableAllocator, &module, &jsGenerator)); if (inheritContext) @@ -378,11 +382,13 @@ QV4::ReturnedValue Script::evaluate(ExecutionEngine *engine, const QString &scr QV4::Script qmlScript(engine, scopeObject, script, QString()); QV4::ExecutionContext *ctx = engine->current; - try { - qmlScript.parse(); - return qmlScript.run(); - } catch (...) { + qmlScript.parse(); + QV4::ScopedValue result(scope); + if (!scope.engine->hasException) + result = qmlScript.run(); + if (scope.engine->hasException) { ctx->catchException(); + return Encode::undefined(); } - return Encode::undefined(); + return result.asReturnedValue(); } diff --git a/src/qml/qml/qqmlcomponent.cpp b/src/qml/qml/qqmlcomponent.cpp index ab08fb7..0314e4d 100644 --- a/src/qml/qml/qqmlcomponent.cpp +++ b/src/qml/qml/qqmlcomponent.cpp @@ -1573,7 +1573,7 @@ void QmlIncubatorObject::statusChanged(QQmlIncubator::Status s) callData->args[0] = QV4::Primitive::fromUInt32(s); f->call(callData); } catch (...) { - QQmlError error = QV4::ExecutionEngine::convertJavaScriptException(ctx); + QQmlError error = QV4::ExecutionEngine::catchExceptionAsQmlError(ctx); QQmlEnginePrivate::warning(QQmlEnginePrivate::get(v8->engine()), error); } } diff --git a/src/qml/qml/qqmljavascriptexpression.cpp b/src/qml/qml/qqmljavascriptexpression.cpp index 37a9e39..d115268 100644 --- a/src/qml/qml/qqmljavascriptexpression.cpp +++ b/src/qml/qml/qqmljavascriptexpression.cpp @@ -86,7 +86,7 @@ void QQmlDelayedError::setErrorObject(QObject *object) void QQmlDelayedError::catchJavaScriptException(QV4::ExecutionContext *context) { - m_error = QV4::ExecutionEngine::convertJavaScriptException(context); + m_error = QV4::ExecutionEngine::catchExceptionAsQmlError(context); } @@ -302,11 +302,11 @@ QQmlJavaScriptExpression::evalFunction(QQmlContextData *ctxt, QObject *scopeObje QV4::ScopedObject qmlScopeObject(scope, QV4::QmlContextWrapper::qmlScope(ep->v8engine(), ctxt, scopeObject)); QV4::Script script(v4, qmlScopeObject, code, filename, line); QV4::ScopedValue result(scope); - try { - script.parse(); + script.parse(); + if (!v4->hasException) result = script.run(); - } catch (...) { - QQmlError error = QV4::ExecutionEngine::convertJavaScriptException(ctx); + if (v4->hasException) { + QQmlError error = QV4::ExecutionEngine::catchExceptionAsQmlError(ctx); if (error.description().isEmpty()) error.setDescription(QLatin1String("Exception occurred during function evaluation")); if (error.line() == -1) @@ -336,11 +336,11 @@ QV4::ReturnedValue QQmlJavaScriptExpression::qmlBinding(QQmlContextData *ctxt, Q QV4::ScopedObject qmlScopeObject(scope, QV4::QmlContextWrapper::qmlScope(ep->v8engine(), ctxt, qmlScope)); QV4::Script script(v4, qmlScopeObject, code, filename, line); QV4::ScopedValue result(scope); - try { - script.parse(); + script.parse(); + if (!v4->hasException) result = script.qmlBinding(); - } catch (...) { - QQmlError error = QV4::ExecutionEngine::convertJavaScriptException(ctx); + if (v4->hasException) { + QQmlError error = QV4::ExecutionEngine::catchExceptionAsQmlError(ctx); if (error.description().isEmpty()) error.setDescription(QLatin1String("Exception occurred during function evaluation")); if (error.line() == -1) diff --git a/src/qml/qml/qqmltypeloader.cpp b/src/qml/qml/qqmltypeloader.cpp index eb565ba..733f057 100644 --- a/src/qml/qml/qqmltypeloader.cpp +++ b/src/qml/qml/qqmltypeloader.cpp @@ -2786,11 +2786,10 @@ QV4::PersistentValue QQmlScriptData::scriptValueForContext(QQmlContextData *pare QV4::QmlContextWrapper::takeContextOwnership(qmlglobal); QV4::ExecutionContext *ctx = QV8Engine::getV4(v8engine)->current; - try { - m_program->qml = qmlglobal; - m_program->run(); - } catch (...) { - QQmlError error = QV4::ExecutionEngine::convertJavaScriptException(ctx); + m_program->qml = qmlglobal; + m_program->run(); + if (scope.engine->hasException) { + QQmlError error = QV4::ExecutionEngine::catchExceptionAsQmlError(ctx); if (error.isValid()) ep->warning(error); } diff --git a/src/qml/qml/qqmlvmemetaobject.cpp b/src/qml/qml/qqmlvmemetaobject.cpp index 5752033..4b141b5 100644 --- a/src/qml/qml/qqmlvmemetaobject.cpp +++ b/src/qml/qml/qqmlvmemetaobject.cpp @@ -960,7 +960,7 @@ int QQmlVMEMetaObject::metaCall(QMetaObject::Call c, int _id, void **a) result = function->call(callData); if (a[0]) *(QVariant *)a[0] = ep->v8engine()->toVariant(result, 0); } catch (...) { - QQmlError error = QV4::ExecutionEngine::convertJavaScriptException(ctx); + QQmlError error = QV4::ExecutionEngine::catchExceptionAsQmlError(ctx); if (error.isValid()) ep->warning(error); if (a[0]) *(QVariant *)a[0] = QVariant(); diff --git a/src/qml/qml/qqmlxmlhttprequest.cpp b/src/qml/qml/qqmlxmlhttprequest.cpp index aff0cf2..8445b58 100644 --- a/src/qml/qml/qqmlxmlhttprequest.cpp +++ b/src/qml/qml/qqmlxmlhttprequest.cpp @@ -1572,7 +1572,7 @@ void QQmlXMLHttpRequest::dispatchCallback(const ValueRef me) // the source is changed). We do nothing in this case, as the evaluation // cannot succeed. } catch (...) { - QQmlError error = QV4::ExecutionEngine::convertJavaScriptException(ctx); + QQmlError error = QV4::ExecutionEngine::catchExceptionAsQmlError(ctx); QQmlEnginePrivate::warning(QQmlEnginePrivate::get(v4->v8Engine->engine()), error); } } diff --git a/src/qml/types/qquickworkerscript.cpp b/src/qml/types/qquickworkerscript.cpp index 322b4b8..c0f6c73 100644 --- a/src/qml/types/qquickworkerscript.cpp +++ b/src/qml/types/qquickworkerscript.cpp @@ -230,8 +230,10 @@ void QQuickWorkerScriptEnginePrivate::WorkerEngine::init() QV4::Scope scope(m_v4Engine); onmessage = QV4::Script(m_v4Engine->rootContext, CALL_ONMESSAGE_SCRIPT).run(); + Q_ASSERT(!scope.engine->hasException); QV4::Script createsendscript(m_v4Engine->rootContext, SEND_MESSAGE_CREATE_SCRIPT); QV4::Scoped createsendconstructor(scope, createsendscript.run()); + Q_ASSERT(!scope.engine->hasException); QV4::ScopedString name(scope, m_v4Engine->newString(QStringLiteral("sendMessage"))); QV4::ScopedValue function(scope, m_v4Engine->newBuiltinFunction(m_v4Engine->rootContext, name, QQuickWorkerScriptEnginePrivate::method_sendMessage)); @@ -367,7 +369,7 @@ void QQuickWorkerScriptEnginePrivate::processMessage(int id, const QByteArray &d callData->args[1] = value; f->call(callData); } catch (...) { - QQmlError error = QV4::ExecutionEngine::convertJavaScriptException(ctx); + QQmlError error = QV4::ExecutionEngine::catchExceptionAsQmlError(ctx); reportScriptException(script, error); } } @@ -400,11 +402,11 @@ void QQuickWorkerScriptEnginePrivate::processLoad(int id, const QUrl &url) QV4::Script program(v4, activation, sourceCode, url.toString()); QV4::ExecutionContext *ctx = v4->current; - try { - program.parse(); + program.parse(); + if (!v4->hasException) program.run(); - } catch (...) { - QQmlError error = QV4::ExecutionEngine::convertJavaScriptException(ctx); + if (v4->hasException) { + QQmlError error = QV4::ExecutionEngine::catchExceptionAsQmlError(ctx); reportScriptException(script, error); } } else { diff --git a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp index e764ae7..5caf572 100644 --- a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp +++ b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp @@ -2257,15 +2257,16 @@ static inline bool evaluate_error(QV8Engine *engine, const QV4::ValueRef o, cons QV4::ExecutionContext *ctx = QV8Engine::getV4(engine)->current; QV4::Scope scope(ctx); - try { - QV4::Scoped function(scope, program.run()); - if (!function) - return false; - QV4::ScopedCallData d(scope, 1); - d->args[0] = o; - d->thisObject = engine->global(); - function->call(d); - } catch (...) { + QV4::Scoped function(scope, program.run()); + if (scope.engine->hasException) { + ctx->catchException(); + return true; + } + QV4::ScopedCallData d(scope, 1); + d->args[0] = o; + d->thisObject = engine->global(); + function->call(d); + if (scope.engine->hasException) { ctx->catchException(); return true; } @@ -2284,21 +2285,24 @@ static inline bool evaluate_value(QV8Engine *engine, const QV4::ValueRef o, QV4::ExecutionContext *ctx = QV8Engine::getV4(engine)->current; QV4::Scope scope(ctx); - try { - QV4::Scoped function(scope, program.run()); - if (!function) - return false; - - QV4::ScopedValue value(scope); - QV4::ScopedCallData d(scope, 1); - d->args[0] = o; - d->thisObject = engine->global(); - value = function->call(d); - return __qmljs_strict_equal(value, result); - } catch (...) { + QV4::Scoped function(scope, program.run()); + if (scope.engine->hasException) { ctx->catchException(); + return false; } - return false; + if (!function) + return false; + + QV4::ScopedValue value(scope); + QV4::ScopedCallData d(scope, 1); + d->args[0] = o; + d->thisObject = engine->global(); + value = function->call(d); + if (scope.engine->hasException) { + ctx->catchException(); + return false; + } + return __qmljs_strict_equal(value, result); } static inline QV4::ReturnedValue evaluate(QV8Engine *engine, const QV4::ValueRef o, @@ -2312,18 +2316,23 @@ static inline QV4::ReturnedValue evaluate(QV8Engine *engine, const QV4::ValueRef QV4::Script program(QV8Engine::getV4(engine)->rootContext, functionSource); program.inheritContext = true; - try { - QV4::Scoped function(scope, program.run()); - if (!function) - return QV4::Encode::undefined(); - QV4::ScopedCallData d(scope, 1); - d->args[0] = o; - d->thisObject = engine->global(); - return function->call(d); - } catch (...) { + + QV4::Scoped function(scope, program.run()); + if (scope.engine->hasException) { + ctx->catchException(); + return QV4::Encode::undefined(); + } + if (!function) + return QV4::Encode::undefined(); + QV4::ScopedCallData d(scope, 1); + d->args[0] = o; + d->thisObject = engine->global(); + QV4::ScopedValue result(scope, function->call(d)); + if (scope.engine->hasException) { ctx->catchException(); + return QV4::Encode::undefined(); } - return QV4::Encode::undefined(); + return result.asReturnedValue(); } #define EVALUATE_ERROR(source) evaluate_error(engine, object, source) diff --git a/tools/qmljs/main.cpp b/tools/qmljs/main.cpp index 2d31dbf..32a45d9 100644 --- a/tools/qmljs/main.cpp +++ b/tools/qmljs/main.cpp @@ -202,22 +202,22 @@ int main(int argc, char *argv[]) const QString code = QString::fromUtf8(file.readAll()); file.close(); - try { - QV4::Script script(ctx, code, fn); - script.parseAsBinding = runAsQml; - script.parse(); - QV4::ScopedValue result(scope, script.run()); - if (!result->isUndefined()) { - if (! qgetenv("SHOW_EXIT_VALUE").isEmpty()) - std::cout << "exit value: " << qPrintable(result->toString(ctx)->toQString()) << std::endl; - } - } catch (...) { + QV4::ScopedValue result(scope); + QV4::Script script(ctx, code, fn); + script.parseAsBinding = runAsQml; + script.parse(); + if (!scope.engine->hasException) + result = script.run(); + if (scope.engine->hasException) { QV4::StackTrace trace; QV4::ScopedValue ex(scope, ctx->catchException(&trace)); showException(ctx, ex, trace); return EXIT_FAILURE; } - + if (!result->isUndefined()) { + if (! qgetenv("SHOW_EXIT_VALUE").isEmpty()) + std::cout << "exit value: " << qPrintable(result->toString(ctx)->toQString()) << std::endl; + } } else { std::cerr << "Error: cannot open file " << fn.toUtf8().constData() << std::endl; return EXIT_FAILURE; -- 2.7.4