[PATCH 0/1] MR1604: vbscript: Make redim fail on fixed-size arrays.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=53964 Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1604
From: Nikolay Sivov <nsivov(a)codeweavers.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=53964 Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- dlls/vbscript/interp.c | 14 +++++++++++--- dlls/vbscript/tests/lang.vbs | 11 ++++++++++- dlls/vbscript/tests/run.c | 34 ++++++++++++++++++++++++++++++++-- 3 files changed, 53 insertions(+), 6 deletions(-) diff --git a/dlls/vbscript/interp.c b/dlls/vbscript/interp.c index 2d33982839e..1e80767a2d3 100644 --- a/dlls/vbscript/interp.c +++ b/dlls/vbscript/interp.c @@ -1246,6 +1246,7 @@ static HRESULT interp_dim(exec_ctx_t *ctx) *array_ref = SafeArrayCreate(VT_VARIANT, array_desc->dim_cnt, array_desc->bounds); if(!*array_ref) return E_OUTOFMEMORY; + (*array_ref)->fFeatures |= (FADF_FIXEDSIZE | FADF_STATIC); } V_VT(v) = VT_ARRAY|VT_BYREF|VT_VARIANT; @@ -1311,14 +1312,21 @@ static HRESULT interp_redim(exec_ctx_t *ctx) if(!array) return E_OUTOFMEMORY; - /* FIXME: We should check if we're not modifying an existing static array here */ - v = ref.u.v; if(V_VT(v) == (VT_VARIANT|VT_BYREF)) { v = V_VARIANTREF(v); } + if(V_ISARRAY(v)) { + SAFEARRAY *sa = V_ISBYREF(v) ? *V_ARRAYREF(v) : V_ARRAY(v); + if(sa->fFeatures & FADF_FIXEDSIZE) + { + SafeArrayDestroy(array); + return MAKE_VBSERROR(VBSE_ARRAY_LOCKED); + } + } + VariantClear(v); V_VT(v) = VT_ARRAY|VT_VARIANT; V_ARRAY(v) = array; @@ -1361,7 +1369,7 @@ static HRESULT interp_redim_preserve(exec_ctx_t *ctx) return E_FAIL; } - array = V_ARRAY(v); + array = V_ISBYREF(v) ? *V_ARRAYREF(v) : V_ARRAY(v); hres = array_bounds_from_stack(ctx, dim_cnt, &bounds); if(FAILED(hres)) diff --git a/dlls/vbscript/tests/lang.vbs b/dlls/vbscript/tests/lang.vbs index ddfaf20a19d..04492e04474 100644 --- a/dlls/vbscript/tests/lang.vbs +++ b/dlls/vbscript/tests/lang.vbs @@ -1536,7 +1536,16 @@ on error resume next redim staticarray(3) e = err.number on error goto 0 -todo_wine_ok e = 10, "e = " & e +ok e = 10, "e = " & e +ok isArrayFixed(staticarray), "Expected fixed size array" + +dim staticarray2(4) +on error resume next +redim preserve staticarray2(5) +e = err.number +on error goto 0 +ok e = 10, "e = " & e +ok isArrayFixed(staticarray2), "Expected fixed size array" sub TestReDimList dim x, y diff --git a/dlls/vbscript/tests/run.c b/dlls/vbscript/tests/run.c index 31ac03ab8d7..fd53ea37e75 100644 --- a/dlls/vbscript/tests/run.c +++ b/dlls/vbscript/tests/run.c @@ -146,6 +146,7 @@ DEFINE_EXPECT(OnLeaveScript); #define DISPID_GLOBAL_PROPARGSET 1025 #define DISPID_GLOBAL_UNKOBJ 1026 #define DISPID_GLOBAL_THROWEXCEPTION 1027 +#define DISPID_GLOBAL_ISARRAYFIXED 1028 #define DISPID_TESTOBJ_PROPGET 2000 #define DISPID_TESTOBJ_PROPPUT 2001 @@ -568,7 +569,6 @@ static void test_safearray(SAFEARRAY *safearray, unsigned indims) if(!exdims) exdims = 1; ok(safearray->cDims == exdims, "safearray->cDims = %d, expected %d\n", safearray->cDims, exdims); - todo_wine ok(safearray->fFeatures == (FADF_VARIANT|FADF_HAVEVARTYPE|FADF_FIXEDSIZE|FADF_STATIC), "safearray->fFeatures = %x\n", safearray->fFeatures); ok(safearray->cbElements == sizeof(VARIANT), "safearray->cbElements = %lx\n", safearray->cbElements); @@ -1163,7 +1163,8 @@ static HRESULT WINAPI Global_GetDispID(IDispatchEx *iface, BSTR bstrName, DWORD { L"testOptionalArg", DISPID_GLOBAL_TESTOPTIONALARG }, { L"testErrorObject", DISPID_GLOBAL_TESTERROROBJECT }, { L"throwWithDesc", DISPID_GLOBAL_THROWWITHDESC }, - { L"unkObj", DISPID_GLOBAL_UNKOBJ } + { L"unkObj", DISPID_GLOBAL_UNKOBJ }, + { L"isArrayFixed", DISPID_GLOBAL_ISARRAYFIXED }, }; test_grfdex(grfdex, fdexNameCaseInsensitive); @@ -1737,6 +1738,35 @@ static HRESULT WINAPI Global_InvokeEx(IDispatchEx *iface, DISPID id, LCID lcid, V_VT(pvarRes) = VT_UNKNOWN; V_UNKNOWN(pvarRes) = &unkObj; return S_OK; + + case DISPID_GLOBAL_ISARRAYFIXED: + { + BOOL is_fixed = FALSE; + VARIANT *v; + + ok(pdp != NULL, "pdp == NULL\n"); + ok(pdp->rgvarg != NULL, "rgvarg == NULL\n"); + ok(!pdp->rgdispidNamedArgs, "rgdispidNamedArgs != NULL\n"); + ok(pdp->cArgs == 1, "cArgs = %d\n", pdp->cArgs); + ok(!pdp->cNamedArgs, "cNamedArgs = %d\n", pdp->cNamedArgs); + ok(pvarRes != NULL, "pvarRes == NULL\n"); + ok(V_VT(pvarRes) == VT_EMPTY, "V_VT(pvarRes) = %d\n", V_VT(pvarRes)); + ok(pei != NULL, "pei == NULL\n"); + + ok(V_VT(pdp->rgvarg) == (VT_VARIANT|VT_BYREF), "Unexpected argument type %d.\n", V_VT(pdp->rgvarg)); + v = V_VARIANTREF(pdp->rgvarg); + ok(V_VT(v) == (VT_VARIANT|VT_ARRAY|VT_BYREF), "Unexpected argument type %d.\n", V_VT(v)); + if (V_ISARRAY(v)) + { + SAFEARRAY *sa = V_ISBYREF(v) ? *V_ARRAYREF(v) : V_ARRAY(v); + is_fixed = (sa->fFeatures & (FADF_FIXEDSIZE | FADF_STATIC)) == (FADF_FIXEDSIZE | FADF_STATIC); + sa->fFeatures &= ~FADF_FIXEDSIZE; + } + + V_VT(pvarRes) = VT_BOOL; + V_BOOL(pvarRes) = is_fixed ? VARIANT_TRUE : VARIANT_FALSE; + return S_OK; + } } ok(0, "unexpected call %ld\n", id); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1604
Jacek Caban (@jacek) commented about dlls/vbscript/tests/run.c:
+ ok(pdp->rgvarg != NULL, "rgvarg == NULL\n"); + ok(!pdp->rgdispidNamedArgs, "rgdispidNamedArgs != NULL\n"); + ok(pdp->cArgs == 1, "cArgs = %d\n", pdp->cArgs); + ok(!pdp->cNamedArgs, "cNamedArgs = %d\n", pdp->cNamedArgs); + ok(pvarRes != NULL, "pvarRes == NULL\n"); + ok(V_VT(pvarRes) == VT_EMPTY, "V_VT(pvarRes) = %d\n", V_VT(pvarRes)); + ok(pei != NULL, "pei == NULL\n"); + + ok(V_VT(pdp->rgvarg) == (VT_VARIANT|VT_BYREF), "Unexpected argument type %d.\n", V_VT(pdp->rgvarg)); + v = V_VARIANTREF(pdp->rgvarg); + ok(V_VT(v) == (VT_VARIANT|VT_ARRAY|VT_BYREF), "Unexpected argument type %d.\n", V_VT(v)); + if (V_ISARRAY(v)) + { + SAFEARRAY *sa = V_ISBYREF(v) ? *V_ARRAYREF(v) : V_ARRAY(v); + is_fixed = (sa->fFeatures & (FADF_FIXEDSIZE | FADF_STATIC)) == (FADF_FIXEDSIZE | FADF_STATIC); + sa->fFeatures &= ~FADF_FIXEDSIZE; Is modifying fFeatures a leftover from some experimenting?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1604#note_18008
Jacek Caban (@jacek) commented about dlls/vbscript/interp.c:
- /* FIXME: We should check if we're not modifying an existing static array here */ - v = ref.u.v;
if(V_VT(v) == (VT_VARIANT|VT_BYREF)) { v = V_VARIANTREF(v); }
+ if(V_ISARRAY(v)) { + SAFEARRAY *sa = V_ISBYREF(v) ? *V_ARRAYREF(v) : V_ARRAY(v); + if(sa->fFeatures & FADF_FIXEDSIZE) + { + SafeArrayDestroy(array); + return MAKE_VBSERROR(VBSE_ARRAY_LOCKED); + } + } I think this should be checked earlier, before `array_bounds_from_stack()` call. This would avoid creating the new array and my testing confirms that's what Windows does (if I try to use "abc" as redim argument, VBSE_ARRAY_LOCKED is still returned on Windows rather than `to_int()` error).
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1604#note_18009
On Wed Nov 30 10:36:48 2022 +0000, Jacek Caban wrote:
Is modifying fFeatures a leftover from some experimenting? Yes, it is. The experiment was to see if "fixed" state is in direct relation to safearray flags, or tracked separately, e.g. per identifier.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1604#note_18010
participants (3)
-
Jacek Caban (＠jacek) -
Nikolay Sivov
-
Nikolay Sivov (＠nsivov)