[PATCH v4 0/1] MR10500: vbscript: Defer For loop control variable assignment until all expressions are evaluated.
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. -- v4: vbscript: Defer For loop control variable assignment until all expressions are evaluated. https://gitlab.winehq.org/wine/wine/-/merge_requests/10500
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 | 72 +++++++++++++++++++++++++++-------- dlls/vbscript/interp.c | 16 ++++++-- dlls/vbscript/tests/error.vbs | 10 ++--- dlls/vbscript/tests/lang.vbs | 41 ++++++++++++++++++++ 4 files changed, 116 insertions(+), 23 deletions(-) diff --git a/dlls/vbscript/compile.c b/dlls/vbscript/compile.c index 3777624c6d4..a934cdb59b1 100644 --- a/dlls/vbscript/compile.c +++ b/dlls/vbscript/compile.c @@ -871,7 +871,7 @@ 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; + unsigned step_instr, instr, expr_err_label, past_err_label, body_label; BSTR identifier; HRESULT hres; @@ -879,23 +879,23 @@ static HRESULT compile_forto_statement(compile_ctx_t *ctx, forto_statement_t *st if(!identifier) return E_OUTOFMEMORY; - hres = compile_expression(ctx, stat->from_expr); - if(FAILED(hres)) - return hres; - if(!push_instr(ctx, OP_numval)) + expr_err_label = alloc_label(ctx); + if(!expr_err_label) return E_OUTOFMEMORY; - /* FIXME: Assign should happen after both expressions evaluation. */ - instr = push_instr(ctx, OP_assign_ident); - if(!instr) + past_err_label = alloc_label(ctx); + if(!past_err_label) + return E_OUTOFMEMORY; + + body_label = alloc_label(ctx); + if(!body_label) return E_OUTOFMEMORY; - instr_ptr(ctx, instr)->arg1.bstr = identifier; - instr_ptr(ctx, instr)->arg2.uint = 0; + /* Evaluate all three expressions before assignment, so that + * the control variable is not modified if any expression fails. */ hres = compile_expression(ctx, stat->to_expr); if(FAILED(hres)) return hres; - if(!push_instr(ctx, OP_numval)) return E_OUTOFMEMORY; @@ -912,6 +912,25 @@ static HRESULT compile_forto_statement(compile_ctx_t *ctx, forto_statement_t *st return hres; } + hres = compile_expression(ctx, stat->from_expr); + if(FAILED(hres)) + return hres; + if(!push_instr(ctx, OP_numval)) + return E_OUTOFMEMORY; + + /* If any expression failed, clean up and enter body with empty to/step. */ + if(!emit_catch_jmp(ctx, 0, expr_err_label)) + return E_OUTOFMEMORY; + + 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; @@ -925,13 +944,13 @@ static HRESULT compile_forto_statement(compile_ctx_t *ctx, forto_statement_t *st if(!emit_catch(ctx, 2)) 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 +962,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, 2); if(FAILED(hres)) return hres; @@ -952,6 +975,25 @@ 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 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; + + 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..5934175968e 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, 2); + instr_jmp(ctx, ctx->instr->arg1.uint); + return S_OK; + } V_VT(&zero) = VT_I2; V_I2(&zero) = 0; @@ -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..ed145d4bab8 100644 --- a/dlls/vbscript/tests/lang.vbs +++ b/dlls/vbscript/tests/lang.vbs @@ -855,6 +855,47 @@ 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 + if null then call ok(false, "if null evaluated") while null -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10500
Jacek Caban (@jacek) commented about dlls/vbscript/compile.c:
if(!identifier) return E_OUTOFMEMORY;
- hres = compile_expression(ctx, stat->from_expr); - if(FAILED(hres)) - return hres; - if(!push_instr(ctx, OP_numval)) + expr_err_label = alloc_label(ctx); + if(!expr_err_label) return E_OUTOFMEMORY;
This changes the order in which those expressions are evaluated, which can potentially change the script’s behavior. Is that intentional? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10500#note_134965
On Fri Apr 3 18:58:25 2026 +0000, Jacek Caban wrote:
This changes the order in which those expressions are evaluated, which can potentially change the script’s behavior. Is that intentional? adding a test to validate evaluation order on windows
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10500#note_135054
participants (3)
-
Francis De Brabandere -
Francis De Brabandere (＠francisdb)
-
Jacek Caban (＠jacek)