[PATCH v6 0/9] MR2942: jscript: Misc fixes and improvements.
Some of these fixes are subtle (like the first patch) and very annoying to debug. Although the first patch looks like a hack, surprisingly, it's how the spec itself says it is! It's not even an IE quirk, but a special case in the spec. For example, the variable name (which holds the builtin eval func) **does** matter: if it's called something other than 'eval', it gets treated differently (as if indirect), and this is verified by the tests + the spec's wording (so Microsoft's implementation follows it). Most of the patches other than the first 2 are pretty small so they're in same MR. -- v6: jscript: Store ref to the function code instead of the function instance jscript: Start from the last argument when adding them to named locals. jscript: Store detached locals into a scope's specialized buffer. jscript: Detach the frame's scope before inserting eval() variables into it. jscript: Don't use iface_to_jsdisp where it's not necessary to grab it. jscript: Get rid of jsobj in scope_chain_t. jscript: Fix function leak in scope_init_locals. jscript: Fix addressing invalid memory if ref is an argument. jscript: Correctly implement context for indirect eval calls in ES5+ modes. https://gitlab.winehq.org/wine/wine/-/merge_requests/2942
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/jscript/compile.c | 14 ++++++--- dlls/jscript/engine.c | 34 +++++++++++++++++++++ dlls/jscript/engine.h | 1 + dlls/jscript/function.c | 6 ++++ dlls/jscript/global.c | 9 ++++-- dlls/jscript/jscript.h | 2 ++ dlls/jscript/tests/api.js | 43 ++++++++++++++++++++++++++ dlls/mshtml/tests/documentmode.js | 51 +++++++++++++++++++++++++++++++ 8 files changed, 153 insertions(+), 7 deletions(-) diff --git a/dlls/jscript/compile.c b/dlls/jscript/compile.c index 9d7944fcf72..e2690935212 100644 --- a/dlls/jscript/compile.c +++ b/dlls/jscript/compile.c @@ -720,16 +720,20 @@ static HRESULT compile_new_expression(compiler_ctx_t *ctx, call_expression_t *ex static HRESULT compile_call_expression(compiler_ctx_t *ctx, call_expression_t *expr, BOOL emit_ret) { - unsigned arg_cnt = 0, extra_args; + unsigned arg_cnt = 0, extra_args = 0; + HRESULT hres = S_OK; argument_t *arg; unsigned instr; jsop_t op; - HRESULT hres; if(is_memberid_expr(expr->expression->type)) { - op = OP_call_member; - extra_args = 2; - hres = compile_memberid_expression(ctx, expr->expression, 0); + if(expr->expression->type == EXPR_IDENT && !wcscmp(((identifier_expression_t*)expr->expression)->identifier, L"eval")) + op = OP_call_eval; + else { + op = OP_call_member; + extra_args = 2; + hres = compile_memberid_expression(ctx, expr->expression, 0); + } }else { op = OP_call; extra_args = 1; diff --git a/dlls/jscript/engine.c b/dlls/jscript/engine.c index b0557065e69..9e375294cb5 100644 --- a/dlls/jscript/engine.c +++ b/dlls/jscript/engine.c @@ -1459,6 +1459,40 @@ static HRESULT interp_call_member(script_ctx_t *ctx) argn, stack_args(ctx, argn), do_ret ? &ctx->acc : NULL); } +/* ECMA-262 5th Edition 15.1.2.1.1 */ +static HRESULT interp_call_eval(script_ctx_t *ctx) +{ + const unsigned argn = get_op_uint(ctx, 0); + const int do_ret = get_op_int(ctx, 1); + HRESULT hres = S_OK; + exprval_t exprval; + jsdisp_t *jsdisp; + BSTR identifier; + jsval_t v; + + TRACE("%d %d\n", argn, do_ret); + + identifier = SysAllocString(L"eval"); + hres = identifier_eval(ctx, identifier, &exprval); + SysFreeString(identifier); + if(FAILED(hres)) + return hres; + + clear_acc(ctx); + hres = exprval_propget(ctx, &exprval, &v); + if(SUCCEEDED(hres)) { + if(is_object_instance(v) && (jsdisp = to_jsdisp(get_object(v))) && jsdisp->ctx == ctx && is_builtin_eval_func(jsdisp)) + hres = builtin_eval(ctx, ctx->call_ctx, DISPATCH_METHOD | DISPATCH_JSCRIPT_CALLEREXECSSOURCE, + argn, stack_args(ctx, argn), do_ret ? &ctx->acc : NULL); + else + hres = exprval_call(ctx, &exprval, DISPATCH_METHOD | DISPATCH_JSCRIPT_CALLEREXECSSOURCE, + argn, stack_args(ctx, argn), do_ret ? &ctx->acc : NULL); + jsval_release(v); + } + exprval_release(&exprval); + return hres; +} + /* ECMA-262 3rd Edition 11.1.1 */ static HRESULT interp_this(script_ctx_t *ctx) { diff --git a/dlls/jscript/engine.h b/dlls/jscript/engine.h index f8046cbaafb..987e1cfa52d 100644 --- a/dlls/jscript/engine.h +++ b/dlls/jscript/engine.h @@ -25,6 +25,7 @@ X(bool, 1, ARG_INT, 0) \ X(bneg, 1, 0,0) \ X(call, 1, ARG_UINT, ARG_UINT) \ + X(call_eval, 1, ARG_UINT, ARG_UINT) \ X(call_member,1, ARG_UINT, ARG_UINT) \ X(carray, 1, ARG_UINT, 0) \ X(carray_set, 1, ARG_UINT, 0) \ diff --git a/dlls/jscript/function.c b/dlls/jscript/function.c index 5f210a04050..018fd3955db 100644 --- a/dlls/jscript/function.c +++ b/dlls/jscript/function.c @@ -1140,6 +1140,12 @@ static HRESULT FunctionProt_value(script_ctx_t *ctx, jsval_t vthis, WORD flags, return E_NOTIMPL; } +BOOL is_builtin_eval_func(jsdisp_t *jsdisp) +{ + return is_class(jsdisp, JSCLASS_FUNCTION) && function_from_jsdisp(jsdisp)->vtbl == &NativeFunctionVtbl && + ((NativeFunction*)function_from_jsdisp(jsdisp))->proc == JSGlobal_eval; +} + HRESULT init_function_constr(script_ctx_t *ctx, jsdisp_t *object_prototype) { NativeFunction *prot, *constr; diff --git a/dlls/jscript/global.c b/dlls/jscript/global.c index 9dd969aa334..997b2542a9e 100644 --- a/dlls/jscript/global.c +++ b/dlls/jscript/global.c @@ -130,10 +130,9 @@ static HRESULT JSGlobal_escape(script_ctx_t *ctx, jsval_t vthis, WORD flags, uns } /* ECMA-262 3rd Edition 15.1.2.1 */ -HRESULT JSGlobal_eval(script_ctx_t *ctx, jsval_t vthis, WORD flags, unsigned argc, jsval_t *argv, +HRESULT builtin_eval(script_ctx_t *ctx, call_frame_t *frame, WORD flags, unsigned argc, jsval_t *argv, jsval_t *r) { - call_frame_t *frame = ctx->call_ctx; DWORD exec_flags = EXEC_EVAL; bytecode_t *code; const WCHAR *src; @@ -174,6 +173,12 @@ HRESULT JSGlobal_eval(script_ctx_t *ctx, jsval_t vthis, WORD flags, unsigned arg return hres; } +HRESULT JSGlobal_eval(script_ctx_t *ctx, jsval_t vthis, WORD flags, unsigned argc, jsval_t *argv, + jsval_t *r) +{ + return builtin_eval(ctx, ctx->version < SCRIPTLANGUAGEVERSION_ES5 ? ctx->call_ctx : NULL, flags, argc, argv, r); +} + static HRESULT JSGlobal_isNaN(script_ctx_t *ctx, jsval_t vthis, WORD flags, unsigned argc, jsval_t *argv, jsval_t *r) { diff --git a/dlls/jscript/jscript.h b/dlls/jscript/jscript.h index 06c446b0c76..e91aa6dfabd 100644 --- a/dlls/jscript/jscript.h +++ b/dlls/jscript/jscript.h @@ -464,6 +464,8 @@ BOOL bool_obj_value(jsdisp_t*) DECLSPEC_HIDDEN; unsigned array_get_length(jsdisp_t*) DECLSPEC_HIDDEN; HRESULT localize_number(script_ctx_t*,DOUBLE,BOOL,jsstr_t**) DECLSPEC_HIDDEN; +BOOL is_builtin_eval_func(jsdisp_t*) DECLSPEC_HIDDEN; +HRESULT builtin_eval(script_ctx_t*,struct _call_frame_t*,WORD,unsigned,jsval_t*,jsval_t*) DECLSPEC_HIDDEN; HRESULT JSGlobal_eval(script_ctx_t*,jsval_t,WORD,unsigned,jsval_t*,jsval_t*) DECLSPEC_HIDDEN; HRESULT Object_get_proto_(script_ctx_t*,jsval_t,WORD,unsigned,jsval_t*,jsval_t*) DECLSPEC_HIDDEN; HRESULT Object_set_proto_(script_ctx_t*,jsval_t,WORD,unsigned,jsval_t*,jsval_t*) DECLSPEC_HIDDEN; diff --git a/dlls/jscript/tests/api.js b/dlls/jscript/tests/api.js index a3b2bbcbba5..481e0612be5 100644 --- a/dlls/jscript/tests/api.js +++ b/dlls/jscript/tests/api.js @@ -2446,6 +2446,49 @@ ok(bool.toLocaleString() === bool.toString(), "bool.toLocaleString() = " + bool. tmp = Object.prototype.valueOf.call(nullDisp); ok(tmp === nullDisp, "nullDisp.valueOf != nullDisp"); +(function(global) { + var i, context, code = "this.foobar = 1234"; + + var direct = [ + function() { eval(code); }, + function() { (eval)(code); }, + function() { (function(eval) { eval(code); }).call(this, eval); }, + function() { eval("eval(" + code + ")"); } + ]; + + for(i = 0; i < direct.length; i++) { + context = {}; + direct[i].call(context); + ok(context.foobar === 1234, "direct[" + i + "] context foobar = " + context.foobar); + } + + var indirect = [ + function() { (true, eval)(code); }, + function() { (eval, eval)(code); }, + function() { (true ? eval : false)(code); }, + function() { [eval][0](code); }, + function() { eval.call(this, code); }, + function() { var f; (f = eval)(code); }, + function() { var f = eval; f(code); }, + function() { (function(f) { f(code); }).call(this, eval); }, + function() { (function(f) { return f; }).call(this, eval)(code); }, + function() { (function() { arguments[0](code) }).call(this, eval); }, + function() { eval("eval")(code); } + ]; + + for(i = 0; i < indirect.length; i++) { + context = {}; + ok(!("foobar" in global), "indirect[" + i + "] has global foobar before call"); + indirect[i].call(context); + ok(context.foobar === 1234, "indirect[" + i + "] context foobar = " + context.foobar); + ok(!("foobar" in global), "indirect[" + i + "] has global foobar"); + } + + context = {}; + (function(eval) { eval(code); })(function() { context.barfoo = 4321; }); + ok(context.barfoo === 4321, "context.barfoo = " + context.barfoo); +})(this); + ok(ActiveXObject instanceof Function, "ActiveXObject is not instance of Function"); ok(ActiveXObject.prototype instanceof Object, "ActiveXObject.prototype is not instance of Object"); diff --git a/dlls/mshtml/tests/documentmode.js b/dlls/mshtml/tests/documentmode.js index 1a9f36927d0..9007d451bdb 100644 --- a/dlls/mshtml/tests/documentmode.js +++ b/dlls/mshtml/tests/documentmode.js @@ -756,6 +756,57 @@ sync_test("JS objs", function() { test_parses("if(false) { o.if; }", v >= 9); }); +sync_test("eval", function() { + var i, context, code = "this.foobar = 1234", v = document.documentMode; + + var direct = [ + function() { eval(code); }, + function() { (eval)(code); }, + function() { (function(eval) { eval(code); }).call(this, eval); }, + function() { eval("eval(" + code + ")"); } + ]; + + for(i = 0; i < direct.length; i++) { + context = {}; + direct[i].call(context); + ok(context.foobar === 1234, "direct[" + i + "] context foobar = " + context.foobar); + } + + var indirect = [ + function() { (true, eval)(code); }, + function() { (eval, eval)(code); }, + function() { (true ? eval : false)(code); }, + function() { [eval][0](code); }, + function() { eval.call(this, code); }, + function() { var f; (f = eval)(code); }, + function() { var f = eval; f(code); }, + function() { (function(f) { f(code); }).call(this, eval); }, + function() { (function(f) { return f; }).call(this, eval)(code); }, + function() { (function() { arguments[0](code) }).call(this, eval); }, + function() { window.eval(code); }, + function() { window["eval"](code); }, + function() { eval("eval")(code); } + ]; + + for(i = 0; i < indirect.length; i++) { + context = {}; + ok(!("foobar" in window), "indirect[" + i + "] has global foobar before call"); + indirect[i].call(context); + if(v < 9) { + ok(context.foobar === 1234, "indirect[" + i + "] context foobar = " + context.foobar); + ok(!("foobar" in window), "indirect[" + i + "] has global foobar"); + }else { + ok(!("foobar" in context), "indirect[" + i + "] has foobar"); + ok(window.foobar === 1234, "indirect[" + i + "] global foobar = " + context.foobar); + delete window.foobar; + } + } + + context = {}; + (function(eval) { eval(code); })(function() { context.barfoo = 4321; }); + ok(context.barfoo === 4321, "context.barfoo = " + context.barfoo); +}); + sync_test("for..in", function() { var v = document.documentMode, found = 0, r; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2942
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> `ref` can be negative in case it refers to an argument. Even though scope != frame->base_scope would rule this out (because only base scopes have args), it was checked *after* the memory access, which would read out of bounds memory first. This didn't appear as an issue in practice since it's using the heap pool, so there's probably valid memory before it, but it's still wrong. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/jscript/engine.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/jscript/engine.c b/dlls/jscript/engine.c index 9e375294cb5..a4b416ba8ed 100644 --- a/dlls/jscript/engine.c +++ b/dlls/jscript/engine.c @@ -657,7 +657,7 @@ static HRESULT detach_scope(script_ctx_t *ctx, call_frame_t *frame, scope_chain_ if (FAILED(hres = jsdisp_propput_name(scope->jsobj, name, ctx->stack[local_off(frame, ref)]))) return hres; - if (frame->function->variables[ref].func_id != -1 && scope != frame->base_scope + if (scope != frame->base_scope && frame->function->variables[ref].func_id != -1 && FAILED(hres = jsdisp_propput_name(frame->variable_obj, name, ctx->stack[local_off(frame, ref)]))) return hres; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2942
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/jscript/engine.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/dlls/jscript/engine.c b/dlls/jscript/engine.c index a4b416ba8ed..b48202ed16f 100644 --- a/dlls/jscript/engine.c +++ b/dlls/jscript/engine.c @@ -978,7 +978,10 @@ static HRESULT scope_init_locals(script_ctx_t *ctx) return hres; val = jsval_obj(func_obj); if (detached_vars && FAILED(hres = jsdisp_propput_name(frame->variable_obj, name, jsval_obj(func_obj)))) + { + jsdisp_release(func_obj); return hres; + } } else { @@ -987,7 +990,9 @@ static HRESULT scope_init_locals(script_ctx_t *ctx) if (detached_vars) { - if (FAILED(hres = jsdisp_propput_name(scope->jsobj, name, val))) + hres = jsdisp_propput_name(scope->jsobj, name, val); + jsval_release(val); + if (FAILED(hres)) return hres; } else -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2942
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> It was confusing and aliased to obj when it was a jsdisp (and shared ref), but we can obtain that already with helpers as needed (as_jsdisp and to_jsdisp), no reason to keep it so confusing and a separate field. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/jscript/engine.c | 57 +++++++++++++++++++++-------------------- dlls/jscript/engine.h | 1 - dlls/jscript/function.c | 11 ++++---- 3 files changed, 35 insertions(+), 34 deletions(-) diff --git a/dlls/jscript/engine.c b/dlls/jscript/engine.c index b48202ed16f..d5075e13500 100644 --- a/dlls/jscript/engine.c +++ b/dlls/jscript/engine.c @@ -207,6 +207,7 @@ static BOOL stack_topn_exprval(script_ctx_t *ctx, unsigned n, exprval_t *r) unsigned off = get_number(v); if(!frame->base_scope->frame && off >= frame->arguments_off) { + jsdisp_t *jsobj; DISPID id; BSTR name; HRESULT hres = E_FAIL; @@ -227,7 +228,7 @@ static BOOL stack_topn_exprval(script_ctx_t *ctx, unsigned n, exprval_t *r) while (1) { - if (scope->jsobj && SUCCEEDED(hres = jsdisp_get_id(scope->jsobj, name, 0, &id))) + if ((jsobj = to_jsdisp(scope->obj)) && SUCCEEDED(hres = jsdisp_get_id(jsobj, name, 0, &id))) break; if (scope == frame->base_scope) { @@ -238,10 +239,10 @@ static BOOL stack_topn_exprval(script_ctx_t *ctx, unsigned n, exprval_t *r) scope = scope->next; } - *stack_top_ref(ctx, n+1) = jsval_obj(jsdisp_addref(scope->jsobj)); + *stack_top_ref(ctx, n+1) = jsval_obj(jsdisp_addref(jsobj)); *stack_top_ref(ctx, n) = jsval_number(id); r->type = EXPRVAL_IDREF; - r->u.idref.disp = scope->obj; + r->u.idref.disp = to_disp(jsobj); r->u.idref.id = id; return TRUE; } @@ -437,6 +438,7 @@ static void scope_destructor(jsdisp_t *dispex) static HRESULT scope_gc_traverse(struct gc_ctx *gc_ctx, enum gc_traverse_op op, jsdisp_t *dispex) { scope_chain_t *scope = CONTAINING_RECORD(dispex, scope_chain_t, dispex); + jsdisp_t *jsobj; HRESULT hres; if(scope->next) { @@ -454,7 +456,7 @@ static HRESULT scope_gc_traverse(struct gc_ctx *gc_ctx, enum gc_traverse_op op, return S_OK; } - return scope->jsobj ? gc_process_linked_obj(gc_ctx, op, dispex, scope->jsobj, (void**)&scope->obj) : S_OK; + return scope->obj && (jsobj = to_jsdisp(scope->obj)) ? gc_process_linked_obj(gc_ctx, op, dispex, jsobj, (void**)&scope->obj) : S_OK; } static const builtin_info_t scope_info = { @@ -470,7 +472,7 @@ static const builtin_info_t scope_info = { scope_gc_traverse }; -static HRESULT scope_push(script_ctx_t *ctx, scope_chain_t *scope, jsdisp_t *jsobj, IDispatch *obj, scope_chain_t **ret) +static HRESULT scope_push(script_ctx_t *ctx, scope_chain_t *scope, IDispatch *obj, scope_chain_t **ret) { scope_chain_t *new_scope; HRESULT hres; @@ -487,7 +489,6 @@ static HRESULT scope_push(script_ctx_t *ctx, scope_chain_t *scope, jsdisp_t *jso if (obj) IDispatch_AddRef(obj); - new_scope->jsobj = jsobj; new_scope->obj = obj; new_scope->frame = NULL; new_scope->next = scope ? scope_addref(scope) : NULL; @@ -628,6 +629,7 @@ static HRESULT detach_scope(script_ctx_t *ctx, call_frame_t *frame, scope_chain_ { function_code_t *func = frame->function; unsigned int i, index; + jsdisp_t *jsobj; HRESULT hres; if (!scope->frame) @@ -636,18 +638,18 @@ static HRESULT detach_scope(script_ctx_t *ctx, call_frame_t *frame, scope_chain_ assert(scope->frame == frame); scope->frame = NULL; - if (!scope->jsobj) + if (!scope->obj) { - assert(!scope->obj); - - if (FAILED(hres = create_object(ctx, NULL, &scope->jsobj))) + if (FAILED(hres = create_object(ctx, NULL, &jsobj))) return hres; - scope->obj = to_disp(scope->jsobj); + scope->obj = to_disp(jsobj); } + else + jsobj = as_jsdisp(scope->obj); if (scope == frame->base_scope && func->name && func->local_ref == INVALID_LOCAL_REF && ctx->version >= SCRIPTLANGUAGEVERSION_ES5) - jsdisp_propput_name(scope->jsobj, func->name, jsval_obj(jsdisp_addref(frame->function_instance))); + jsdisp_propput_name(jsobj, func->name, jsval_obj(jsdisp_addref(frame->function_instance))); index = scope->scope_index; for(i = 0; i < frame->function->local_scopes[index].locals_cnt; i++) @@ -655,7 +657,7 @@ static HRESULT detach_scope(script_ctx_t *ctx, call_frame_t *frame, scope_chain_ WCHAR *name = frame->function->local_scopes[index].locals[i].name; int ref = frame->function->local_scopes[index].locals[i].ref; - if (FAILED(hres = jsdisp_propput_name(scope->jsobj, name, ctx->stack[local_off(frame, ref)]))) + if (FAILED(hres = jsdisp_propput_name(jsobj, name, ctx->stack[local_off(frame, ref)]))) return hres; if (scope != frame->base_scope && frame->function->variables[ref].func_id != -1 && FAILED(hres = jsdisp_propput_name(frame->variable_obj, name, ctx->stack[local_off(frame, ref)]))) @@ -687,7 +689,7 @@ static HRESULT detach_variable_object(script_ctx_t *ctx, call_frame_t *frame, BO TRACE("detaching %p\n", frame); assert(frame == frame->base_scope->frame); - assert(frame->variable_obj == frame->base_scope->jsobj); + assert(to_disp(frame->variable_obj) == frame->base_scope->obj); if(!from_release && !frame->arguments_obj) { hres = setup_arguments_object(ctx, frame); @@ -784,13 +786,10 @@ static HRESULT identifier_eval(script_ctx_t *ctx, BSTR identifier, exprval_t *re } } - if (!scope->jsobj && !scope->obj) + if (!scope->obj) continue; - if(scope->jsobj) - hres = jsdisp_get_id(scope->jsobj, identifier, fdexNameImplicit, &id); - else - hres = disp_get_id(ctx, scope->obj, identifier, identifier, fdexNameImplicit, &id); + hres = disp_get_id(ctx, scope->obj, identifier, identifier, fdexNameImplicit, &id); if(SUCCEEDED(hres)) { exprval_set_disp_ref(ret, scope->obj, id); return S_OK; @@ -943,6 +942,7 @@ static HRESULT scope_init_locals(script_ctx_t *ctx) unsigned int i, off, index; scope_chain_t *scope; BOOL detached_vars; + jsdisp_t *jsobj; HRESULT hres; scope = frame->scope; @@ -954,13 +954,14 @@ static HRESULT scope_init_locals(script_ctx_t *ctx) assert(frame->base_scope->frame == frame); frame->scope->frame = ctx->call_ctx; } - else if (!scope->jsobj) + else if (!scope->obj) { - assert(!scope->obj); - if (FAILED(hres = create_object(ctx, NULL, &scope->jsobj))) + if (FAILED(hres = create_object(ctx, NULL, &jsobj))) return hres; - scope->obj = to_disp(scope->jsobj); + scope->obj = to_disp(jsobj); } + else + jsobj = as_jsdisp(scope->obj); for(i = 0; i < frame->function->local_scopes[index].locals_cnt; i++) { @@ -990,7 +991,7 @@ static HRESULT scope_init_locals(script_ctx_t *ctx) if (detached_vars) { - hres = jsdisp_propput_name(scope->jsobj, name, val); + hres = jsdisp_propput_name(jsobj, name, val); jsval_release(val); if (FAILED(hres)) return hres; @@ -1020,7 +1021,7 @@ static HRESULT interp_push_with_scope(script_ctx_t *ctx) if(FAILED(hres)) return hres; - hres = scope_push(ctx, ctx->call_ctx->scope, to_jsdisp(disp), disp, &ctx->call_ctx->scope); + hres = scope_push(ctx, ctx->call_ctx->scope, disp, &ctx->call_ctx->scope); IDispatch_Release(disp); return hres; } @@ -1034,7 +1035,7 @@ static HRESULT interp_push_block_scope(script_ctx_t *ctx) TRACE("scope_index %u.\n", scope_index); - hres = scope_push(ctx, ctx->call_ctx->scope, NULL, NULL, &frame->scope); + hres = scope_push(ctx, ctx->call_ctx->scope, NULL, &frame->scope); if (FAILED(hres) || !scope_index) return hres; @@ -1246,7 +1247,7 @@ static HRESULT interp_enter_catch(script_ctx_t *ctx) hres = jsdisp_propput_name(scope_obj, ident, v); jsval_release(v); if(SUCCEEDED(hres)) - hres = scope_push(ctx, ctx->call_ctx->scope, scope_obj, to_disp(scope_obj), &ctx->call_ctx->scope); + hres = scope_push(ctx, ctx->call_ctx->scope, to_disp(scope_obj), &ctx->call_ctx->scope); jsdisp_release(scope_obj); return hres; } @@ -3270,7 +3271,7 @@ static HRESULT setup_scope(script_ctx_t *ctx, call_frame_t *frame, scope_chain_t frame->pop_variables = i; - hres = scope_push(ctx, scope_chain, variable_object, to_disp(variable_object), &scope); + hres = scope_push(ctx, scope_chain, to_disp(variable_object), &scope); if(FAILED(hres)) { stack_popn(ctx, ctx->stack_top - orig_stack); return hres; diff --git a/dlls/jscript/engine.h b/dlls/jscript/engine.h index 987e1cfa52d..9b29373a0da 100644 --- a/dlls/jscript/engine.h +++ b/dlls/jscript/engine.h @@ -224,7 +224,6 @@ static inline bytecode_t *bytecode_addref(bytecode_t *code) typedef struct _scope_chain_t { jsdisp_t dispex; /* FIXME: don't wrap it in a jsdisp (it holds ref and traverse for the garbage collector) */ - jsdisp_t *jsobj; IDispatch *obj; unsigned int scope_index; struct _call_frame_t *frame; diff --git a/dlls/jscript/function.c b/dlls/jscript/function.c index 018fd3955db..7f53122001c 100644 --- a/dlls/jscript/function.c +++ b/dlls/jscript/function.c @@ -145,7 +145,7 @@ static HRESULT Arguments_idx_get(jsdisp_t *jsdisp, unsigned idx, jsval_t *r) return jsval_copy(*ref, r); /* FIXME: Accessing by name won't work for duplicated argument names */ - return jsdisp_propget_name(arguments->frame->base_scope->jsobj, + return jsdisp_propget_name(as_jsdisp(arguments->frame->base_scope->obj), arguments->function->func_code->params[idx], r); } @@ -169,7 +169,7 @@ static HRESULT Arguments_idx_put(jsdisp_t *jsdisp, unsigned idx, jsval_t val) } /* FIXME: Accessing by name won't work for duplicated argument names */ - return jsdisp_propput_name(arguments->frame->base_scope->jsobj, + return jsdisp_propput_name(as_jsdisp(arguments->frame->base_scope->obj), arguments->function->func_code->params[idx], val); } @@ -227,7 +227,7 @@ HRESULT setup_arguments_object(script_ctx_t *ctx, call_frame_t *frame) hres = jsdisp_define_data_property(&args->jsdisp, L"callee", PROPF_WRITABLE | PROPF_CONFIGURABLE, jsval_obj(&args->function->function.dispex)); if(SUCCEEDED(hres)) - hres = jsdisp_propput(frame->base_scope->jsobj, L"arguments", PROPF_WRITABLE, TRUE, jsval_obj(&args->jsdisp)); + hres = jsdisp_propput(as_jsdisp(frame->base_scope->obj), L"arguments", PROPF_WRITABLE, TRUE, jsval_obj(&args->jsdisp)); if(FAILED(hres)) { jsdisp_release(&args->jsdisp); return hres; @@ -242,11 +242,12 @@ void detach_arguments_object(jsdisp_t *args_disp) ArgumentsInstance *arguments = arguments_from_jsdisp(args_disp); call_frame_t *frame = arguments->frame; const BOOL on_stack = frame->base_scope->frame == frame; + jsdisp_t *jsobj = as_jsdisp(frame->base_scope->obj); HRESULT hres; /* Reset arguments value to cut the reference cycle. Note that since all activation contexts have * their own arguments property, it's impossible to use prototype's one during name lookup */ - jsdisp_propput_name(frame->base_scope->jsobj, L"arguments", jsval_undefined()); + jsdisp_propput_name(jsobj, L"arguments", jsval_undefined()); arguments->frame = NULL; /* Don't bother coppying arguments if call frame holds the last reference. */ @@ -259,7 +260,7 @@ void detach_arguments_object(jsdisp_t *args_disp) if(on_stack || i >= frame->function->param_cnt) hres = jsval_copy(arguments->jsdisp.ctx->stack[frame->arguments_off + i], arguments->buf+i); else - hres = jsdisp_propget_name(frame->base_scope->jsobj, frame->function->params[i], arguments->buf+i); + hres = jsdisp_propget_name(jsobj, frame->function->params[i], arguments->buf+i); if(FAILED(hres)) arguments->buf[i] = jsval_undefined(); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2942
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/jscript/array.c | 3 +-- dlls/jscript/dispex.c | 3 +-- dlls/jscript/engine.c | 20 ++++++-------------- dlls/jscript/json.c | 3 +-- dlls/jscript/jsregexp.c | 16 ++++------------ 5 files changed, 13 insertions(+), 32 deletions(-) diff --git a/dlls/jscript/array.c b/dlls/jscript/array.c index 341505a335c..4e1af864a8e 100644 --- a/dlls/jscript/array.c +++ b/dlls/jscript/array.c @@ -1663,9 +1663,8 @@ static HRESULT ArrayConstr_isArray(script_ctx_t *ctx, jsval_t vthis, WORD flags, return S_OK; } - obj = iface_to_jsdisp(get_object(argv[0])); + obj = to_jsdisp(get_object(argv[0])); if(r) *r = jsval_bool(obj && is_class(obj, JSCLASS_ARRAY)); - if(obj) jsdisp_release(obj); return S_OK; } diff --git a/dlls/jscript/dispex.c b/dlls/jscript/dispex.c index c3cc41ce782..e8f7d599463 100644 --- a/dlls/jscript/dispex.c +++ b/dlls/jscript/dispex.c @@ -101,10 +101,9 @@ static inline BOOL is_function_prop(dispex_prop_t *prop) if (is_object_instance(prop->u.val)) { - jsdisp_t *jsdisp = iface_to_jsdisp(get_object(prop->u.val)); + jsdisp_t *jsdisp = to_jsdisp(get_object(prop->u.val)); if (jsdisp) ret = is_class(jsdisp, JSCLASS_FUNCTION); - jsdisp_release(jsdisp); } return ret; } diff --git a/dlls/jscript/engine.c b/dlls/jscript/engine.c index d5075e13500..ceaa01b0648 100644 --- a/dlls/jscript/engine.c +++ b/dlls/jscript/engine.c @@ -514,12 +514,9 @@ static HRESULT disp_get_id(script_ctx_t *ctx, IDispatch *disp, const WCHAR *name BSTR bstr; HRESULT hres; - jsdisp = iface_to_jsdisp(disp); - if(jsdisp) { - hres = jsdisp_get_id(jsdisp, name, flags, id); - jsdisp_release(jsdisp); - return hres; - } + jsdisp = to_jsdisp(disp); + if(jsdisp) + return jsdisp_get_id(jsdisp, name, flags, id); if(name_bstr) { bstr = name_bstr; @@ -1771,7 +1768,7 @@ static HRESULT interp_obj_prop(script_ctx_t *ctx) jsdisp_t *func; assert(is_object_instance(val)); - func = iface_to_jsdisp(get_object(val)); + func = to_jsdisp(get_object(val)); desc.mask = desc.flags; if(type == PROPERTY_DEFINITION_GETTER) { @@ -1783,7 +1780,6 @@ static HRESULT interp_obj_prop(script_ctx_t *ctx) } hres = jsdisp_define_property(obj, name, &desc); - jsdisp_release(func); } jsval_release(val); @@ -1927,15 +1923,12 @@ static HRESULT interp_instanceof(script_ctx_t *ctx) hres = JS_E_OBJECT_EXPECTED; else if(is_object_instance(prot)) { if(is_object_instance(v)) - tmp = iface_to_jsdisp(get_object(v)); + tmp = to_jsdisp(get_object(v)); for(iter = tmp; !ret && iter; iter = iter->prototype) { hres = disp_cmp(get_object(prot), to_disp(iter), &ret); if(FAILED(hres)) break; } - - if(tmp) - jsdisp_release(tmp); }else { FIXME("prototype is not an object\n"); hres = E_FAIL; @@ -2221,9 +2214,8 @@ static HRESULT typeof_string(jsval_t v, const WCHAR **ret) case JSV_OBJECT: { jsdisp_t *dispex; - if((dispex = iface_to_jsdisp(get_object(v)))) { + if((dispex = to_jsdisp(get_object(v)))) { *ret = is_class(dispex, JSCLASS_FUNCTION) ? L"function" : L"object"; - jsdisp_release(dispex); }else { *ret = L"object"; } diff --git a/dlls/jscript/json.c b/dlls/jscript/json.c index e36c4973ef2..7826000f87d 100644 --- a/dlls/jscript/json.c +++ b/dlls/jscript/json.c @@ -754,14 +754,13 @@ static HRESULT stringify(stringify_ctx_t *ctx, jsdisp_t *object, const WCHAR *na jsdisp_t *obj; DISPID id; - obj = iface_to_jsdisp(get_object(value)); + obj = to_jsdisp(get_object(value)); if(!obj) { jsval_release(value); return S_FALSE; } hres = jsdisp_get_id(obj, L"toJSON", 0, &id); - jsdisp_release(obj); if(hres == S_OK) FIXME("Use toJSON.\n"); } diff --git a/dlls/jscript/jsregexp.c b/dlls/jscript/jsregexp.c index 7b97a2951df..378dee9bc53 100644 --- a/dlls/jscript/jsregexp.c +++ b/dlls/jscript/jsregexp.c @@ -666,17 +666,14 @@ HRESULT create_regexp_var(script_ctx_t *ctx, jsval_t src_arg, jsval_t *flags_arg if(is_object_instance(src_arg)) { jsdisp_t *obj; - obj = iface_to_jsdisp(get_object(src_arg)); + obj = to_jsdisp(get_object(src_arg)); if(obj) { if(is_class(obj, JSCLASS_REGEXP)) { RegExpInstance *regexp = regexp_from_jsdisp(obj); hres = create_regexp(ctx, regexp->str, regexp->jsregexp->flags, ret); - jsdisp_release(obj); return hres; } - - jsdisp_release(obj); } } @@ -913,21 +910,16 @@ static HRESULT RegExpConstr_value(script_ctx_t *ctx, jsval_t vthis, WORD flags, case DISPATCH_METHOD: if(argc) { if(is_object_instance(argv[0])) { - jsdisp_t *jsdisp = iface_to_jsdisp(get_object(argv[0])); + jsdisp_t *jsdisp = to_jsdisp(get_object(argv[0])); if(jsdisp) { if(is_class(jsdisp, JSCLASS_REGEXP)) { - if(argc > 1 && !is_undefined(argv[1])) { - jsdisp_release(jsdisp); + if(argc > 1 && !is_undefined(argv[1])) return JS_E_REGEXP_SYNTAX; - } if(r) - *r = jsval_obj(jsdisp); - else - jsdisp_release(jsdisp); + *r = jsval_obj(jsdisp_addref(jsdisp)); return S_OK; } - jsdisp_release(jsdisp); } } } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2942
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> This is no-op now, but is needed when we use specialized detached locals buffer instead of the variable obj. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/jscript/engine.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/dlls/jscript/engine.c b/dlls/jscript/engine.c index ceaa01b0648..8ce2efb85d4 100644 --- a/dlls/jscript/engine.c +++ b/dlls/jscript/engine.c @@ -3343,7 +3343,11 @@ HRESULT exec_source(script_ctx_t *ctx, DWORD flags, bytecode_t *bytecode, functi } if((flags & EXEC_EVAL) && ctx->call_ctx) { - variable_obj = jsdisp_addref(ctx->call_ctx->variable_obj); + variable_obj = ctx->call_ctx->variable_obj; + hres = detach_variable_object(ctx, ctx->call_ctx, FALSE); + if(FAILED(hres)) + return hres; + jsdisp_addref(variable_obj); }else if(!(flags & (EXEC_GLOBAL | EXEC_EVAL))) { hres = create_dispex(ctx, NULL, NULL, &variable_obj); if(FAILED(hres)) return hres; @@ -3397,12 +3401,6 @@ HRESULT exec_source(script_ctx_t *ctx, DWORD flags, bytecode_t *bytecode, functi this_obj = NULL; } - if(ctx->call_ctx && (flags & EXEC_EVAL)) { - hres = detach_variable_object(ctx, ctx->call_ctx, FALSE); - if(FAILED(hres)) - goto fail; - } - frame = calloc(1, sizeof(*frame)); if(!frame) { hres = E_OUTOFMEMORY; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2942
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> Rather than using the variable obj for it, unless necessary. Since it's an implementation detail, the scope's dispex object accesses them using index props (using same indices as the buffer's). Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/jscript/compile.c | 5 +- dlls/jscript/dispex.c | 8 ++ dlls/jscript/engine.c | 223 ++++++++++++++++++++++++++++++++++++---- dlls/jscript/engine.h | 12 ++- dlls/jscript/function.c | 42 +++----- dlls/jscript/jscript.h | 1 + 6 files changed, 238 insertions(+), 53 deletions(-) diff --git a/dlls/jscript/compile.c b/dlls/jscript/compile.c index e2690935212..8ccd7cfaae1 100644 --- a/dlls/jscript/compile.c +++ b/dlls/jscript/compile.c @@ -2517,9 +2517,9 @@ static HRESULT init_code(compiler_ctx_t *compiler, const WCHAR *source, UINT64 s static HRESULT compile_function(compiler_ctx_t *ctx, statement_t *source, function_expression_t *func_expr, BOOL from_eval, function_code_t *func) { + unsigned off, i, scope, scope_var_offs; function_expression_t *iter; function_local_t *local; - unsigned off, i, scope; HRESULT hres; TRACE("\n"); @@ -2594,15 +2594,18 @@ static HRESULT compile_function(compiler_ctx_t *ctx, statement_t *source, functi func->local_scopes[scope].locals_cnt = ctx->local_scopes[scope].locals_cnt; i = 0; + scope_var_offs = 0; WINE_RB_FOR_EACH_ENTRY(local, &ctx->local_scopes[scope].locals, function_local_t, entry) { func->local_scopes[scope].locals[i].name = local->name; func->local_scopes[scope].locals[i].ref = local->ref; if(local->ref >= 0) { func->variables[local->ref].name = local->name; + func->variables[local->ref].scope_offs = scope_var_offs++; func->variables[local->ref].func_id = -1; } i++; } + func->local_scopes[scope].vars_cnt = scope_var_offs; assert(i == ctx->local_scopes[scope].locals_cnt); } diff --git a/dlls/jscript/dispex.c b/dlls/jscript/dispex.c index e8f7d599463..b5fc3699212 100644 --- a/dlls/jscript/dispex.c +++ b/dlls/jscript/dispex.c @@ -2332,6 +2332,14 @@ HRESULT jsdisp_get_id(jsdisp_t *jsdisp, const WCHAR *name, DWORD flags, DISPID * return DISP_E_UNKNOWNNAME; } +HRESULT jsdisp_get_idx_id(jsdisp_t *jsdisp, DWORD idx, DISPID *id) +{ + WCHAR name[11]; + + swprintf(name, ARRAY_SIZE(name), L"%u", idx); + return jsdisp_get_id(jsdisp, name, 0, id); +} + HRESULT jsdisp_call_value(jsdisp_t *jsfunc, jsval_t vthis, WORD flags, unsigned argc, jsval_t *argv, jsval_t *r) { HRESULT hres; diff --git a/dlls/jscript/engine.c b/dlls/jscript/engine.c index 8ce2efb85d4..5472df02cf0 100644 --- a/dlls/jscript/engine.c +++ b/dlls/jscript/engine.c @@ -162,6 +162,25 @@ static inline BSTR local_name(call_frame_t *frame, int ref) return ref < 0 ? frame->function->params[-ref-1] : frame->function->variables[ref].name; } +static jsval_t *get_detached_local_ref(scope_chain_t *scope, const WCHAR *name) +{ + struct locals_buffer *detached_locals = scope->detached_locals; + function_code_t *func; + local_ref_t *ref; + + if(!detached_locals) + return NULL; + func = detached_locals->func_code; + ref = lookup_local(func, name, scope->scope_index); + return ref ? &detached_locals->local[(ref->ref < 0) ? -ref->ref - 1 : detached_locals->var_offs + func->variables[ref->ref].scope_offs] : NULL; +} + +static HRESULT get_detached_local_dispid(scope_chain_t *scope, const WCHAR *name, DISPID *id) +{ + jsval_t *local_ref = get_detached_local_ref(scope, name); + return local_ref ? jsdisp_get_idx_id(&scope->dispex, local_ref - scope->detached_locals->local, id) : DISP_E_UNKNOWNNAME; +} + /* Steals input reference even on failure. */ static HRESULT stack_push_exprval(script_ctx_t *ctx, exprval_t *val) { @@ -228,6 +247,18 @@ static BOOL stack_topn_exprval(script_ctx_t *ctx, unsigned n, exprval_t *r) while (1) { + hres = get_detached_local_dispid(scope, name, &id); + if (hres != DISP_E_UNKNOWNNAME) + { + if (FAILED(hres)) + { + r->type = EXPRVAL_INVALID; + r->u.hres = hres; + return FALSE; + } + jsobj = &scope->dispex; + break; + } if ((jsobj = to_jsdisp(scope->obj)) && SUCCEEDED(hres = jsdisp_get_id(jsobj, name, 0, &id))) break; if (scope == frame->base_scope) @@ -423,9 +454,24 @@ static inline void clear_acc(script_ctx_t *ctx) ctx->acc = jsval_undefined(); } +static inline scope_chain_t *scope_from_dispex(jsdisp_t *dispex) +{ + return CONTAINING_RECORD(dispex, scope_chain_t, dispex); +} + static void scope_destructor(jsdisp_t *dispex) { - scope_chain_t *scope = CONTAINING_RECORD(dispex, scope_chain_t, dispex); + scope_chain_t *scope = scope_from_dispex(dispex); + + if(scope->detached_locals) { + struct locals_buffer *locals = scope->detached_locals; + unsigned i, cnt = locals->local_cnt; + + release_bytecode(locals->func_code->bytecode); + for(i = 0; i < cnt; i++) + jsval_release(locals->local[i]); + free(locals); + } if(scope->next) scope_release(scope->next); @@ -435,12 +481,53 @@ static void scope_destructor(jsdisp_t *dispex) free(scope); } +static unsigned scope_idx_length(jsdisp_t *dispex) +{ + scope_chain_t *scope = scope_from_dispex(dispex); + + return scope->detached_locals->local_cnt; +} + +static HRESULT scope_idx_get(jsdisp_t *dispex, unsigned idx, jsval_t *r) +{ + scope_chain_t *scope = scope_from_dispex(dispex); + + return jsval_copy(scope->detached_locals->local[idx], r); +} + +static HRESULT scope_idx_put(jsdisp_t *dispex, unsigned idx, jsval_t val) +{ + scope_chain_t *scope = scope_from_dispex(dispex); + jsval_t copy, *ref; + HRESULT hres; + + hres = jsval_copy(val, ©); + if(FAILED(hres)) + return hres; + + ref = &scope->detached_locals->local[idx]; + jsval_release(*ref); + *ref = copy; + return S_OK; +} + static HRESULT scope_gc_traverse(struct gc_ctx *gc_ctx, enum gc_traverse_op op, jsdisp_t *dispex) { - scope_chain_t *scope = CONTAINING_RECORD(dispex, scope_chain_t, dispex); + scope_chain_t *scope = scope_from_dispex(dispex); jsdisp_t *jsobj; HRESULT hres; + if(scope->detached_locals) { + struct locals_buffer *locals = scope->detached_locals; + unsigned i, cnt = locals->local_cnt; + + for(i = 0; i < cnt; i++) { + hres = gc_process_linked_val(gc_ctx, op, dispex, &locals->local[i]); + if(FAILED(hres)) + return hres; + } + } + if(scope->next) { hres = gc_process_linked_obj(gc_ctx, op, dispex, &scope->next->dispex, (void**)&scope->next); if(FAILED(hres)) @@ -466,9 +553,9 @@ static const builtin_info_t scope_info = { NULL, scope_destructor, NULL, - NULL, - NULL, - NULL, + scope_idx_length, + scope_idx_get, + scope_idx_put, scope_gc_traverse }; @@ -622,10 +709,42 @@ HRESULT jsval_strict_equal(jsval_t lval, jsval_t rval, BOOL *ret) return S_OK; } +static HRESULT alloc_detached_locals(script_ctx_t *ctx, call_frame_t *frame, scope_chain_t *scope) +{ + function_code_t *func = frame->function; + unsigned i, argc, local_cnt; + jsval_t *detached_local; + HRESULT hres; + + argc = (scope == frame->base_scope) ? max(frame->argc, func->param_cnt) : 0; + local_cnt = argc + func->local_scopes[scope->scope_index].vars_cnt; + + if(!(scope->detached_locals = malloc(FIELD_OFFSET(struct locals_buffer, local[local_cnt])))) + return E_OUTOFMEMORY; + detached_local = scope->detached_locals->local; + scope->detached_locals->local_cnt = local_cnt; + scope->detached_locals->var_offs = argc; + scope->detached_locals->func_code = func; + bytecode_addref(func->bytecode); + + for(i = argc; i < local_cnt; i++) + detached_local[i] = jsval_undefined(); + for(i = 0; i < argc; i++) { + hres = jsval_copy(ctx->stack[frame->arguments_off + i], &detached_local[i]); + if(FAILED(hres)) { + do detached_local[i++] = jsval_undefined(); while(i < argc); + return hres; + } + } + + return S_OK; +} + static HRESULT detach_scope(script_ctx_t *ctx, call_frame_t *frame, scope_chain_t *scope) { function_code_t *func = frame->function; - unsigned int i, index; + unsigned int i, index, var_offs; + jsval_t *detached_local; jsdisp_t *jsobj; HRESULT hres; @@ -635,6 +754,12 @@ static HRESULT detach_scope(script_ctx_t *ctx, call_frame_t *frame, scope_chain_ assert(scope->frame == frame); scope->frame = NULL; + hres = alloc_detached_locals(ctx, frame, scope); + if (FAILED(hres)) + return hres; + detached_local = scope->detached_locals->local; + var_offs = scope->detached_locals->var_offs; + if (!scope->obj) { if (FAILED(hres = create_object(ctx, NULL, &jsobj))) @@ -649,16 +774,33 @@ static HRESULT detach_scope(script_ctx_t *ctx, call_frame_t *frame, scope_chain_ jsdisp_propput_name(jsobj, func->name, jsval_obj(jsdisp_addref(frame->function_instance))); index = scope->scope_index; - for(i = 0; i < frame->function->local_scopes[index].locals_cnt; i++) + for (i = 0; i < func->local_scopes[index].locals_cnt; i++) { - WCHAR *name = frame->function->local_scopes[index].locals[i].name; - int ref = frame->function->local_scopes[index].locals[i].ref; + WCHAR *name = func->local_scopes[index].locals[i].name; + int ref = func->local_scopes[index].locals[i].ref; - if (FAILED(hres = jsdisp_propput_name(jsobj, name, ctx->stack[local_off(frame, ref)]))) - return hres; - if (scope != frame->base_scope && frame->function->variables[ref].func_id != -1 - && FAILED(hres = jsdisp_propput_name(frame->variable_obj, name, ctx->stack[local_off(frame, ref)]))) + if (ref < 0) + continue; + if (FAILED(hres = jsval_copy(ctx->stack[local_off(frame, ref)], &detached_local[var_offs + func->variables[ref].scope_offs]))) return hres; + if (scope != frame->base_scope && func->variables[ref].func_id != -1) + { + jsval_t *base_ref = get_detached_local_ref(frame->base_scope, name); + if (base_ref) + { + jsval_t tmp; + hres = jsval_copy(ctx->stack[local_off(frame, ref)], &tmp); + if (SUCCEEDED(hres)) + { + jsval_release(*base_ref); + *base_ref = tmp; + } + } + else + hres = jsdisp_propput_name(frame->variable_obj, name, ctx->stack[local_off(frame, ref)]); + if (FAILED(hres)) + return hres; + } } return S_OK; } @@ -781,6 +923,10 @@ static HRESULT identifier_eval(script_ctx_t *ctx, BSTR identifier, exprval_t *re ret->u.val = jsval_obj(jsdisp_addref(scope->frame->function_instance)); return S_OK; } + }else if((hres = get_detached_local_dispid(scope, identifier, &id)) != DISP_E_UNKNOWNNAME) { + if(SUCCEEDED(hres)) + exprval_set_disp_ref(ret, to_disp(&scope->dispex), id); + return hres; } if (!scope->obj) @@ -960,6 +1106,14 @@ static HRESULT scope_init_locals(script_ctx_t *ctx) else jsobj = as_jsdisp(scope->obj); + if (detached_vars) + { + assert(!scope->detached_locals); + hres = alloc_detached_locals(ctx, frame, scope); + if (FAILED(hres)) + return hres; + } + for(i = 0; i < frame->function->local_scopes[index].locals_cnt; i++) { WCHAR *name = frame->function->local_scopes[index].locals[i].name; @@ -975,10 +1129,20 @@ static HRESULT scope_init_locals(script_ctx_t *ctx) + frame->function->variables[ref].func_id, ctx->call_ctx->scope, &func_obj))) return hres; val = jsval_obj(func_obj); - if (detached_vars && FAILED(hres = jsdisp_propput_name(frame->variable_obj, name, jsval_obj(func_obj)))) + if (detached_vars) { - jsdisp_release(func_obj); - return hres; + jsval_t *base_ref; + + if (frame->base_scope && (base_ref = get_detached_local_ref(frame->base_scope, name))) + { + jsval_release(*base_ref); + *base_ref = jsval_obj(jsdisp_addref(func_obj)); + } + else if (FAILED(hres = jsdisp_propput_name(frame->variable_obj, name, jsval_obj(func_obj)))) + { + jsdisp_release(func_obj); + return hres; + } } } else @@ -988,7 +1152,10 @@ static HRESULT scope_init_locals(script_ctx_t *ctx) if (detached_vars) { - hres = jsdisp_propput_name(jsobj, name, val); + if (scope->detached_locals) + hres = jsval_copy(val, &scope->detached_locals->local[scope->detached_locals->var_offs + frame->function->variables[ref].scope_offs]); + else + hres = jsdisp_propput_name(jsobj, name, val); jsval_release(val); if (FAILED(hres)) return hres; @@ -3297,6 +3464,7 @@ static HRESULT setup_scope(script_ctx_t *ctx, call_frame_t *frame, scope_chain_t HRESULT exec_source(script_ctx_t *ctx, DWORD flags, bytecode_t *bytecode, function_code_t *function, scope_chain_t *scope, IDispatch *this_obj, jsdisp_t *function_instance, unsigned argc, jsval_t *argv, jsval_t *r) { + scope_chain_t *locals_scope = NULL; jsdisp_t *variable_obj; call_frame_t *frame; unsigned i; @@ -3343,6 +3511,7 @@ HRESULT exec_source(script_ctx_t *ctx, DWORD flags, bytecode_t *bytecode, functi } if((flags & EXEC_EVAL) && ctx->call_ctx) { + locals_scope = ctx->call_ctx->base_scope; variable_obj = ctx->call_ctx->variable_obj; hres = detach_variable_object(ctx, ctx->call_ctx, FALSE); if(FAILED(hres)) @@ -3365,6 +3534,7 @@ HRESULT exec_source(script_ctx_t *ctx, DWORD flags, bytecode_t *bytecode, functi TRACE("[%d] %s %d\n", i, debugstr_w(function->variables[i].name), function->variables[i].func_id); if(function->variables[i].func_id != -1) { jsdisp_t *func_obj; + jsval_t *local_ref; if (function->funcs[function->variables[i].func_id].scope_index && flags & EXEC_EVAL) { @@ -3376,8 +3546,15 @@ HRESULT exec_source(script_ctx_t *ctx, DWORD flags, bytecode_t *bytecode, functi if(FAILED(hres)) goto fail; - hres = jsdisp_propput_name(variable_obj, function->variables[i].name, jsval_obj(func_obj)); - jsdisp_release(func_obj); + if(locals_scope && (local_ref = get_detached_local_ref(locals_scope, function->variables[i].name))) { + jsval_release(*local_ref); + *local_ref = jsval_obj(func_obj); + }else { + hres = jsdisp_propput_name(variable_obj, function->variables[i].name, jsval_obj(func_obj)); + jsdisp_release(func_obj); + if(FAILED(hres)) + goto fail; + } continue; } @@ -3388,9 +3565,11 @@ HRESULT exec_source(script_ctx_t *ctx, DWORD flags, bytecode_t *bytecode, functi if(!item && (flags & EXEC_GLOBAL) && lookup_global_members(ctx, function->variables[i].name, NULL)) continue; - hres = jsdisp_get_id(variable_obj, function->variables[i].name, fdexNameEnsure, &id); - if(FAILED(hres)) - goto fail; + if(!locals_scope || !get_detached_local_ref(locals_scope, function->variables[i].name)) { + hres = jsdisp_get_id(variable_obj, function->variables[i].name, fdexNameEnsure, &id); + if(FAILED(hres)) + goto fail; + } } } diff --git a/dlls/jscript/engine.h b/dlls/jscript/engine.h index 9b29373a0da..3918ec36989 100644 --- a/dlls/jscript/engine.h +++ b/dlls/jscript/engine.h @@ -151,6 +151,7 @@ typedef struct { typedef struct { unsigned locals_cnt; + unsigned vars_cnt; local_ref_t *locals; } local_ref_scopes_t; @@ -169,6 +170,7 @@ typedef struct _function_code_t { unsigned var_cnt; struct { BSTR name; + unsigned scope_offs; int func_id; /* -1 if not a function */ } *variables; @@ -222,10 +224,18 @@ static inline bytecode_t *bytecode_addref(bytecode_t *code) return code; } +struct locals_buffer { + function_code_t *func_code; + unsigned local_cnt; + unsigned var_offs; + jsval_t local[]; +}; + typedef struct _scope_chain_t { - jsdisp_t dispex; /* FIXME: don't wrap it in a jsdisp (it holds ref and traverse for the garbage collector) */ + jsdisp_t dispex; IDispatch *obj; unsigned int scope_index; + struct locals_buffer *detached_locals; struct _call_frame_t *frame; struct _scope_chain_t *next; } scope_chain_t; diff --git a/dlls/jscript/function.c b/dlls/jscript/function.c index 7f53122001c..da89b7f2b3a 100644 --- a/dlls/jscript/function.c +++ b/dlls/jscript/function.c @@ -129,48 +129,36 @@ static jsval_t *get_argument_ref(ArgumentsInstance *arguments, unsigned idx) { if(arguments->buf) return arguments->buf + idx; - if(arguments->frame->base_scope->frame || idx >= arguments->frame->function->param_cnt) + if(!arguments->frame->base_scope->detached_locals) return arguments->jsdisp.ctx->stack + arguments->frame->arguments_off + idx; - return NULL; + return arguments->frame->base_scope->detached_locals->local + idx; } static HRESULT Arguments_idx_get(jsdisp_t *jsdisp, unsigned idx, jsval_t *r) { ArgumentsInstance *arguments = arguments_from_jsdisp(jsdisp); - jsval_t *ref; TRACE("%p[%u]\n", arguments, idx); - if((ref = get_argument_ref(arguments, idx))) - return jsval_copy(*ref, r); - - /* FIXME: Accessing by name won't work for duplicated argument names */ - return jsdisp_propget_name(as_jsdisp(arguments->frame->base_scope->obj), - arguments->function->func_code->params[idx], r); + return jsval_copy(*get_argument_ref(arguments, idx), r); } static HRESULT Arguments_idx_put(jsdisp_t *jsdisp, unsigned idx, jsval_t val) { ArgumentsInstance *arguments = arguments_from_jsdisp(jsdisp); - jsval_t *ref; + jsval_t copy, *ref; HRESULT hres; TRACE("%p[%u] = %s\n", arguments, idx, debugstr_jsval(val)); - if((ref = get_argument_ref(arguments, idx))) { - jsval_t copy; - hres = jsval_copy(val, ©); - if(FAILED(hres)) - return hres; - - jsval_release(*ref); - *ref = copy; - return S_OK; - } + hres = jsval_copy(val, ©); + if(FAILED(hres)) + return hres; - /* FIXME: Accessing by name won't work for duplicated argument names */ - return jsdisp_propput_name(as_jsdisp(arguments->frame->base_scope->obj), - arguments->function->func_code->params[idx], val); + ref = get_argument_ref(arguments, idx); + jsval_release(*ref); + *ref = copy; + return S_OK; } static HRESULT Arguments_gc_traverse(struct gc_ctx *gc_ctx, enum gc_traverse_op op, jsdisp_t *jsdisp) @@ -243,7 +231,6 @@ void detach_arguments_object(jsdisp_t *args_disp) call_frame_t *frame = arguments->frame; const BOOL on_stack = frame->base_scope->frame == frame; jsdisp_t *jsobj = as_jsdisp(frame->base_scope->obj); - HRESULT hres; /* Reset arguments value to cut the reference cycle. Note that since all activation contexts have * their own arguments property, it's impossible to use prototype's one during name lookup */ @@ -254,14 +241,11 @@ void detach_arguments_object(jsdisp_t *args_disp) if(arguments->jsdisp.ref > 1) { arguments->buf = malloc(arguments->argc * sizeof(*arguments->buf)); if(arguments->buf) { + const jsval_t *args = on_stack ? arguments->jsdisp.ctx->stack + frame->arguments_off : frame->base_scope->detached_locals->local; int i; for(i = 0; i < arguments->argc ; i++) { - if(on_stack || i >= frame->function->param_cnt) - hres = jsval_copy(arguments->jsdisp.ctx->stack[frame->arguments_off + i], arguments->buf+i); - else - hres = jsdisp_propget_name(jsobj, frame->function->params[i], arguments->buf+i); - if(FAILED(hres)) + if(FAILED(jsval_copy(args[i], &arguments->buf[i]))) arguments->buf[i] = jsval_undefined(); } }else { diff --git a/dlls/jscript/jscript.h b/dlls/jscript/jscript.h index e91aa6dfabd..bd80528c020 100644 --- a/dlls/jscript/jscript.h +++ b/dlls/jscript/jscript.h @@ -254,6 +254,7 @@ HRESULT jsdisp_propput_idx(jsdisp_t*,DWORD,jsval_t) DECLSPEC_HIDDEN; HRESULT jsdisp_propget_name(jsdisp_t*,LPCWSTR,jsval_t*) DECLSPEC_HIDDEN; HRESULT jsdisp_get_idx(jsdisp_t*,DWORD,jsval_t*) DECLSPEC_HIDDEN; HRESULT jsdisp_get_id(jsdisp_t*,const WCHAR*,DWORD,DISPID*) DECLSPEC_HIDDEN; +HRESULT jsdisp_get_idx_id(jsdisp_t*,DWORD,DISPID*) DECLSPEC_HIDDEN; HRESULT disp_delete(IDispatch*,DISPID,BOOL*) DECLSPEC_HIDDEN; HRESULT disp_delete_name(script_ctx_t*,IDispatch*,jsstr_t*,BOOL*) DECLSPEC_HIDDEN; HRESULT jsdisp_delete_idx(jsdisp_t*,DWORD) DECLSPEC_HIDDEN; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2942
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> This is needed for duplicated argument names, as the last arg will shadow all the prior ones when it comes to name lookup. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/jscript/compile.c | 2 +- dlls/jscript/tests/lang.js | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/dlls/jscript/compile.c b/dlls/jscript/compile.c index 8ccd7cfaae1..aaeea4035d3 100644 --- a/dlls/jscript/compile.c +++ b/dlls/jscript/compile.c @@ -2567,7 +2567,7 @@ static HRESULT compile_function(compiler_ctx_t *ctx, statement_t *source, functi } } - for(i = 0; i < func->param_cnt; i++) { + for(i = func->param_cnt; i--;) { if(!find_local(ctx, func->params[i], 0) && !alloc_local(ctx, func->params[i], -i-1, 0)) return E_OUTOFMEMORY; } diff --git a/dlls/jscript/tests/lang.js b/dlls/jscript/tests/lang.js index 5c201d5bf26..30d8a338fd3 100644 --- a/dlls/jscript/tests/lang.js +++ b/dlls/jscript/tests/lang.js @@ -306,6 +306,38 @@ argumentsTest(); ok(arguments === 1, "arguments = " + arguments); })(); +// duplicated argument names are shadowed by the last argument with the same name +(function() { + var args, get_a, set_a; + + (function(a, a, b, c) { + get_a = function() { return a; } + set_a = function(v) { a = v; } + ok(get_a() === 2, "function(a, a, b, c) get_a() = " + get_a()); + ok(a === 2, "function(a, a, b, c) a = " + a); + ok(b === 3, "function(a, a, b, c) b = " + b); + ok(c === 4, "function(a, a, b, c) c = " + c); + a = 42; + ok(arguments[0] === 1, "function(a, a, b, c) arguments[0] = " + arguments[0]); + ok(arguments[1] === 42, "function(a, a, b, c) arguments[1] = " + arguments[1]); + ok(get_a() === 42, "function(a, a, b, c) get_a() = " + get_a() + " expected 42"); + args = arguments; + })(1, 2, 3, 4); + + ok(get_a() === 42, "function(a, a, b, c) get_a() after detach = " + get_a()); + set_a(100); + ok(get_a() === 100, "function(a, a, b, c) get_a() = " + get_a() + " expected 100"); + ok(args[0] === 1, "function(a, a, b, c) detached args[0] = " + args[0]); + ok(args[1] === 42, "function(a, a, b, c) detached args[1] = " + args[1]); + + (function(a, a) { + eval("var a = 7;"); + ok(a === 7, "function(a, a) a = " + a); + ok(arguments[0] === 5, "function(a, a) arguments[0] = " + arguments[0]); + ok(arguments[1] === 7, "function(a, a) arguments[1] = " + arguments[1]); + })(5, 6); +})(); + (function callAsExprTest() { ok(callAsExprTest.arguments === null, "callAsExprTest.arguments = " + callAsExprTest.arguments); })(1,2); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2942
From: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> We don't need the function instance anymore, and this simplifies the traversal and removes possible cyclic refs with it. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/jscript/function.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/dlls/jscript/function.c b/dlls/jscript/function.c index da89b7f2b3a..3836ab7439f 100644 --- a/dlls/jscript/function.c +++ b/dlls/jscript/function.c @@ -65,7 +65,7 @@ typedef struct { typedef struct { jsdisp_t jsdisp; - InterpretedFunction *function; + function_code_t *func_code; jsval_t *buf; call_frame_t *frame; unsigned argc; @@ -114,8 +114,7 @@ static void Arguments_destructor(jsdisp_t *jsdisp) free(arguments->buf); } - if(arguments->function) - jsdisp_release(&arguments->function->function.dispex); + release_bytecode(arguments->func_code->bytecode); free(arguments); } @@ -175,7 +174,7 @@ static HRESULT Arguments_gc_traverse(struct gc_ctx *gc_ctx, enum gc_traverse_op } } - return gc_process_linked_obj(gc_ctx, op, jsdisp, &arguments->function->function.dispex, (void**)&arguments->function); + return S_OK; } static const builtin_info_t Arguments_info = { @@ -205,7 +204,8 @@ HRESULT setup_arguments_object(script_ctx_t *ctx, call_frame_t *frame) return hres; } - args->function = (InterpretedFunction*)function_from_jsdisp(jsdisp_addref(frame->function_instance)); + args->func_code = ((InterpretedFunction*)function_from_jsdisp(frame->function_instance))->func_code; + bytecode_addref(args->func_code->bytecode); args->argc = frame->argc; args->frame = frame; @@ -213,7 +213,7 @@ HRESULT setup_arguments_object(script_ctx_t *ctx, call_frame_t *frame) jsval_number(args->argc)); if(SUCCEEDED(hres)) hres = jsdisp_define_data_property(&args->jsdisp, L"callee", PROPF_WRITABLE | PROPF_CONFIGURABLE, - jsval_obj(&args->function->function.dispex)); + jsval_obj(frame->function_instance)); if(SUCCEEDED(hres)) hres = jsdisp_propput(as_jsdisp(frame->base_scope->obj), L"arguments", PROPF_WRITABLE, TRUE, jsval_obj(&args->jsdisp)); if(FAILED(hres)) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2942
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=133845 Your paranoid android. === debian11 (32 bit report) === user32: msg.c:6897: Test failed: SetFocus(hwnd) on a button: 6: the msg 0x002b was expected, but got msg 0x001c instead msg.c:6897: Test failed: SetFocus(hwnd) on a button: 7: the msg 0x0111 was expected, but got msg 0x0086 instead msg.c:6897: Test failed: SetFocus(hwnd) on a button: 8: the msg 0x8000 was expected, but got msg 0x0006 instead msg.c:6897: Test failed: SetFocus(hwnd) on a button: 9: the msg sequence is not complete: expected 0000 - actual 0009
participants (2)
-
Gabriel Ivăncescu -
Marvin