From: Francis De Brabandere <francisdb@gmail.com> On Windows, when a For loop expression (from, to, or step) fails with On Error Resume Next, the control variable is not modified and error 92 (For loop not initialized) is set at the next iteration. Wine previously assigned the from value before evaluating to/step. Restructure compile_forto_statement to evaluate all three expressions before assigning to the control variable. On expression failure, error recovery enters the body with empty sentinel values so that OP_step detects the uninitialized state and sets error 92. Also return error 92 from OP_incc when VarAdd fails. --- dlls/vbscript/compile.c | 83 ++++++++++++++++++++++++++++------- dlls/vbscript/interp.c | 18 ++++++-- dlls/vbscript/tests/error.vbs | 10 ++--- dlls/vbscript/tests/lang.vbs | 58 ++++++++++++++++++++++++ 4 files changed, 145 insertions(+), 24 deletions(-) diff --git a/dlls/vbscript/compile.c b/dlls/vbscript/compile.c index 3777624c6d4..b2113aee4c4 100644 --- a/dlls/vbscript/compile.c +++ b/dlls/vbscript/compile.c @@ -870,8 +870,8 @@ static HRESULT compile_foreach_statement(compile_ctx_t *ctx, foreach_statement_t static HRESULT compile_forto_statement(compile_ctx_t *ctx, forto_statement_t *stat) { - statement_ctx_t loop_ctx = {2}; - unsigned step_instr, loop_start, instr; + statement_ctx_t loop_ctx = {3}; + unsigned step_instr, instr, expr_err_label, past_err_label, body_label, from_offset; BSTR identifier; HRESULT hres; @@ -879,23 +879,32 @@ static HRESULT compile_forto_statement(compile_ctx_t *ctx, forto_statement_t *st if(!identifier) return E_OUTOFMEMORY; + expr_err_label = alloc_label(ctx); + if(!expr_err_label) + return E_OUTOFMEMORY; + + past_err_label = alloc_label(ctx); + if(!past_err_label) + return E_OUTOFMEMORY; + + body_label = alloc_label(ctx); + if(!body_label) + return E_OUTOFMEMORY; + + /* Evaluate all three expressions (from, to, step) before assignment, + * so that the control variable is not modified if any expression fails. + * Stack layout after evaluation: [from, to, step] (step on top). */ + from_offset = stack_offset(ctx); + hres = compile_expression(ctx, stat->from_expr); if(FAILED(hres)) return hres; if(!push_instr(ctx, OP_numval)) return E_OUTOFMEMORY; - /* FIXME: Assign should happen after both expressions evaluation. */ - instr = push_instr(ctx, OP_assign_ident); - if(!instr) - return E_OUTOFMEMORY; - instr_ptr(ctx, instr)->arg1.bstr = identifier; - instr_ptr(ctx, instr)->arg2.uint = 0; - hres = compile_expression(ctx, stat->to_expr); if(FAILED(hres)) return hres; - if(!push_instr(ctx, OP_numval)) return E_OUTOFMEMORY; @@ -912,6 +921,24 @@ static HRESULT compile_forto_statement(compile_ctx_t *ctx, forto_statement_t *st return hres; } + /* If any expression failed, clean up and enter body with empty sentinels. */ + if(!emit_catch_jmp(ctx, 0, expr_err_label)) + return E_OUTOFMEMORY; + + /* Copy from (buried at depth 2) to top and assign to control variable. */ + hres = push_instr_uint(ctx, OP_stack, from_offset); + if(FAILED(hres)) + return hres; + + instr = push_instr(ctx, OP_assign_ident); + if(!instr) + return E_OUTOFMEMORY; + instr_ptr(ctx, instr)->arg1.bstr = identifier; + instr_ptr(ctx, instr)->arg2.uint = 0; + + if(!emit_catch_jmp(ctx, 0, expr_err_label)) + return E_OUTOFMEMORY; + loop_ctx.for_end_label = alloc_label(ctx); if(!loop_ctx.for_end_label) return E_OUTOFMEMORY; @@ -922,16 +949,16 @@ static HRESULT compile_forto_statement(compile_ctx_t *ctx, forto_statement_t *st instr_ptr(ctx, step_instr)->arg2.bstr = identifier; instr_ptr(ctx, step_instr)->arg1.uint = loop_ctx.for_end_label; - if(!emit_catch(ctx, 2)) + if(!emit_catch(ctx, 3)) return E_OUTOFMEMORY; - loop_start = ctx->instr_cnt; + label_set_addr(ctx, body_label); + hres = compile_statement(ctx, &loop_ctx, stat->body); if(FAILED(hres)) return hres; - /* We need a separated OP_step here so that errors jump to the end-of-loop catch. */ - ctx->loc = stat->stat.loc; + instr = push_instr(ctx, OP_incc); if(!instr) return E_OUTOFMEMORY; @@ -943,7 +970,11 @@ static HRESULT compile_forto_statement(compile_ctx_t *ctx, forto_statement_t *st instr_ptr(ctx, instr)->arg2.bstr = identifier; instr_ptr(ctx, instr)->arg1.uint = loop_ctx.for_end_label; - hres = push_instr_addr(ctx, OP_jmp, loop_start); + hres = push_instr_addr(ctx, OP_jmp, step_instr); + if(FAILED(hres)) + return hres; + + hres = push_instr_uint(ctx, OP_pop, 3); if(FAILED(hres)) return hres; @@ -952,6 +983,28 @@ static HRESULT compile_forto_statement(compile_ctx_t *ctx, forto_statement_t *st if(!emit_catch(ctx, 0)) return E_OUTOFMEMORY; + hres = push_instr_addr(ctx, OP_jmp, past_err_label); + if(FAILED(hres)) + return hres; + + /* Expression error path: push empty from/to/step sentinels and enter body. */ + label_set_addr(ctx, expr_err_label); + + if(!push_instr(ctx, OP_empty)) + return E_OUTOFMEMORY; + + if(!push_instr(ctx, OP_empty)) + return E_OUTOFMEMORY; + + if(!push_instr(ctx, OP_empty)) + return E_OUTOFMEMORY; + + hres = push_instr_addr(ctx, OP_jmp, body_label); + if(FAILED(hres)) + return hres; + + label_set_addr(ctx, past_err_label); + return S_OK; } diff --git a/dlls/vbscript/interp.c b/dlls/vbscript/interp.c index 2a73a7fb51a..58be3bde0a2 100644 --- a/dlls/vbscript/interp.c +++ b/dlls/vbscript/interp.c @@ -1548,8 +1548,18 @@ static HRESULT interp_step(exec_ctx_t *ctx) TRACE("%s\n", debugstr_w(ident)); - if(V_VT(stack_top(ctx, 0)) == VT_EMPTY || V_VT(stack_top(ctx, 1)) == VT_EMPTY) - return MAKE_VBSERROR(VBSE_FOR_LOOP_NOT_INITIALIZED); + /* If to and step are VT_EMPTY, the For loop was not properly initialized + * (expression evaluation failed during On Error Resume Next). Set error 92 + * and exit the loop. */ + if(V_VT(stack_top(ctx, 0)) == VT_EMPTY && V_VT(stack_top(ctx, 1)) == VT_EMPTY) { + WARN("For loop not initialized\n"); + clear_ei(&ctx->script->ei); + ctx->script->ei.scode = MAKE_VBSERROR(VBSE_FOR_LOOP_NOT_INITIALIZED); + map_vbs_exception(&ctx->script->ei); + stack_popn(ctx, 3); + instr_jmp(ctx, ctx->instr->arg1.uint); + return S_OK; + } V_VT(&zero) = VT_I2; V_I2(&zero) = 0; @@ -1575,7 +1585,7 @@ static HRESULT interp_step(exec_ctx_t *ctx) if(hres == VARCMP_EQ || hres == (gteq_zero ? VARCMP_LT : VARCMP_GT)) { ctx->instr++; }else { - stack_popn(ctx, 2); + stack_popn(ctx, 3); instr_jmp(ctx, ctx->instr->arg1.uint); } return S_OK; @@ -2502,7 +2512,7 @@ static HRESULT interp_incc(exec_ctx_t *ctx) hres = VarAdd(stack_top(ctx, 0), ref.u.v, &v); if(FAILED(hres)) - return hres; + return MAKE_VBSERROR(VBSE_FOR_LOOP_NOT_INITIALIZED); VariantClear(ref.u.v); *ref.u.v = v; diff --git a/dlls/vbscript/tests/error.vbs b/dlls/vbscript/tests/error.vbs index ce08bcec67a..fa9ca6b7902 100644 --- a/dlls/vbscript/tests/error.vbs +++ b/dlls/vbscript/tests/error.vbs @@ -216,11 +216,11 @@ sub testThrow x = 6 for x = 100 to throwInt(E_TESTERROR) call ok(Err.Number = E_TESTERROR, "Err.Number = " & Err.Number) - call todo_wine_ok(x = 6, "x = " & x) + call ok(x = 6, "x = " & x) y = y+1 next call ok(y = 1, "y = " & y) - call todo_wine_ok(x = 6, "x = " & x) + call ok(x = 6, "x = " & x) call ok(Err.Number = VB_E_FORLOOPNOTINITIALIZED, "Err.Number = " & Err.Number) Err.clear() @@ -228,12 +228,12 @@ sub testThrow x = 6 for x = 100 to 200 step throwInt(E_TESTERROR) call ok(Err.Number = E_TESTERROR, "Err.Number = " & Err.Number) - call todo_wine_ok(x = 6, "x = " & x) + call ok(x = 6, "x = " & x) y = y+1 next call ok(y = 1, "y = " & y) - call todo_wine_ok(x = 6, "x = " & x) - call todo_wine_ok(Err.Number = VB_E_FORLOOPNOTINITIALIZED, "Err.Number = " & Err.Number) + call ok(x = 6, "x = " & x) + call ok(Err.Number = VB_E_FORLOOPNOTINITIALIZED, "Err.Number = " & Err.Number) select case throwInt(E_TESTERROR) case true diff --git a/dlls/vbscript/tests/lang.vbs b/dlls/vbscript/tests/lang.vbs index ed483602176..09d96743fba 100644 --- a/dlls/vbscript/tests/lang.vbs +++ b/dlls/vbscript/tests/lang.vbs @@ -855,6 +855,64 @@ do while true next loop +' For loop control variable should not be modified when expression evaluation fails +on error resume next + +x = 6 +y = 0 +err.clear +for x = 1/0 to 100 + y = y + 1 +next +call ok(err.number = 92, "for (from error): err.number = " & err.number) +call ok(x = 6, "for (from error): x = " & x) + +x = 6 +y = 0 +err.clear +for x = 100 to 1/0 + y = y + 1 +next +call ok(err.number = 92, "for (to error): err.number = " & err.number) +call ok(x = 6, "for (to error): x = " & x) + +x = 6 +y = 0 +err.clear +for x = 100 to 200 step 1/0 + y = y + 1 +next +call ok(err.number = 92, "for (step error): err.number = " & err.number) +call ok(x = 6, "for (step error): x = " & x) + +z = 99 +y = 0 +err.clear +for z = 1 to UBound(empty) + y = y + 1 +next +call ok(err.number = 92, "for (UBound(empty)): err.number = " & err.number) +call ok(z = 99, "for (UBound(empty)): z = " & z) + +on error goto 0 + +' For loop expression evaluation order: from, to, step +dim eval_order +function trackEval(val, label) + eval_order = eval_order & label + trackEval = val +end function + +eval_order = "" +for x = trackEval(1, "F") to trackEval(5, "T") step trackEval(1, "S") +next +call ok(eval_order = "FTS", "for eval order = " & eval_order) + +eval_order = "" +for x = trackEval(1, "F") to trackEval(5, "T") +next +call ok(eval_order = "FT", "for eval order (no step) = " & eval_order) + if null then call ok(false, "if null evaluated") while null -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10500