[PATCH v2 0/4] MR10766: Draft: 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)) Expanding test coverage and fixing a divergence from Windows VBScript that the original change didn't cover. -- v2: vbscript: String-compare BSTR vs VT_UI1 / VT_CY. 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> VarCmp falls into its "BSTR always greater" branch for BSTR vs VT_BOOL, so every "True" = True / "False" = False returned False on Wine. VBScript coerces the boolean to its CStr form ("True"/"False") and string-compares; comparison is binary, not locale-aware (matches "abc" > True returning True on Windows). VarBstrFromBool / VariantChangeType yield "-1"/"0" (OLE numeric convention) and don't match what VBScript uses, so the strings are hardcoded. --- dlls/vbscript/interp.c | 20 ++++++++++++++++++++ dlls/vbscript/tests/lang.vbs | 24 +++++++++++++++++++++++- 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/dlls/vbscript/interp.c b/dlls/vbscript/interp.c index 58afe4c6b41..ce4cdd8cf87 100644 --- a/dlls/vbscript/interp.c +++ b/dlls/vbscript/interp.c @@ -2136,6 +2136,26 @@ static HRESULT var_cmp(exec_ctx_t *ctx, VARIANT *l, VARIANT *r) return VARCMP_EQ; } + /* VarCmp returns "BSTR always greater" for BSTR vs VT_BOOL; VBScript instead + * coerces the boolean to its CStr form ("True"/"False") and string-compares. + * (VarBstrFromBool yields "-1"/"0" — OLE numeric convention, not what VBScript + * uses.) */ + if((V_VT(l) == VT_BSTR && V_VT(r) == VT_BOOL) || + (V_VT(r) == VT_BSTR && V_VT(l) == VT_BOOL)) { + VARIANT_BOOL b = V_VT(l) == VT_BOOL ? V_BOOL(l) : V_BOOL(r); + VARIANT bool_str; + HRESULT hres; + + V_VT(&bool_str) = VT_BSTR; + V_BSTR(&bool_str) = SysAllocString(b ? L"True" : L"False"); + if(!V_BSTR(&bool_str)) + return E_OUTOFMEMORY; + hres = V_VT(l) == VT_BOOL ? VarCmp(&bool_str, r, 0, 0) + : VarCmp(l, &bool_str, 0, 0); + VariantClear(&bool_str); + return hres; + } + return VarCmp(l, r, ctx->script->lcid, 0); } diff --git a/dlls/vbscript/tests/lang.vbs b/dlls/vbscript/tests/lang.vbs index b1bffe0ab45..4328b75272f 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 -- 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 4328b75272f..1e10e28e39e 100644 --- a/dlls/vbscript/tests/lang.vbs +++ b/dlls/vbscript/tests/lang.vbs @@ -197,6 +197,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> VarCmp falls into its "BSTR always greater" branch for BSTR vs VT_UI1 / VT_CY; VBScript instead coerces the numeric side to its CStr form and string-compares. Non-numeric BSTRs return False with no error (unlike VT_I2/I4 which raise type-mismatch), and relational operators use lex order. Also strips VT_BYREF before dispatching so a ByRef BSTR doesn't silently fall through to plain VarCmp. Folds the VT_UI1 / VT_CY divergences out of is_numeric_vt into a new is_string_compared_vt, and routes VT_BOOL through the same branch (the only difference being that bool's BSTR form is hardcoded "True"/"False" rather than what VarBstrFromBool yields). --- dlls/vbscript/interp.c | 57 ++++++++++++++++++++++++------------ dlls/vbscript/tests/lang.vbs | 27 ++++------------- 2 files changed, 45 insertions(+), 39 deletions(-) diff --git a/dlls/vbscript/interp.c b/dlls/vbscript/interp.c index ce4cdd8cf87..823585ccad5 100644 --- a/dlls/vbscript/interp.c +++ b/dlls/vbscript/interp.c @@ -2110,16 +2110,29 @@ static HRESULT interp_imp(exec_ctx_t *ctx) 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_DATE; +} + +/* VBScript treats some numeric types (VT_UI1, VT_CY) by converting them to + * BSTR and string-comparing, rather than coercing the BSTR side to numeric. + * Non-numeric BSTRs return False with no error and relational operators use + * lex order. VT_BOOL behaves the same way but with hardcoded "True"/"False" + * since VarBstrFromBool yields "-1"/"0". */ +static inline BOOL is_string_compared_vt(VARTYPE vt) +{ + return vt == VT_UI1 || vt == VT_CY; } static HRESULT var_cmp(exec_ctx_t *ctx, VARIANT *l, VARIANT *r) { + 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)))) { + if((lvt == VT_BSTR && is_numeric_vt(rvt)) || + (rvt == VT_BSTR && is_numeric_vt(lvt))) { double dl, dr; HRESULT hres; @@ -2136,23 +2149,31 @@ static HRESULT var_cmp(exec_ctx_t *ctx, VARIANT *l, VARIANT *r) return VARCMP_EQ; } - /* VarCmp returns "BSTR always greater" for BSTR vs VT_BOOL; VBScript instead - * coerces the boolean to its CStr form ("True"/"False") and string-compares. - * (VarBstrFromBool yields "-1"/"0" — OLE numeric convention, not what VBScript - * uses.) */ - if((V_VT(l) == VT_BSTR && V_VT(r) == VT_BOOL) || - (V_VT(r) == VT_BSTR && V_VT(l) == VT_BOOL)) { - VARIANT_BOOL b = V_VT(l) == VT_BOOL ? V_BOOL(l) : V_BOOL(r); - VARIANT bool_str; + /* VarCmp returns "BSTR always greater" for BSTR vs VT_BOOL/UI1/CY; VBScript + * instead coerces the numeric side to its CStr form and string-compares. + * (VarBstrFromBool yields "-1"/"0" rather than "True"/"False", so the bool + * strings are hardcoded.) */ + if((lvt == VT_BSTR && (rvt == VT_BOOL || is_string_compared_vt(rvt))) || + (rvt == VT_BSTR && (lvt == VT_BOOL || is_string_compared_vt(lvt)))) { + VARIANT *num = lvt == VT_BSTR ? r : l; + VARIANT *str = lvt == VT_BSTR ? l : r; + VARIANT num_str; HRESULT hres; - V_VT(&bool_str) = VT_BSTR; - V_BSTR(&bool_str) = SysAllocString(b ? L"True" : L"False"); - if(!V_BSTR(&bool_str)) - return E_OUTOFMEMORY; - hres = V_VT(l) == VT_BOOL ? VarCmp(&bool_str, r, 0, 0) - : VarCmp(l, &bool_str, 0, 0); - VariantClear(&bool_str); + 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; } diff --git a/dlls/vbscript/tests/lang.vbs b/dlls/vbscript/tests/lang.vbs index 1e10e28e39e..ccee919f238 100644 --- a/dlls/vbscript/tests/lang.vbs +++ b/dlls/vbscript/tests/lang.vbs @@ -222,28 +222,13 @@ 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)") Call ok(getVT(false) = "VT_BOOL", "getVT(false) is not VT_BOOL") Call ok(getVT(true) = "VT_BOOL", "getVT(true) is not VT_BOOL") -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10766
participants (2)
-
Francis De Brabandere -
Francis De Brabandere (＠francisdb)