[Bug 53767] New: vbscript fails to handle ReDim when variable is not yet created

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=53767

Bug ID: 53767
           Summary: vbscript fails to handle ReDim when variable is not
                    yet created
           Product: Wine
           Version: 7.16
          Hardware: x86-64
                OS: Linux
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: vbscript
          Assignee: [e-mail]
          Reporter: [e-mail]
      Distribution: ---

While working on leveraging the vbscript engine of Wine for a macos/linux port
of Visual Pinball, I finally have the example table working and am starting to
work through the runtime errors.

Since the table scripts are often copies of copies of copied code, sometimes
bad practices make their way in.

Take the following code:

Const tnob = 5 ' total number of balls
ReDim rolling(tnob)

This works in real vbscript, but fails here.

As a work around, I've updated the script to:

Const tnob = 5 ' total number of balls
Dim rolling()
ReDim rolling(tnob)

According to the rules I've found:

It's possible to create a new dynamic array within a procedure using the ReDim
statement if the array to which it refers doesn't already exist at either
module or level. Typically, this results from an error of omission; the
programmer forgets to explicitly define the array using Dim, Public, or
Private. Since this method of creating an array can cause conflicts if a
variable or array of the same name is subsequently defined explicitly, ReDim
should be used only to redimension an existing array, not to define a new one.

[Bug 53767] vbscript fails to handle ReDim when variable is not yet created

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=53767

Nikolay Sivov  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW

--- Comment #1 from Nikolay Sivov  ---
Confirming. It should be easy to fix in interp_redim(). lookup_identifier()
never fails apparently, missing symbol in case of redim() will result in
REF_NONE type, when that happens we could probably create a new array variable
on a spot.

[Bug 53767] vbscript fails to handle ReDim when variable is not yet created

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=53767

Robert Wilhelm  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

[Bug 53767] vbscript fails to handle ReDim when variable is not yet created

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=53767

--- Comment #2 from Jason Millard  ---
Thanks!

I messed with this for quite a while, but I'm missing something.. I even tried
to just calling dim when REF_NONE, but then it asserts in various places.

This is the last script bug, before I have a working "un-modified" example
table script.

[Bug 53767] vbscript fails to handle ReDim when variable is not yet created

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=53767

--- Comment #3 from Robert Wilhelm  ---
Jason, as proof of concept, I hacked together some random lines which I think
is along Nikolay's suggestion.

It needs quite some cleanup, as there is no error handling and no handling of
multidimensional arrays. We could also make use of already existing helper
funtions.

At least my testcase works with it.

Const tnob = 5 ' total number of balls
ReDim rolling(tnob)
rolling(2) = 3
MsgBox rolling(2)

diff --git a/dlls/vbscript/interp.c b/dlls/vbscript/interp.c
index c43b5383dee..0b9d86ce864 100644
--- a/dlls/vbscript/interp.c
+++ b/dlls/vbscript/interp.c
@@ -1297,8 +1306,20 @@ static HRESULT interp_redim(exec_ctx_t *ctx)
     }

if(ref.type != REF_VAR) {
-        FIXME("got ref.type = %d\n", ref.type);
-        return E_FAIL;
+        int dim;
+        ctx->func->array_cnt++;
+        array_desc_t *array_desc;
+        array_desc = (array_desc_t *)malloc (sizeof(array_desc_t));
+         array_desc->dim_cnt = 1;
+         array_desc->bounds = (SAFEARRAYBOUND *)
malloc(sizeof(SAFEARRAYBOUND));
+         to_int(stack_top(ctx, 0), &dim);
+         stack_popn(ctx, 1);
+         array_desc->bounds->cElements = dim;
+         array_desc->bounds->lLbound = 0;
+         ctx->func->array_descs = array_desc;
+        VARIANT *new;
+        hres = add_dynamic_var(ctx, identifier, FALSE, &new);
+        return interp_dim(ctx);
     }

hres = array_bounds_from_stack(ctx, dim_cnt, &bounds);

[Bug 53767] vbscript fails to handle ReDim when variable is not yet created

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=53767

--- Comment #4 from Jason Millard  ---
Thank you!

I just tried this, but it asserts in interp_dim at the:

assert(array_id < ctx->func->array_cnt);

[vbscript][interp_redim]: L"rolling" 1
[vbscript][interp_dim]: L"rolling"
Assertion failed: (array_id < ctx->func->array_cnt), function interp_dim, file
interp.c, line 1209.

array_id is 1
ctx->func->array_cnt is 1

The code is:

Const tnob = 5 ' total number of balls
ReDim rolling(tnob)

[Bug 53767] vbscript fails to handle ReDim when variable is not yet created

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=53767

