From: Simon Hausmann Date: Fri, 8 Feb 2013 12:33:50 +0000 (+0100) Subject: Fix scoping for catch blocks X-Git-Tag: upstream/5.2.1~669^2~659^2~272 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=8e55a1a9263c651d5db10184f7f44539832feb9b;p=platform%2Fupstream%2Fqtdeclarative.git Fix scoping for catch blocks Based on Lars' idea, done together with Erik. For proper scoping of the exception variable in catch blocks, it is necessary to define the new scope (ExecutionContext for us) at run-time, it cannot be determined at compile time. This patch implements the necessary logic to create the new execution context when entering a try block and re-uses the existing pop_scope infrastructure for destroying it again. Within the catch scope it is necessary to do all lookups by name, so the existing _function->insideWith variable was re-used and renamed to _function->insideWithOrCatch. Additionally the new context also stores the name and value of the separately scoped exception variable that shadows any existing equally named variables in outter scopes. CodeGen::unwindException also had a bug that it would generate the finally code with the wrong _function->insideWithOrCatch level, resulting in name lookups inside finally instead of local index lookups. Change-Id: I5616af38c3558e553e971a6a894ce5239ccb8422 Reviewed-by: Lars Knoll --- diff --git a/src/v4/moth/qv4instr_moth_p.h b/src/v4/moth/qv4instr_moth_p.h index 741ddba..d573713 100644 --- a/src/v4/moth/qv4instr_moth_p.h +++ b/src/v4/moth/qv4instr_moth_p.h @@ -25,6 +25,7 @@ F(CallBuiltinDeleteExceptionHandler, callBuiltinDeleteExceptionHandler) \ F(CallBuiltinGetException, callBuiltinGetException) \ F(CallBuiltinPushScope, callBuiltinPushScope) \ + F(CallBuiltinPushCatchScope, callBuiltinPushCatchScope) \ F(CallBuiltinPopScope, callBuiltinPopScope) \ F(CallBuiltinForeachIteratorObject, callBuiltinForeachIteratorObject) \ F(CallBuiltinForeachNextPropertyName, callBuiltinForeachNextPropertyName) \ @@ -242,6 +243,10 @@ union Instr MOTH_INSTR_HEADER Param arg; }; + struct instr_callBuiltinPushCatchScope { + MOTH_INSTR_HEADER + VM::String *varName; + }; struct instr_callBuiltinPopScope { MOTH_INSTR_HEADER }; @@ -452,6 +457,7 @@ union Instr instr_callBuiltinDeleteExceptionHandler callBuiltinDeleteExceptionHandler; instr_callBuiltinGetException callBuiltinGetException; instr_callBuiltinPushScope callBuiltinPushScope; + instr_callBuiltinPushCatchScope callBuiltinPushCatchScope; instr_callBuiltinPopScope callBuiltinPopScope; instr_callBuiltinForeachIteratorObject callBuiltinForeachIteratorObject; instr_callBuiltinForeachNextPropertyName callBuiltinForeachNextPropertyName; diff --git a/src/v4/moth/qv4isel_moth.cpp b/src/v4/moth/qv4isel_moth.cpp index 2f0a614..87b21aa 100644 --- a/src/v4/moth/qv4isel_moth.cpp +++ b/src/v4/moth/qv4isel_moth.cpp @@ -873,6 +873,13 @@ void InstructionSelection::callBuiltinPushWithScope(IR::Temp *arg) addInstruction(call); } +void InstructionSelection::callBuiltinPushCatchScope(const QString &exceptionVarName) +{ + Instruction::CallBuiltinPushCatchScope call; + call.varName = identifier(exceptionVarName); + addInstruction(call); +} + void InstructionSelection::callBuiltinPopScope() { Instruction::CallBuiltinPopScope call; diff --git a/src/v4/moth/qv4isel_moth_p.h b/src/v4/moth/qv4isel_moth_p.h index 7d57897..012a26b 100644 --- a/src/v4/moth/qv4isel_moth_p.h +++ b/src/v4/moth/qv4isel_moth_p.h @@ -49,6 +49,7 @@ protected: virtual void callBuiltinForeachIteratorObject(IR::Temp *arg, IR::Temp *result); virtual void callBuiltinForeachNextPropertyname(IR::Temp *arg, IR::Temp *result); virtual void callBuiltinPushWithScope(IR::Temp *arg); + virtual void callBuiltinPushCatchScope(const QString &exceptionVarName); virtual void callBuiltinPopScope(); virtual void callBuiltinDeclareVar(bool deletable, const QString &name); virtual void callBuiltinDefineGetterSetter(IR::Temp *object, const QString &name, IR::Temp *getter, IR::Temp *setter); diff --git a/src/v4/moth/qv4vme_moth.cpp b/src/v4/moth/qv4vme_moth.cpp index 8ed3fb1..fc06e90 100644 --- a/src/v4/moth/qv4vme_moth.cpp +++ b/src/v4/moth/qv4vme_moth.cpp @@ -283,6 +283,10 @@ VM::Value VME::operator()(QQmlJS::VM::ExecutionContext *context, const uchar *co context = __qmljs_builtin_push_with_scope(VALUE(instr.arg), context); MOTH_END_INSTR(CallBuiltinPushScope) + MOTH_BEGIN_INSTR(CallBuiltinPushCatchScope) + context = __qmljs_builtin_push_catch_scope(instr.varName, context); + MOTH_END_INSTR(CallBuiltinPushCatchScope) + MOTH_BEGIN_INSTR(CallBuiltinPopScope) context = __qmljs_builtin_pop_scope(context); MOTH_END_INSTR(CallBuiltinPopScope) diff --git a/src/v4/qmljs_environment.cpp b/src/v4/qmljs_environment.cpp index b1339b6..40b8938 100644 --- a/src/v4/qmljs_environment.cpp +++ b/src/v4/qmljs_environment.cpp @@ -178,10 +178,18 @@ ExecutionContext *ExecutionContext::createWithScope(Object *with) return withCtx; } +ExecutionContext *ExecutionContext::createCatchScope(String *exceptionVarName) +{ + ExecutionContext *catchCtx = engine->newContext(); + catchCtx->initForCatch(this, exceptionVarName); + engine->current = catchCtx; + return catchCtx; +} + ExecutionContext *ExecutionContext::popScope() { assert(engine->current == this); - assert(withObject != 0); + assert(withObject != 0 || exceptionVarName != 0); engine->current = parent; parent = 0; @@ -222,6 +230,8 @@ void ExecutionContext::init(ExecutionEngine *eng) arguments = 0; argumentCount = 0; locals = 0; + exceptionVarName = 0; + exceptionValue = Value::undefinedValue(); strictMode = false; activation = 0; withObject = 0; @@ -242,11 +252,31 @@ void ExecutionContext::init(ExecutionContext *p, Object *with) arguments = 0; argumentCount = 0; locals = 0; + exceptionVarName = 0; + exceptionValue = Value::undefinedValue(); strictMode = false; activation = 0; withObject = with; } +void ExecutionContext::initForCatch(ExecutionContext *p, String *exceptionVarName) +{ + engine = p->engine; + parent = p; + outer = p; + thisObject = p->thisObject; + + function = 0; + arguments = 0; + argumentCount = 0; + locals = 0; + this->exceptionVarName = exceptionVarName; + exceptionValue = engine->exception; + strictMode = p->strictMode; + activation = 0; + withObject = 0; +} + void ExecutionContext::destroy() { delete[] arguments; @@ -265,6 +295,8 @@ bool ExecutionContext::deleteProperty(String *name) if (ctx->activation && ctx->activation->__hasProperty__(this, name)) return ctx->activation->__delete__(this, name); } + if (ctx->exceptionVarName && ctx->exceptionVarName->isEqualTo(name)) + return false; if (FunctionObject *f = ctx->function) { if (f->needsActivation || hasWith) { for (unsigned int i = 0; i < f->varCount; ++i) @@ -294,6 +326,9 @@ void ExecutionContext::mark() activation->mark(); if (withObject) withObject->mark(); + if (exceptionVarName) + exceptionVarName->mark(); + exceptionValue.mark(); } void ExecutionContext::setProperty(String *name, Value value) @@ -307,6 +342,9 @@ void ExecutionContext::setProperty(String *name, Value value) w->__put__(ctx, name, value); return; } + } else if (ctx->exceptionVarName && ctx->exceptionVarName->isEqualTo(name)) { + ctx->exceptionValue = value; + return; } else { // qDebug() << ctx << "setting mutable binding"; if (ctx->setMutableBinding(this, name, value)) @@ -326,6 +364,7 @@ Value ExecutionContext::getProperty(String *name) return thisObject; bool hasWith = false; + bool hasCatchScope = false; // qDebug() << "=== getProperty" << name->toQString(); for (ExecutionContext *ctx = this; ctx; ctx = ctx->outer) { if (Object *w = ctx->withObject) { @@ -340,8 +379,14 @@ Value ExecutionContext::getProperty(String *name) continue; } + if (ctx->exceptionVarName) { + hasCatchScope = true; + if (ctx->exceptionVarName->isEqualTo(name)) + return ctx->exceptionValue; + } + if (FunctionObject *f = ctx->function) { - if (f->needsActivation || hasWith) { + if (f->needsActivation || hasWith || hasCatchScope) { for (unsigned int i = 0; i < f->varCount; ++i) if (f->varList[i]->isEqualTo(name)) return ctx->locals[i]; @@ -369,6 +414,7 @@ Value ExecutionContext::getPropertyNoThrow(String *name) return thisObject; bool hasWith = false; + bool hasCatchScope = false; for (ExecutionContext *ctx = this; ctx; ctx = ctx->outer) { if (Object *w = ctx->withObject) { hasWith = true; @@ -379,8 +425,14 @@ Value ExecutionContext::getPropertyNoThrow(String *name) continue; } + if (ctx->exceptionVarName) { + hasCatchScope = true; + if (ctx->exceptionVarName->isEqualTo(name)) + return ctx->exceptionValue; + } + if (FunctionObject *f = ctx->function) { - if (f->needsActivation || hasWith) { + if (f->needsActivation || hasWith || hasCatchScope) { for (unsigned int i = 0; i < f->varCount; ++i) if (f->varList[i]->isEqualTo(name)) return ctx->locals[i]; @@ -408,6 +460,7 @@ Value ExecutionContext::getPropertyAndBase(String *name, Object **base) return thisObject; bool hasWith = false; + bool hasCatchScope = false; for (ExecutionContext *ctx = this; ctx; ctx = ctx->outer) { if (Object *w = ctx->withObject) { hasWith = true; @@ -420,8 +473,14 @@ Value ExecutionContext::getPropertyAndBase(String *name, Object **base) continue; } + if (ctx->exceptionVarName) { + hasCatchScope = true; + if (ctx->exceptionVarName->isEqualTo(name)) + return ctx->exceptionValue; + } + if (FunctionObject *f = ctx->function) { - if (f->needsActivation || hasWith) { + if (f->needsActivation || hasWith || hasCatchScope) { for (unsigned int i = 0; i < f->varCount; ++i) if (f->varList[i]->isEqualTo(name)) return ctx->locals[i]; @@ -503,6 +562,9 @@ void ExecutionContext::initCallContext(ExecutionContext *parent) outer = function->scope; engine->current = this; + exceptionVarName = 0; + exceptionValue = Value::undefinedValue(); + activation = 0; withObject = 0; diff --git a/src/v4/qmljs_environment.h b/src/v4/qmljs_environment.h index 3c4e1fb..f118c63 100644 --- a/src/v4/qmljs_environment.h +++ b/src/v4/qmljs_environment.h @@ -88,6 +88,9 @@ struct ExecutionContext unsigned int argumentCount; Value *locals; + String *exceptionVarName; + Value exceptionValue; + String * const *formals() const; unsigned int formalCount() const; String * const *variables() const; @@ -100,6 +103,7 @@ struct ExecutionContext void init(ExecutionEngine *e); void init(ExecutionContext *p, Object *with); + void initForCatch(ExecutionContext *p, String *exceptionVarName); void destroy(); bool hasBinding(String *name) const; @@ -109,6 +113,7 @@ struct ExecutionContext bool deleteBinding(ExecutionContext *ctx, String *name); ExecutionContext *createWithScope(Object *with); + ExecutionContext *createCatchScope(String* exceptionVarName); ExecutionContext *popScope(); void initCallContext(ExecutionContext *parent); diff --git a/src/v4/qmljs_runtime.cpp b/src/v4/qmljs_runtime.cpp index 51f6b40..4c77715 100644 --- a/src/v4/qmljs_runtime.cpp +++ b/src/v4/qmljs_runtime.cpp @@ -1211,6 +1211,11 @@ ExecutionContext *__qmljs_builtin_push_with_scope(Value o, ExecutionContext *ctx return ctx->createWithScope(obj); } +ExecutionContext *__qmljs_builtin_push_catch_scope(String *exceptionVarName, ExecutionContext *ctx) +{ + return ctx->createCatchScope(exceptionVarName); +} + ExecutionContext *__qmljs_builtin_pop_scope(ExecutionContext *ctx) { return ctx->popScope(); diff --git a/src/v4/qmljs_runtime.h b/src/v4/qmljs_runtime.h index b2efc15..579e832 100644 --- a/src/v4/qmljs_runtime.h +++ b/src/v4/qmljs_runtime.h @@ -119,6 +119,7 @@ Value __qmljs_builtin_post_decrement_element(Value base, Value index, ExecutionC void __qmljs_builtin_throw(Value val, ExecutionContext *context); void __qmljs_builtin_rethrow(ExecutionContext *context); ExecutionContext *__qmljs_builtin_push_with_scope(Value o, ExecutionContext *ctx); +ExecutionContext *__qmljs_builtin_push_catch_scope(String *exceptionVarName, ExecutionContext *ctx); ExecutionContext *__qmljs_builtin_pop_scope(ExecutionContext *ctx); void __qmljs_builtin_declare_var(ExecutionContext *ctx, bool deletable, String *name); void __qmljs_builtin_define_property(Value object, String *name, Value val, ExecutionContext *ctx); diff --git a/src/v4/qv4codegen.cpp b/src/v4/qv4codegen.cpp index eda6b0c..17cf824 100644 --- a/src/v4/qv4codegen.cpp +++ b/src/v4/qv4codegen.cpp @@ -849,7 +849,7 @@ void Codegen::variableDeclaration(VariableDeclaration *ast) assert(expr.code); initializer = *expr; - if (! _env->parent || _function->insideWith) { + if (! _env->parent || _function->insideWithOrCatch) { // it's global code. move(_block->NAME(ast->name.toString(), ast->identifierToken.startLine, ast->identifierToken.startColumn), initializer); } else { @@ -1374,7 +1374,7 @@ IR::Expr *Codegen::identifier(const QString &name, int line, int col) { int index = _env->findMember(name); - if (! _function->hasDirectEval && !_function->insideWith && _env->parent) { + if (! _function->hasDirectEval && !_function->insideWithOrCatch && _env->parent) { if (index != -1) { return _block->TEMP(index); } @@ -2444,23 +2444,20 @@ bool Codegen::visit(TryStatement *ast) move(_block->TEMP(inCatch), _block->CONST(IR::BoolType, true)); move(_block->TEMP(hasException), _block->CONST(IR::BoolType, false)); - const int exception = _block->newTemp(); - move(_block->TEMP(exception), _block->CALL(_block->NAME(IR::Name::builtin_get_exception, 0, 0), 0)); + IR::ExprList *catchScopeArgs = _function->New(); + catchScopeArgs->init(_block->NAME(ast->catchExpression->name.toString(), ast->catchExpression->identifierToken.startLine, ast->catchExpression->identifierToken.startColumn)); + _block->EXP(_block->CALL(_block->NAME(IR::Name::builtin_push_catch_scope, 0, 0), catchScopeArgs)); - // the variable used in the catch statement is local and hides any global - // variable with the same name. - const Environment::Member undefinedMember = { Environment::UndefinedMember, -1 , 0 }; - const Environment::Member catchMember = { Environment::VariableDefinition, exception, 0 }; - Environment::Member m = _env->members.value(ast->catchExpression->name.toString(), undefinedMember); - _env->members.insert(ast->catchExpression->name.toString(), catchMember); - - statement(ast->catchExpression->statement); + ++_function->insideWithOrCatch; + { + ScopeAndFinally scope(_scopeAndFinally); + _scopeAndFinally = &scope; + statement(ast->catchExpression->statement); + _scopeAndFinally = scope.parent; + } + --_function->insideWithOrCatch; - // reset the variable name to the one from the outer scope - if (m.type == Environment::UndefinedMember) - _env->members.remove(ast->catchExpression->name.toString()); - else - _env->members.insert(ast->catchExpression->name.toString(), m); + _block->EXP(_block->CALL(_block->NAME(IR::Name::builtin_pop_scope, 0, 0))); _block->JUMP(finallyBody); } @@ -2489,12 +2486,14 @@ bool Codegen::visit(TryStatement *ast) void Codegen::unwindException(Codegen::ScopeAndFinally *outest) { + int savedDepthForWidthOrCatch = _function->insideWithOrCatch; ScopeAndFinally *scopeAndFinally = _scopeAndFinally; qSwap(_scopeAndFinally, scopeAndFinally); while (_scopeAndFinally != outest) { if (_scopeAndFinally->popScope) { _block->EXP(_block->CALL(_block->NAME(IR::Name::builtin_pop_scope, 0, 0))); _scopeAndFinally = _scopeAndFinally->parent; + --_function->insideWithOrCatch; } else { _block->EXP(_block->CALL(_block->NAME(IR::Name::builtin_delete_exception_handler, 0, 0), _scopeAndFinally->deleteExceptionArgs)); ScopeAndFinally *tc = _scopeAndFinally; @@ -2504,6 +2503,7 @@ void Codegen::unwindException(Codegen::ScopeAndFinally *outest) } } qSwap(_scopeAndFinally, scopeAndFinally); + _function->insideWithOrCatch = savedDepthForWidthOrCatch; } bool Codegen::visit(VariableStatement *ast) @@ -2546,14 +2546,14 @@ bool Codegen::visit(WithStatement *ast) args->init(_block->TEMP(withObject)); _block->EXP(_block->CALL(_block->NAME(IR::Name::builtin_push_with_scope, 0, 0), args)); - ++_function->insideWith; + ++_function->insideWithOrCatch; { ScopeAndFinally scope(_scopeAndFinally); _scopeAndFinally = &scope; statement(ast->statement); _scopeAndFinally = scope.parent; } - --_function->insideWith; + --_function->insideWithOrCatch; _block->EXP(_block->CALL(_block->NAME(IR::Name::builtin_pop_scope, 0, 0), 0)); IR::BasicBlock *next = _function->newBasicBlock(); diff --git a/src/v4/qv4ir.cpp b/src/v4/qv4ir.cpp index 1c2a9d9..fca14df 100644 --- a/src/v4/qv4ir.cpp +++ b/src/v4/qv4ir.cpp @@ -242,6 +242,8 @@ static const char *builtin_to_string(Name::Builtin b) return "builtin_foreach_next_property_name"; case IR::Name::builtin_push_with_scope: return "builtin_push_with_scope"; + case IR::Name::builtin_push_catch_scope: + return "builtin_push_catch_scope"; case IR::Name::builtin_pop_scope: return "builtin_pop_scope"; case IR::Name::builtin_declare_vars: diff --git a/src/v4/qv4ir_p.h b/src/v4/qv4ir_p.h index eefc039..e39ef94 100644 --- a/src/v4/qv4ir_p.h +++ b/src/v4/qv4ir_p.h @@ -284,6 +284,7 @@ struct Name: Expr { builtin_foreach_iterator_object, builtin_foreach_next_property_name, builtin_push_with_scope, + builtin_push_catch_scope, builtin_pop_scope, builtin_declare_vars, builtin_define_property, @@ -612,7 +613,7 @@ struct Function { QList locals; QVector nestedFunctions; - int insideWith; + int insideWithOrCatch; uint hasDirectEval: 1; uint usesArgumentsObject : 1; @@ -626,7 +627,7 @@ struct Function { , pool(&module->pool) , tempCount(0) , maxNumberOfArguments(0) - , insideWith(0) + , insideWithOrCatch(0) , hasDirectEval(false) , usesArgumentsObject(false) , isStrict(false) diff --git a/src/v4/qv4isel_masm.cpp b/src/v4/qv4isel_masm.cpp index 5098a44..355d8a5 100644 --- a/src/v4/qv4isel_masm.cpp +++ b/src/v4/qv4isel_masm.cpp @@ -600,6 +600,11 @@ void InstructionSelection::callBuiltinPushWithScope(IR::Temp *arg) generateFunctionCall(Assembler::ContextRegister, __qmljs_builtin_push_with_scope, arg, Assembler::ContextRegister); } +void InstructionSelection::callBuiltinPushCatchScope(const QString &exceptionVarName) +{ + generateFunctionCall(Assembler::ContextRegister, __qmljs_builtin_push_catch_scope, identifier(exceptionVarName), Assembler::ContextRegister); +} + void InstructionSelection::callBuiltinPopScope() { generateFunctionCall(Assembler::ContextRegister, __qmljs_builtin_pop_scope, Assembler::ContextRegister); diff --git a/src/v4/qv4isel_masm_p.h b/src/v4/qv4isel_masm_p.h index 3662268..9e2c4e6 100644 --- a/src/v4/qv4isel_masm_p.h +++ b/src/v4/qv4isel_masm_p.h @@ -711,6 +711,7 @@ protected: virtual void callBuiltinForeachIteratorObject(IR::Temp *arg, IR::Temp *result); virtual void callBuiltinForeachNextPropertyname(IR::Temp *arg, IR::Temp *result); virtual void callBuiltinPushWithScope(IR::Temp *arg); + virtual void callBuiltinPushCatchScope(const QString &exceptionVarName); virtual void callBuiltinPopScope(); virtual void callBuiltinDeclareVar(bool deletable, const QString &name); virtual void callBuiltinDefineGetterSetter(IR::Temp *object, const QString &name, IR::Temp *getter, IR::Temp *setter); diff --git a/src/v4/qv4isel_p.cpp b/src/v4/qv4isel_p.cpp index 6a050b3..e47f349 100644 --- a/src/v4/qv4isel_p.cpp +++ b/src/v4/qv4isel_p.cpp @@ -339,6 +339,12 @@ void InstructionSelection::callBuiltin(IR::Call *call, IR::Temp *result) callBuiltinPushWithScope(arg); } return; + case IR::Name::builtin_push_catch_scope: { + IR::Name *arg = call->args->expr->asName(); + assert(arg != 0); + callBuiltinPushCatchScope(*arg->id); + } return; + case IR::Name::builtin_pop_scope: callBuiltinPopScope(); return; diff --git a/src/v4/qv4isel_p.h b/src/v4/qv4isel_p.h index 5ec50b3..5a0e828 100644 --- a/src/v4/qv4isel_p.h +++ b/src/v4/qv4isel_p.h @@ -109,6 +109,7 @@ public: // to implement by subclasses: virtual void callBuiltinForeachIteratorObject(IR::Temp *arg, IR::Temp *result) = 0; virtual void callBuiltinForeachNextPropertyname(IR::Temp *arg, IR::Temp *result) = 0; virtual void callBuiltinPushWithScope(IR::Temp *arg) = 0; + virtual void callBuiltinPushCatchScope(const QString &exceptionVarName) = 0; virtual void callBuiltinPopScope() = 0; virtual void callBuiltinDeclareVar(bool deletable, const QString &name) = 0; virtual void callBuiltinDefineGetterSetter(IR::Temp *object, const QString &name, IR::Temp *getter, IR::Temp *setter) = 0; diff --git a/tests/TestExpectations b/tests/TestExpectations index 22cb397..214d5f4 100644 --- a/tests/TestExpectations +++ b/tests/TestExpectations @@ -42,9 +42,7 @@ Sbp_A4_T1 failing Sbp_A4_T2 failing S12.4_A1 failing S15.2.4.4_A14 failing -# Try/catch scoping issue -S12.14_A4 failing # Function declaration inside if / while Sbp_12.5_A9_T3 failing -Sbp_12.6.2_A13_T3 failing \ No newline at end of file +Sbp_12.6.2_A13_T3 failing