[PATCH v4 0/3] MR10783: vbscript: Match native semantics for empty parens on non-callable values
Native VBScript distinguishes bare member access from with-parens access and rejects the latter on non-callable values: - `obj.scalarProp()` / `obj.arrayProp()` on a class instance variant property — error 5 (Invalid procedure call) - `x()` on a Dim scalar variable — error 13 (Type mismatch) - `arr()` on a Dim array — error 9 (Subscript out of range) -- v4: vbscript: Delegate do_icall is_call branch to variant_call. vbscript: Raise illegal-func-call on empty parens of class variant property. vbscript/tests: Cover empty parens on properties, Dim variables, and arrays. https://gitlab.winehq.org/wine/wine/-/merge_requests/10783
From: Francis De Brabandere <francisdb@gmail.com> Native VBScript distinguishes bare member access from with-parens access and rejects the latter on non-callable values: 5 (Invalid procedure call) on a class variant property, 13 (Type mismatch) on a Dim scalar, and 9 (Subscript out of range) on a Dim array. Wine currently returns the value silently in all three cases; mark with todo_wine pending the fix. --- dlls/vbscript/tests/lang.vbs | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/dlls/vbscript/tests/lang.vbs b/dlls/vbscript/tests/lang.vbs index 3a3a668e58b..7357658e0db 100644 --- a/dlls/vbscript/tests/lang.vbs +++ b/dlls/vbscript/tests/lang.vbs @@ -2186,6 +2186,19 @@ Call ok(Err.number = 438, "obj.publicProp(0, 1) Err.number = " & Err.number) Err.Clear obj.publicProp("k") = 5 Call ok(Err.number = 438, "obj.publicProp(""k"") = 5 Err.number = " & Err.number) + +' Empty parens on a variant-typed public property: native raises 5 on get +' (not callable). Set with empty parens succeeds: the source-level () adds +' no positional args, so DISPATCH_PROPERTYPUT receives just the value. +Err.Clear +x = obj.publicProp() +Call todo_wine_ok(Err.number = 5, "obj.publicProp() Err.number = " & Err.number) +Err.Clear +x = obj.publicArrayProp() +Call todo_wine_ok(Err.number = 5, "obj.publicArrayProp() Err.number = " & Err.number) +Err.Clear +obj.publicProp() = 7 +Call ok(Err.number = 0, "obj.publicProp() = 7 Err.number = " & Err.number) Err.Clear On Error Goto 0 @@ -3177,6 +3190,29 @@ sub test_index_non_array x(0, 1) = 1 call ok(err.number = 13, "assign int(0,1): err.number = " & err.number) + ' empty parens on a non-array Dim variable: native raises 13. + x = 42 + err.clear + tmp = x() + call todo_wine_ok(err.number = 13, "read int(): err.number = " & err.number) + + x = "hello" + err.clear + tmp = x() + call todo_wine_ok(err.number = 13, "read str(): err.number = " & err.number) + + x = Empty + err.clear + tmp = x() + call todo_wine_ok(err.number = 13, "read empty(): err.number = " & err.number) + + ' empty parens on a Dim array: native raises 9 (subscript out of range, + ' since () supplies zero indices for a multi-dim access). + Dim arr(2) + err.clear + tmp = arr() + call todo_wine_ok(err.number = 9, "read arr(): err.number = " & err.number) + on error goto 0 end sub call test_index_non_array -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10783
From: Francis De Brabandere <francisdb@gmail.com> Native VBScript raises error 5 (Invalid procedure call or argument) when a class instance's variant-typed public property is read with empty parens (e.g. obj.scalarProp()). Wine previously returned the value as if the parens weren't present. Distinguish bare member access from with-parens member access at the compiler level: compile_member_expression now emits a new OP_mget opcode for bare obj.x, while obj.x() still goes through OP_mcall as before. Plumb a BOOL is_call flag through disp_call -> invoke_vbdisp -> invoke_variant_prop so that the runtime can raise error 5 on the script-internal call form. External IDispatchEx::InvokeEx callers go through the existing path with is_call=FALSE and continue to receive the value. The set path (obj.scalarProp() = 7) is unaffected: the source-level parens add no positional args to the put DISPPARAMS. --- dlls/vbscript/compile.c | 9 ++++++-- dlls/vbscript/global.c | 2 +- dlls/vbscript/interp.c | 40 +++++++++++++++++++++++++++++++----- dlls/vbscript/tests/lang.vbs | 4 ++-- dlls/vbscript/vbdisp.c | 24 ++++++++++++++-------- dlls/vbscript/vbscript.h | 3 ++- 6 files changed, 62 insertions(+), 20 deletions(-) diff --git a/dlls/vbscript/compile.c b/dlls/vbscript/compile.c index dd07d7b0782..3c347f72686 100644 --- a/dlls/vbscript/compile.c +++ b/dlls/vbscript/compile.c @@ -543,9 +543,14 @@ static HRESULT compile_member_call_expression(compile_ctx_t *ctx, member_express static HRESULT compile_member_expression(compile_ctx_t *ctx, member_expression_t *expr) { expression_t *const_expr; + HRESULT hres; - if (expr->obj_expr) /* FIXME: we should probably have a dedicated opcode as well */ - return compile_member_call_expression(ctx, expr, 0, TRUE); + if (expr->obj_expr) { + hres = compile_expression(ctx, expr->obj_expr); + if(FAILED(hres)) + return hres; + return push_instr_bstr(ctx, OP_mget, expr->identifier); + } if (!lookup_dim_decls(ctx, expr->identifier) && !lookup_args_name(ctx, expr->identifier)) { const_expr = lookup_const_decls(ctx, expr->identifier, TRUE); diff --git a/dlls/vbscript/global.c b/dlls/vbscript/global.c index bb6d4ab038d..bd949f7b370 100644 --- a/dlls/vbscript/global.c +++ b/dlls/vbscript/global.c @@ -4521,7 +4521,7 @@ static HRESULT dispatch_to_string(script_ctx_t *ctx, IDispatch *disp, BSTR *ret) if(!disp) return MAKE_VBSERROR(VBSE_OBJECT_VARIABLE_NOT_SET); - hres = disp_call(ctx, disp, DISPID_VALUE, &dp, &v); + hres = disp_call(ctx, disp, DISPID_VALUE, TRUE, &dp, &v); if(FAILED(hres)) return hres; if(V_VT(&v) == VT_BSTR) { diff --git a/dlls/vbscript/interp.c b/dlls/vbscript/interp.c index 76ea15a6e2e..d82c2cd96a7 100644 --- a/dlls/vbscript/interp.c +++ b/dlls/vbscript/interp.c @@ -640,7 +640,7 @@ static HRESULT variant_call(exec_ctx_t *ctx, VARIANT *v, unsigned arg_cnt, VARIA break; case VT_DISPATCH: vbstack_to_dp(ctx, arg_cnt, FALSE, &dp); - hres = disp_call(ctx->script, V_DISPATCH(v), DISPID_VALUE, &dp, res); + hres = disp_call(ctx->script, V_DISPATCH(v), DISPID_VALUE, TRUE, &dp, res); stack_popn(ctx, arg_cnt); return hres; default: @@ -710,7 +710,7 @@ static HRESULT do_icall(exec_ctx_t *ctx, VARIANT *res, BSTR identifier, unsigned break; case REF_DISP: vbstack_to_dp(ctx, arg_cnt, FALSE, &dp); - hres = disp_call(ctx->script, ref.u.d.disp, ref.u.d.id, &dp, res); + hres = disp_call(ctx->script, ref.u.d.disp, ref.u.d.id, is_call, &dp, res); if(FAILED(hres)) return hres; break; @@ -723,7 +723,7 @@ static HRESULT do_icall(exec_ctx_t *ctx, VARIANT *res, BSTR identifier, unsigned case REF_OBJ: if(arg_cnt) { vbstack_to_dp(ctx, arg_cnt, FALSE, &dp); - hres = disp_call(ctx->script, ref.u.obj, DISPID_VALUE, &dp, res); + hres = disp_call(ctx->script, ref.u.obj, DISPID_VALUE, TRUE, &dp, res); if(FAILED(hres)) return hres; break; @@ -847,7 +847,7 @@ static HRESULT do_mcall(exec_ctx_t *ctx, VARIANT *res) hres = disp_get_id(obj, identifier, VBDISP_CALLGET, FALSE, &id); if(SUCCEEDED(hres)) - hres = disp_call(ctx->script, obj, id, &dp, res); + hres = disp_call(ctx->script, obj, id, TRUE, &dp, res); IDispatch_Release(obj); if(FAILED(hres)) return hres; @@ -877,6 +877,36 @@ static HRESULT interp_mcallv(exec_ctx_t *ctx) return do_mcall(ctx, NULL); } +static HRESULT interp_mget(exec_ctx_t *ctx) +{ + const BSTR identifier = ctx->instr->arg1.bstr; + DISPPARAMS dp = {0}; + IDispatch *obj; + VARIANT res; + DISPID id; + HRESULT hres; + + TRACE("\n"); + + hres = stack_pop_disp(ctx, &obj); + if(FAILED(hres)) + return hres; + + if(!obj) { + WARN("NULL obj\n"); + return MAKE_VBSERROR(VBSE_OBJECT_REQUIRED); + } + + hres = disp_get_id(obj, identifier, VBDISP_CALLGET, FALSE, &id); + if(SUCCEEDED(hres)) + hres = disp_call(ctx->script, obj, id, FALSE, &dp, &res); + IDispatch_Release(obj); + if(FAILED(hres)) + return hres; + + return stack_push(ctx, &res); +} + static HRESULT interp_ident(exec_ctx_t *ctx) { BSTR identifier = ctx->instr->arg1.bstr; @@ -1723,7 +1753,7 @@ static HRESULT interp_newenum(exec_ctx_t *ctx) DISPPARAMS dp = {0}; VARIANT iterv; - hres = disp_call(ctx->script, V_ISBYREF(v.v) ? *V_DISPATCHREF(v.v) : V_DISPATCH(v.v), DISPID_NEWENUM, &dp, &iterv); + hres = disp_call(ctx->script, V_ISBYREF(v.v) ? *V_DISPATCHREF(v.v) : V_DISPATCH(v.v), DISPID_NEWENUM, TRUE, &dp, &iterv); release_val(&v); if(FAILED(hres)) return hres; diff --git a/dlls/vbscript/tests/lang.vbs b/dlls/vbscript/tests/lang.vbs index 7357658e0db..94a0949d529 100644 --- a/dlls/vbscript/tests/lang.vbs +++ b/dlls/vbscript/tests/lang.vbs @@ -2192,10 +2192,10 @@ Call ok(Err.number = 438, "obj.publicProp(""k"") = 5 Err.number = " & Err.number ' no positional args, so DISPATCH_PROPERTYPUT receives just the value. Err.Clear x = obj.publicProp() -Call todo_wine_ok(Err.number = 5, "obj.publicProp() Err.number = " & Err.number) +Call ok(Err.number = 5, "obj.publicProp() Err.number = " & Err.number) Err.Clear x = obj.publicArrayProp() -Call todo_wine_ok(Err.number = 5, "obj.publicArrayProp() Err.number = " & Err.number) +Call ok(Err.number = 5, "obj.publicArrayProp() Err.number = " & Err.number) Err.Clear obj.publicProp() = 7 Call ok(Err.number = 0, "obj.publicProp() = 7 Err.number = " & Err.number) diff --git a/dlls/vbscript/vbdisp.c b/dlls/vbscript/vbdisp.c index 90f3d18156b..f1d1bb3d4e9 100644 --- a/dlls/vbscript/vbdisp.c +++ b/dlls/vbscript/vbdisp.c @@ -162,7 +162,7 @@ static HRESULT get_array_from_variant(VARIANT *v, SAFEARRAY **array) } } -static HRESULT invoke_variant_prop(script_ctx_t *ctx, VARIANT *v, WORD flags, DISPPARAMS *dp, VARIANT *res) +static HRESULT invoke_variant_prop(script_ctx_t *ctx, VARIANT *v, WORD flags, BOOL is_call, DISPPARAMS *dp, VARIANT *res) { HRESULT hres; @@ -179,6 +179,12 @@ static HRESULT invoke_variant_prop(script_ctx_t *ctx, VARIANT *v, WORD flags, DI WARN("failed to access array element\n"); return hres; } + }else if(is_call && (flags & DISPATCH_METHOD)) { + /* Empty parens in script source on a variant-typed property: + * not callable. is_call is set only for script-internal call + * forms (`obj.x()`, etc.); bare access (`obj.x`) and external + * IDispatchEx callers still receive the value normally. */ + return MAKE_VBSERROR(VBSE_ILLEGAL_FUNC_CALL); } hres = VariantCopyInd(res, v); @@ -231,7 +237,7 @@ static HRESULT invoke_variant_prop(script_ctx_t *ctx, VARIANT *v, WORD flags, DI return hres; } -static HRESULT invoke_vbdisp(vbdisp_t *This, DISPID id, DWORD flags, BOOL extern_caller, DISPPARAMS *params, VARIANT *res) +static HRESULT invoke_vbdisp(vbdisp_t *This, DISPID id, DWORD flags, BOOL extern_caller, BOOL is_call, DISPPARAMS *params, VARIANT *res) { if(id < 0) return DISP_E_MEMBERNOTFOUND; @@ -315,7 +321,7 @@ static HRESULT invoke_vbdisp(vbdisp_t *This, DISPID id, DWORD flags, BOOL extern return DISP_E_MEMBERNOTFOUND; TRACE("%p->%s\n", This, debugstr_w(This->desc->props[id - This->desc->func_cnt].name)); - return invoke_variant_prop(This->desc->ctx, This->props+(id-This->desc->func_cnt), flags, params, res); + return invoke_variant_prop(This->desc->ctx, This->props+(id-This->desc->func_cnt), flags, is_call, params, res); } static BOOL run_terminator(vbdisp_t *This) @@ -489,7 +495,7 @@ static HRESULT WINAPI DispatchEx_InvokeEx(IDispatchEx *iface, DISPID id, LCID lc if(pspCaller) IServiceProvider_AddRef(pspCaller); - hres = invoke_vbdisp(This, id, wFlags, TRUE, pdp, pvarRes); + hres = invoke_vbdisp(This, id, wFlags, TRUE, FALSE, pdp, pvarRes); This->desc->ctx->vbcaller->caller = prev_caller; if(pspCaller) @@ -1659,7 +1665,7 @@ static HRESULT WINAPI ScriptDisp_InvokeEx(IDispatchEx *iface, DISPID id, LCID lc goto done; } - hres = invoke_variant_prop(This->ctx, &This->global_vars[id - 1]->v, wFlags, pdp, pvarRes); + hres = invoke_variant_prop(This->ctx, &This->global_vars[id - 1]->v, wFlags, FALSE, pdp, pvarRes); done: This->ctx->vbcaller->caller = prev_caller; @@ -1861,7 +1867,7 @@ void map_vbs_exception(EXCEPINFO *ei) ei->bstrDescription = get_vbscript_string(VBS_UNKNOWN_RUNTIME_ERROR); } -HRESULT disp_call(script_ctx_t *ctx, IDispatch *disp, DISPID id, DISPPARAMS *dp, VARIANT *retv) +HRESULT disp_call(script_ctx_t *ctx, IDispatch *disp, DISPID id, BOOL is_call, DISPPARAMS *dp, VARIANT *retv) { const WORD flags = DISPATCH_METHOD|(retv ? DISPATCH_PROPERTYGET : 0); IDispatchEx *dispex; @@ -1875,7 +1881,7 @@ HRESULT disp_call(script_ctx_t *ctx, IDispatch *disp, DISPID id, DISPPARAMS *dp, vbdisp = unsafe_impl_from_IDispatch(disp); if(vbdisp && vbdisp->desc && vbdisp->desc->ctx == ctx) - return invoke_vbdisp(vbdisp, id, flags, FALSE, dp, retv); + return invoke_vbdisp(vbdisp, id, flags, FALSE, is_call, dp, retv); hres = IDispatch_QueryInterface(disp, &IID_IDispatchEx, (void**)&dispex); if(SUCCEEDED(hres)) { @@ -1901,7 +1907,7 @@ HRESULT get_disp_value(script_ctx_t *ctx, IDispatch *disp, VARIANT *v) DISPPARAMS dp = {NULL}; if(!disp) return MAKE_VBSERROR(VBSE_OBJECT_VARIABLE_NOT_SET); - return disp_call(ctx, disp, DISPID_VALUE, &dp, v); + return disp_call(ctx, disp, DISPID_VALUE, TRUE, &dp, v); } HRESULT disp_propput(script_ctx_t *ctx, IDispatch *disp, DISPID id, WORD flags, DISPPARAMS *dp) @@ -1913,7 +1919,7 @@ HRESULT disp_propput(script_ctx_t *ctx, IDispatch *disp, DISPID id, WORD flags, vbdisp = unsafe_impl_from_IDispatch(disp); if(vbdisp && vbdisp->desc && vbdisp->desc->ctx == ctx) - return invoke_vbdisp(vbdisp, id, flags, FALSE, dp, NULL); + return invoke_vbdisp(vbdisp, id, flags, FALSE, FALSE, dp, NULL); hres = IDispatch_QueryInterface(disp, &IID_IDispatchEx, (void**)&dispex); if(SUCCEEDED(hres)) { diff --git a/dlls/vbscript/vbscript.h b/dlls/vbscript/vbscript.h index 7b8315748c9..eda5d8740b4 100644 --- a/dlls/vbscript/vbscript.h +++ b/dlls/vbscript/vbscript.h @@ -171,7 +171,7 @@ HRESULT create_vbdisp(const class_desc_t*,vbdisp_t**); HRESULT disp_get_id(IDispatch*,BSTR,vbdisp_invoke_type_t,BOOL,DISPID*); const WCHAR *vbdisp_class_name(IDispatch*); HRESULT vbdisp_get_id(vbdisp_t*,BSTR,vbdisp_invoke_type_t,BOOL,DISPID*); -HRESULT disp_call(script_ctx_t*,IDispatch*,DISPID,DISPPARAMS*,VARIANT*); +HRESULT disp_call(script_ctx_t*,IDispatch*,DISPID,BOOL,DISPPARAMS*,VARIANT*); HRESULT disp_propput(script_ctx_t*,IDispatch*,DISPID,WORD,DISPPARAMS*); HRESULT get_disp_value(script_ctx_t*,IDispatch*,VARIANT*); void collect_objects(script_ctx_t*); @@ -296,6 +296,7 @@ typedef enum { X(lteq, 1, ARG_UINT, 0) \ X(mcall, 1, ARG_BSTR, ARG_UINT) \ X(mcallv, 1, ARG_BSTR, ARG_UINT) \ + X(mget, 1, ARG_BSTR, 0) \ X(me, 1, 0, 0) \ X(mod, 1, 0, 0) \ X(mul, 1, 0, 0) \ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10783
From: Francis De Brabandere <francisdb@gmail.com> variant_call already handles arg_cnt=0 correctly for the three relevant cases (VT_DISPATCH invokes the default property via the new is_call plumbing in disp_call, an array fails with out-of-bounds, anything else returns type-mismatch), so the inline default-property handling from 02a15b1df6c can be collapsed into a single variant_call call. This brings non-array Dim variables and Dim arrays in line with native behavior: x() raises 13 (Type mismatch) and arr() raises 9 (Subscript out of range) instead of silently returning the value or array. --- dlls/vbscript/interp.c | 19 ++----------------- dlls/vbscript/tests/lang.vbs | 8 ++++---- 2 files changed, 6 insertions(+), 21 deletions(-) diff --git a/dlls/vbscript/interp.c b/dlls/vbscript/interp.c index d82c2cd96a7..219b4fdefda 100644 --- a/dlls/vbscript/interp.c +++ b/dlls/vbscript/interp.c @@ -682,23 +682,8 @@ static HRESULT do_icall(exec_ctx_t *ctx, VARIANT *res, BSTR identifier, unsigned if(arg_cnt) return variant_call(ctx, ref.u.v, arg_cnt, res); - if(is_call) { - VARIANT *v; - - v = V_VT(ref.u.v) == (VT_VARIANT|VT_BYREF) ? V_VARIANTREF(ref.u.v) : ref.u.v; - if(V_VT(v) == VT_DISPATCH) { - VARIANT result; - V_VT(&result) = VT_EMPTY; - hres = get_disp_value(ctx->script, V_DISPATCH(v), &result); - if(FAILED(hres)) - return hres; - if(res) - *res = result; - else - VariantClear(&result); - break; - } - } + if(is_call) + return variant_call(ctx, ref.u.v, 0, res); if(!res) { WARN("REF_VAR no res\n"); diff --git a/dlls/vbscript/tests/lang.vbs b/dlls/vbscript/tests/lang.vbs index 94a0949d529..1b6cdf763e4 100644 --- a/dlls/vbscript/tests/lang.vbs +++ b/dlls/vbscript/tests/lang.vbs @@ -3194,24 +3194,24 @@ sub test_index_non_array x = 42 err.clear tmp = x() - call todo_wine_ok(err.number = 13, "read int(): err.number = " & err.number) + call ok(err.number = 13, "read int(): err.number = " & err.number) x = "hello" err.clear tmp = x() - call todo_wine_ok(err.number = 13, "read str(): err.number = " & err.number) + call ok(err.number = 13, "read str(): err.number = " & err.number) x = Empty err.clear tmp = x() - call todo_wine_ok(err.number = 13, "read empty(): err.number = " & err.number) + call ok(err.number = 13, "read empty(): err.number = " & err.number) ' empty parens on a Dim array: native raises 9 (subscript out of range, ' since () supplies zero indices for a multi-dim access). Dim arr(2) err.clear tmp = arr() - call todo_wine_ok(err.number = 9, "read arr(): err.number = " & err.number) + call ok(err.number = 9, "read arr(): err.number = " & err.number) on error goto 0 end sub -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10783
This merge request was approved by Jacek Caban. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10783
participants (3)
-
Francis De Brabandere -
Francis De Brabandere (＠francisdb)
-
Jacek Caban (＠jacek)