[PATCH v3 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

--
  v3: vbscript: Make redim fail on fixed-size arrays.

https://gitlab.winehq.org/wine/wine/-/merge_requests/1604

[PATCH v3 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       | 23 ++++++++++-------
 dlls/vbscript/tests/lang.vbs | 48 +++++++++++++++++++++++++++++++-----
 dlls/vbscript/tests/run.c    | 33 +++++++++++++++++++++++--
 3 files changed, 87 insertions(+), 17 deletions(-)

diff --git a/dlls/vbscript/interp.c b/dlls/vbscript/interp.c
index 2d33982839e..b3be3bea6cf 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;
@@ -1302,6 +1303,18 @@ static HRESULT interp_redim(exec_ctx_t *ctx)
         return E_FAIL;
     }

+    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)
+            return MAKE_VBSERROR(VBSE_ARRAY_LOCKED);
+    }
+
     hres = array_bounds_from_stack(ctx, dim_cnt, &bounds);
     if(FAILED(hres))
         return hres;
@@ -1311,14 +1324,6 @@ 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);
-    }
-
     VariantClear(v);
     V_VT(v) = VT_ARRAY|VT_VARIANT;
     V_ARRAY(v) = array;
@@ -1361,7 +1366,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..d27277508c0 100644
--- a/dlls/vbscript/tests/lang.vbs
+++ b/dlls/vbscript/tests/lang.vbs
@@ -1531,12 +1531,48 @@ e = err.number
 on error goto 0
 ok e = 9, "e = " & e ' VBSE_OUT_OF_BOUNDS, can only change rightmost dimension

-dim staticarray(4)
-on error resume next
-redim staticarray(3)
-e = err.number
-on error goto 0
-todo_wine_ok e = 10, "e = " & e
+sub TestReDimFixed
+    on error resume next
+
+    dim staticarray(4)
+    err.clear
+    redim staticarray(3)
+    call ok(err.number = 10, "err.number = " & err.number)
+    call ok(isArrayFixed(staticarray), "Expected fixed size array")
+
+    err.clear
+    redim staticarray("abc")
+    call ok(err.number = 10, "err.number = " & err.number)
+
+    dim staticarray2(4)
+    err.clear
+    redim preserve staticarray2(5)
+    call ok(err.number = 10, "err.number = " & err.number)
+    call ok(isArrayFixed(staticarray2), "Expected fixed size array")
+
+    err.clear
+    redim preserve staticarray2("abc")
+    ' Win10+ builds return INVALID_CALL (5)
+    call ok(err.number = 5 or err.number = 13, "err.number = " & err.number)
+end sub
+Call TestRedimFixed
+
+sub TestRedimInputArg
+    on error resume next
+
+    dim x
+
+    x = Array(1)
+    err.clear
+    redim x("abc")
+    call ok(err.number = 13, "err.number = " & err.number)
+
+    err.clear
+    redim preserve x("abc")
+    ' Win10+ builds return INVALID_CALL (5)
+    call ok(err.number = 5 or err.number = 13, "err.number = " & err.number)
+end sub
+Call TestRedimInputArg

sub TestReDimList
     dim x, y
diff --git a/dlls/vbscript/tests/run.c b/dlls/vbscript/tests/run.c
index 31ac03ab8d7..022d714dea5 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,34 @@ 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);
+        }
+
+        V_VT(pvarRes) = VT_BOOL;
+        V_BOOL(pvarRes) = is_fixed ? VARIANT_TRUE : VARIANT_FALSE;
+        return S_OK;
+    }
     }

ok(0, "unexpected call %ld\n", id);

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

[Marvin]

Hi,

It looks like your patch introduced the new failures shown below.
Please investigate and fix them before resubmitting your patch.
If they are not new, fixing them anyway would help a lot. Otherwise
please ask for the known failures list to be updated.

The tests also ran into some preexisting test failures. If you know how
to fix them that would be helpful. See the TestBot job for the details:

The full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=127004

Your paranoid android.

=== w7u_el (testbot log) ===

WineRunTask.pl:error: The previous 1 run(s) terminated abnormally

[Nikolay Sivov (＠nsivov)]

Okay, one more update to accept both error codes.

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

[Jacek Caban (＠jacek)]

This merge request was approved by Jacek Caban.