[PATCH 0/1] MR1604: vbscript: Make redim fail on fixed-size arrays.

[Nikolay Sivov (＠nsivov)]

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=53964
Signed-off-by: Nikolay Sivov

[PATCH 1/1] vbscript: Make redim fail on fixed-size arrays.

[Nikolay Sivov]

From: Nikolay Sivov

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=53964
Signed-off-by: Nikolay Sivov 
---
 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);

[Jacek Caban (＠jacek)]

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?

[Jacek Caban (＠jacek)]

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).

[Nikolay Sivov (＠nsivov)]

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.