From 34bf0139c75de861c948391737af3c8c2a42703c Mon Sep 17 00:00:00 2001 From: Lars Knoll Date: Mon, 21 Oct 2013 17:07:45 +0200 Subject: Rework IR code generation for try/catch/finally Simplify the generated code. Add a special block to catch exceptions thrown inside a catch() statement. store the exception on the stack when entering finally and rethrow it at the end. This ensure correct behavior for break/continue/return statements inside finally. Don't check for exceptions after calling push_catch_scope and pop_scope in the JIT'ed code. This can lead to infinite loops when throwing inside an exception handler. Change-Id: I67e9325794e2fd25b0773b21e02fbaadb43faab0 Change-Id: Ic1ea9c0c43eec1d49177dc1ab4552a1da04e96fe Reviewed-by: Simon Hausmann --- src/qml/compiler/qv4codegen.cpp | 97 +++++++++++++++++++-------------------- src/qml/compiler/qv4codegen_p.h | 7 ++- src/qml/compiler/qv4isel_masm.cpp | 11 +++-- src/qml/compiler/qv4isel_masm_p.h | 11 ++++- src/qml/compiler/qv4isel_moth.cpp | 5 ++ src/qml/compiler/qv4isel_moth_p.h | 1 + src/qml/compiler/qv4isel_p.cpp | 4 ++ src/qml/compiler/qv4isel_p.h | 1 + src/qml/compiler/qv4jsir.cpp | 2 + src/qml/compiler/qv4jsir_p.h | 1 + src/qml/compiler/qv4regalloc.cpp | 1 + src/qml/jsruntime/qv4engine.cpp | 2 +- src/qml/jsruntime/qv4runtime.cpp | 12 ++++- src/qml/jsruntime/qv4runtime_p.h | 3 +- 14 files changed, 97 insertions(+), 61 deletions(-) (limited to 'src') diff --git a/src/qml/compiler/qv4codegen.cpp b/src/qml/compiler/qv4codegen.cpp index b7ea7491d6..b366582ade 100644 --- a/src/qml/compiler/qv4codegen.cpp +++ b/src/qml/compiler/qv4codegen.cpp @@ -2328,57 +2328,47 @@ bool Codegen::visit(TryStatement *ast) (ast->catchExpression->name == QLatin1String("eval") || ast->catchExpression->name == QLatin1String("arguments"))) throwSyntaxError(ast->catchExpression->identifierToken, QCoreApplication::translate("qv4codegen", "Catch variable name may not be eval or arguments in strict mode")); + V4IR::BasicBlock *surroundingExceptionHandler = exceptionHandler(); + // We always need a finally body to clean up the exception handler // exceptions thrown in finally get catched by the surrounding catch block - V4IR::BasicBlock *finallyBody = _function->newBasicBlock(groupStartBlock(), exceptionHandler(), V4IR::Function::DontInsertBlock); + V4IR::BasicBlock *finallyBody = 0; + V4IR::BasicBlock *catchBody = 0; + V4IR::BasicBlock *catchExceptionHandler = 0; + V4IR::BasicBlock *end = _function->newBasicBlock(groupStartBlock(), surroundingExceptionHandler, V4IR::Function::DontInsertBlock); - // the Catch block for catch is the block itself. We protect against recursion by checking the - // hasException boolean - // all basic blocks within try and catch get catched by catchBody - V4IR::BasicBlock *catchBody = _function->newBasicBlock(groupStartBlock(), 0, V4IR::Function::DontInsertBlock); - catchBody->catchBlock = catchBody; - pushExceptionHandler(catchBody); + if (ast->finallyExpression) + finallyBody = _function->newBasicBlock(groupStartBlock(), surroundingExceptionHandler, V4IR::Function::DontInsertBlock); - int hasException = _block->newTemp(); - move(_block->TEMP(hasException), _block->CONST(V4IR::BoolType, false)); + if (ast->catchExpression) { + // exception handler for the catch body + catchExceptionHandler = _function->newBasicBlock(groupStartBlock(), 0, V4IR::Function::DontInsertBlock); + pushExceptionHandler(catchExceptionHandler); + catchBody = _function->newBasicBlock(groupStartBlock(), catchExceptionHandler, V4IR::Function::DontInsertBlock); + popExceptionHandler(); + pushExceptionHandler(catchBody); + } else { + Q_ASSERT(finallyBody); + pushExceptionHandler(finallyBody); + } V4IR::BasicBlock *tryBody = _function->newBasicBlock(groupStartBlock(), exceptionHandler()); _block->JUMP(tryBody); - // ### Remove - // Pass the hidden "needRethrow" TEMP to the - // builtin_delete_exception_handler, in order to have those TEMPs alive for - // the duration of the exception handling block. - V4IR::ExprList *finishTryArgs = _function->New(); - finishTryArgs->init(_block->TEMP(hasException)); - - ScopeAndFinally tcf(_scopeAndFinally, ast->finallyExpression, finishTryArgs); + ScopeAndFinally tcf(_scopeAndFinally, ast->finallyExpression); _scopeAndFinally = &tcf; _block = tryBody; statement(ast->statement); - _block->JUMP(finallyBody); + _block->JUMP(finallyBody ? finallyBody : end); - _function->insertBasicBlock(catchBody); - _block = catchBody; + popExceptionHandler(); if (ast->catchExpression) { - // check if an exception got thrown within catch. Go to finally - // and then rethrow - V4IR::BasicBlock *cleanupCatchScope = _function->newBasicBlock(groupStartBlock(), exceptionHandler()); - V4IR::BasicBlock *b = _function->newBasicBlock(groupStartBlock(), exceptionHandler()); - _block->CJUMP(_block->TEMP(hasException), cleanupCatchScope, b); - - _block = cleanupCatchScope; - _block->EXP(_block->CALL(_block->NAME(V4IR::Name::builtin_pop_scope, 0, 0), 0)); - _block->JUMP(finallyBody); + pushExceptionHandler(catchExceptionHandler); + _function->insertBasicBlock(catchBody); + _block = catchBody; - _block = b; - } - - move(_block->TEMP(hasException), _block->CONST(V4IR::BoolType, true)); - - if (ast->catchExpression) { ++_function->insideWithOrCatch; V4IR::ExprList *catchArgs = _function->New(); catchArgs->init(_block->STRING(_function->newString(ast->catchExpression->name.toString()))); @@ -2391,28 +2381,37 @@ bool Codegen::visit(TryStatement *ast) } _block->EXP(_block->CALL(_block->NAME(V4IR::Name::builtin_pop_scope, 0, 0), 0)); --_function->insideWithOrCatch; - move(_block->TEMP(hasException), _block->CONST(V4IR::BoolType, false)); + _block->JUMP(finallyBody ? finallyBody : end); + popExceptionHandler(); + + _function->insertBasicBlock(catchExceptionHandler); + catchExceptionHandler->EXP(catchExceptionHandler->CALL(catchExceptionHandler->NAME(V4IR::Name::builtin_pop_scope, 0, 0), 0)); + if (finallyBody || surroundingExceptionHandler) + catchExceptionHandler->JUMP(finallyBody ? finallyBody : surroundingExceptionHandler); + else + catchExceptionHandler->EXP(catchExceptionHandler->CALL(catchExceptionHandler->NAME(V4IR::Name::builtin_rethrow, 0, 0), 0)); } - _block->JUMP(finallyBody); _scopeAndFinally = tcf.parent; - // exceptions thrown in finally get catched by the surrounding catch statement - popExceptionHandler(); + if (finallyBody) { + _function->insertBasicBlock(finallyBody); + _block = finallyBody; - _function->insertBasicBlock(finallyBody); + int hasException = _block->newTemp(); + move(_block->TEMP(hasException), _block->CALL(_block->NAME(V4IR::Name::builtin_unwind_exception, /*line*/0, /*column*/0), 0)); - _block = finallyBody; - if (ast->finallyExpression && ast->finallyExpression->statement) - statement(ast->finallyExpression->statement); + if (ast->finallyExpression && ast->finallyExpression->statement) + statement(ast->finallyExpression->statement); - V4IR::BasicBlock *rethrowBlock = _function->newBasicBlock(groupStartBlock(), exceptionHandler()); - V4IR::BasicBlock *after = _function->newBasicBlock(groupStartBlock(), exceptionHandler()); - _block->CJUMP(_block->TEMP(hasException), rethrowBlock, after); - _block = rethrowBlock; - _block->EXP(_block->CALL(_block->NAME(V4IR::Name::builtin_rethrow, /*line*/0, /*column*/0), 0)); + V4IR::ExprList *arg = _function->New(); + arg->expr = _block->TEMP(hasException); + _block->EXP(_block->CALL(_block->NAME(V4IR::Name::builtin_throw, /*line*/0, /*column*/0), arg)); + _block->JUMP(end); + } - _block = after; + _function->insertBasicBlock(end); + _block = end; return false; } diff --git a/src/qml/compiler/qv4codegen_p.h b/src/qml/compiler/qv4codegen_p.h index 652f395a88..b0d1962bca 100644 --- a/src/qml/compiler/qv4codegen_p.h +++ b/src/qml/compiler/qv4codegen_p.h @@ -238,12 +238,11 @@ protected: ScopeAndFinally *parent; AST::Finally *finally; - V4IR::ExprList *finishTryArgs; ScopeType type; - ScopeAndFinally(ScopeAndFinally *parent, ScopeType t = WithScope) : parent(parent), finally(0), finishTryArgs(0), type(t) {} - ScopeAndFinally(ScopeAndFinally *parent, AST::Finally *finally, V4IR::ExprList *finishTryArgs) - : parent(parent), finally(finally), finishTryArgs(finishTryArgs), type(TryScope) + ScopeAndFinally(ScopeAndFinally *parent, ScopeType t = WithScope) : parent(parent), finally(0), type(t) {} + ScopeAndFinally(ScopeAndFinally *parent, AST::Finally *finally) + : parent(parent), finally(finally), type(TryScope) {} }; diff --git a/src/qml/compiler/qv4isel_masm.cpp b/src/qml/compiler/qv4isel_masm.cpp index 6fe0f27ec3..2d13b4d8c8 100644 --- a/src/qml/compiler/qv4isel_masm.cpp +++ b/src/qml/compiler/qv4isel_masm.cpp @@ -818,9 +818,8 @@ void InstructionSelection::callBuiltinDeleteValue(V4IR::Temp *result) void InstructionSelection::callBuiltinThrow(V4IR::Expr *arg) { - generateFunctionCall(Assembler::Void, __qmljs_throw, Assembler::ContextRegister, + generateFunctionCall(Assembler::ReturnValueRegister, __qmljs_throw, Assembler::ContextRegister, Assembler::PointerToValue(arg)); - _as->jumpToExceptionHandler(); } void InstructionSelection::callBuiltinReThrow() @@ -828,10 +827,16 @@ void InstructionSelection::callBuiltinReThrow() _as->jumpToExceptionHandler(); } +void InstructionSelection::callBuiltinUnwindException(V4IR::Temp *result) +{ + generateFunctionCall(result, __qmljs_builtin_unwind_exception, Assembler::ContextRegister); + +} + void InstructionSelection::callBuiltinPushCatchScope(const QString &exceptionName) { Assembler::Pointer s = _as->loadStringAddress(Assembler::ScratchRegister, exceptionName); - generateFunctionCall(Assembler::ContextRegister, __qmljs_builtin_push_catch_scope, s, Assembler::ContextRegister); + generateFunctionCall(Assembler::ContextRegister, __qmljs_builtin_push_catch_scope, Assembler::ContextRegister, s); } void InstructionSelection::callBuiltinForeachIteratorObject(V4IR::Temp *arg, V4IR::Temp *result) diff --git a/src/qml/compiler/qv4isel_masm_p.h b/src/qml/compiler/qv4isel_masm_p.h index 3d1b271cdc..40d1aa5275 100644 --- a/src/qml/compiler/qv4isel_masm_p.h +++ b/src/qml/compiler/qv4isel_masm_p.h @@ -95,7 +95,15 @@ template struct ExceptionCheck { enum { NeedsCheck = 1 }; }; - +// push_catch and pop context methods shouldn't check for exceptions +template <> +struct ExceptionCheck { + enum { NeedsCheck = 0 }; +}; +template +struct ExceptionCheck { + enum { NeedsCheck = 0 }; +}; class Assembler : public JSC::MacroAssembler { @@ -1444,6 +1452,7 @@ protected: virtual void callBuiltinDeleteValue(V4IR::Temp *result); virtual void callBuiltinThrow(V4IR::Expr *arg); virtual void callBuiltinReThrow(); + virtual void callBuiltinUnwindException(V4IR::Temp *); virtual void callBuiltinPushCatchScope(const QString &exceptionName); virtual void callBuiltinForeachIteratorObject(V4IR::Temp *arg, V4IR::Temp *result); virtual void callBuiltinForeachNextPropertyname(V4IR::Temp *arg, V4IR::Temp *result); diff --git a/src/qml/compiler/qv4isel_moth.cpp b/src/qml/compiler/qv4isel_moth.cpp index 2e31aa10a0..06fb9caac8 100644 --- a/src/qml/compiler/qv4isel_moth.cpp +++ b/src/qml/compiler/qv4isel_moth.cpp @@ -777,6 +777,11 @@ void InstructionSelection::callBuiltinReThrow() // ### } +void InstructionSelection::callBuiltinUnwindException(V4IR::Temp *) +{ + // ### +} + void InstructionSelection::callBuiltinPushCatchScope(const QString &exceptionName) { diff --git a/src/qml/compiler/qv4isel_moth_p.h b/src/qml/compiler/qv4isel_moth_p.h index 8be28b5605..f5a14e15ae 100644 --- a/src/qml/compiler/qv4isel_moth_p.h +++ b/src/qml/compiler/qv4isel_moth_p.h @@ -94,6 +94,7 @@ protected: virtual void callBuiltinDeleteValue(V4IR::Temp *result); virtual void callBuiltinThrow(V4IR::Expr *arg); virtual void callBuiltinReThrow(); + virtual void callBuiltinUnwindException(V4IR::Temp *); virtual void callBuiltinPushCatchScope(const QString &exceptionName); virtual void callBuiltinForeachIteratorObject(V4IR::Temp *arg, V4IR::Temp *result); virtual void callBuiltinForeachNextPropertyname(V4IR::Temp *arg, V4IR::Temp *result); diff --git a/src/qml/compiler/qv4isel_p.cpp b/src/qml/compiler/qv4isel_p.cpp index 7d10230f43..851d5661ff 100644 --- a/src/qml/compiler/qv4isel_p.cpp +++ b/src/qml/compiler/qv4isel_p.cpp @@ -270,6 +270,10 @@ void IRDecoder::callBuiltin(V4IR::Call *call, V4IR::Temp *result) callBuiltinReThrow(); } return; + case V4IR::Name::builtin_unwind_exception: { + callBuiltinUnwindException(result); + } return; + case V4IR::Name::builtin_push_catch_scope: { V4IR::String *s = call->args->expr->asString(); Q_ASSERT(s); diff --git a/src/qml/compiler/qv4isel_p.h b/src/qml/compiler/qv4isel_p.h index 9895e2fd04..e3146add66 100644 --- a/src/qml/compiler/qv4isel_p.h +++ b/src/qml/compiler/qv4isel_p.h @@ -120,6 +120,7 @@ public: // to implement by subclasses: virtual void callBuiltinDeleteValue(V4IR::Temp *result) = 0; virtual void callBuiltinThrow(V4IR::Expr *arg) = 0; virtual void callBuiltinReThrow() = 0; + virtual void callBuiltinUnwindException(V4IR::Temp *) = 0; virtual void callBuiltinPushCatchScope(const QString &exceptionName) = 0; virtual void callBuiltinForeachIteratorObject(V4IR::Temp *arg, V4IR::Temp *result) = 0; virtual void callBuiltinForeachNextPropertyname(V4IR::Temp *arg, V4IR::Temp *result) = 0; diff --git a/src/qml/compiler/qv4jsir.cpp b/src/qml/compiler/qv4jsir.cpp index f0a37895b5..1c9620e1f5 100644 --- a/src/qml/compiler/qv4jsir.cpp +++ b/src/qml/compiler/qv4jsir.cpp @@ -397,6 +397,8 @@ static const char *builtin_to_string(Name::Builtin b) return "builtin_throw"; case Name::builtin_rethrow: return "builtin_rethrow"; + case Name::builtin_unwind_exception: + return "builtin_unwind_exception"; case Name::builtin_push_catch_scope: return "builtin_push_catch_scope"; case V4IR::Name::builtin_foreach_iterator_object: diff --git a/src/qml/compiler/qv4jsir_p.h b/src/qml/compiler/qv4jsir_p.h index ee8e0c0636..fe8425ab71 100644 --- a/src/qml/compiler/qv4jsir_p.h +++ b/src/qml/compiler/qv4jsir_p.h @@ -311,6 +311,7 @@ struct Name: Expr { builtin_delete, builtin_throw, builtin_rethrow, + builtin_unwind_exception, builtin_push_catch_scope, builtin_foreach_iterator_object, builtin_foreach_next_property_name, diff --git a/src/qml/compiler/qv4regalloc.cpp b/src/qml/compiler/qv4regalloc.cpp index 5d1265b4b6..9c90388c9e 100644 --- a/src/qml/compiler/qv4regalloc.cpp +++ b/src/qml/compiler/qv4regalloc.cpp @@ -194,6 +194,7 @@ protected: // IRDecoder virtual void callBuiltinDeleteValue(V4IR::Temp *) {} virtual void callBuiltinThrow(V4IR::Expr *) {} virtual void callBuiltinReThrow() {} + virtual void callBuiltinUnwindException(V4IR::Temp *) {} virtual void callBuiltinPushCatchScope(const QString &) {}; virtual void callBuiltinForeachIteratorObject(V4IR::Temp *, V4IR::Temp *) {} virtual void callBuiltinForeachNextProperty(V4IR::Temp *, V4IR::Temp *) {} diff --git a/src/qml/jsruntime/qv4engine.cpp b/src/qml/jsruntime/qv4engine.cpp index 3449c1aa3c..73db54174c 100644 --- a/src/qml/jsruntime/qv4engine.cpp +++ b/src/qml/jsruntime/qv4engine.cpp @@ -834,7 +834,7 @@ ReturnedValue ExecutionEngine::catchException(ExecutionContext *catchingContext, exceptionStackTrace.clear(); hasException = false; ReturnedValue res = exceptionValue.asReturnedValue(); - exceptionValue = Encode::undefined(); + exceptionValue = Primitive::emptyValue(); return res; } diff --git a/src/qml/jsruntime/qv4runtime.cpp b/src/qml/jsruntime/qv4runtime.cpp index 9b16f4e45a..1eed693c3d 100644 --- a/src/qml/jsruntime/qv4runtime.cpp +++ b/src/qml/jsruntime/qv4runtime.cpp @@ -902,7 +902,8 @@ ReturnedValue __qmljs_construct_property(ExecutionContext *context, const ValueR void __qmljs_throw(ExecutionContext *context, const ValueRef value) { - context->throwError(value); + if (!value->isEmpty()) + context->throwError(value); } ReturnedValue __qmljs_builtin_typeof(ExecutionContext *ctx, const ValueRef value) @@ -967,7 +968,14 @@ ExecutionContext *__qmljs_builtin_push_with_scope(const ValueRef o, ExecutionCon return ctx->newWithContext(obj); } -ExecutionContext *__qmljs_builtin_push_catch_scope(const StringRef exceptionVarName, ExecutionContext *ctx) +ReturnedValue __qmljs_builtin_unwind_exception(ExecutionContext *ctx) +{ + if (!ctx->engine->hasException) + return Primitive::emptyValue().asReturnedValue(); + return ctx->engine->catchException(ctx, 0); +} + +ExecutionContext *__qmljs_builtin_push_catch_scope(ExecutionContext *ctx, const StringRef exceptionVarName) { Scope scope(ctx); ScopedValue v(scope, ctx->engine->catchException(ctx, 0)); diff --git a/src/qml/jsruntime/qv4runtime_p.h b/src/qml/jsruntime/qv4runtime_p.h index 50a1ce17d1..f61a9805ac 100644 --- a/src/qml/jsruntime/qv4runtime_p.h +++ b/src/qml/jsruntime/qv4runtime_p.h @@ -129,8 +129,9 @@ QV4::ReturnedValue __qmljs_builtin_typeof_element(QV4::ExecutionContext* context void __qmljs_builtin_rethrow(QV4::ExecutionContext *context); QV4::ExecutionContext *__qmljs_builtin_push_with_scope(const QV4::ValueRef o, QV4::ExecutionContext *ctx); -QV4::ExecutionContext *__qmljs_builtin_push_catch_scope(const QV4::StringRef exceptionVarName, QV4::ExecutionContext *ctx); +QV4::ExecutionContext *__qmljs_builtin_push_catch_scope(QV4::ExecutionContext *ctx, const QV4::StringRef exceptionVarName); QV4::ExecutionContext *__qmljs_builtin_pop_scope(QV4::ExecutionContext *ctx); +ReturnedValue __qmljs_builtin_unwind_exception(ExecutionContext *ctx); void __qmljs_builtin_declare_var(QV4::ExecutionContext *ctx, bool deletable, const QV4::StringRef name); void __qmljs_builtin_define_property(QV4::ExecutionContext *ctx, const QV4::ValueRef object, const QV4::StringRef name, QV4::ValueRef val); QV4::ReturnedValue __qmljs_builtin_define_array(QV4::ExecutionContext *ctx, QV4::Value *values, uint length); -- cgit v1.2.3