[Bug 53767] New: vbscript fails to handle ReDim when variable is not yet created
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: wine-bugs(a)winehq.org Reporter: jsm174(a)gmail.com 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. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=53767 Nikolay Sivov <bunglehead(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Ever confirmed|0 |1 Status|UNCONFIRMED |NEW --- Comment #1 from Nikolay Sivov <bunglehead(a)gmail.com> --- 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. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=53767 Robert Wilhelm <sloper42(a)yahoo.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |sloper42(a)yahoo.com -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=53767 --- Comment #2 from Jason Millard <jsm174(a)gmail.com> --- 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. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=53767 --- Comment #3 from Robert Wilhelm <sloper42(a)yahoo.com> --- 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); -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=53767 --- Comment #4 from Jason Millard <jsm174(a)gmail.com> --- 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) -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=53767 --- Comment #5 from Robert Wilhelm <sloper42(a)yahoo.com> --- 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); -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=53767 --- Comment #6 from Jason Millard <jsm174(a)gmail.com> --- 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 -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=53767 --- Comment #7 from Robert Wilhelm <sloper42(a)yahoo.com> --- Please change array_desc->bounds->cElements = dim; to array_desc->bounds->cElements = dim + 1; -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=53767 --- Comment #8 from Jason Millard <jsm174(a)gmail.com> --- 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 -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=53767 --- Comment #9 from Robert Wilhelm <sloper42(a)yahoo.com> --- Nice video. Congrats to your achievement porting Visual Pinball. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=53767 --- Comment #10 from Jason Millard <jsm174(a)gmail.com> --- 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 -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=53767 Jason Millard <jsm174(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jsm174(a)gmail.com -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=53767 --- Comment #11 from Jason Millard <jsm174(a)gmail.com> --- 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. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
https://bugs.winehq.org/show_bug.cgi?id=53767 francisdb <francisdb(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |francisdb(a)gmail.com --- Comment #12 from francisdb <francisdb(a)gmail.com> --- https://gitlab.winehq.org/wine/wine/-/merge_requests/7070 -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=53767 Zeb Figura <z.figura12(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |z.figura12(a)gmail.com Resolution|--- |FIXED Status|NEW |RESOLVED Fixed by SHA1| |10651f22ebe0ccdc3375f00e048 | |75a52dcc5fe52 --- Comment #13 from Zeb Figura <z.figura12(a)gmail.com> --- Fixed upstream by 10651f22ebe0ccdc3375f00e04875a52dcc5fe52. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=53767 Alexandre Julliard <julliard(a)winehq.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED --- Comment #14 from Alexandre Julliard <julliard(a)winehq.org> --- Closing bugs fixed in 10.16. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
participants (1)
-
WineHQ Bugzilla