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 16e661c98ff..3bfe91f836a 100644 --- a/dlls/vbscript/compile.c +++ b/dlls/vbscript/compile.c @@ -530,9 +530,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 b32104a4e6b..628a8ab6bae 100644 --- a/dlls/vbscript/global.c +++ b/dlls/vbscript/global.c @@ -4306,7 +4306,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 51099e67d0e..2dea36ccc33 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; @@ -841,7 +841,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; @@ -871,6 +871,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; @@ -1651,7 +1681,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 e7fd8aed852..ed3ddeeb44e 100644 --- a/dlls/vbscript/tests/lang.vbs +++ b/dlls/vbscript/tests/lang.vbs @@ -1800,10 +1800,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 5cf7b5ae596..1f3538a1928 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) @@ -1653,7 +1659,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; @@ -1855,7 +1861,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; @@ -1869,7 +1875,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)) { @@ -1895,7 +1901,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) @@ -1907,7 +1913,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 e9b76b6cda8..32510f2eb2a 100644 --- a/dlls/vbscript/vbscript.h +++ b/dlls/vbscript/vbscript.h @@ -170,7 +170,7 @@ typedef struct named_item_t { HRESULT create_vbdisp(const class_desc_t*,vbdisp_t**); HRESULT disp_get_id(IDispatch*,BSTR,vbdisp_invoke_type_t,BOOL,DISPID*); 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*); @@ -288,6 +288,7 @@ typedef enum { X(lteq, 1, 0, 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