[PATCH v4 0/1] MR10380: vbscript: Return error 92 for uninitialized For loops.
Emit two OP_step instructions in for-to loops (matching the for-each pattern with two OP_enumnext), so that errors in the second OP_step are caught by the end-of-loop handler and exit the loop instead of re-entering the body. Convert VT_EMPTY to VT_I2(0) in OP_numval so that legitimate Empty values become numeric while error-handler-padded VT_EMPTY values remain distinguishable. Detect VT_EMPTY step or limit values in OP_step and return MAKE_VBSERROR(VBSE_FOR_LOOP_NOT_INITIALIZED) (error 92). -- v4: vbscript: Return error 92 for uninitialized For loops. https://gitlab.winehq.org/wine/wine/-/merge_requests/10380
From: Francis De Brabandere <francisdb@gmail.com> Emit two OP_step instructions in for-to loops (matching the for-each pattern with two OP_enumnext), so that errors in the second OP_step are caught by the end-of-loop handler and exit the loop instead of re-entering the body. Convert VT_EMPTY to VT_I2(0) in OP_numval so that legitimate Empty values become numeric while error-handler-padded VT_EMPTY values remain distinguishable. Detect VT_EMPTY step or limit values in OP_step and return MAKE_VBSERROR(VBSE_FOR_LOOP_NOT_INITIALIZED) (error 92). --- dlls/vbscript/compile.c | 17 ++++++++++------- dlls/vbscript/interp.c | 10 ++++++++++ dlls/vbscript/tests/error.vbs | 14 +++++++++++++- dlls/vbscript/tests/lang.vbs | 15 +++++++++++++++ dlls/vbscript/vbscript.rc | 1 + dlls/vbscript/vbscript_defs.h | 1 + 6 files changed, 50 insertions(+), 8 deletions(-) diff --git a/dlls/vbscript/compile.c b/dlls/vbscript/compile.c index 8c82d849a1b..5b5801c7056 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, loop_start, instr; BSTR identifier; HRESULT hres; @@ -918,27 +918,30 @@ 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; 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. */ + /* 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; instr_ptr(ctx, instr)->arg1.bstr = identifier; - hres = push_instr_addr(ctx, OP_jmp, step_instr); - if(FAILED(hres)) - return hres; + instr = push_instr(ctx, OP_step); + if(!instr) + return E_OUTOFMEMORY; + instr_ptr(ctx, instr)->arg2.bstr = identifier; + instr_ptr(ctx, instr)->arg1.uint = loop_ctx.for_end_label; - hres = push_instr_uint(ctx, OP_pop, 2); + hres = push_instr_addr(ctx, OP_jmp, loop_start); if(FAILED(hres)) return hres; label_set_addr(ctx, loop_ctx.for_end_label); - /* FIXME: reconsider after OP_incc fixup. */ if(!emit_catch(ctx, 0)) return E_OUTOFMEMORY; diff --git a/dlls/vbscript/interp.c b/dlls/vbscript/interp.c index d8f0f80dfaa..009de839839 100644 --- a/dlls/vbscript/interp.c +++ b/dlls/vbscript/interp.c @@ -1120,6 +1120,13 @@ static HRESULT interp_numval(exec_ctx_t *ctx) if(FAILED(hres)) return hres; + if(V_VT(val.v) == VT_EMPTY) { + release_val(&val); + V_VT(&v) = VT_I2; + V_I2(&v) = 0; + return stack_push(ctx, &v); + } + if (V_VT(val.v) == VT_BSTR) { V_VT(&v) = VT_EMPTY; hres = VariantChangeType(&v, val.v, 0, VT_R8); @@ -1454,6 +1461,9 @@ 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); + V_VT(&zero) = VT_I2; V_I2(&zero) = 0; hres = VarCmp(stack_top(ctx, 0), &zero, ctx->script->lcid, 0); diff --git a/dlls/vbscript/tests/error.vbs b/dlls/vbscript/tests/error.vbs index 1ffe88318e9..ce08bcec67a 100644 --- a/dlls/vbscript/tests/error.vbs +++ b/dlls/vbscript/tests/error.vbs @@ -209,7 +209,7 @@ 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 @@ -221,6 +221,18 @@ sub testThrow next call ok(y = 1, "y = " & y) call todo_wine_ok(x = 6, "x = " & x) + call ok(Err.Number = VB_E_FORLOOPNOTINITIALIZED, "Err.Number = " & Err.Number) + + Err.clear() + y = 0 + 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) + 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) select case throwInt(E_TESTERROR) diff --git a/dlls/vbscript/tests/lang.vbs b/dlls/vbscript/tests/lang.vbs index 00807619d00..6b130f58265 100644 --- a/dlls/vbscript/tests/lang.vbs +++ b/dlls/vbscript/tests/lang.vbs @@ -809,6 +809,21 @@ if (isEnglishLang) then Call testfor ("1", "2.5", "0.5", 4) end if +' Empty is treated as 0 in for-to loop expressions +y = 0 +for x = empty to 3 + y = y + 1 +next +call ok(y = 4, "for empty to 3: y = " & y) +call ok(x = 4, "for empty to 3: x = " & x) + +y = 0 +for x = 0 to empty + y = y + 1 +next +call ok(y = 1, "for 0 to empty: y = " & y) +call ok(x = 1, "for 0 to empty: x = " & x) + for x = 1.5 to 1 Call ok(false, "for..to called when unexpected") next diff --git a/dlls/vbscript/vbscript.rc b/dlls/vbscript/vbscript.rc index a0f7381bcf0..6bac38bd756 100644 --- a/dlls/vbscript/vbscript.rc +++ b/dlls/vbscript/vbscript.rc @@ -41,6 +41,7 @@ STRINGTABLE VBSE_PATH_FILE_ACCESS "Path/File access error" VBSE_PATH_NOT_FOUND "Path not found" VBSE_OBJECT_VARIABLE_NOT_SET "Object variable not set" + VBSE_FOR_LOOP_NOT_INITIALIZED "For loop not initialized" VBSE_ILLEGAL_NULL_USE "Invalid use of Null" VBSE_CANT_CREATE_TMP_FILE "Can't create necessary temporary file" VBSE_OBJECT_REQUIRED "Object required" 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/10380
Jacek Caban (@jacek) commented about dlls/vbscript/interp.c:
if(FAILED(hres)) return hres;
+ if(V_VT(val.v) == VT_EMPTY) { + release_val(&val);
The patch looks mostly good to me, but `VT_EMPTY` does not need to be released, so `release_val` call here may be skipped. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10380#note_134603
participants (3)
-
Francis De Brabandere -
Francis De Brabandere (＠francisdb)
-
Jacek Caban (＠jacek)