[PATCH v7 0/4] MR10673: Draft: oleaut32: Fix Null handling in three-valued logical ops.
VarAnd only inspected the non-Null operand one way and left V_BOOL/V_I4 uninitialized. VarImp treated VT_UI1 0xFF as VARIANT_TRUE (it isn't) and its VT_CY -1 check compared unscaled .int64 to -1 instead of -10000. props to @jsm174 for this one: https://github.com/vpinball/libwinevbs/commit/7e877fafe34483623ca33d26ab76fd... -- v7: oleaut32: Fix VT_DATE Null handling in VarImp. https://gitlab.winehq.org/wine/wine/-/merge_requests/10673
From: Francis De Brabandere <francisdb@gmail.com> Handle both operand orderings symmetrically and always write the result payload. Convert VARAND_TODOCOMM back to VARAND for the cases the fix makes commutative, and drop the now-unused macro. --- dlls/oleaut32/tests/vartest.c | 43 ++++-------- dlls/oleaut32/variant.c | 124 +++++++++++++++++++++++----------- dlls/vbscript/tests/lang.vbs | 10 +++ 3 files changed, 111 insertions(+), 66 deletions(-) diff --git a/dlls/oleaut32/tests/vartest.c b/dlls/oleaut32/tests/vartest.c index db357cf77ff..c362f9854fe 100644 --- a/dlls/oleaut32/tests/vartest.c +++ b/dlls/oleaut32/tests/vartest.c @@ -55,8 +55,6 @@ static WCHAR sz12_true[32]; /* Has I8/UI8 data type? */ static BOOL has_i8; -static BOOL is_win; - /* When comparing floating point values we cannot expect an exact match * because the rounding errors depend on the exact algorithm. */ @@ -434,8 +432,6 @@ static void init(void) BSTR bstr; HRESULT res; - is_win = !strcmp(winetest_platform, "windows"); - res = VarBstrFromBool(VARIANT_TRUE, LANG_USER_DEFAULT, VAR_LOCALBOOL, &bstr); ok(res == S_OK && bstr[0], "Expected localized string for 'True'\n"); /* lstrcpyW / lstrcatW do not work on win95 */ @@ -6800,15 +6796,6 @@ static HRESULT (WINAPI *pVarAnd)(LPVARIANT,LPVARIANT,LPVARIANT); V_VT(&exp) = VT_##rvt; V_##rvt(&exp) = rval; \ test_var_call2_commutative( __LINE__, pVarAnd, &left, &right, &exp ) -#define VARAND_TODOCOMM(vt1,val1,vt2,val2,rvt,rval) \ - V_VT(&left) = VT_##vt1; V_##vt1(&left) = val1; \ - V_VT(&right) = VT_##vt2; V_##vt2(&right) = val2; \ - V_VT(&exp) = VT_##rvt; V_##rvt(&exp) = rval; \ - if (is_win) \ - test_var_call2_commutative( __LINE__, pVarAnd, &left, &right, &exp ); \ - else \ - test_var_call2( __LINE__, pVarAnd, &left, &right, &exp ); \ - #define VARANDCY(vt1,val1,val2,rvt,rval) \ V_VT(&left) = VT_##vt1; V_##vt1(&left) = val1; \ V_VT(&right) = VT_CY; V_CY(&right).int64 = val2; \ @@ -6990,35 +6977,35 @@ static void test_VarAnd(void) VARAND(NULL,0,NULL,0,NULL,0); VARAND(NULL,1,NULL,0,NULL,0); VARAND(NULL,0,I1,0,I4,0); - VARAND_TODOCOMM(NULL,0,I1,1,NULL,0); + VARAND(NULL,0,I1,1,NULL,0); VARAND(NULL,0,UI1,0,UI1,0); - VARAND_TODOCOMM(NULL,0,UI1,1,NULL,0); + VARAND(NULL,0,UI1,1,NULL,0); VARAND(NULL,0,I2,0,I2,0); - VARAND_TODOCOMM(NULL,0,I2,1,NULL,0); + VARAND(NULL,0,I2,1,NULL,0); VARAND(NULL,0,UI2,0,I4,0); - VARAND_TODOCOMM(NULL,0,UI2,1,NULL,0); + VARAND(NULL,0,UI2,1,NULL,0); VARAND(NULL,0,I4,0,I4,0); - VARAND_TODOCOMM(NULL,0,I4,1,NULL,0); + VARAND(NULL,0,I4,1,NULL,0); VARAND(NULL,0,UI4,0,I4,0); - VARAND_TODOCOMM(NULL,0,UI4,1,NULL,0); + VARAND(NULL,0,UI4,1,NULL,0); if (has_i8) { VARAND(NULL,0,I8,0,I8,0); - VARAND_TODOCOMM(NULL,0,I8,1,NULL,0); + VARAND(NULL,0,I8,1,NULL,0); VARAND(NULL,0,UI8,0,I4,0); - VARAND_TODOCOMM(NULL,0,UI8,1,NULL,0); + VARAND(NULL,0,UI8,1,NULL,0); } VARAND(NULL,0,INT,0,I4,0); - VARAND_TODOCOMM(NULL,0,INT,1,NULL,0); + VARAND(NULL,0,INT,1,NULL,0); VARAND(NULL,0,UINT,0,I4,0); - VARAND_TODOCOMM(NULL,0,UINT,1,NULL,0); - VARAND_TODOCOMM(NULL,0,BOOL,0,BOOL,0); - VARAND_TODOCOMM(NULL,0,BOOL,1,NULL,0); + VARAND(NULL,0,UINT,1,NULL,0); + VARAND(NULL,0,BOOL,0,BOOL,0); + VARAND(NULL,0,BOOL,1,NULL,0); VARAND(NULL,0,R4,0,I4,0); - VARAND_TODOCOMM(NULL,0,R4,1,NULL,0); + VARAND(NULL,0,R4,1,NULL,0); VARAND(NULL,0,R8,0,I4,0); - VARAND_TODOCOMM(NULL,0,R8,1,NULL,0); - VARAND_TODOCOMM(NULL,0,BSTR,false_str,BOOL,0); + VARAND(NULL,0,R8,1,NULL,0); + VARAND(NULL,0,BSTR,false_str,BOOL,0); VARAND(NULL,0,BSTR,true_str,NULL,VARIANT_FALSE); VARANDCY(NULL,0,10000,NULL,0); VARANDCY(NULL,0,0,I4,0); diff --git a/dlls/oleaut32/variant.c b/dlls/oleaut32/variant.c index 48851ba05ec..4695ca66bb0 100644 --- a/dlls/oleaut32/variant.c +++ b/dlls/oleaut32/variant.c @@ -3048,52 +3048,100 @@ HRESULT WINAPI VarAnd(LPVARIANT left, LPVARIANT right, LPVARIANT result) if (leftvt == VT_NULL || rightvt == VT_NULL) { - /* - * Special cases for when left variant is VT_NULL - * (VT_NULL & 0 = VT_NULL, VT_NULL & value = value) - */ + /* VBScript three-valued logic for `And` with Null: + * zero And Null = zero (typed by the resvt computed above) + * nonzero And Null = Null + * Null And Null = Null + * + * Both orderings must produce the same result. The previous version + * only inspected the right operand when the left was Null, and never + * wrote the result payload — so the V_BOOL/V_I4 slot was uninitialized + * for every case that fell through. */ + VARIANT *other; + VARTYPE othervt; + BOOL nonzero = FALSE; + + if (leftvt == VT_NULL && rightvt == VT_NULL) + { + V_VT(result) = VT_NULL; + goto VarAnd_Exit; + } + if (leftvt == VT_NULL) + { + other = right; + othervt = rightvt; + } + else + { + other = left; + othervt = leftvt; + } + + switch (othervt) + { + case VT_EMPTY: break; + case VT_I1: nonzero = V_I1(other) != 0; break; + case VT_UI1: nonzero = V_UI1(other) != 0; break; + case VT_I2: nonzero = V_I2(other) != 0; break; + case VT_UI2: nonzero = V_UI2(other) != 0; break; + case VT_I4: nonzero = V_I4(other) != 0; break; + case VT_UI4: nonzero = V_UI4(other) != 0; break; + case VT_I8: nonzero = V_I8(other) != 0; break; + case VT_UI8: nonzero = V_UI8(other) != 0; break; + case VT_INT: nonzero = V_INT(other) != 0; break; + case VT_UINT: nonzero = V_UINT(other) != 0; break; + case VT_BOOL: nonzero = V_BOOL(other) != VARIANT_FALSE; break; + case VT_R4: nonzero = V_R4(other) != 0.0f; break; + case VT_R8: nonzero = V_R8(other) != 0.0; break; + case VT_DATE: nonzero = V_DATE(other) != 0.0; break; + case VT_CY: nonzero = V_CY(other).int64 != 0; break; + case VT_DECIMAL: nonzero = V_DECIMAL(other).Hi32 || V_DECIMAL(other).Lo64; break; + case VT_BSTR: { VARIANT_BOOL b; - switch(rightvt) - { - case VT_I1: if (V_I1(right)) resvt = VT_NULL; break; - case VT_UI1: if (V_UI1(right)) resvt = VT_NULL; break; - case VT_I2: if (V_I2(right)) resvt = VT_NULL; break; - case VT_UI2: if (V_UI2(right)) resvt = VT_NULL; break; - case VT_I4: if (V_I4(right)) resvt = VT_NULL; break; - case VT_UI4: if (V_UI4(right)) resvt = VT_NULL; break; - case VT_I8: if (V_I8(right)) resvt = VT_NULL; break; - case VT_UI8: if (V_UI8(right)) resvt = VT_NULL; break; - case VT_INT: if (V_INT(right)) resvt = VT_NULL; break; - case VT_UINT: if (V_UINT(right)) resvt = VT_NULL; break; - case VT_BOOL: if (V_BOOL(right)) resvt = VT_NULL; break; - case VT_R4: if (V_R4(right)) resvt = VT_NULL; break; - case VT_R8: if (V_R8(right)) resvt = VT_NULL; break; - case VT_CY: - if(V_CY(right).int64) - resvt = VT_NULL; - break; - case VT_DECIMAL: - if (V_DECIMAL(right).Hi32 || V_DECIMAL(right).Lo64) - resvt = VT_NULL; - break; - case VT_BSTR: - hres = VarBoolFromStr(V_BSTR(right), + hres = VarBoolFromStr(V_BSTR(other), LOCALE_USER_DEFAULT, VAR_LOCALBOOL, &b); - if (FAILED(hres)) - return hres; - else if (b) - V_VT(result) = VT_NULL; - else - { - V_VT(result) = VT_BOOL; - V_BOOL(result) = b; - } + if (FAILED(hres)) goto VarAnd_Exit; + if (b) + V_VT(result) = VT_NULL; + else + { + V_VT(result) = VT_BOOL; + V_BOOL(result) = VARIANT_FALSE; } + goto VarAnd_Exit; + } + default: + V_VT(result) = VT_NULL; + goto VarAnd_Exit; + } + + if (nonzero) + { + V_VT(result) = VT_NULL; + goto VarAnd_Exit; } + V_VT(result) = resvt; + switch (resvt) + { + case VT_BOOL: V_BOOL(result) = VARIANT_FALSE; break; + case VT_I1: V_I1(result) = 0; break; + case VT_UI1: V_UI1(result) = 0; break; + case VT_I2: V_I2(result) = 0; break; + case VT_UI2: V_UI2(result) = 0; break; + case VT_I4: V_I4(result) = 0; break; + case VT_UI4: V_UI4(result) = 0; break; + case VT_I8: V_I8(result) = 0; break; + case VT_UI8: V_UI8(result) = 0; break; + case VT_INT: V_INT(result) = 0; break; + case VT_UINT: V_UINT(result) = 0; break; + default: + V_VT(result) = VT_NULL; + break; + } goto VarAnd_Exit; } diff --git a/dlls/vbscript/tests/lang.vbs b/dlls/vbscript/tests/lang.vbs index d6dae1a9498..b990863ab2d 100644 --- a/dlls/vbscript/tests/lang.vbs +++ b/dlls/vbscript/tests/lang.vbs @@ -196,6 +196,16 @@ call ok(false imp false, "false does not imp false?") call ok(not (true imp false), "true imp false?") call ok(false imp null, "false imp null is false?") +' Smoke check that VBScript's `And` operator reaches VarAnd correctly and +' propagates the result payload. The full VarAnd+Null conformance table +' lives in dlls/oleaut32/tests/vartest.c. +Call ok((False And Null) = False, "False And Null is not False") +Call ok(isNull(True And Null), "True And Null is not Null") +Call ok(isNull(Null And Null), "Null And Null is not Null") +Call ok((CInt(0) And Null) = 0, "CInt(0) And Null is not 0") +Call ok(getVT(CInt(0) And Null) = "VT_I2", "getVT(CInt(0) And Null) = " & getVT(CInt(0) And Null)) +Call ok(isNull(CInt(5) And Null), "CInt(5) And Null is not Null") + Call ok(2 >= 1, "! 2 >= 1") Call ok(2 >= 2, "! 2 >= 2") Call ok(2 => 1, "! 2 => 1") -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10673
From: Francis De Brabandere <francisdb@gmail.com> VT_CY stores values scaled by 10000 in .int64, so the all-ones-Imp- Null check must compare to -10000 (not -1) for CCur(-1). Add right- Null conformance rows for I2/I4/R8/UI1 and a manual CY -1 case. --- dlls/oleaut32/tests/vartest.c | 20 ++++++++++++++++++++ dlls/oleaut32/variant.c | 3 ++- dlls/vbscript/tests/lang.vbs | 10 ++++++++++ 3 files changed, 32 insertions(+), 1 deletion(-) diff --git a/dlls/oleaut32/tests/vartest.c b/dlls/oleaut32/tests/vartest.c index c362f9854fe..dc85f04c0ae 100644 --- a/dlls/oleaut32/tests/vartest.c +++ b/dlls/oleaut32/tests/vartest.c @@ -9759,6 +9759,26 @@ static void test_VarImp(void) V_I8(&result), -3); } + /* All-ones left Imp Null returns Null for every numeric type: the + * three-valued "True Imp unknown = unknown" rule applies to any bit + * pattern that matches VARIANT_TRUE in the operand's width, including + * VT_UI1 0xFF (despite being unsigned). */ + VARIMP(I2,-1,NULL,0,NULL,0); + VARIMP(I4,-1,NULL,0,NULL,0); + VARIMP(R8,-1.0,NULL,0,NULL,0); + VARIMP(UI1,255,NULL,0,NULL,0); + + /* VT_CY stores values scaled by 10000 in .int64, so CY -1 has .int64 of + * -10000 (not -1). Previously Wine's all-ones check compared against -1 + * and never fired, producing a bitwise result instead of Null. */ + V_VT(&left) = VT_CY; + V_CY(&left).int64 = -10000; + V_VT(&right) = VT_NULL; + V_I4(&right) = 0; + V_VT(&exp) = VT_NULL; + V_I4(&exp) = 0; + test_var_call2(__LINE__, pVarImp, &left, &right, &exp); + SysFreeString(false_str); SysFreeString(true_str); } diff --git a/dlls/oleaut32/variant.c b/dlls/oleaut32/variant.c index 4695ca66bb0..6f78b52423b 100644 --- a/dlls/oleaut32/variant.c +++ b/dlls/oleaut32/variant.c @@ -5960,7 +5960,8 @@ HRESULT WINAPI VarImp(LPVARIANT left, LPVARIANT right, LPVARIANT result) case VT_BOOL: if (V_BOOL(left) == VARIANT_TRUE) resvt = VT_NULL; break; case VT_R4: if (V_R4(left) == -1.0) resvt = VT_NULL; break; case VT_R8: if (V_R8(left) == -1.0) resvt = VT_NULL; break; - case VT_CY: if (V_CY(left).int64 == -1) resvt = VT_NULL; break; + /* VT_CY stores values scaled by 10000, so -1 is -10000 in .int64. */ + case VT_CY: if (V_CY(left).int64 == -10000) resvt = VT_NULL; break; case VT_DECIMAL: if (V_DECIMAL(left).Hi32 == 0xffffffff) resvt = VT_NULL; diff --git a/dlls/vbscript/tests/lang.vbs b/dlls/vbscript/tests/lang.vbs index b990863ab2d..4062c3f826d 100644 --- a/dlls/vbscript/tests/lang.vbs +++ b/dlls/vbscript/tests/lang.vbs @@ -206,6 +206,16 @@ Call ok((CInt(0) And Null) = 0, "CInt(0) And Null is not 0") Call ok(getVT(CInt(0) And Null) = "VT_I2", "getVT(CInt(0) And Null) = " & getVT(CInt(0) And Null)) Call ok(isNull(CInt(5) And Null), "CInt(5) And Null is not Null") +' Smoke checks that VBScript's sibling logical operators reach Var* +' correctly and propagate the result payload. The full conformance tables +' live in dlls/oleaut32/tests/vartest.c. +Call ok((True Or Null) = True, "True Or Null is not True") +Call ok(isNull(False Or Null), "False Or Null is not Null") +Call ok(isNull(CInt(5) Xor Null), "CInt(5) Xor Null is not Null") +Call ok(isNull(CInt(5) Eqv Null), "CInt(5) Eqv Null is not Null") +Call ok(isNull(CByte(255) Imp Null), "CByte(255) Imp Null is not Null") +Call ok((Not CLng(0)) = -1, "Not CLng(0) is not -1") + Call ok(2 >= 1, "! 2 >= 1") Call ok(2 >= 2, "! 2 >= 2") Call ok(2 => 1, "! 2 => 1") -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10673
From: Francis De Brabandere <francisdb@gmail.com> Native VarImp returns VT_NULL for UI1 0xFF Imp Null via the three- valued all-ones rule, but native VBScript keeps UI1 width and returns the bitwise ~left. Handle the narrow case directly in interp_imp. --- dlls/vbscript/interp.c | 13 ++++++++++++- dlls/vbscript/tests/lang.vbs | 12 +++++++++++- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/dlls/vbscript/interp.c b/dlls/vbscript/interp.c index e7ce691ba2c..e71e642b98f 100644 --- a/dlls/vbscript/interp.c +++ b/dlls/vbscript/interp.c @@ -2098,7 +2098,18 @@ static HRESULT interp_imp(exec_ctx_t *ctx) hres = stack_pop_val(ctx, &l); if(SUCCEEDED(hres)) { - hres = VarImp(l.v, r.v, &v); + /* Native VarImp returns VT_NULL for UI1 0xFF Imp Null under the + * three-valued "all-ones Imp unknown = unknown" rule, but native + * VBScript keeps UI1 width and returns the bitwise complement of + * the left operand. Handle UI1 Imp Null directly. */ + if ((V_VT(l.v) & VT_TYPEMASK) == VT_UI1 && + (V_VT(r.v) & VT_TYPEMASK) == VT_NULL) { + V_VT(&v) = VT_UI1; + V_UI1(&v) = ~V_UI1(l.v); + hres = S_OK; + } else { + hres = VarImp(l.v, r.v, &v); + } release_val(&l); } release_val(&r); diff --git a/dlls/vbscript/tests/lang.vbs b/dlls/vbscript/tests/lang.vbs index 4062c3f826d..7d38297f12a 100644 --- a/dlls/vbscript/tests/lang.vbs +++ b/dlls/vbscript/tests/lang.vbs @@ -213,9 +213,19 @@ Call ok((True Or Null) = True, "True Or Null is not True") Call ok(isNull(False Or Null), "False Or Null is not Null") Call ok(isNull(CInt(5) Xor Null), "CInt(5) Xor Null is not Null") Call ok(isNull(CInt(5) Eqv Null), "CInt(5) Eqv Null is not Null") -Call ok(isNull(CByte(255) Imp Null), "CByte(255) Imp Null is not Null") Call ok((Not CLng(0)) = -1, "Not CLng(0) is not -1") +' VBScript-specific: for VT_UI1 Imp VT_NULL, native VBScript keeps UI1 +' width and returns the bitwise complement of the left operand, rather +' than applying VarImp's three-valued "all-ones Imp unknown = unknown" +' rule (which returns VT_NULL at the C level for UI1 0xFF). interp_imp +' has a narrow special case to match this native behavior. +Call ok((CByte(0) Imp Null) = 255, "CByte(0) Imp Null is not 255") +Call ok(getVT(CByte(0) Imp Null) = "VT_UI1", "getVT(CByte(0) Imp Null) = " & getVT(CByte(0) Imp Null)) +Call ok((CByte(170) Imp Null) = 85, "CByte(170) Imp Null is not 85") +Call ok((CByte(255) Imp Null) = 0, "CByte(255) Imp Null is not 0") +Call ok(getVT(CByte(255) Imp Null) = "VT_UI1", "getVT(CByte(255) Imp Null) = " & getVT(CByte(255) Imp Null)) + Call ok(2 >= 1, "! 2 >= 1") Call ok(2 >= 2, "! 2 >= 2") Call ok(2 => 1, "! 2 => 1") -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10673
From: Francis De Brabandere <francisdb@gmail.com> VarImp's right-Null switch was missing a VT_DATE case, so CDate(-1) Imp Null fell through to the main computation and returned VT_I4 0 instead of Null. Add the all-ones check using V_DATE. --- dlls/oleaut32/tests/vartest.c | 1 + dlls/oleaut32/variant.c | 1 + dlls/vbscript/tests/lang.vbs | 1 + 3 files changed, 3 insertions(+) diff --git a/dlls/oleaut32/tests/vartest.c b/dlls/oleaut32/tests/vartest.c index dc85f04c0ae..e5bbecbdd3f 100644 --- a/dlls/oleaut32/tests/vartest.c +++ b/dlls/oleaut32/tests/vartest.c @@ -9766,6 +9766,7 @@ static void test_VarImp(void) VARIMP(I2,-1,NULL,0,NULL,0); VARIMP(I4,-1,NULL,0,NULL,0); VARIMP(R8,-1.0,NULL,0,NULL,0); + VARIMP(DATE,-1.0,NULL,0,NULL,0); VARIMP(UI1,255,NULL,0,NULL,0); /* VT_CY stores values scaled by 10000 in .int64, so CY -1 has .int64 of diff --git a/dlls/oleaut32/variant.c b/dlls/oleaut32/variant.c index 6f78b52423b..3f80d621f5d 100644 --- a/dlls/oleaut32/variant.c +++ b/dlls/oleaut32/variant.c @@ -5960,6 +5960,7 @@ HRESULT WINAPI VarImp(LPVARIANT left, LPVARIANT right, LPVARIANT result) case VT_BOOL: if (V_BOOL(left) == VARIANT_TRUE) resvt = VT_NULL; break; case VT_R4: if (V_R4(left) == -1.0) resvt = VT_NULL; break; case VT_R8: if (V_R8(left) == -1.0) resvt = VT_NULL; break; + case VT_DATE: if (V_DATE(left) == -1.0) resvt = VT_NULL; break; /* VT_CY stores values scaled by 10000, so -1 is -10000 in .int64. */ case VT_CY: if (V_CY(left).int64 == -10000) resvt = VT_NULL; break; case VT_DECIMAL: diff --git a/dlls/vbscript/tests/lang.vbs b/dlls/vbscript/tests/lang.vbs index 7d38297f12a..21e272c96b6 100644 --- a/dlls/vbscript/tests/lang.vbs +++ b/dlls/vbscript/tests/lang.vbs @@ -213,6 +213,7 @@ Call ok((True Or Null) = True, "True Or Null is not True") Call ok(isNull(False Or Null), "False Or Null is not Null") Call ok(isNull(CInt(5) Xor Null), "CInt(5) Xor Null is not Null") Call ok(isNull(CInt(5) Eqv Null), "CInt(5) Eqv Null is not Null") +Call ok(isNull(CDate(-1) Imp Null), "CDate(-1) Imp Null is not Null") Call ok((Not CLng(0)) = -1, "Not CLng(0) is not -1") ' VBScript-specific: for VT_UI1 Imp VT_NULL, native VBScript keeps UI1 -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10673
participants (2)
-
Francis De Brabandere -
Francis De Brabandere (＠francisdb)