[PATCH v2 0/7] 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. -- v2: jscript: Store ref to the function code instead of the function instance jscript: Properly implement duplicated argument names in functions. 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: Move arguments_obj from the frame to the base scope. 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 | 39 +++++++++++++++++++++++++ dlls/mshtml/tests/documentmode.js | 47 +++++++++++++++++++++++++++++++ 8 files changed, 145 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..ed0756451e2 100644 --- a/dlls/jscript/tests/api.js +++ b/dlls/jscript/tests/api.js @@ -2446,6 +2446,45 @@ 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"); + } +})(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..48641187f5c 100644 --- a/dlls/mshtml/tests/documentmode.js +++ b/dlls/mshtml/tests/documentmode.js @@ -756,6 +756,53 @@ 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; + } + } +}); + 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> We will need it when looking up arguments by name. Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/jscript/engine.c | 32 ++++++++++++++++++++------------ dlls/jscript/engine.h | 4 ++-- dlls/jscript/function.c | 14 +++++++++----- 3 files changed, 31 insertions(+), 19 deletions(-) diff --git a/dlls/jscript/engine.c b/dlls/jscript/engine.c index a4b416ba8ed..393e5429d3a 100644 --- a/dlls/jscript/engine.c +++ b/dlls/jscript/engine.c @@ -428,7 +428,8 @@ static void scope_destructor(jsdisp_t *dispex) if(scope->next) scope_release(scope->next); - + if(scope->arguments_obj) + jsdisp_release(scope->arguments_obj); if(scope->obj) IDispatch_Release(scope->obj); free(scope); @@ -445,6 +446,12 @@ static HRESULT scope_gc_traverse(struct gc_ctx *gc_ctx, enum gc_traverse_op op, return hres; } + if(scope->arguments_obj) { + hres = gc_process_linked_obj(gc_ctx, op, dispex, scope->arguments_obj, (void**)&scope->arguments_obj); + if(FAILED(hres)) + return hres; + } + if(op == GC_TRAVERSE_UNLINK) { IDispatch *obj = scope->obj; if(obj) { @@ -689,7 +696,7 @@ static HRESULT detach_variable_object(script_ctx_t *ctx, call_frame_t *frame, BO assert(frame == frame->base_scope->frame); assert(frame->variable_obj == frame->base_scope->jsobj); - if(!from_release && !frame->arguments_obj) { + if(!from_release && !frame->base_scope->arguments_obj) { hres = setup_arguments_object(ctx, frame); if(FAILED(hres)) return hres; @@ -2987,23 +2994,24 @@ OP_LIST static void pop_call_frame(script_ctx_t *ctx) { call_frame_t *frame = ctx->call_ctx; + scope_chain_t *scope = frame->scope; frame->stack_base -= frame->pop_locals + frame->pop_variables; assert(frame->scope == frame->base_scope); - /* If current scope will be kept alive, we need to transfer local variables to its variable object. */ - if(frame->scope && frame->scope->dispex.ref > 1) { - HRESULT hres = detach_variable_object(ctx, frame, TRUE); - if(FAILED(hres)) - ERR("Failed to detach variable object: %08lx\n", hres); + if(scope) { + /* If current scope will be kept alive, we need to transfer local variables to its variable object. */ + if(scope->dispex.ref > 1) { + HRESULT hres = detach_variable_object(ctx, frame, TRUE); + if(FAILED(hres)) + ERR("Failed to detach variable object: %08lx\n", hres); + } + if(scope->arguments_obj) + detach_arguments_object(scope->arguments_obj, frame); + scope_release(scope); } - if(frame->arguments_obj) - detach_arguments_object(frame->arguments_obj); - if(frame->scope) - scope_release(frame->scope); - if(frame->pop_variables) stack_popn(ctx, frame->pop_variables); stack_popn(ctx, frame->pop_locals); diff --git a/dlls/jscript/engine.h b/dlls/jscript/engine.h index 987e1cfa52d..6a7a16d2923 100644 --- a/dlls/jscript/engine.h +++ b/dlls/jscript/engine.h @@ -226,6 +226,7 @@ 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; + jsdisp_t *arguments_obj; unsigned int scope_index; struct _call_frame_t *frame; struct _scope_chain_t *next; @@ -279,7 +280,6 @@ typedef struct _call_frame_t { IDispatch *this_obj; jsdisp_t *function_instance; jsdisp_t *variable_obj; - jsdisp_t *arguments_obj; DWORD flags; unsigned argc; @@ -304,4 +304,4 @@ HRESULT exec_source(script_ctx_t*,DWORD,bytecode_t*,function_code_t*,scope_chain HRESULT create_source_function(script_ctx_t*,bytecode_t*,function_code_t*,scope_chain_t*,jsdisp_t**) DECLSPEC_HIDDEN; HRESULT setup_arguments_object(script_ctx_t*,call_frame_t*) DECLSPEC_HIDDEN; -void detach_arguments_object(jsdisp_t*) DECLSPEC_HIDDEN; +void detach_arguments_object(jsdisp_t*,call_frame_t*) DECLSPEC_HIDDEN; diff --git a/dlls/jscript/function.c b/dlls/jscript/function.c index 018fd3955db..f0677948093 100644 --- a/dlls/jscript/function.c +++ b/dlls/jscript/function.c @@ -233,20 +233,24 @@ HRESULT setup_arguments_object(script_ctx_t *ctx, call_frame_t *frame) return hres; } - frame->arguments_obj = &args->jsdisp; + frame->base_scope->arguments_obj = &args->jsdisp; return S_OK; } -void detach_arguments_object(jsdisp_t *args_disp) +void detach_arguments_object(jsdisp_t *args_disp, call_frame_t *popped_frame) { ArgumentsInstance *arguments = arguments_from_jsdisp(args_disp); call_frame_t *frame = arguments->frame; const BOOL on_stack = frame->base_scope->frame == frame; HRESULT hres; + if(frame != popped_frame) + return; + /* 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()); + frame->base_scope->arguments_obj = NULL; arguments->frame = NULL; /* Don't bother coppying arguments if call frame holds the last reference. */ @@ -269,7 +273,7 @@ void detach_arguments_object(jsdisp_t *args_disp) } } - jsdisp_release(frame->arguments_obj); + jsdisp_release(&arguments->jsdisp); } HRESULT Function_invoke(jsdisp_t *func_this, jsval_t vthis, WORD flags, unsigned argc, jsval_t *argv, jsval_t *r) @@ -542,12 +546,12 @@ static HRESULT Function_get_arguments(script_ctx_t *ctx, jsdisp_t *jsthis, jsval for(frame = ctx->call_ctx; frame; frame = frame->prev_frame) { if(frame->function_instance == &function->dispex) { - if(!frame->arguments_obj) { + if(!frame->base_scope->arguments_obj) { hres = setup_arguments_object(ctx, frame); if(FAILED(hres)) return hres; } - *r = jsval_obj(jsdisp_addref(frame->arguments_obj)); + *r = jsval_obj(jsdisp_addref(frame->base_scope->arguments_obj)); return S_OK; } } -- 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 | 12 +++++---- 3 files changed, 36 insertions(+), 34 deletions(-) diff --git a/dlls/jscript/engine.c b/dlls/jscript/engine.c index 393e5429d3a..3becd1fc042 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; } @@ -438,6 +439,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) { @@ -461,7 +463,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 = { @@ -477,7 +479,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; @@ -494,7 +496,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; @@ -635,6 +636,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) @@ -643,18 +645,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++) @@ -662,7 +664,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)]))) @@ -694,7 +696,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->base_scope->arguments_obj) { hres = setup_arguments_object(ctx, frame); @@ -791,13 +793,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; @@ -950,6 +949,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; @@ -961,13 +961,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++) { @@ -994,7 +995,7 @@ static HRESULT scope_init_locals(script_ctx_t *ctx) if (detached_vars) { - if (FAILED(hres = jsdisp_propput_name(scope->jsobj, name, val))) + if (FAILED(hres = jsdisp_propput_name(jsobj, name, val))) return hres; } else @@ -1022,7 +1023,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; } @@ -1036,7 +1037,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; @@ -1248,7 +1249,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; } @@ -3273,7 +3274,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 6a7a16d2923..3688972f29f 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; jsdisp_t *arguments_obj; unsigned int scope_index; diff --git a/dlls/jscript/function.c b/dlls/jscript/function.c index f0677948093..b1d7e249923 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,14 +242,16 @@ void detach_arguments_object(jsdisp_t *args_disp, call_frame_t *popped_frame) 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; HRESULT hres; if(frame != popped_frame) return; + jsobj = as_jsdisp(frame->base_scope->obj); /* 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()); frame->base_scope->arguments_obj = NULL; arguments->frame = NULL; @@ -263,7 +265,7 @@ void detach_arguments_object(jsdisp_t *args_disp, call_frame_t *popped_frame) 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 3becd1fc042..d24b54ec800 100644 --- a/dlls/jscript/engine.c +++ b/dlls/jscript/engine.c @@ -521,12 +521,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; @@ -1773,7 +1770,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) { @@ -1785,7 +1782,6 @@ static HRESULT interp_obj_prop(script_ctx_t *ctx) } hres = jsdisp_define_property(obj, name, &desc); - jsdisp_release(func); } jsval_release(val); @@ -1929,15 +1925,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; @@ -2223,9 +2216,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 7da365f4e6c..e82c91d9ae1 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); } } @@ -911,21 +908,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> Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- dlls/jscript/compile.c | 2 +- dlls/jscript/engine.c | 34 ++++++++++++- dlls/jscript/engine.h | 1 + dlls/jscript/function.c | 101 ++++++++++++++++--------------------- dlls/jscript/tests/lang.js | 10 ++++ 5 files changed, 88 insertions(+), 60 deletions(-) diff --git a/dlls/jscript/compile.c b/dlls/jscript/compile.c index e2690935212..2d2c2c9f40a 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/engine.c b/dlls/jscript/engine.c index d24b54ec800..3270547cf7b 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) { + WCHAR name_idx[11]; jsdisp_t *jsobj; DISPID id; BSTR name; @@ -219,16 +220,24 @@ static BOOL stack_topn_exprval(script_ctx_t *ctx, unsigned n, exprval_t *r) { name = frame->function->variables[off - frame->variables_off].name; scope = frame->scope; + jsobj = to_jsdisp(scope->obj); } else { name = frame->function->params[off - frame->arguments_off]; scope = frame->base_scope; + jsobj = to_jsdisp(scope->obj); + if (scope->arguments_obj && off - frame->arguments_off < frame->argc) + { + name = name_idx; + jsobj = scope->arguments_obj; + swprintf(name_idx, ARRAY_SIZE(name_idx), L"%u", off - frame->arguments_off); + } } while (1) { - if ((jsobj = to_jsdisp(scope->obj)) && SUCCEEDED(hres = jsdisp_get_id(jsobj, name, 0, &id))) + if (jsobj && SUCCEEDED(hres = jsdisp_get_id(jsobj, name, 0, &id))) break; if (scope == frame->base_scope) { @@ -237,6 +246,7 @@ static BOOL stack_topn_exprval(script_ctx_t *ctx, unsigned n, exprval_t *r) return FALSE; } scope = scope->next; + jsobj = to_jsdisp(scope->obj); } *stack_top_ref(ctx, n+1) = jsval_obj(jsdisp_addref(jsobj)); @@ -632,7 +642,7 @@ HRESULT jsval_strict_equal(jsval_t lval, jsval_t rval, BOOL *ret) 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, args_len = 0; jsdisp_t *jsobj; HRESULT hres; @@ -651,6 +661,17 @@ static HRESULT detach_scope(script_ctx_t *ctx, call_frame_t *frame, scope_chain_ else jsobj = as_jsdisp(scope->obj); + if (scope->arguments_obj) + { + args_len = frame->argc; + for (i = 0; i < args_len; i++) + { + hres = scope->arguments_obj->builtin_info->idx_put(scope->arguments_obj, i, ctx->stack[frame->arguments_off + i]); + if(FAILED(hres)) + return hres; + } + } + if (scope == frame->base_scope && func->name && func->local_ref == INVALID_LOCAL_REF && ctx->version >= SCRIPTLANGUAGEVERSION_ES5) jsdisp_propput_name(jsobj, func->name, jsval_obj(jsdisp_addref(frame->function_instance))); @@ -661,6 +682,8 @@ 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(ref < 0 && -ref - 1 < args_len) + continue; 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 @@ -788,6 +811,13 @@ 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(scope->arguments_obj) { + hres = get_arguments_identifier_id(scope->arguments_obj, identifier, scope->scope_index, &id); + if(hres != DISP_E_MEMBERNOTFOUND) { + if(SUCCEEDED(hres)) + exprval_set_disp_ref(ret, to_disp(scope->arguments_obj), id); + return hres; + } } if (!scope->obj) diff --git a/dlls/jscript/engine.h b/dlls/jscript/engine.h index 3688972f29f..af0aa0e08b4 100644 --- a/dlls/jscript/engine.h +++ b/dlls/jscript/engine.h @@ -304,3 +304,4 @@ HRESULT exec_source(script_ctx_t*,DWORD,bytecode_t*,function_code_t*,scope_chain HRESULT create_source_function(script_ctx_t*,bytecode_t*,function_code_t*,scope_chain_t*,jsdisp_t**) DECLSPEC_HIDDEN; HRESULT setup_arguments_object(script_ctx_t*,call_frame_t*) DECLSPEC_HIDDEN; void detach_arguments_object(jsdisp_t*,call_frame_t*) DECLSPEC_HIDDEN; +HRESULT get_arguments_identifier_id(jsdisp_t*,const WCHAR*,unsigned,DISPID*) DECLSPEC_HIDDEN; diff --git a/dlls/jscript/function.c b/dlls/jscript/function.c index b1d7e249923..ec037449e08 100644 --- a/dlls/jscript/function.c +++ b/dlls/jscript/function.c @@ -66,9 +66,9 @@ typedef struct { typedef struct { jsdisp_t jsdisp; InterpretedFunction *function; - jsval_t *buf; call_frame_t *frame; unsigned argc; + jsval_t buf[]; } ArgumentsInstance; static HRESULT create_bind_function(script_ctx_t*,FunctionInstance*,jsval_t,unsigned,jsval_t*,jsdisp_t**r); @@ -104,15 +104,12 @@ static HRESULT Arguments_value(script_ctx_t *ctx, jsval_t vthis, WORD flags, uns static void Arguments_destructor(jsdisp_t *jsdisp) { ArgumentsInstance *arguments = arguments_from_jsdisp(jsdisp); + unsigned i; TRACE("(%p)\n", arguments); - if(arguments->buf) { - unsigned i; - for(i = 0; i < arguments->argc; i++) - jsval_release(arguments->buf[i]); - free(arguments->buf); - } + for(i = 0; i < arguments->argc; i++) + jsval_release(arguments->buf[i]); if(arguments->function) jsdisp_release(&arguments->function->function.dispex); @@ -127,50 +124,36 @@ static unsigned Arguments_idx_length(jsdisp_t *jsdisp) 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 && arguments->frame->base_scope->frame) return arguments->jsdisp.ctx->stack + arguments->frame->arguments_off + idx; - return NULL; + return arguments->buf + 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) @@ -179,12 +162,10 @@ static HRESULT Arguments_gc_traverse(struct gc_ctx *gc_ctx, enum gc_traverse_op HRESULT hres; unsigned i; - if(arguments->buf) { - for(i = 0; i < arguments->argc; i++) { - hres = gc_process_linked_val(gc_ctx, op, jsdisp, &arguments->buf[i]); - if(FAILED(hres)) - return hres; - } + for(i = 0; i < arguments->argc; i++) { + hres = gc_process_linked_val(gc_ctx, op, jsdisp, &arguments->buf[i]); + if(FAILED(hres)) + return hres; } return gc_process_linked_obj(gc_ctx, op, jsdisp, &arguments->function->function.dispex, (void**)&arguments->function); @@ -206,8 +187,9 @@ HRESULT setup_arguments_object(script_ctx_t *ctx, call_frame_t *frame) { ArgumentsInstance *args; HRESULT hres; + unsigned i; - args = calloc(1, sizeof(*args)); + args = calloc(1, FIELD_OFFSET(ArgumentsInstance, buf[frame->argc])); if(!args) return E_OUTOFMEMORY; @@ -220,6 +202,8 @@ HRESULT setup_arguments_object(script_ctx_t *ctx, call_frame_t *frame) args->function = (InterpretedFunction*)function_from_jsdisp(jsdisp_addref(frame->function_instance)); args->argc = frame->argc; args->frame = frame; + for(i = 0; i < args->argc; i++) + args->buf[i] = jsval_undefined(); hres = jsdisp_define_data_property(&args->jsdisp, L"length", PROPF_WRITABLE | PROPF_CONFIGURABLE, jsval_number(args->argc)); @@ -241,9 +225,7 @@ void detach_arguments_object(jsdisp_t *args_disp, call_frame_t *popped_frame) { 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; - HRESULT hres; if(frame != popped_frame) return; @@ -255,29 +237,34 @@ void detach_arguments_object(jsdisp_t *args_disp, call_frame_t *popped_frame) frame->base_scope->arguments_obj = NULL; arguments->frame = NULL; - /* Don't bother coppying arguments if call frame holds the last reference. */ - if(arguments->jsdisp.ref > 1) { - arguments->buf = malloc(arguments->argc * sizeof(*arguments->buf)); - if(arguments->buf) { - int i; + /* Don't bother copying arguments if call frame holds the last reference. */ + if(arguments->jsdisp.ref > 1 && frame->base_scope->frame == frame) { + unsigned 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)) - arguments->buf[i] = jsval_undefined(); - } - }else { - ERR("out of memory\n"); - arguments->argc = 0; + for(i = 0; i < arguments->argc; i++) { + assert(is_undefined(arguments->buf[i])); + jsval_copy(arguments->jsdisp.ctx->stack[frame->arguments_off + i], &arguments->buf[i]); } } jsdisp_release(&arguments->jsdisp); } +HRESULT get_arguments_identifier_id(jsdisp_t *args_disp, const WCHAR *identifier, unsigned scope_index, DISPID *id) +{ + ArgumentsInstance *arguments = arguments_from_jsdisp(args_disp); + local_ref_t *ref; + WCHAR buf[11]; + + ref = lookup_local(arguments->function->func_code, identifier, scope_index); + + if(!ref || ref->ref >= 0 || -ref->ref - 1 >= arguments->argc) + return DISP_E_MEMBERNOTFOUND; + + swprintf(buf, ARRAY_SIZE(buf), L"%u", -ref->ref - 1); + return jsdisp_get_id(&arguments->jsdisp, buf, fdexNameImplicit, id); +} + HRESULT Function_invoke(jsdisp_t *func_this, jsval_t vthis, WORD flags, unsigned argc, jsval_t *argv, jsval_t *r) { FunctionInstance *function; diff --git a/dlls/jscript/tests/lang.js b/dlls/jscript/tests/lang.js index 5c201d5bf26..adb5be67c8e 100644 --- a/dlls/jscript/tests/lang.js +++ b/dlls/jscript/tests/lang.js @@ -306,6 +306,16 @@ argumentsTest(); ok(arguments === 1, "arguments = " + arguments); })(); +// duplicated argument names are shadowed by the last argument with the same name +(function(a, a, b, c) { + 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]); +})(1, 2, 3, 4); + (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 | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/dlls/jscript/function.c b/dlls/jscript/function.c index ec037449e08..56feda7f03f 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; call_frame_t *frame; unsigned argc; jsval_t buf[]; @@ -111,8 +111,7 @@ static void Arguments_destructor(jsdisp_t *jsdisp) for(i = 0; i < arguments->argc; i++) jsval_release(arguments->buf[i]); - if(arguments->function) - jsdisp_release(&arguments->function->function.dispex); + release_bytecode(arguments->func_code->bytecode); free(arguments); } @@ -168,7 +167,7 @@ static HRESULT Arguments_gc_traverse(struct gc_ctx *gc_ctx, enum gc_traverse_op return hres; } - 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 = { @@ -199,7 +198,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; for(i = 0; i < args->argc; i++) @@ -209,7 +209,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)) { @@ -256,7 +256,7 @@ HRESULT get_arguments_identifier_id(jsdisp_t *args_disp, const WCHAR *identifier local_ref_t *ref; WCHAR buf[11]; - ref = lookup_local(arguments->function->func_code, identifier, scope_index); + ref = lookup_local(arguments->func_code, identifier, scope_index); if(!ref || ref->ref >= 0 || -ref->ref - 1 >= arguments->argc) return DISP_E_MEMBERNOTFOUND; -- 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=133426 Your paranoid android. === w10pro64_zh_CN (64 bit report) === mshtml: htmldoc.c:351: Test failed: expected Exec_SETTITLE === debian11 (32 bit report) === user32: msg.c:6897: Test failed: SetFocus(hwnd) on a button: 4: the msg 0x0007 was expected, but got msg 0x0005 instead msg.c:6897: Test failed: SetFocus(hwnd) on a button: 5: the msg 0x0138 was expected, but got msg 0x030f instead msg.c:6897: Test failed: SetFocus(hwnd) on a button: 6: the msg 0x0111 was expected, but got msg 0x001c instead msg.c:6897: Test failed: SetFocus(hwnd) on a button: 8: the msg 0x8000 was expected, but got msg 0x0086 instead msg.c:6897: Test failed: SetFocus(hwnd) on a button: 9: the msg sequence is not complete: expected 0000 - actual 0006
This was way more complicated than I anticipated, but I'm satisfied with how it turned out. I removed the other patches past the duplicate arguments one, because now the MR would have been too large. I'll send them as a follow up MR after this. Now I moved the arguments_obj from the frame to the scope since we need it for lookup (and I had to take care of eval frames, which aren't the same as the scope's frame). Then I had to patch all places where identifier lookup is done (two of them) so that it looks into the argument obj if it fits the requirements; so instead of an IDREF to the scope's jsobj, it's an IDREF into the arguments obj. I also took the liberty to simplify some other parts of the code, in separate commits of course. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2942#note_34678
Jacek Caban (@jacek) commented about dlls/mshtml/tests/documentmode.js:
+ 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; + } + } It would be nice to have some simple tests for cases where `interp_call_eval` is used with call that's not a builtin `eval`.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2942#note_34803
Jacek Caban (@jacek) commented about dlls/jscript/tests/lang.js:
ok(arguments === 1, "arguments = " + arguments); })();
+// duplicated argument names are shadowed by the last argument with the same name +(function(a, a, b, c) { + 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]); +})(1, 2, 3, 4);
It would be nice to test arguments object with detached frames as well. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2942#note_34804
Jacek Caban (@jacek) commented about dlls/jscript/function.c:
typedef struct { jsdisp_t jsdisp; InterpretedFunction *function; - jsval_t *buf; call_frame_t *frame; unsigned argc; + jsval_t buf[]; I'm not sure that an improvement (and at very least could be a separated patch). Note that we expect vast majority of arguments objects to never need this buffer.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2942#note_34806
Jacek Caban (@jacek) commented about dlls/jscript/engine.c:
{ name = frame->function->variables[off - frame->variables_off].name; scope = frame->scope; + jsobj = to_jsdisp(scope->obj); } else { name = frame->function->params[off - frame->arguments_off]; scope = frame->base_scope; + jsobj = to_jsdisp(scope->obj); + if (scope->arguments_obj && off - frame->arguments_off < frame->argc)
You could move that check one level above. In fact, it would be probably more readable to just call `jsdisp_get_id()` (or even an arguments-specific function) separately instead of complicating the existing loop. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2942#note_34805
On Tue Jun 6 15:38:45 2023 +0000, Jacek Caban wrote:
I'm not sure that an improvement (and at very least could be a separated patch). Note that we expect vast majority of arguments objects to never need this buffer. Oh, I did it that way because it was simpler, since I always allocated the buffer in `setup_arguments_object` unconditionally. That's because it's used a lot more with this patch (for all argument lookups that aren't on the stack). Before, it was used only when the frame was detached.
Should I try to allocate it on demand? Note that detaching the variable obj will make use of it now (so it will allocate it). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2942#note_34809
On 06/06/2023 19:49, Gabriel Ivăncescu (@insn) wrote:
On Tue Jun 6 15:38:45 2023 +0000, Jacek Caban wrote:
I'm not sure that an improvement (and at very least could be a separated patch). Note that we expect vast majority of arguments objects to never need this buffer. Oh, I did it that way because it was simpler, since I always allocated the buffer in `setup_arguments_object` unconditionally. That's because it's used a lot more with this patch (for all argument lookups that aren't on the stack). Before, it was used only when the frame was detached.
Should I try to allocate it on demand? Note that detaching the variable obj will make use of it now (so it will allocate it).
Nevermind, I found a subtle problem. We'll need **two** buffers. The original buffer has to be used when the arguments object is detached from execution context. The new buffer I added was just to store the detached variable obj props that would otherwise go into the variable object, except that it works for duplicated arg names (shadowed). This buffer should be allocated unconditionally, so I'll use variable length to avoid two allocations and complicating the setup. Both buffers will be needed: the original buf for detached arguments object, and the new buf to hold the parameters for detached variable obj.
On Tue Jun 6 18:36:43 2023 +0000, **** wrote:
Gabriel Ivăncescu replied on the mailing list: ``` On 06/06/2023 19:49, Gabriel Ivăncescu (@insn) wrote:
On Tue Jun 6 15:38:45 2023 +0000, Jacek Caban wrote:
I'm not sure that an improvement (and at very least could be a separated patch). Note that we expect vast majority of arguments objects to never need this buffer. Oh, I did it that way because it was simpler, since I always allocated the buffer in `setup_arguments_object` unconditionally. That's because it's used a lot more with this patch (for all argument lookups that aren't on the stack). Before, it was used only when the frame was detached.
Should I try to allocate it on demand? Note that detaching the variable obj will make use of it now (so it will allocate it).
Nevermind, I found a subtle problem. We'll need **two** buffers. The original buffer has to be used when the arguments object is detached from execution context. The new buffer I added was just to store the detached variable obj props that would otherwise go into the variable object, except that it works for duplicated arg names (shadowed). This buffer should be allocated unconditionally, so I'll use variable length to avoid two allocations and complicating the setup. Both buffers will be needed: the original buf for detached arguments object, and the new buf to hold the parameters for detached variable obj. ``` It may be interesting to try to store all locals in a buffer and eliminate variable object for most cases.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2942#note_34815
participants (4)
-
Gabriel Ivăncescu -
Gabriel Ivăncescu -
Jacek Caban (＠jacek)
-
Marvin