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 | 64 ++++++++++++++++++++++++++++------- dlls/vbscript/interp.c | 15 +++++++- dlls/vbscript/tests/error.vbs | 8 ++--- dlls/vbscript/tests/lang.vbs | 41 ++++++++++++++++++++++ dlls/vbscript/vbscript_defs.h | 1 + 5 files changed, 111 insertions(+), 18 deletions(-) diff --git a/dlls/vbscript/compile.c b/dlls/vbscript/compile.c index 8c82d849a1b..73cb484a614 100644 --- a/dlls/vbscript/compile.c +++ b/dlls/vbscript/compile.c @@ -864,7 +864,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, instr; + unsigned step_instr, instr, expr_err_label, past_err_label, body_label; BSTR identifier; HRESULT hres; @@ -872,23 +872,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; - instr_ptr(ctx, instr)->arg1.bstr = identifier; - instr_ptr(ctx, instr)->arg2.uint = 0; + body_label = alloc_label(ctx); + if(!body_label) + return E_OUTOFMEMORY; + + /* 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; @@ -905,6 +905,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; @@ -918,11 +937,12 @@ static HRESULT compile_forto_statement(compile_ctx_t *ctx, forto_statement_t *st if(!emit_catch(ctx, 2)) return E_OUTOFMEMORY; + label_set_addr(ctx, body_label); + hres = compile_statement(ctx, &loop_ctx, stat->body); if(FAILED(hres)) return hres; - /* FIXME: Error handling can't be done compatible with native using OP_incc here. */ instr = push_instr(ctx, OP_incc); if(!instr) return E_OUTOFMEMORY; @@ -938,10 +958,28 @@ static HRESULT compile_forto_statement(compile_ctx_t *ctx, forto_statement_t *st label_set_addr(ctx, loop_ctx.for_end_label); - /* FIXME: reconsider after OP_incc fixup. */ 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 d8f0f80dfaa..20308411ffd 100644 --- a/dlls/vbscript/interp.c +++ b/dlls/vbscript/interp.c @@ -1454,6 +1454,19 @@ static HRESULT interp_step(exec_ctx_t *ctx) TRACE("%s\n", debugstr_w(ident)); + /* 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; hres = VarCmp(stack_top(ctx, 0), &zero, ctx->script->lcid, 0); @@ -2405,7 +2418,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 1ffe88318e9..45c831888cd 100644 --- a/dlls/vbscript/tests/error.vbs +++ b/dlls/vbscript/tests/error.vbs @@ -209,19 +209,19 @@ sub testThrow next call ok(y = 1, "y = " & y) call ok(x = 6, "x = " & x) - call todo_wine_ok(Err.Number = VB_E_FORLOOPNOTINITIALIZED, "Err.Number = " & Err.Number) + call ok(Err.Number = VB_E_FORLOOPNOTINITIALIZED, "Err.Number = " & Err.Number) Err.clear() y = 0 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 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 c0d22606ba6..8f409e89356 100644 --- a/dlls/vbscript/tests/lang.vbs +++ b/dlls/vbscript/tests/lang.vbs @@ -840,6 +840,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 diff --git a/dlls/vbscript/vbscript_defs.h b/dlls/vbscript/vbscript_defs.h index 06d3b8990fb..e5c896f9158 100644 --- a/dlls/vbscript/vbscript_defs.h +++ b/dlls/vbscript/vbscript_defs.h @@ -252,6 +252,7 @@ #define VBSE_PATH_FILE_ACCESS 75 #define VBSE_PATH_NOT_FOUND 76 #define VBSE_OBJECT_VARIABLE_NOT_SET 91 +#define VBSE_FOR_LOOP_NOT_INITIALIZED 92 #define VBSE_ILLEGAL_NULL_USE 94 #define VBSE_CANT_CREATE_TMP_FILE 322 #define VBSE_OBJECT_REQUIRED 424 -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10500