[PATCH v13 0/4] MR10766: vbscript: Fix and expand test coverage for BSTR comparison.
Follow-up to !10314 ([Wine-Bug 56281](https://bugs.winehq.org/show_bug.cgi?id=56281)) aligning BSTR-vs-numeric and BSTR-vs-Boolean comparison with real Windows VBScript. The `VT_RESERVED` literal-marker mechanism is the documented `VarCmp` contract in Wine's `oleaut32/variant.c` (the existing comments at lines 2761–2769 describe how `VT_RESERVED` distinguishes string vs numeric paths in BSTR comparisons). -- v13: vbscript: Match Windows BSTR-vs-numeric and BSTR-vs-Boolean comparison. https://gitlab.winehq.org/wine/wine/-/merge_requests/10766
From: Francis De Brabandere <francisdb@gmail.com> Add assertions for locale-invariant numeric-string forms ("+5", "-0", "05", " 5", "5 ", "5e0") and for type-mismatch errors raised when an unparseable BSTR ("abc", "", " ") is compared to a number with =, <> or a relational operator. --- dlls/vbscript/tests/lang.vbs | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/dlls/vbscript/tests/lang.vbs b/dlls/vbscript/tests/lang.vbs index e0de413be93..b1bffe0ab45 100644 --- a/dlls/vbscript/tests/lang.vbs +++ b/dlls/vbscript/tests/lang.vbs @@ -136,13 +136,35 @@ Call ok(30 > "0", "30 > ""0"" should be true") Call ok(9 < "10", "9 < ""10"" should be true") Call ok(42 = "42", "42 = ""42"" should be true") Call ok(not ("10" > "9"), """10"" > ""9"" should be false (string comparison)") +' Locale-invariant numeric strings: parse to the same value as the literal in any locale +Call ok("+5" = 5, """+5"" = 5 should be true") +Call ok("-0" = 0, """-0"" = 0 should be true") +Call ok("05" = 5, """05"" = 5 should be true") +Call ok(" 5" = 5, """ 5"" = 5 should be true") +Call ok("5 " = 5, """5 "" = 5 should be true") +Call ok("5e0" = 5, """5e0"" = 5 should be true") ' String vs Boolean uses string comparison, not numeric conversion Call ok(not ("1" = True), """1"" = True should be false") Call ok(not ("-1" = True), """-1"" = True should be false") Call ok(not ("0" = False), """0"" = False should be false") -' Non-numeric string compared to number should raise type mismatch +' Non-numeric BSTR compared to number raises type mismatch (= / <> / relational) on error resume next err.clear +x = ("abc" = 5) +Call ok(err.number = 13, """abc"" = 5 err.number = " & err.number) +err.clear +x = ("" = 5) +Call ok(err.number = 13, """"" = 5 err.number = " & err.number) +err.clear +x = (" " = 0) +Call ok(err.number = 13, """ "" = 0 err.number = " & err.number) +err.clear +x = ("abc" <> 5) +Call ok(err.number = 13, """abc"" <> 5 err.number = " & err.number) +err.clear +x = ("" <> 5) +Call ok(err.number = 13, """"" <> 5 err.number = " & err.number) +err.clear x = ("abc" > 5) Call ok(err.number = 13, """abc"" > 5 err.number = " & err.number) err.clear -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10766
From: Francis De Brabandere <francisdb@gmail.com> ByRef parameter holding a BSTR still coerces to numeric for comparison ("5" = 5 is True). For VT_UI1/VT_CY operands, Windows string-compares against CStr(numeric) rather than numeric-coercing, so non-numeric BSTRs return False with no error and relational operators use lex order. Hex ("&hff") and scientific ("1e2") BSTRs parse as numeric. The VT_UI1/VT_CY divergences are marked todo_wine pending the var_cmp fix. --- dlls/vbscript/tests/lang.vbs | 48 ++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/dlls/vbscript/tests/lang.vbs b/dlls/vbscript/tests/lang.vbs index b1bffe0ab45..ee8152ce6f8 100644 --- a/dlls/vbscript/tests/lang.vbs +++ b/dlls/vbscript/tests/lang.vbs @@ -175,6 +175,54 @@ x = (5 > "abc") Call ok(err.number = 13, "5 > ""abc"" err.number = " & err.number) on error goto 0 +' BSTR coerces to numeric for comparison even when it carries VT_BYREF +' (e.g. ByRef parameter holding a string). +Sub TestByRefStrEq5(ByRef x) + Call ok(x = 5, "ByRef ""5"" = 5 should be true") + Call ok(x < 6, "ByRef ""5"" < 6 should be true") +End Sub +TestByRefStrEq5 "5" + +' BSTR vs each numeric VT VBScript can produce. +Call ok("5" = CByte(5), """5"" = CByte(5) should be true") +Call ok("5" = CInt(5), """5"" = CInt(5) should be true") +Call ok("5" = CLng(5), """5"" = CLng(5) should be true") +Call ok("5" = CSng(5), """5"" = CSng(5) should be true") +Call ok("5" = CDbl(5), """5"" = CDbl(5) should be true") +Call ok("5" = CCur(5), """5"" = CCur(5) should be true") +Call ok(CByte(5) = "5", "CByte(5) = ""5"" should be true") +Call ok(CCur(5) = "5", "CCur(5) = ""5"" should be true") + +' Hex / scientific BSTRs parse as numeric. +Call ok("1e2" = 100, """1e2"" = 100 should be true") +Call ok("&hff" = 255, """&hff"" = 255 should be true") +Call ok("&H1F" = 31, """&H1F"" = 31 should be true") + +' VT_UI1/VT_CY vs BSTR diverge from VT_I2: string-compare against CStr(numeric). +' Non-numeric BSTR returns False with NO error (VT_I2 raises 13 for the same). +on error resume next +err.clear +x = ("abc" = CByte(5)) +saved_err = err.number +err.clear +Call todo_wine_ok(saved_err = 0, """abc"" = CByte(5) should not raise error (err=" & saved_err & ")") +x = ("abc" = CCur(5)) +saved_err = err.number +err.clear +Call todo_wine_ok(saved_err = 0, """abc"" = CCur(5) should not raise error (err=" & saved_err & ")") +x = ("" = CByte(0)) +saved_err = err.number +err.clear +Call todo_wine_ok(saved_err = 0, """"" = CByte(0) should not raise error (err=" & saved_err & ")") +x = ("" = CCur(0)) +saved_err = err.number +err.clear +Call todo_wine_ok(saved_err = 0, """"" = CCur(0) should not raise error (err=" & saved_err & ")") +on error goto 0 +' Relational confirms lex compare: 10 > 5 numerically would be true; lex "10" < "5". +Call todo_wine_ok(not ("10" > CByte(5)), """10"" > CByte(5) should be false (lex)") +Call todo_wine_ok(not ("5" < CCur(10)), """5"" < CCur(10) should be false (lex)") + Call ok(getVT(false) = "VT_BOOL", "getVT(false) is not VT_BOOL") Call ok(getVT(true) = "VT_BOOL", "getVT(true) is not VT_BOOL") Call ok(getVT("") = "VT_BSTR", "getVT("""") is not VT_BSTR") -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10766
From: Francis De Brabandere <francisdb@gmail.com> Numeric literals raise type-mismatch (error 13) on unparseable BSTR; variables, arithmetic, C-coercion, function returns and parameters use string-compare instead. Cases needing the var_cmp rework are marked todo_wine. --- dlls/vbscript/tests/lang.vbs | 115 +++++++++++++++++++++++++++++++++++ 1 file changed, 115 insertions(+) diff --git a/dlls/vbscript/tests/lang.vbs b/dlls/vbscript/tests/lang.vbs index ee8152ce6f8..0e5462348b7 100644 --- a/dlls/vbscript/tests/lang.vbs +++ b/dlls/vbscript/tests/lang.vbs @@ -223,6 +223,121 @@ on error goto 0 Call todo_wine_ok(not ("10" > CByte(5)), """10"" > CByte(5) should be false (lex)") Call todo_wine_ok(not ("5" < CCur(10)), """5"" < CCur(10) should be false (lex)") +' --- BSTR vs numeric LITERAL: BSTR coerces to a number, parse failure raises 13. --- +Dim saved_err +on error resume next +err.clear +x = ("abc" = 5.5) : saved_err = err.number : err.clear +Call ok(saved_err = 13, "literal R8: ""abc"" = 5.5 should error 13 (got " & saved_err & ")") +x = ("abc" = 1e2) : saved_err = err.number : err.clear +Call ok(saved_err = 13, "literal sci: ""abc"" = 1e2 should error 13 (got " & saved_err & ")") +x = ("abc" = &hff) : saved_err = err.number : err.clear +Call ok(saved_err = 13, "literal hex: ""abc"" = &hff should error 13 (got " & saved_err & ")") +x = ("abc" = 100000) : saved_err = err.number : err.clear +Call ok(saved_err = 13, "literal I4: ""abc"" = 100000 should error 13 (got " & saved_err & ")") +x = ("abc" = #1/15/2024#): saved_err = err.number : err.clear +Call ok(saved_err = 13, "literal date: ""abc"" = #1/15/2024# should error 13 (got " & saved_err & ")") +on error goto 0 + +' --- Variable holding a numeric loses "literal" status: string compare, no error. --- +Dim n5 : n5 = 5 +Dim n10 : n10 = 10 +on error resume next +err.clear +x = ("abc" = n5) : saved_err = err.number : err.clear +on error goto 0 +Call todo_wine_ok(saved_err = 0, "var: ""abc"" = (n=5) should not raise (got " & saved_err & ")") +Call todo_wine_ok(not ("abc" = n5), "var: ""abc"" = (n=5) should be false") +Call todo_wine_ok(not ("010" = n10), "var: ""010"" = (n=10) should be false (string)") + +' --- Arithmetic / unary produce a fresh non-literal value. --- +on error resume next +err.clear +x = ("abc" = (5+0)) : saved_err = err.number : err.clear +on error goto 0 +Call todo_wine_ok(saved_err = 0, "arith: ""abc"" = (5+0) should not raise (got " & saved_err & ")") +Call todo_wine_ok(not ("010" = (5+5)), "arith: ""010"" = (5+5) should be false") +Call todo_wine_ok(not ("010" = (10*1)), "arith: ""010"" = (10*1) should be false") +on error resume next +err.clear +x = ("abc" = -5) : saved_err = err.number : err.clear +on error goto 0 +Call todo_wine_ok(saved_err = 0, "neg: ""abc"" = -5 should not raise (got " & saved_err & ")") + +' --- C-coercion functions return non-literal values. --- +Call todo_wine_ok(not ("010" = CInt(10)), "CInt: ""010"" = CInt(10) should be false") +Call todo_wine_ok(not ("010" = CLng(10)), "CLng: ""010"" = CLng(10) should be false") +Call todo_wine_ok(not ("010" = CSng(10)), "CSng: ""010"" = CSng(10) should be false") +Call todo_wine_ok(not ("010" = CDbl(10)), "CDbl: ""010"" = CDbl(10) should be false") +on error resume next +err.clear +x = ("abc" = CInt(5)) : saved_err = err.number : err.clear +Call todo_wine_ok(saved_err = 0, "CInt: ""abc"" = CInt(5) should not raise (got " & saved_err & ")") +err.clear +x = ("abc" = CLng(5)) : saved_err = err.number : err.clear +Call todo_wine_ok(saved_err = 0, "CLng: ""abc"" = CLng(5) should not raise (got " & saved_err & ")") +err.clear +x = ("abc" = CSng(5)) : saved_err = err.number : err.clear +Call todo_wine_ok(saved_err = 0, "CSng: ""abc"" = CSng(5) should not raise (got " & saved_err & ")") +err.clear +x = ("abc" = CDbl(5)) : saved_err = err.number : err.clear +Call todo_wine_ok(saved_err = 0, "CDbl: ""abc"" = CDbl(5) should not raise (got " & saved_err & ")") +on error goto 0 + +' VT_R4 / VT_R8 from CSng/CDbl: relational uses lex compare. +Call todo_wine_ok(not ("10" > CDbl(5)), """10"" > CDbl(5) should be false (lex)") +Call todo_wine_ok(not ("10" > CSng(5)), """10"" > CSng(5) should be false (lex)") +Call todo_wine_ok(not ("9" < CDbl(10)), """9"" < CDbl(10) should be false (lex)") + +' --- VT_DATE from CDate is non-literal: string compare, no error. --- +Dim cdt : cdt = CDate("2024-01-15") +on error resume next +err.clear +x = ("abc" = cdt) : saved_err = err.number : err.clear +on error goto 0 +Call todo_wine_ok(saved_err = 0, "CDate: ""abc"" = CDate(...) should not raise (got " & saved_err & ")") + +' --- Function return / ByVal / ByRef strip "literal" status. --- +Function GetFiveLit() + GetFiveLit = 5 +End Function +on error resume next +err.clear +x = ("abc" = GetFiveLit()) : saved_err = err.number : err.clear +on error goto 0 +Call todo_wine_ok(saved_err = 0, "fn return: ""abc"" = GetFiveLit() should not raise (got " & saved_err & ")") + +Sub TestByValStripsLit(ByVal v) + Dim local_err + on error resume next + err.clear + x = ("abc" = v) : local_err = err.number : err.clear + on error goto 0 + Call todo_wine_ok(local_err = 0, "ByVal: ""abc"" = v should not raise (got " & local_err & ")") +End Sub +TestByValStripsLit 5 + +Sub TestByRefStripsLit(ByRef v) + Dim local_err + on error resume next + err.clear + x = ("abc" = v) : local_err = err.number : err.clear + on error goto 0 + Call todo_wine_ok(local_err = 0, "ByRef: ""abc"" = v should not raise (got " & local_err & ")") +End Sub +Dim litvar : litvar = 5 +TestByRefStripsLit litvar + +' --- Const inlines at compile-time in Wine, so the value is treated as a +' literal and raises error 13; on Windows Const acts like a variable. +' Tracked separately from the var_cmp fix; remains todo_wine for now. --- +Const FIVE_C = 5 +on error resume next +err.clear +x = ("abc" = FIVE_C) : saved_err = err.number : err.clear +on error goto 0 +Call todo_wine_ok(saved_err = 0, "Const: ""abc"" = FIVE should not raise (got " & saved_err & "; needs compile-time fix)") + Call ok(getVT(false) = "VT_BOOL", "getVT(false) is not VT_BOOL") Call ok(getVT(true) = "VT_BOOL", "getVT(true) is not VT_BOOL") Call ok(getVT("") = "VT_BSTR", "getVT("""") is not VT_BSTR") -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10766
From: Francis De Brabandere <francisdb@gmail.com> VT_RESERVED is set on numeric literal pushes (interp_int / interp_double / interp_date) and stripped at every boundary that isn't the comparison consumer; var_cmp dispatches on the flag so BSTR vs literal numeric goes through numeric coercion (error 13 on parse failure) while BSTR vs non-literal numeric uses string compare against CStr(numeric). VT_BOOL is handled in the same branch with hardcoded "True"/"False" since VarBstrFromBool yields "-1"/"0". Const references are inlined at compile time, so a new OP_strip_reserved opcode is emitted after the inlined expression to clear the flag and make Const behave like a variable read. --- dlls/vbscript/compile.c | 18 +++-- dlls/vbscript/global.c | 5 ++ dlls/vbscript/interp.c | 106 ++++++++++++++++++++++++--- dlls/vbscript/tests/lang.vbs | 134 ++++++++++++++++++++--------------- dlls/vbscript/vbdisp.c | 3 + dlls/vbscript/vbscript.h | 7 ++ 6 files changed, 199 insertions(+), 74 deletions(-) diff --git a/dlls/vbscript/compile.c b/dlls/vbscript/compile.c index 16e661c98ff..5ae679d7828 100644 --- a/dlls/vbscript/compile.c +++ b/dlls/vbscript/compile.c @@ -510,8 +510,13 @@ static HRESULT compile_member_call_expression(compile_ctx_t *ctx, member_express expression_t *const_expr; const_expr = lookup_const_decls(ctx, expr->identifier, TRUE); - if(const_expr) - return compile_expression(ctx, const_expr); + if(const_expr) { + HRESULT hres; + hres = compile_expression(ctx, const_expr); + if(FAILED(hres)) + return hres; + return push_instr(ctx, OP_strip_reserved) ? S_OK : E_OUTOFMEMORY; + } } if(expr->obj_expr) { @@ -536,8 +541,13 @@ static HRESULT compile_member_expression(compile_ctx_t *ctx, member_expression_t if (!lookup_dim_decls(ctx, expr->identifier) && !lookup_args_name(ctx, expr->identifier)) { const_expr = lookup_const_decls(ctx, expr->identifier, TRUE); - if(const_expr) - return compile_expression(ctx, const_expr); + if(const_expr) { + HRESULT hres; + hres = compile_expression(ctx, const_expr); + if(FAILED(hres)) + return hres; + return push_instr(ctx, OP_strip_reserved) ? S_OK : E_OUTOFMEMORY; + } } return push_instr_bstr(ctx, OP_ident, expr->identifier); } diff --git a/dlls/vbscript/global.c b/dlls/vbscript/global.c index 7c0041a76ce..0e8604d7154 100644 --- a/dlls/vbscript/global.c +++ b/dlls/vbscript/global.c @@ -259,6 +259,7 @@ static HRESULT WINAPI Builtin_Invoke(IDispatch *iface, DISPID id, REFIID riid, L args[i] = *V_VARIANTREF(dp->rgvarg+dp->cArgs-i-1); else args[i] = dp->rgvarg[dp->cArgs-i-1]; + strip_vt_reserved(&args[i]); } hres = prop->proc(This, args, dp->cArgs, res); @@ -416,6 +417,7 @@ HRESULT to_int(VARIANT *v, int *ret) VARIANT r; HRESULT hres; + strip_vt_reserved(v); V_VT(&r) = VT_EMPTY; hres = VariantChangeType(&r, v, 0, VT_I4); if(FAILED(hres)) @@ -430,6 +432,7 @@ HRESULT to_double(VARIANT *v, double *ret) VARIANT dst; HRESULT hres; + strip_vt_reserved(v); V_VT(&dst) = VT_EMPTY; hres = VariantChangeType(&dst, v, 0, VT_R8); if(FAILED(hres)) @@ -444,6 +447,7 @@ static HRESULT to_float(VARIANT *v, float *ret) VARIANT dst; HRESULT hres; + strip_vt_reserved(v); V_VT(&dst) = VT_EMPTY; hres = VariantChangeType(&dst, v, 0, VT_R4); if(FAILED(hres)) @@ -458,6 +462,7 @@ static HRESULT to_string(VARIANT *v, BSTR *ret) VARIANT dst; HRESULT hres; + strip_vt_reserved(v); V_VT(&dst) = VT_EMPTY; hres = VariantChangeType(&dst, v, VARIANT_LOCALBOOL, VT_BSTR); if(FAILED(hres)) diff --git a/dlls/vbscript/interp.c b/dlls/vbscript/interp.c index 58afe4c6b41..c66b6dcd7a5 100644 --- a/dlls/vbscript/interp.c +++ b/dlls/vbscript/interp.c @@ -436,6 +436,11 @@ static HRESULT stack_pop_val(exec_ctx_t *ctx, variant_val_t *r) r->v = &r->store; } + /* cmp_oper / interp_case capture VT_RESERVED from stack_top before + * calling this; everywhere else we want it gone. */ + if(r->owned) + strip_vt_reserved(r->v); + return S_OK; } @@ -576,6 +581,14 @@ static void vbstack_to_dp(exec_ctx_t *ctx, unsigned arg_cnt, BOOL is_propput, DI }else { dp->rgvarg = is_propput ? ctx->stack+ctx->top-1 : NULL; } + + /* Once a literal value flows into a function arg or property put it + * loses its literal-marker. */ + if(dp->rgvarg) { + unsigned i; + for(i = 0; i < dp->cArgs; i++) + strip_vt_reserved(&dp->rgvarg[i]); + } } HRESULT array_access(SAFEARRAY *array, DISPPARAMS *dp, VARIANT **ret) @@ -1142,6 +1155,8 @@ static HRESULT interp_const(exec_ctx_t *ctx) return hres; *v = *stack_pop(ctx); + /* A Const stored from a literal value must not carry the literal flag. */ + strip_vt_reserved(v); return S_OK; } @@ -1886,6 +1901,16 @@ static HRESULT interp_string(exec_ctx_t *ctx) return stack_push(ctx, &v); } +/* Clears VT_RESERVED on the top-of-stack variant. Emitted after a Const + * reference is inlined into a literal-emitting expression: the inlined value + * must not be treated as a bare literal by the comparison dispatcher. */ +static HRESULT interp_strip_reserved(exec_ctx_t *ctx) +{ + TRACE("\n"); + strip_vt_reserved(stack_top(ctx, 0)); + return S_OK; +} + static HRESULT interp_date(exec_ctx_t *ctx) { const DATE *d = ctx->instr->arg1.date; @@ -1893,7 +1918,10 @@ static HRESULT interp_date(exec_ctx_t *ctx) TRACE("%lf\n",*d); - V_VT(&v) = VT_DATE; + /* VT_RESERVED marks bare numeric literals so that the comparison + * dispatcher in var_cmp can apply the literal-vs-non-literal rule. + * Stripped at any boundary other than the immediate comparison. */ + V_VT(&v) = VT_DATE | VT_RESERVED; V_DATE(&v) = *d; return stack_push(ctx, &v); @@ -1907,10 +1935,10 @@ static HRESULT interp_int(exec_ctx_t *ctx) TRACE("%ld\n", arg); if(arg == (INT16)arg) { - V_VT(&v) = VT_I2; + V_VT(&v) = VT_I2 | VT_RESERVED; V_I2(&v) = arg; }else { - V_VT(&v) = VT_I4; + V_VT(&v) = VT_I4 | VT_RESERVED; V_I4(&v) = arg; } return stack_push(ctx, &v); @@ -1923,7 +1951,7 @@ static HRESULT interp_double(exec_ctx_t *ctx) TRACE("%lf\n", *arg); - V_VT(&v) = VT_R8; + V_VT(&v) = VT_R8 | VT_RESERVED; V_R8(&v) = *arg; return stack_push(ctx, &v); } @@ -2107,19 +2135,30 @@ static HRESULT interp_imp(exec_ctx_t *ctx) return stack_push(ctx, &v); } +/* Any numeric variant type. together with the VT_RESERVED literal marker + * (set on interp_int / interp_double / interp_date) this drives the + * literal-vs-non-literal dispatch in var_cmp. */ static inline BOOL is_numeric_vt(VARTYPE vt) { return vt == VT_I2 || vt == VT_I4 || vt == VT_R4 || vt == VT_R8 - || vt == VT_CY || vt == VT_DATE || vt == VT_UI1; + || vt == VT_CY || vt == VT_DATE || vt == VT_UI1 || vt == VT_DECIMAL + || vt == VT_I1 || vt == VT_I8 || vt == VT_UI2 || vt == VT_UI4 + || vt == VT_UI8 || vt == VT_INT || vt == VT_UINT; } -static HRESULT var_cmp(exec_ctx_t *ctx, VARIANT *l, VARIANT *r) +static HRESULT var_cmp(exec_ctx_t *ctx, VARIANT *l, VARIANT *r, BOOL l_lit, BOOL r_lit) { + VARTYPE lvt = V_VT(l) & VT_TYPEMASK; + VARTYPE rvt = V_VT(r) & VT_TYPEMASK; + TRACE("%s %s\n", debugstr_variant(l), debugstr_variant(r)); - /* VarCmp would use string comparison; VBScript converts the string to a number. */ - if((V_VT(l) == VT_BSTR && is_numeric_vt(V_VT(r))) || - (V_VT(r) == VT_BSTR && is_numeric_vt(V_VT(l)))) { + /* BSTR vs numeric LITERAL: coerce BSTR to a number and numeric-compare; + * parse failure raises type-mismatch (error 13). VT_RESERVED is set by + * interp_int / interp_double / interp_date and stripped at any boundary + * other than the immediate comparison consumer. */ + if((lvt == VT_BSTR && is_numeric_vt(rvt) && r_lit) || + (rvt == VT_BSTR && is_numeric_vt(lvt) && l_lit)) { double dl, dr; HRESULT hres; @@ -2136,21 +2175,56 @@ static HRESULT var_cmp(exec_ctx_t *ctx, VARIANT *l, VARIANT *r) return VARCMP_EQ; } + /* BSTR vs numeric (function-call result, variable, etc.) or VT_BOOL: + * coerce the numeric/bool side to its CStr form and string-compare. No + * error on unparseable BSTR; relational uses binary lex order. + * VarBstrFromBool yields "-1"/"0" rather than VBScript's "True"/"False", + * so bool strings are hardcoded. */ + if((lvt == VT_BSTR && (is_numeric_vt(rvt) || rvt == VT_BOOL)) || + (rvt == VT_BSTR && (is_numeric_vt(lvt) || lvt == VT_BOOL))) { + VARIANT *num = lvt == VT_BSTR ? r : l; + VARIANT *str = lvt == VT_BSTR ? l : r; + VARIANT num_str; + HRESULT hres; + + VariantInit(&num_str); + if((V_VT(num) & VT_TYPEMASK) == VT_BOOL) { + V_VT(&num_str) = VT_BSTR; + V_BSTR(&num_str) = SysAllocString(V_BOOL(num) ? L"True" : L"False"); + if(!V_BSTR(&num_str)) + return E_OUTOFMEMORY; + }else { + hres = VariantChangeType(&num_str, num, 0, VT_BSTR); + if(FAILED(hres)) + return hres; + } + hres = lvt == VT_BSTR ? VarCmp(str, &num_str, 0, 0) + : VarCmp(&num_str, str, 0, 0); + VariantClear(&num_str); + return hres; + } + return VarCmp(l, r, ctx->script->lcid, 0); } static HRESULT cmp_oper(exec_ctx_t *ctx) { variant_val_t l, r; + BOOL l_lit, r_lit; HRESULT hres; + /* Capture VT_RESERVED ("this side is a numeric literal") before + * stack_pop_val strips it. */ + r_lit = (V_VT(stack_top(ctx, 0)) & VT_RESERVED) != 0; + l_lit = (V_VT(stack_top(ctx, 1)) & VT_RESERVED) != 0; + hres = stack_pop_val(ctx, &r); if(FAILED(hres)) return hres; hres = stack_pop_val(ctx, &l); if(SUCCEEDED(hres)) { - hres = var_cmp(ctx, l.v, r.v); + hres = var_cmp(ctx, l.v, r.v, l_lit, r_lit); release_val(&l); } @@ -2270,15 +2344,20 @@ static HRESULT interp_case(exec_ctx_t *ctx) { const unsigned arg = ctx->instr->arg1.uint; variant_val_t v; + BOOL test_lit, case_lit; HRESULT hres; TRACE("%d\n", arg); + /* Capture VT_RESERVED before stack_pop_val strips it. */ + case_lit = (V_VT(stack_top(ctx, 0)) & VT_RESERVED) != 0; + test_lit = (V_VT(stack_top(ctx, 1)) & VT_RESERVED) != 0; + hres = stack_pop_val(ctx, &v); if(FAILED(hres)) return hres; - hres = var_cmp(ctx, stack_top(ctx, 0), v.v); + hres = var_cmp(ctx, stack_top(ctx, 0), v.v, test_lit, case_lit); release_val(&v); if(FAILED(hres)) { if(hres == DISP_E_TYPEMISMATCH) { @@ -2579,6 +2658,9 @@ static HRESULT interp_incc(exec_ctx_t *ctx) return E_FAIL; } + /* The default For step is a numeric literal still on the stack. */ + strip_vt_reserved(stack_top(ctx, 0)); + hres = VarAdd(stack_top(ctx, 0), ref.u.v, &v); if(FAILED(hres)) return hres; @@ -2882,6 +2964,8 @@ HRESULT exec_script(script_ctx_t *ctx, BOOL extern_caller, function_t *func, vbd if(SUCCEEDED(hres) && res) { *res = exec.ret_val; V_VT(&exec.ret_val) = VT_EMPTY; + /* External callers (e.g. ParseScriptText) shouldn't see the flag. */ + strip_vt_reserved(res); } ctx->current_named_item = prev_named_item; diff --git a/dlls/vbscript/tests/lang.vbs b/dlls/vbscript/tests/lang.vbs index 0e5462348b7..88751371c7a 100644 --- a/dlls/vbscript/tests/lang.vbs +++ b/dlls/vbscript/tests/lang.vbs @@ -143,10 +143,32 @@ Call ok("05" = 5, """05"" = 5 should be true") Call ok(" 5" = 5, """ 5"" = 5 should be true") Call ok("5 " = 5, """5 "" = 5 should be true") Call ok("5e0" = 5, """5e0"" = 5 should be true") -' String vs Boolean uses string comparison, not numeric conversion +' BSTR vs Boolean: string-compare against CStr(bool) ("True"/"False"), +' case-sensitive, no whitespace trimming, no numeric coercion, no type-mismatch. +Call ok("True" = True, """True"" = True should be true") +Call ok("False" = False, """False"" = False should be true") +Call ok(not ("True" = False), """True"" = False should be false") +Call ok(not ("False" = True), """False"" = True should be false") +Call ok(not ("true" = True), """true"" = True should be false (case-sensitive)") +Call ok(not ("TRUE" = True), """TRUE"" = True should be false (case-sensitive)") +Call ok(not ("True " = True), """True "" = True should be false (no trim)") +Call ok(not (" True" = True), """ True"" = True should be false (no trim)") Call ok(not ("1" = True), """1"" = True should be false") Call ok(not ("-1" = True), """-1"" = True should be false") Call ok(not ("0" = False), """0"" = False should be false") +Call ok(not ("-1" = False), """-1"" = False should be false") +Call ok(not ("True" <> True), """True"" <> True should be false") +Call ok("False" <> True, """False"" <> True should be true") +Call ok(not ("abc" = True), """abc"" = True should be false (no error)") +Call ok(not ("" = True), """"" = True should be false (no error)") +Call ok(not ("" = False), """"" = False should be false (no error)") +Call ok(True = "True", "True = ""True"" should be true") +Call ok(False = "False", "False = ""False"" should be true") +' Relational: lexicographic string comparison after coercing bool to BSTR +Call ok("True" > False, """True"" > False should be true (lex)") +Call ok(not ("True" < False), """True"" < False should be false (lex)") +Call ok(not ("False" > True), """False"" > True should be false (lex)") +Call ok("abc" > True, """abc"" > True should be true (lex)") ' Non-numeric BSTR compared to number raises type mismatch (= / <> / relational) on error resume next err.clear @@ -200,94 +222,86 @@ Call ok("&H1F" = 31, """&H1F"" = 31 should be true") ' VT_UI1/VT_CY vs BSTR diverge from VT_I2: string-compare against CStr(numeric). ' Non-numeric BSTR returns False with NO error (VT_I2 raises 13 for the same). -on error resume next -err.clear -x = ("abc" = CByte(5)) -saved_err = err.number -err.clear -Call todo_wine_ok(saved_err = 0, """abc"" = CByte(5) should not raise error (err=" & saved_err & ")") -x = ("abc" = CCur(5)) -saved_err = err.number -err.clear -Call todo_wine_ok(saved_err = 0, """abc"" = CCur(5) should not raise error (err=" & saved_err & ")") -x = ("" = CByte(0)) -saved_err = err.number -err.clear -Call todo_wine_ok(saved_err = 0, """"" = CByte(0) should not raise error (err=" & saved_err & ")") -x = ("" = CCur(0)) -saved_err = err.number -err.clear -Call todo_wine_ok(saved_err = 0, """"" = CCur(0) should not raise error (err=" & saved_err & ")") -on error goto 0 +Call ok(not ("abc" = CByte(5)), """abc"" = CByte(5) should be false (no error)") +Call ok(not ("abc" = CCur(5)), """abc"" = CCur(5) should be false (no error)") +Call ok(not ("" = CByte(0)), """"" = CByte(0) should be false (no error)") +Call ok(not ("" = CCur(0)), """"" = CCur(0) should be false (no error)") ' Relational confirms lex compare: 10 > 5 numerically would be true; lex "10" < "5". -Call todo_wine_ok(not ("10" > CByte(5)), """10"" > CByte(5) should be false (lex)") -Call todo_wine_ok(not ("5" < CCur(10)), """5"" < CCur(10) should be false (lex)") +Call ok(not ("10" > CByte(5)), """10"" > CByte(5) should be false (lex)") +Call ok(not ("5" < CCur(10)), """5"" < CCur(10) should be false (lex)") ' --- BSTR vs numeric LITERAL: BSTR coerces to a number, parse failure raises 13. --- Dim saved_err on error resume next err.clear x = ("abc" = 5.5) : saved_err = err.number : err.clear -Call ok(saved_err = 13, "literal R8: ""abc"" = 5.5 should error 13 (got " & saved_err & ")") +Call ok(saved_err = 13, "literal R8: ""abc"" = 5.5 should error 13 (got " & saved_err & "; needs literal tracking)") x = ("abc" = 1e2) : saved_err = err.number : err.clear -Call ok(saved_err = 13, "literal sci: ""abc"" = 1e2 should error 13 (got " & saved_err & ")") +Call ok(saved_err = 13, "literal sci: ""abc"" = 1e2 should error 13 (got " & saved_err & "; needs literal tracking)") x = ("abc" = &hff) : saved_err = err.number : err.clear Call ok(saved_err = 13, "literal hex: ""abc"" = &hff should error 13 (got " & saved_err & ")") x = ("abc" = 100000) : saved_err = err.number : err.clear Call ok(saved_err = 13, "literal I4: ""abc"" = 100000 should error 13 (got " & saved_err & ")") x = ("abc" = #1/15/2024#): saved_err = err.number : err.clear -Call ok(saved_err = 13, "literal date: ""abc"" = #1/15/2024# should error 13 (got " & saved_err & ")") +Call ok(saved_err = 13, "literal date: ""abc"" = #1/15/2024# should error 13 (got " & saved_err & "; needs literal tracking)") on error goto 0 -' --- Variable holding a numeric loses "literal" status: string compare, no error. --- +' --- Variable holding a numeric / arithmetic / unary / function return --- +' VBScript on Windows treats these as non-literal: BSTR vs numeric uses +' string-compare, non-numeric BSTR returns False with no error. Wine doesn't +' track literal-ness, so VT_I2/I4 values still go through the numeric path +' and raise error 13, so these stay todo_wine until the literal-tracking work +' lands. We wrap each block in "on error resume next" so the script doesn't +' abort on the unwanted error. Dim n5 : n5 = 5 Dim n10 : n10 = 10 on error resume next err.clear x = ("abc" = n5) : saved_err = err.number : err.clear -on error goto 0 -Call todo_wine_ok(saved_err = 0, "var: ""abc"" = (n=5) should not raise (got " & saved_err & ")") -Call todo_wine_ok(not ("abc" = n5), "var: ""abc"" = (n=5) should be false") -Call todo_wine_ok(not ("010" = n10), "var: ""010"" = (n=10) should be false (string)") - -' --- Arithmetic / unary produce a fresh non-literal value. --- -on error resume next +Call ok(saved_err = 0, "var: ""abc"" = (n=5) should not raise (got " & saved_err & ")") +err.clear +x = ("abc" = n5) : Call ok(err.number = 0 and not x, "var: ""abc"" = (n=5) should be false") +err.clear +x = ("010" = n10) : Call ok(err.number = 0 and not x, "var: ""010"" = (n=10) should be false (string)") err.clear x = ("abc" = (5+0)) : saved_err = err.number : err.clear -on error goto 0 -Call todo_wine_ok(saved_err = 0, "arith: ""abc"" = (5+0) should not raise (got " & saved_err & ")") -Call todo_wine_ok(not ("010" = (5+5)), "arith: ""010"" = (5+5) should be false") -Call todo_wine_ok(not ("010" = (10*1)), "arith: ""010"" = (10*1) should be false") -on error resume next +Call ok(saved_err = 0, "arith: ""abc"" = (5+0) should not raise (got " & saved_err & ")") +err.clear +x = ("010" = (5+5)) : Call ok(err.number = 0 and not x, "arith: ""010"" = (5+5) should be false") +err.clear +x = ("010" = (10*1)) : Call ok(err.number = 0 and not x, "arith: ""010"" = (10*1) should be false") err.clear x = ("abc" = -5) : saved_err = err.number : err.clear +Call ok(saved_err = 0, "neg: ""abc"" = -5 should not raise (got " & saved_err & ")") on error goto 0 -Call todo_wine_ok(saved_err = 0, "neg: ""abc"" = -5 should not raise (got " & saved_err & ")") ' --- C-coercion functions return non-literal values. --- -Call todo_wine_ok(not ("010" = CInt(10)), "CInt: ""010"" = CInt(10) should be false") -Call todo_wine_ok(not ("010" = CLng(10)), "CLng: ""010"" = CLng(10) should be false") -Call todo_wine_ok(not ("010" = CSng(10)), "CSng: ""010"" = CSng(10) should be false") -Call todo_wine_ok(not ("010" = CDbl(10)), "CDbl: ""010"" = CDbl(10) should be false") +' VT_I2/I4 from CInt/CLng still go through the numeric path on Wine (no literal +' tracking), so these remain todo_wine. +Call ok(not ("010" = CInt(10)), "CInt: ""010"" = CInt(10) should be false") +Call ok(not ("010" = CLng(10)), "CLng: ""010"" = CLng(10) should be false") +' VT_R4/VT_R8 from CSng/CDbl take the string-compare path now. +Call ok(not ("010" = CSng(10)), "CSng: ""010"" = CSng(10) should be false") +Call ok(not ("010" = CDbl(10)), "CDbl: ""010"" = CDbl(10) should be false") on error resume next err.clear x = ("abc" = CInt(5)) : saved_err = err.number : err.clear -Call todo_wine_ok(saved_err = 0, "CInt: ""abc"" = CInt(5) should not raise (got " & saved_err & ")") +Call ok(saved_err = 0, "CInt: ""abc"" = CInt(5) should not raise (got " & saved_err & ")") err.clear x = ("abc" = CLng(5)) : saved_err = err.number : err.clear -Call todo_wine_ok(saved_err = 0, "CLng: ""abc"" = CLng(5) should not raise (got " & saved_err & ")") +Call ok(saved_err = 0, "CLng: ""abc"" = CLng(5) should not raise (got " & saved_err & ")") err.clear x = ("abc" = CSng(5)) : saved_err = err.number : err.clear -Call todo_wine_ok(saved_err = 0, "CSng: ""abc"" = CSng(5) should not raise (got " & saved_err & ")") +Call ok(saved_err = 0, "CSng: ""abc"" = CSng(5) should not raise (got " & saved_err & ")") err.clear x = ("abc" = CDbl(5)) : saved_err = err.number : err.clear -Call todo_wine_ok(saved_err = 0, "CDbl: ""abc"" = CDbl(5) should not raise (got " & saved_err & ")") +Call ok(saved_err = 0, "CDbl: ""abc"" = CDbl(5) should not raise (got " & saved_err & ")") on error goto 0 ' VT_R4 / VT_R8 from CSng/CDbl: relational uses lex compare. -Call todo_wine_ok(not ("10" > CDbl(5)), """10"" > CDbl(5) should be false (lex)") -Call todo_wine_ok(not ("10" > CSng(5)), """10"" > CSng(5) should be false (lex)") -Call todo_wine_ok(not ("9" < CDbl(10)), """9"" < CDbl(10) should be false (lex)") +Call ok(not ("10" > CDbl(5)), """10"" > CDbl(5) should be false (lex)") +Call ok(not ("10" > CSng(5)), """10"" > CSng(5) should be false (lex)") +Call ok(not ("9" < CDbl(10)), """9"" < CDbl(10) should be false (lex)") ' --- VT_DATE from CDate is non-literal: string compare, no error. --- Dim cdt : cdt = CDate("2024-01-15") @@ -295,7 +309,7 @@ on error resume next err.clear x = ("abc" = cdt) : saved_err = err.number : err.clear on error goto 0 -Call todo_wine_ok(saved_err = 0, "CDate: ""abc"" = CDate(...) should not raise (got " & saved_err & ")") +Call ok(saved_err = 0, "CDate: ""abc"" = CDate(...) should not raise (got " & saved_err & ")") ' --- Function return / ByVal / ByRef strip "literal" status. --- Function GetFiveLit() @@ -305,7 +319,7 @@ on error resume next err.clear x = ("abc" = GetFiveLit()) : saved_err = err.number : err.clear on error goto 0 -Call todo_wine_ok(saved_err = 0, "fn return: ""abc"" = GetFiveLit() should not raise (got " & saved_err & ")") +Call ok(saved_err = 0, "fn return: ""abc"" = GetFiveLit() should not raise (got " & saved_err & ")") Sub TestByValStripsLit(ByVal v) Dim local_err @@ -313,7 +327,7 @@ Sub TestByValStripsLit(ByVal v) err.clear x = ("abc" = v) : local_err = err.number : err.clear on error goto 0 - Call todo_wine_ok(local_err = 0, "ByVal: ""abc"" = v should not raise (got " & local_err & ")") + Call ok(local_err = 0, "ByVal: ""abc"" = v should not raise (got " & local_err & ")") End Sub TestByValStripsLit 5 @@ -323,20 +337,22 @@ Sub TestByRefStripsLit(ByRef v) err.clear x = ("abc" = v) : local_err = err.number : err.clear on error goto 0 - Call todo_wine_ok(local_err = 0, "ByRef: ""abc"" = v should not raise (got " & local_err & ")") + Call ok(local_err = 0, "ByRef: ""abc"" = v should not raise (got " & local_err & ")") End Sub Dim litvar : litvar = 5 TestByRefStripsLit litvar -' --- Const inlines at compile-time in Wine, so the value is treated as a -' literal and raises error 13; on Windows Const acts like a variable. -' Tracked separately from the var_cmp fix; remains todo_wine for now. --- +' Const reads through normal variable lookup at runtime; the stored value +' has VT_RESERVED stripped, so "abc" = FIVE goes through the BSTR-vs-numeric +' string-compare path and returns False with no error. Const FIVE_C = 5 on error resume next err.clear -x = ("abc" = FIVE_C) : saved_err = err.number : err.clear +x = ("abc" = FIVE_C) +saved_err = err.number +err.clear on error goto 0 -Call todo_wine_ok(saved_err = 0, "Const: ""abc"" = FIVE should not raise (got " & saved_err & "; needs compile-time fix)") +Call ok(saved_err = 0, "Const: ""abc"" = FIVE should not raise (got " & saved_err & ")") Call ok(getVT(false) = "VT_BOOL", "getVT(false) is not VT_BOOL") Call ok(getVT(true) = "VT_BOOL", "getVT(true) is not VT_BOOL") diff --git a/dlls/vbscript/vbdisp.c b/dlls/vbscript/vbdisp.c index 9f6cf2e446a..975e94447f5 100644 --- a/dlls/vbscript/vbdisp.c +++ b/dlls/vbscript/vbdisp.c @@ -188,6 +188,9 @@ static HRESULT invoke_variant_prop(script_ctx_t *ctx, VARIANT *v, WORD flags, DI if(res) V_VT(res) = VT_EMPTY; + /* Stored variables must not carry the literal flag. */ + strip_vt_reserved(&put_val); + if(own_val) *v = put_val; else diff --git a/dlls/vbscript/vbscript.h b/dlls/vbscript/vbscript.h index 6ff6e8ff8eb..8ef70c57f63 100644 --- a/dlls/vbscript/vbscript.h +++ b/dlls/vbscript/vbscript.h @@ -50,6 +50,12 @@ void heap_pool_clear(heap_pool_t*); void heap_pool_free(heap_pool_t*); heap_pool_t *heap_pool_mark(heap_pool_t*); +/* VT_RESERVED is set on numeric literal pushes (interp_int / interp_double / + * interp_date) to mark them for the literal-vs-BSTR comparison rule in + * var_cmp. The flag must be stripped at every other boundary because + * oleaut32's VariantChangeType / VarAdd / etc. reject any extra flags. */ +static inline void strip_vt_reserved(VARIANT *v) { V_VT(v) &= ~VT_RESERVED; } + typedef struct _function_t function_t; typedef struct _vbscode_t vbscode_t; typedef struct _script_ctx_t script_ctx_t; @@ -310,6 +316,7 @@ typedef enum { X(step, 0, ARG_ADDR, ARG_BSTR) \ X(stop, 1, 0, 0) \ X(string, 1, ARG_STR, 0) \ + X(strip_reserved, 1, 0, 0) \ X(sub, 1, 0, 0) \ X(val, 1, 0, 0) \ X(vcall, 1, ARG_UINT, 0) \ -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10766
Working on an alternative solution that does not use `VT_RESERVED` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10766#note_138161
Closing in favor of !10775 -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10766#note_138241
This merge request was closed by Francis De Brabandere. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10766
participants (2)
-
Francis De Brabandere -
Francis De Brabandere (＠francisdb)