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.
-- v5: 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: Use the scope's dispex as the default variable obj. 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
Signed-off-by: Gabriel Ivăncescu gabrielopcode@gmail.com --- dlls/jscript/engine.c | 23 ++++++++++++----------- dlls/jscript/engine.h | 4 ++-- 2 files changed, 14 insertions(+), 13 deletions(-)
diff --git a/dlls/jscript/engine.c b/dlls/jscript/engine.c index ceaa01b0648..d5659d55f17 100644 --- a/dlls/jscript/engine.c +++ b/dlls/jscript/engine.c @@ -430,7 +430,7 @@ static void scope_destructor(jsdisp_t *dispex) if(scope->next) scope_release(scope->next);
- if(scope->obj) + if(scope->obj && scope->obj != to_disp(&scope->dispex)) IDispatch_Release(scope->obj); free(scope); } @@ -451,12 +451,14 @@ static HRESULT scope_gc_traverse(struct gc_ctx *gc_ctx, enum gc_traverse_op op, IDispatch *obj = scope->obj; if(obj) { scope->obj = NULL; - IDispatch_Release(obj); + if(obj != to_disp(&scope->dispex)) + IDispatch_Release(obj); } return S_OK; }
- return scope->obj && (jsobj = to_jsdisp(scope->obj)) ? gc_process_linked_obj(gc_ctx, op, dispex, jsobj, (void**)&scope->obj) : S_OK; + return scope->obj && scope->obj != to_disp(&scope->dispex) && (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 = { @@ -637,8 +639,7 @@ static HRESULT detach_scope(script_ctx_t *ctx, call_frame_t *frame, scope_chain_
if (!scope->obj) { - if (FAILED(hres = create_object(ctx, NULL, &jsobj))) - return hres; + jsobj = &scope->dispex; scope->obj = to_disp(jsobj); } else @@ -953,8 +954,7 @@ static HRESULT scope_init_locals(script_ctx_t *ctx) } else if (!scope->obj) { - if (FAILED(hres = create_object(ctx, NULL, &jsobj))) - return hres; + jsobj = &scope->dispex; scope->obj = to_disp(jsobj); } else @@ -3263,7 +3263,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, to_disp(variable_object), &scope); + hres = scope_push(ctx, scope_chain, variable_object ? to_disp(variable_object) : NULL, &scope); if(FAILED(hres)) { stack_popn(ctx, ctx->stack_top - orig_stack); return hres; @@ -3345,8 +3345,7 @@ 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); }else if(!(flags & (EXEC_GLOBAL | EXEC_EVAL))) { - hres = create_dispex(ctx, NULL, NULL, &variable_obj); - if(FAILED(hres)) return hres; + variable_obj = NULL; /* set to scope's dispex later */ }else if(bytecode->named_item) { variable_obj = jsdisp_addref(bytecode->named_item->script_obj); }else { @@ -3415,12 +3414,14 @@ HRESULT exec_source(script_ctx_t *ctx, DWORD flags, bytecode_t *bytecode, functi frame->bytecode = bytecode_addref(bytecode);
if(!(flags & (EXEC_GLOBAL|EXEC_EVAL))) { - hres = setup_scope(ctx, frame, scope, variable_obj, argc, argv); + hres = setup_scope(ctx, frame, scope, NULL, argc, argv); if(FAILED(hres)) { release_bytecode(frame->bytecode); free(frame); goto fail; } + variable_obj = jsdisp_addref(&frame->scope->dispex); + frame->scope->obj = to_disp(variable_obj); }else if(scope) { frame->base_scope = frame->scope = scope_addref(scope); } diff --git a/dlls/jscript/engine.h b/dlls/jscript/engine.h index 9b29373a0da..b6f26232059 100644 --- a/dlls/jscript/engine.h +++ b/dlls/jscript/engine.h @@ -223,8 +223,8 @@ 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) */ - IDispatch *obj; + jsdisp_t dispex; + IDispatch *obj; /* Does not hold ref when it's set to scope's dispex (prevents cyclic ref for a very common case) */ unsigned int scope_index; struct _call_frame_t *frame; struct _scope_chain_t *next;
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 d5659d55f17..0b044076676 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))) { variable_obj = NULL; /* set to scope's dispex later */ }else if(bytecode->named_item) { @@ -3396,12 +3400,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/engine.c | 229 ++++++++++++++++++++++++++++++++++++---- dlls/jscript/engine.h | 10 ++ dlls/jscript/function.c | 42 +++----- 4 files changed, 234 insertions(+), 52 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/engine.c b/dlls/jscript/engine.c index 0b044076676..7149293c170 100644 --- a/dlls/jscript/engine.c +++ b/dlls/jscript/engine.c @@ -162,6 +162,31 @@ 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); + WCHAR name_idx[11]; + + if(!local_ref) + return DISP_E_UNKNOWNNAME; + + swprintf(name_idx, ARRAY_SIZE(name_idx), L"%u", local_ref - scope->detached_locals->local); + return jsdisp_get_id(&scope->dispex, name_idx, 0, id); +} + /* Steals input reference even on failure. */ static HRESULT stack_push_exprval(script_ctx_t *ctx, exprval_t *val) { @@ -228,6 +253,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 +460,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 +487,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)) @@ -468,9 +561,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 };
@@ -624,10 +717,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;
@@ -637,6 +762,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) { jsobj = &scope->dispex; @@ -650,16 +781,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; } @@ -782,6 +930,10 @@ static HRESULT identifier_eval(script_ctx_t *ctx, BSTR identifier, exprval_t *re ret->u.val = jsval_obj(jsdisp_addref(scope->frame->function_instance)); return S_OK; } + }else if((hres = get_detached_local_dispid(scope, identifier, &id)) != DISP_E_UNKNOWNNAME) { + if(SUCCEEDED(hres)) + exprval_set_disp_ref(ret, to_disp(&scope->dispex), id); + return hres; }
if (!scope->obj) @@ -960,6 +1112,14 @@ static HRESULT scope_init_locals(script_ctx_t *ctx) else jsobj = as_jsdisp(scope->obj);
+ if (detached_vars) + { + assert(!scope->detached_locals); + hres = alloc_detached_locals(ctx, frame, scope); + if (FAILED(hres)) + return hres; + } + for(i = 0; i < frame->function->local_scopes[index].locals_cnt; i++) { WCHAR *name = frame->function->local_scopes[index].locals[i].name; @@ -975,10 +1135,20 @@ static HRESULT scope_init_locals(script_ctx_t *ctx) + frame->function->variables[ref].func_id, ctx->call_ctx->scope, &func_obj))) return hres; val = jsval_obj(func_obj); - if (detached_vars && FAILED(hres = jsdisp_propput_name(frame->variable_obj, name, jsval_obj(func_obj)))) + if (detached_vars) { - jsdisp_release(func_obj); - return hres; + jsval_t *base_ref; + + if (frame->base_scope && (base_ref = get_detached_local_ref(frame->base_scope, name))) + { + jsval_release(*base_ref); + *base_ref = jsval_obj(jsdisp_addref(func_obj)); + } + else if (FAILED(hres = jsdisp_propput_name(frame->variable_obj, name, jsval_obj(func_obj)))) + { + jsdisp_release(func_obj); + return hres; + } } } else @@ -988,7 +1158,10 @@ static HRESULT scope_init_locals(script_ctx_t *ctx)
if (detached_vars) { - hres = jsdisp_propput_name(jsobj, name, val); + if (scope->detached_locals) + hres = jsval_copy(val, &scope->detached_locals->local[scope->detached_locals->var_offs + frame->function->variables[ref].scope_offs]); + else + hres = jsdisp_propput_name(jsobj, name, val); jsval_release(val); if (FAILED(hres)) return hres; @@ -3297,6 +3470,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 +3517,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)) @@ -3364,6 +3539,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) { @@ -3375,8 +3551,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; }
@@ -3387,9 +3570,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 b6f26232059..2454b0abdc3 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; IDispatch *obj; /* Does not hold ref when it's set to scope's dispex (prevents cyclic ref for a very common case) */ 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 {
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)) {
New version stores detached locals buffer into the scope itself, but only when needed to avoid waste for non-detached scopes. It first stores the args, followed by the local vars (which are offset by var_offs), so index 0 into the buffer is first arg, and `var_offs + 0` is the first local var.
I had to introduce a new index accessing for the local scope vars. `ref` can't be used since it indexes them based on where they're found in the code, including other scopes, and we can't change that since stuff like the TypeInfo relies on it (and tests would fail). Instead, I introduced `scope_offs` as an index of a given var into the scope's detached locals.
I also made use of the scope's dispex now instead of allocating the default variable object, because we're already using it, and there's no reason to have *two* objects just for some couple extra props like `arguments` or the function's name in ES5, which feels too much like a waste. We don't keep a ref to it in this special case though, to prevent cyclic refs in the very common case, so it can be freed immediately instead of waiting for the garbage collector.
I also made use of the scope's dispex now instead of allocating the default variable object, because we're already using it, and there's no reason to have *two* objects just for some couple extra props like `arguments` or the function's name in ES5, which feels too much like a waste.
We shouldn't need variable object for function name. `identifier_eval` already has handing for that, I would suggest to make it work for detached scope (probably by storing function instance in the buffer) and remove its setting from `detach_scope`. It even uses `EXPRVAL_JSVAL`, so there is no problem with addressing that.
arguments object is similar: we already special-case it in `identifier_eval`. We could probably just reserve the first or last slot of scope indexes.
Also, I would suggest introducing a helper like `dispex_get_idx_dispid` that would return DISPID for given index.
On Fri Jun 16 13:03:45 2023 +0000, Jacek Caban wrote:
I also made use of the scope's dispex now instead of allocating the
default variable object, because we're already using it, and there's no reason to have *two* objects just for some couple extra props like `arguments` or the function's name in ES5, which feels too much like a waste. We shouldn't need variable object for function name. `identifier_eval` already has handing for that, I would suggest to make it work for detached scope (probably by storing function instance in the buffer) and remove its setting from `detach_scope`. It even uses `EXPRVAL_JSVAL`, so there is no problem with addressing that. arguments object is similar: we already special-case it in `identifier_eval`. We could probably just reserve the first or last slot of scope indexes. Also, I would suggest introducing a helper like `dispex_get_idx_dispid` that would return DISPID for given index.
What about functions in inner scopes exposed back to the base scope? (the last jsdisp_propput_name in detach_scope). Won't we still need variable obj for them, since they can be names not found in the base scope's locals at all?
And if we still need it then I think special casing those two isn't worth it. Unless you have an idea how to handle it in acceptable way?
On Fri Jun 16 14:55:24 2023 +0000, Gabriel Ivăncescu wrote:
What about functions in inner scopes exposed back to the base scope? (the last jsdisp_propput_name in detach_scope). Won't we still need variable obj for them, since they can be names not found in the base scope's locals at all? And if we still need it then I think special casing those two isn't worth it. Unless you have an idea how to handle it in acceptable way?
Unless I'm missing something, we currently need function-named property in variable object only because `identifier_eval` can't handle it otherwise for detached scopes. But if we make `identifier_eval` able to handle it for detached cases, there is no need to involve variable object.
BTW, I know that it's quite a large side quest for what you intended to fix and it's fine if you'd limit the scope only to arguments. I'm mentioning it because, due to above, the patch using scope object as variable object looks questionable to me, but you don't really need that part for the original bug.
On Fri Jun 16 16:08:09 2023 +0000, Jacek Caban wrote:
Unless I'm missing something, we currently need function-named property in variable object only because `identifier_eval` can't handle it otherwise for detached scopes. But if we make `identifier_eval` able to handle it for detached cases, there is no need to involve variable object. BTW, I know that it's quite a large side quest for what you intended to fix and it's fine if you'd limit the scope only to arguments. I'm mentioning it because, due to above, the patch using scope object as variable object looks questionable to me, but you don't really need that part for the original bug.
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`:
```C 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?
-
Gabriel Ivăncescu -
Gabriel Ivăncescu (＠insn)
-
Jacek Caban (＠jacek)