You must not lookup return value for recursive call.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=50758 Signed-off-by: Robert Wilhelm robert.wilhelm@gmx.net --- dlls/vbscript/interp.c | 4 +++- dlls/vbscript/tests/lang.vbs | 6 ++++++ 2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/dlls/vbscript/interp.c b/dlls/vbscript/interp.c index f648f073bc8..18446ffb4a2 100644 --- a/dlls/vbscript/interp.c +++ b/dlls/vbscript/interp.c @@ -135,7 +135,9 @@ static HRESULT lookup_identifier(exec_ctx_t *ctx, BSTR name, vbdisp_invoke_type_ HRESULT hres;
if((ctx->func->type == FUNC_FUNCTION || ctx->func->type == FUNC_PROPGET) - && !wcsicmp(name, ctx->func->name)) { + && !wcsicmp(name, ctx->func->name) + /* you must not use return value for recursive call */ + && ((invoke_type != VBDISP_CALLGET) || ((invoke_type == VBDISP_CALLGET) && ( V_VT(&ctx->ret_val) == VT_DISPATCH)))) { ref->type = REF_VAR; ref->u.v = &ctx->ret_val; return S_OK; diff --git a/dlls/vbscript/tests/lang.vbs b/dlls/vbscript/tests/lang.vbs index d7865301784..cea34fa40a8 100644 --- a/dlls/vbscript/tests/lang.vbs +++ b/dlls/vbscript/tests/lang.vbs @@ -1882,6 +1882,12 @@ set arr(0) = new TestPropSyntax arr(0).prop = 1 ok arr(0).prop = 1, "arr(0) = " & arr(0).prop
+function recursingfunction(x) + if (x) then exit function + call recursingfunction(True) +end function +call recursingfunction(False) + function f2(x,y) end function
-- 2.31.1
Hi Robert,
Sorry for the delay.
On 9/16/21 10:25 PM, Robert Wilhelm wrote:
if((ctx->func->type == FUNC_FUNCTION || ctx->func->type == FUNC_PROPGET)
&& !wcsicmp(name, ctx->func->name)) {
&& !wcsicmp(name, ctx->func->name)/* you must not use return value for recursive call */&& ((invoke_type != VBDISP_CALLGET) || ((invoke_type == VBDISP_CALLGET) && ( V_VT(&ctx->ret_val) == VT_DISPATCH)))) {
I experimented a bit with this, see attached test. It seems that we should use the current function even if ret_val is modified and not an object.
Thanks,
Jacek
Hi,
While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=98606
Your paranoid android.
=== build (build log) ===
error: patch failed: dlls/vbscript/tests/lang.vbs:1884 Task: Patch failed to apply
=== debiant2 (build log) ===
error: patch failed: dlls/vbscript/tests/lang.vbs:1884 Task: Patch failed to apply
=== debiant2 (build log) ===
error: patch failed: dlls/vbscript/tests/lang.vbs:1884 Task: Patch failed to apply
Hi Jacek,
thank you for the review and the testcase.
It seems it cannot be solved in interpreter alone, because bytecode for y=f and y=f() is same in following example.
I would be glad on advice how to fix this...
Thanks, Robert
function f y = f ' y =Empty y = f() ' recursive call end function call f()
00f4:trace:vbscript_disas:dump_code 1: icallv L"f" 0 00f4:trace:vbscript_disas:dump_code 2: catch 2 0 00f4:trace:vbscript_disas:dump_code 3: ret 00f4:trace:vbscript_disas:dump_code 4: icall L"f" 0 00f4:trace:vbscript_disas:dump_code 5: assign_ident L"y" 0 00f4:trace:vbscript_disas:dump_code 6: catch 6 0 00f4:trace:vbscript_disas:dump_code 7: icall L"f" 0 00f4:trace:vbscript_disas:dump_code 8: assign_ident L"y" 0 00f4:trace:vbscript_disas:dump_code 9: catch 9 0 00f4:trace:vbscript_disas:dump_code 10: ret
On Thu, 2021-09-23 at 14:39 +0200, Jacek Caban wrote:
Hi Robert,
Sorry for the delay.
On 9/16/21 10:25 PM, Robert Wilhelm wrote:
if((ctx->func->type == FUNC_FUNCTION || ctx->func->type == FUNC_PROPGET) - && !wcsicmp(name, ctx->func->name)) { + && !wcsicmp(name, ctx->func->name) + /* you must not use return value for recursive call */ + && ((invoke_type != VBDISP_CALLGET) || ((invoke_type == VBDISP_CALLGET) && ( V_VT(&ctx->ret_val) == VT_DISPATCH)))) {
I experimented a bit with this, see attached test. It seems that we should use the current function even if ret_val is modified and not an object.
Thanks,
Jacek
Hi Robert,
On 9/24/21 6:02 PM, Robert Wilhelm wrote:
Hi Jacek,
thank you for the review and the testcase.
It seems it cannot be solved in interpreter alone, because bytecode for y=f and y=f() is same in following example.
I would be glad on advice how to fix this...
Something like the attached patch should take care of required compiler changes. The interpreter part just replicates the existing behaviour, but it should be possible to implement proper behaviour on top of that.
Thanks,
Jacek
Hi Jacek,
On Mon, 2021-09-27 at 17:22 +0200, Jacek Caban wrote:
Something like the attached patch should take care of required compiler changes. The interpreter part just replicates the existing behaviour, but it should be possible to implement proper behaviour on top of that.
Thanks for the patch. For my testcases bytecode looks right with new "ident" op. I will try to make interpreter changes, add testcase and submit it later this week.
Robert
Patch by Jacek Caban.
Signed-off-by: Robert Wilhelm robert.wilhelm@gmx.net --- dlls/vbscript/compile.c | 21 ++++++++++++++++++--- dlls/vbscript/interp.c | 29 ++++++++++++++++++++++++----- dlls/vbscript/vbscript.h | 1 + 3 files changed, 43 insertions(+), 8 deletions(-)
diff --git a/dlls/vbscript/compile.c b/dlls/vbscript/compile.c index 821d85310b6..da56eac74b2 100644 --- a/dlls/vbscript/compile.c +++ b/dlls/vbscript/compile.c @@ -446,7 +446,8 @@ static HRESULT compile_args(compile_ctx_t *ctx, expression_t *args, unsigned *re return S_OK; }
-static HRESULT compile_member_expression(compile_ctx_t *ctx, member_expression_t *expr, unsigned arg_cnt, BOOL ret_val) +static HRESULT compile_member_call_expression(compile_ctx_t *ctx, member_expression_t *expr, + unsigned arg_cnt, BOOL ret_val) { HRESULT hres;
@@ -471,6 +472,20 @@ static HRESULT compile_member_expression(compile_ctx_t *ctx, member_expression_t return hres; }
+static HRESULT compile_member_expression(compile_ctx_t *ctx, member_expression_t *expr) +{ + expression_t *const_expr; + + if (expr->obj_expr) /* FIXME: we should probably have a dedicated opcode as well */ + return compile_member_call_expression(ctx, expr, 0, TRUE); + + const_expr = lookup_const_decls(ctx, expr->identifier, TRUE); + if(const_expr) + return compile_expression(ctx, const_expr); + + return push_instr_bstr(ctx, OP_ident, expr->identifier); +} + static HRESULT compile_call_expression(compile_ctx_t *ctx, call_expression_t *expr, BOOL ret_val) { unsigned arg_cnt = 0; @@ -484,7 +499,7 @@ static HRESULT compile_call_expression(compile_ctx_t *ctx, call_expression_t *ex for(call = expr->call_expr; call->type == EXPR_BRACKETS; call = ((unary_expression_t*)call)->subexpr);
if(call->type == EXPR_MEMBER) - return compile_member_expression(ctx, (member_expression_t*)call, arg_cnt, ret_val); + return compile_member_call_expression(ctx, (member_expression_t*)call, arg_cnt, ret_val);
hres = compile_expression(ctx, call); if(FAILED(hres)) @@ -582,7 +597,7 @@ static HRESULT compile_expression(compile_ctx_t *ctx, expression_t *expr) case EXPR_ME: return push_instr(ctx, OP_me) ? S_OK : E_OUTOFMEMORY; case EXPR_MEMBER: - return compile_member_expression(ctx, (member_expression_t*)expr, 0, TRUE); + return compile_member_expression(ctx, (member_expression_t*)expr); case EXPR_MOD: return compile_binary_expression(ctx, (binary_expression_t*)expr, OP_mod); case EXPR_MUL: diff --git a/dlls/vbscript/interp.c b/dlls/vbscript/interp.c index f648f073bc8..e2c1d5cc53d 100644 --- a/dlls/vbscript/interp.c +++ b/dlls/vbscript/interp.c @@ -615,10 +615,8 @@ static HRESULT variant_call(exec_ctx_t *ctx, VARIANT *v, unsigned arg_cnt, VARIA return S_OK; }
-static HRESULT do_icall(exec_ctx_t *ctx, VARIANT *res) +static HRESULT do_icall(exec_ctx_t *ctx, VARIANT *res, BSTR identifier, unsigned arg_cnt) { - BSTR identifier = ctx->instr->arg1.bstr; - const unsigned arg_cnt = ctx->instr->arg2.uint; DISPPARAMS dp; ref_t ref; HRESULT hres; @@ -687,12 +685,14 @@ static HRESULT do_icall(exec_ctx_t *ctx, VARIANT *res)
static HRESULT interp_icall(exec_ctx_t *ctx) { + BSTR identifier = ctx->instr->arg1.bstr; + const unsigned arg_cnt = ctx->instr->arg2.uint; VARIANT v; HRESULT hres;
TRACE("\n");
- hres = do_icall(ctx, &v); + hres = do_icall(ctx, &v, identifier, arg_cnt); if(FAILED(hres)) return hres;
@@ -701,8 +701,12 @@ static HRESULT interp_icall(exec_ctx_t *ctx)
static HRESULT interp_icallv(exec_ctx_t *ctx) { + BSTR identifier = ctx->instr->arg1.bstr; + const unsigned arg_cnt = ctx->instr->arg2.uint; + TRACE("\n"); - return do_icall(ctx, NULL); + + return do_icall(ctx, NULL, identifier, arg_cnt); }
static HRESULT interp_vcall(exec_ctx_t *ctx) @@ -788,6 +792,21 @@ static HRESULT interp_mcallv(exec_ctx_t *ctx) return do_mcall(ctx, NULL); }
+static HRESULT interp_ident(exec_ctx_t *ctx) +{ + BSTR identifier = ctx->instr->arg1.bstr; + VARIANT v; + HRESULT hres; + + TRACE("%s\n", debugstr_w(identifier)); + + hres = do_icall(ctx, &v, identifier, 0); + if(FAILED(hres)) + return hres; + + return stack_push(ctx, &v); +} + static HRESULT assign_value(exec_ctx_t *ctx, VARIANT *dst, VARIANT *src, WORD flags) { VARIANT value; diff --git a/dlls/vbscript/vbscript.h b/dlls/vbscript/vbscript.h index 9ef2cae81e5..f5353b33cae 100644 --- a/dlls/vbscript/vbscript.h +++ b/dlls/vbscript/vbscript.h @@ -242,6 +242,7 @@ typedef enum { X(gteq, 1, 0, 0) \ X(icall, 1, ARG_BSTR, ARG_UINT) \ X(icallv, 1, ARG_BSTR, ARG_UINT) \ + X(ident, 1, ARG_BSTR, 0) \ X(idiv, 1, 0, 0) \ X(imp, 1, 0, 0) \ X(incc, 1, ARG_BSTR, 0) \ -- 2.31.1
Patch by Jacek Caban.
Signed-off-by: Robert Wilhelm robert.wilhelm@gmx.net --- v2: no changes. --- dlls/vbscript/compile.c | 21 ++++++++++++++++++--- dlls/vbscript/interp.c | 29 ++++++++++++++++++++++++----- dlls/vbscript/vbscript.h | 1 + 3 files changed, 43 insertions(+), 8 deletions(-)
diff --git a/dlls/vbscript/compile.c b/dlls/vbscript/compile.c index 821d85310b6..da56eac74b2 100644 --- a/dlls/vbscript/compile.c +++ b/dlls/vbscript/compile.c @@ -446,7 +446,8 @@ static HRESULT compile_args(compile_ctx_t *ctx, expression_t *args, unsigned *re return S_OK; }
-static HRESULT compile_member_expression(compile_ctx_t *ctx, member_expression_t *expr, unsigned arg_cnt, BOOL ret_val) +static HRESULT compile_member_call_expression(compile_ctx_t *ctx, member_expression_t *expr, + unsigned arg_cnt, BOOL ret_val) { HRESULT hres;
@@ -471,6 +472,20 @@ static HRESULT compile_member_expression(compile_ctx_t *ctx, member_expression_t return hres; }
+static HRESULT compile_member_expression(compile_ctx_t *ctx, member_expression_t *expr) +{ + expression_t *const_expr; + + if (expr->obj_expr) /* FIXME: we should probably have a dedicated opcode as well */ + return compile_member_call_expression(ctx, expr, 0, TRUE); + + const_expr = lookup_const_decls(ctx, expr->identifier, TRUE); + if(const_expr) + return compile_expression(ctx, const_expr); + + return push_instr_bstr(ctx, OP_ident, expr->identifier); +} + static HRESULT compile_call_expression(compile_ctx_t *ctx, call_expression_t *expr, BOOL ret_val) { unsigned arg_cnt = 0; @@ -484,7 +499,7 @@ static HRESULT compile_call_expression(compile_ctx_t *ctx, call_expression_t *ex for(call = expr->call_expr; call->type == EXPR_BRACKETS; call = ((unary_expression_t*)call)->subexpr);
if(call->type == EXPR_MEMBER) - return compile_member_expression(ctx, (member_expression_t*)call, arg_cnt, ret_val); + return compile_member_call_expression(ctx, (member_expression_t*)call, arg_cnt, ret_val);
hres = compile_expression(ctx, call); if(FAILED(hres)) @@ -582,7 +597,7 @@ static HRESULT compile_expression(compile_ctx_t *ctx, expression_t *expr) case EXPR_ME: return push_instr(ctx, OP_me) ? S_OK : E_OUTOFMEMORY; case EXPR_MEMBER: - return compile_member_expression(ctx, (member_expression_t*)expr, 0, TRUE); + return compile_member_expression(ctx, (member_expression_t*)expr); case EXPR_MOD: return compile_binary_expression(ctx, (binary_expression_t*)expr, OP_mod); case EXPR_MUL: diff --git a/dlls/vbscript/interp.c b/dlls/vbscript/interp.c index f648f073bc8..e2c1d5cc53d 100644 --- a/dlls/vbscript/interp.c +++ b/dlls/vbscript/interp.c @@ -615,10 +615,8 @@ static HRESULT variant_call(exec_ctx_t *ctx, VARIANT *v, unsigned arg_cnt, VARIA return S_OK; }
-static HRESULT do_icall(exec_ctx_t *ctx, VARIANT *res) +static HRESULT do_icall(exec_ctx_t *ctx, VARIANT *res, BSTR identifier, unsigned arg_cnt) { - BSTR identifier = ctx->instr->arg1.bstr; - const unsigned arg_cnt = ctx->instr->arg2.uint; DISPPARAMS dp; ref_t ref; HRESULT hres; @@ -687,12 +685,14 @@ static HRESULT do_icall(exec_ctx_t *ctx, VARIANT *res)
static HRESULT interp_icall(exec_ctx_t *ctx) { + BSTR identifier = ctx->instr->arg1.bstr; + const unsigned arg_cnt = ctx->instr->arg2.uint; VARIANT v; HRESULT hres;
TRACE("\n");
- hres = do_icall(ctx, &v); + hres = do_icall(ctx, &v, identifier, arg_cnt); if(FAILED(hres)) return hres;
@@ -701,8 +701,12 @@ static HRESULT interp_icall(exec_ctx_t *ctx)
static HRESULT interp_icallv(exec_ctx_t *ctx) { + BSTR identifier = ctx->instr->arg1.bstr; + const unsigned arg_cnt = ctx->instr->arg2.uint; + TRACE("\n"); - return do_icall(ctx, NULL); + + return do_icall(ctx, NULL, identifier, arg_cnt); }
static HRESULT interp_vcall(exec_ctx_t *ctx) @@ -788,6 +792,21 @@ static HRESULT interp_mcallv(exec_ctx_t *ctx) return do_mcall(ctx, NULL); }
+static HRESULT interp_ident(exec_ctx_t *ctx) +{ + BSTR identifier = ctx->instr->arg1.bstr; + VARIANT v; + HRESULT hres; + + TRACE("%s\n", debugstr_w(identifier)); + + hres = do_icall(ctx, &v, identifier, 0); + if(FAILED(hres)) + return hres; + + return stack_push(ctx, &v); +} + static HRESULT assign_value(exec_ctx_t *ctx, VARIANT *dst, VARIANT *src, WORD flags) { VARIANT value; diff --git a/dlls/vbscript/vbscript.h b/dlls/vbscript/vbscript.h index 9ef2cae81e5..f5353b33cae 100644 --- a/dlls/vbscript/vbscript.h +++ b/dlls/vbscript/vbscript.h @@ -242,6 +242,7 @@ typedef enum { X(gteq, 1, 0, 0) \ X(icall, 1, ARG_BSTR, ARG_UINT) \ X(icallv, 1, ARG_BSTR, ARG_UINT) \ + X(ident, 1, ARG_BSTR, 0) \ X(idiv, 1, 0, 0) \ X(imp, 1, 0, 0) \ X(incc, 1, ARG_BSTR, 0) \ -- 2.31.1
Patch by Jacek Caban.
Signed-off-by: Robert Wilhelm robert.wilhelm@gmx.net --- v2: no changes. v3: no changes. --- dlls/vbscript/compile.c | 21 ++++++++++++++++++--- dlls/vbscript/interp.c | 29 ++++++++++++++++++++++++----- dlls/vbscript/vbscript.h | 1 + 3 files changed, 43 insertions(+), 8 deletions(-)
diff --git a/dlls/vbscript/compile.c b/dlls/vbscript/compile.c index 821d85310b6..da56eac74b2 100644 --- a/dlls/vbscript/compile.c +++ b/dlls/vbscript/compile.c @@ -446,7 +446,8 @@ static HRESULT compile_args(compile_ctx_t *ctx, expression_t *args, unsigned *re return S_OK; }
-static HRESULT compile_member_expression(compile_ctx_t *ctx, member_expression_t *expr, unsigned arg_cnt, BOOL ret_val) +static HRESULT compile_member_call_expression(compile_ctx_t *ctx, member_expression_t *expr, + unsigned arg_cnt, BOOL ret_val) { HRESULT hres;
@@ -471,6 +472,20 @@ static HRESULT compile_member_expression(compile_ctx_t *ctx, member_expression_t return hres; }
+static HRESULT compile_member_expression(compile_ctx_t *ctx, member_expression_t *expr) +{ + expression_t *const_expr; + + if (expr->obj_expr) /* FIXME: we should probably have a dedicated opcode as well */ + return compile_member_call_expression(ctx, expr, 0, TRUE); + + const_expr = lookup_const_decls(ctx, expr->identifier, TRUE); + if(const_expr) + return compile_expression(ctx, const_expr); + + return push_instr_bstr(ctx, OP_ident, expr->identifier); +} + static HRESULT compile_call_expression(compile_ctx_t *ctx, call_expression_t *expr, BOOL ret_val) { unsigned arg_cnt = 0; @@ -484,7 +499,7 @@ static HRESULT compile_call_expression(compile_ctx_t *ctx, call_expression_t *ex for(call = expr->call_expr; call->type == EXPR_BRACKETS; call = ((unary_expression_t*)call)->subexpr);
if(call->type == EXPR_MEMBER) - return compile_member_expression(ctx, (member_expression_t*)call, arg_cnt, ret_val); + return compile_member_call_expression(ctx, (member_expression_t*)call, arg_cnt, ret_val);
hres = compile_expression(ctx, call); if(FAILED(hres)) @@ -582,7 +597,7 @@ static HRESULT compile_expression(compile_ctx_t *ctx, expression_t *expr) case EXPR_ME: return push_instr(ctx, OP_me) ? S_OK : E_OUTOFMEMORY; case EXPR_MEMBER: - return compile_member_expression(ctx, (member_expression_t*)expr, 0, TRUE); + return compile_member_expression(ctx, (member_expression_t*)expr); case EXPR_MOD: return compile_binary_expression(ctx, (binary_expression_t*)expr, OP_mod); case EXPR_MUL: diff --git a/dlls/vbscript/interp.c b/dlls/vbscript/interp.c index f648f073bc8..e2c1d5cc53d 100644 --- a/dlls/vbscript/interp.c +++ b/dlls/vbscript/interp.c @@ -615,10 +615,8 @@ static HRESULT variant_call(exec_ctx_t *ctx, VARIANT *v, unsigned arg_cnt, VARIA return S_OK; }
-static HRESULT do_icall(exec_ctx_t *ctx, VARIANT *res) +static HRESULT do_icall(exec_ctx_t *ctx, VARIANT *res, BSTR identifier, unsigned arg_cnt) { - BSTR identifier = ctx->instr->arg1.bstr; - const unsigned arg_cnt = ctx->instr->arg2.uint; DISPPARAMS dp; ref_t ref; HRESULT hres; @@ -687,12 +685,14 @@ static HRESULT do_icall(exec_ctx_t *ctx, VARIANT *res)
static HRESULT interp_icall(exec_ctx_t *ctx) { + BSTR identifier = ctx->instr->arg1.bstr; + const unsigned arg_cnt = ctx->instr->arg2.uint; VARIANT v; HRESULT hres;
TRACE("\n");
- hres = do_icall(ctx, &v); + hres = do_icall(ctx, &v, identifier, arg_cnt); if(FAILED(hres)) return hres;
@@ -701,8 +701,12 @@ static HRESULT interp_icall(exec_ctx_t *ctx)
static HRESULT interp_icallv(exec_ctx_t *ctx) { + BSTR identifier = ctx->instr->arg1.bstr; + const unsigned arg_cnt = ctx->instr->arg2.uint; + TRACE("\n"); - return do_icall(ctx, NULL); + + return do_icall(ctx, NULL, identifier, arg_cnt); }
static HRESULT interp_vcall(exec_ctx_t *ctx) @@ -788,6 +792,21 @@ static HRESULT interp_mcallv(exec_ctx_t *ctx) return do_mcall(ctx, NULL); }
+static HRESULT interp_ident(exec_ctx_t *ctx) +{ + BSTR identifier = ctx->instr->arg1.bstr; + VARIANT v; + HRESULT hres; + + TRACE("%s\n", debugstr_w(identifier)); + + hres = do_icall(ctx, &v, identifier, 0); + if(FAILED(hres)) + return hres; + + return stack_push(ctx, &v); +} + static HRESULT assign_value(exec_ctx_t *ctx, VARIANT *dst, VARIANT *src, WORD flags) { VARIANT value; diff --git a/dlls/vbscript/vbscript.h b/dlls/vbscript/vbscript.h index 9ef2cae81e5..f5353b33cae 100644 --- a/dlls/vbscript/vbscript.h +++ b/dlls/vbscript/vbscript.h @@ -242,6 +242,7 @@ typedef enum { X(gteq, 1, 0, 0) \ X(icall, 1, ARG_BSTR, ARG_UINT) \ X(icallv, 1, ARG_BSTR, ARG_UINT) \ + X(ident, 1, ARG_BSTR, 0) \ X(idiv, 1, 0, 0) \ X(imp, 1, 0, 0) \ X(incc, 1, ARG_BSTR, 0) \ -- 2.31.1
-
Jacek Caban -
Marvin -
Robert Wilhelm