--- Comment #5 from Robert Wilhelm  ---
Sorry, there was one line missing. Add
 ctx->instr->arg2.uint = 0
before add_dynamic_var call.

This should work
@@ -1297,8 +1306,22 @@ static HRESULT interp_redim(exec_ctx_t *ctx)
     }

if(ref.type != REF_VAR) {
+        int dim;
         FIXME("got ref.type = %d\n", ref.type);
-        return E_FAIL;
+        ctx->func->array_cnt++;
+        array_desc_t *array_desc;
+        array_desc = (array_desc_t *)malloc (sizeof(array_desc_t));
+         array_desc->dim_cnt = 1;
+         array_desc->bounds = (SAFEARRAYBOUND *)
malloc(sizeof(SAFEARRAYBOUND));
+         to_int(stack_top(ctx, 0), &dim);
+         stack_popn(ctx, 1);
+         array_desc->bounds->cElements = dim;
+         array_desc->bounds->lLbound = 0;
+         ctx->func->array_descs = array_desc;
+        VARIANT *new;
+        ctx->instr->arg2.uint = 0;
+        hres = add_dynamic_var(ctx, identifier, FALSE, &new);
+        return interp_dim(ctx);
     }

hres = array_bounds_from_stack(ctx, dim_cnt, &bounds);

[Bug 53767] vbscript fails to handle ReDim when variable is not yet created

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=53767

--- Comment #6 from Jason Millard  ---
So that gets past the assert, however, another issue cropped up.

This is the actual code:

Const tnob = 5 ' total number of balls
ReDim rolling(tnob)
InitRolling

Sub InitRolling
    Dim i
    For i = 0 to tnob
        rolling(i) = False
    Next
End Sub

It fails on rolling(i) = False

This was the workaround code:

Const tnob = 5 ' total number of balls
Dim rolling()
ReDim rolling(tnob)
InitRolling

Sub InitRolling
    Dim i
    For i = 0 to tnob
        rolling(i) = False
    Next
End Sub

[Bug 53767] vbscript fails to handle ReDim when variable is not yet created

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=53767

--- Comment #7 from Robert Wilhelm  ---

Please change
array_desc->bounds->cElements = dim;
to
array_desc->bounds->cElements = dim + 1;

[Bug 53767] vbscript fails to handle ReDim when variable is not yet created

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=53767

--- Comment #8 from Jason Millard  ---
Amazing! That was it.

First table to run without any script modifications!

If your curious, I just uploaded a video:

https://www.youtube.com/watch?v=Iko2nGw0JfU

[Bug 53767] vbscript fails to handle ReDim when variable is not yet created

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=53767

--- Comment #9 from Robert Wilhelm  ---
Nice video. Congrats to your achievement porting Visual Pinball.

[Bug 53767] vbscript fails to handle ReDim when variable is not yet created

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=53767

--- Comment #10 from Jason Millard  ---
So I resolved a few more issues and had to circle back to this as I think the
patch might have an issue.

It's almost like a redim on a new veriable will mess up future dims:

Example:

dim A(20)
WScript.Echo "A = " & UBound(A)

redim B(40)
WScript.Echo "B = " & UBound(B)

redim C(35)
WScript.Echo "C = " & UBound(C)

dim D(50)
WScript.Echo "D = " & UBound(D)

A = 20
B = 40
C = 35
VBSE_OUT_OF_MEMORY

Example:

redim B(40)
WScript.Echo "B = " & UBound(B)

redim C(35)
WScript.Echo "C = " & UBound(C)

dim D(50)
WScript.Echo "D = " & UBound(D)

Debug: B = 40
Debug: C = 35
Debug: D = 35

[Bug 53767] vbscript fails to handle ReDim when variable is not yet created

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=53767

Jason Millard  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

[Bug 53767] vbscript fails to handle ReDim when variable is not yet created

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=53767

--- Comment #11 from Jason Millard  ---
I think with the current approach ctx->func->array_descs is getting corrupted.
I think it would need to grow or realloc, but couldn't figure out array_id.

I traced how this works:

Dim d
Redim d(10)

I then simplified what you had to this:

if(ref.type != REF_VAR) {
        ref.type = REF_VAR;
        hres = add_dynamic_var(ctx, identifier, FALSE, &ref.u.v);
    }

hres = array_bounds_from_stack(ctx, dim_cnt, &bounds);
    if(FAILED(hres))
        return hres;

.
    .
    .

Somehow this is working, but obviously I could be missing something.

[Bug 53767] vbscript fails to handle ReDim when variable is not yet created

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=53767

francisdb  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #12 from francisdb  ---
https://gitlab.winehq.org/wine/wine/-/merge_requests/7070