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.
-- v7: 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.
From: Gabriel Ivăncescu gabrielopcode@gmail.com
Signed-off-by: Gabriel Ivăncescu gabrielopcode@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;
From: Gabriel Ivăncescu gabrielopcode@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@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; }
From: Gabriel Ivăncescu gabrielopcode@gmail.com
Signed-off-by: Gabriel Ivăncescu gabrielopcode@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
From: Gabriel Ivăncescu gabrielopcode@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@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(); }
From: Gabriel Ivăncescu gabrielopcode@gmail.com
Signed-off-by: Gabriel Ivăncescu gabrielopcode@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); } } }
From: Gabriel Ivăncescu gabrielopcode@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@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;
From: Gabriel Ivăncescu gabrielopcode@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@gmail.com --- dlls/jscript/compile.c | 5 +- dlls/jscript/dispex.c | 8 ++ dlls/jscript/engine.c | 227 ++++++++++++++++++++++++++++++++++------ dlls/jscript/engine.h | 12 ++- dlls/jscript/function.c | 42 +++----- dlls/jscript/jscript.h | 1 + 6 files changed, 231 insertions(+), 64 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..7c949a3d2bc 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) @@ -939,7 +1085,6 @@ 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; @@ -951,14 +1096,13 @@ static HRESULT scope_init_locals(script_ctx_t *ctx) assert(frame->base_scope->frame == frame); frame->scope->frame = ctx->call_ctx; } - else if (!scope->obj) + else { - if (FAILED(hres = create_object(ctx, NULL, &jsobj))) + assert(!scope->detached_locals); + hres = alloc_detached_locals(ctx, frame, scope); + if (FAILED(hres)) return hres; - scope->obj = to_disp(jsobj); } - else - jsobj = as_jsdisp(scope->obj);
for(i = 0; i < frame->function->local_scopes[index].locals_cnt; i++) { @@ -975,10 +1119,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 @@ -987,12 +1141,7 @@ static HRESULT scope_init_locals(script_ctx_t *ctx) }
if (detached_vars) - { - hres = jsdisp_propput_name(jsobj, name, val); - jsval_release(val); - if (FAILED(hres)) - return hres; - } + scope->detached_locals->local[scope->detached_locals->var_offs + frame->function->variables[ref].scope_offs] = val; else { off = local_off(frame, ref); @@ -3297,6 +3446,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 +3493,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 +3516,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 +3528,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 +3547,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;
From: Gabriel Ivăncescu gabrielopcode@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@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);
From: Gabriel Ivăncescu gabrielopcode@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@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)) {
I simplified `scope_init_locals` since we don't need the variable obj for the scope there, anymore.
Jacek Caban (@jacek) commented about dlls/jscript/tests/lang.js:
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]);
This test does not work in MSHTML and, as expected, uses 100 here. This looks more like a bug in jscript.dll than anything intended, so maybe we could just implement the MSHTML behavior.
I was surprised that you still need a separated buffer in arguments object and it won't work for the proper implementation.
On Fri Jun 16 16:27:33 2023 +0000, Gabriel Ivăncescu wrote:
Yeah, using the scope object was just an optimization I wanted to make since the variable object is barely used now in such cases, so it feels like a bit of a waste. But as far as I can see `identifier_eval` only treats the "arguments" and the function's name itself as special, though? What I'm talking about is functions in nested scopes getting exposed on the base scope. This call in `detach_scope`:
jsdisp_propput_name(frame->variable_obj, name, ctx->stack[local_off(frame, ref)]);or the equivalent in `scope_init_locals` when `detached_vars` is true. An example of such tests are in "functions scope" test in mshtml/es5.js, at the beginning of it. Even though it exposes them using same name, so base scope does have locals named `f`, but that's just coincidence I think. That said, probably that should go into separate follow up MR, anyway. Anyway, I'll leave the scope obj optimization out of this MR, do you think I should resend it later or you still don't like it?
If it's done right, sure, it would be nice to have.
On Mon Jun 19 09:36:07 2023 +0000, Jacek Caban wrote:
This test does not work in MSHTML and, as expected, uses 100 here. This looks more like a bug in jscript.dll than anything intended, so maybe we could just implement the MSHTML behavior. I was surprised that you still need a separated buffer in arguments object and it won't work for the proper implementation.
Interesting. Maybe it's intended or something makes use of it, as we already had tests exercising something similar (slightly above in the same file); I was confused at first too when I was changing it and had to make it work.
I can certainly fix it for MSHTML, but I think the current arguments buf should definitely stay, because it will be required for "strict mode" arguments object anyway, so it's not the "wrong way forward" at least (those are *always* detached from the local vars, so we could use the buf on initialization—not that I plan to implement it soon, though, just saying it will be needed at some point).
Should I attempt the MSHTML fix in this MR or the follow up? Obviously a separate commit.
Interesting. Maybe it's intended or something makes use of it, as we already had tests exercising something similar (slightly above in the same file); I was confused at first too when I was changing it and had to make it work.
I don't remember details, but from the context, it looks like I wrote this test when working on static binding optimizations and needed some tests for corner cases (this was a huge perf win, BTW, that's why I think it would be nice to extend it for more cases at some point).
It seems that on top of those changes it should be possible to handle both modes so we may as well do it, but let's not complicate this MR.
I can certainly fix it for MSHTML, but I think the current arguments buf should definitely stay, because it will be required for "strict mode" arguments object anyway, so it's not the "wrong way forward" at least (those are *always* detached from the local vars, so we could use the buf on initialization—not that I plan to implement it soon, though, just saying it will be needed at some point).
Yes, strict mode arguments will always need it and will need to make a copy on entry (or at least before the first argument value modification). The nice thing about strict mode is that it will allow us to statically analyze code and know if arguments object will ever be used ahead of time, so it should be easy to optimize in the compiler and simply not create arguments at all when not needed.
Should I attempt the MSHTML fix in this MR or the follow up? Obviously a separate commit.
Follow up, please. This one is already tricky to review.
I'm not yet sure about `scope_offs`. Typeinfo should not care about variables or scopes, only about arguments. It seems to me like it could replace `ref` eventually (or `ref` could be converted to what you need). Anyway, the patch is tricky to review and this part could be just skipped for now. Could you please try to move only arguments to the detached scope's buffer and leave variables for later?
Jacek Caban (@jacek) commented about dlls/jscript/function.c:
typedef struct { jsdisp_t jsdisp;
- InterpretedFunction *function;
- function_code_t *func_code;
It seems that we also don't need func_code, can we just remove it?
On Tue Jun 20 11:35:01 2023 +0000, Jacek Caban wrote:
Interesting. Maybe it's intended or something makes use of it, as we
already had tests exercising something similar (slightly above in the same file); I was confused at first too when I was changing it and had to make it work. I don't remember details, but from the context, it looks like I wrote this test when working on static binding optimizations and needed some tests for corner cases (this was a huge perf win, BTW, that's why I think it would be nice to extend it for more cases at some point). It seems that on top of those changes it should be possible to handle both modes so we may as well do it, but let's not complicate this MR.
I can certainly fix it for MSHTML, but I think the current arguments
buf should definitely stay, because it will be required for "strict mode" arguments object anyway, so it's not the "wrong way forward" at least (those are *always* detached from the local vars, so we could use the buf on initialization—not that I plan to implement it soon, though, just saying it will be needed at some point). Yes, strict mode arguments will always need it and will need to make a copy on entry (or at least before the first argument value modification). The nice thing about strict mode is that it will allow us to statically analyze code and know if arguments object will ever be used ahead of time, so it should be easy to optimize in the compiler and simply not create arguments at all when not needed.
Should I attempt the MSHTML fix in this MR or the follow up? Obviously
a separate commit. Follow up, please. This one is already tricky to review. I'm not yet sure about `scope_offs`. Typeinfo should not care about variables or scopes, only about arguments. It seems to me like it could replace `ref` eventually (or `ref` could be converted to what you need). Anyway, the patch is tricky to review and this part could be just skipped for now. Could you please try to move only arguments to the detached scope's buffer and leave variables for later?
I didn't dig too much into it, so there may be a way to use ref directly somehow. The way it allocates now it's the same as it sees variables in the code, so if you get a nested scope it will assign next refs there, and then have a "skip" for the original scope's refs (creating waste and so on if we use an array).
For the TypeInfo, AFAIK when I converted refs to store same order as scope_offs (except not resetting on each scope), a lot of the tests that cared about variable ordering with the typeinfo failed. Since it iterates through the variables and expects the order they're seen in the code. But that can probably be fixed in different way, I didn't look too much into it.
I'll make it arguments only.
-
Gabriel Ivăncescu -
Gabriel Ivăncescu (＠insn)
-
Jacek Caban (＠jacek)