[Bug 53766] New: vbscript fails to handle SAFEARRAY assignment, access, UBounds, LBounds
https://bugs.winehq.org/show_bug.cgi?id=53766 Bug ID: 53766 Summary: vbscript fails to handle SAFEARRAY assignment, access, UBounds, LBounds 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. The table script requests an array of balls on the table and uses that array to drop ball shadows and generate rolling sounds. Take the following code: Const tnob = 5 Dim rolling() ReDim rolling(tnob) Sub RollingTimer_Timer() Dim BOT, b BOT = GetBalls For b = 0 to UBound(BOT) If BOT(b).z < 30 Then rolling(b) = True End If Next End Sub GetBalls is in the application code and returns a SAFEARRAY of IDispatch pointers: STDMETHODIMP ScriptGlobalTable::GetBalls(LPSAFEARRAY *pVal) Currently this will fail at BOT assign_value because VariantCopyInd calls VariantCopy which calls VARIANT_ValidateType which then returns DISP_E_BADVARTYPE I believe I have this now working but I'm not sure if it would impact anything else for an official patch: inc/wine/dlls/oleaut32/variant.c: HRESULT WINAPI VariantCopy(VARIANTARG* pvargDest, const VARIANTARG* pvargSrc) { HRESULT hres = S_OK; TRACE("(%s,%s)\n", debugstr_variant(pvargDest), debugstr_variant(pvargSrc)); #ifndef __SAFEARRAYFIX__ if (V_TYPE(pvargSrc) == VT_CLSID || /* VT_CLSID is a special case */ FAILED(VARIANT_ValidateType(V_VT(pvargSrc)))) return DISP_E_BADVARTYPE; #else if (V_TYPE(pvargSrc) != VT_SAFEARRAY && (V_TYPE(pvargSrc) == VT_CLSID || /* VT_CLSID is a special case */ FAILED(VARIANT_ValidateType(V_VT(pvargSrc))))) return DISP_E_BADVARTYPE; #endif if (pvargSrc != pvargDest && SUCCEEDED(hres = VariantClear(pvargDest))) { *pvargDest = *pvargSrc; /* Shallow copy the value */ if (!V_ISBYREF(pvargSrc)) { switch (V_VT(pvargSrc)) { case VT_BSTR: V_BSTR(pvargDest) = SysAllocStringByteLen((char*)V_BSTR(pvargSrc), SysStringByteLen(V_BSTR(pvargSrc))); if (!V_BSTR(pvargDest)) hres = E_OUTOFMEMORY; break; case VT_RECORD: hres = VARIANT_CopyIRecordInfo(pvargDest, pvargSrc); break; case VT_DISPATCH: case VT_UNKNOWN: V_UNKNOWN(pvargDest) = V_UNKNOWN(pvargSrc); if (V_UNKNOWN(pvargSrc)) IUnknown_AddRef(V_UNKNOWN(pvargSrc)); break; #ifdef __SAFEARRAYFIX__ case VT_SAFEARRAY: hres = SafeArrayCopy(V_ARRAY(pvargSrc), &V_ARRAY(pvargDest)); break; #endif default: if (V_ISARRAY(pvargSrc)) hres = SafeArrayCopy(V_ARRAY(pvargSrc), &V_ARRAY(pvargDest)); } } } return hres; } inc/wine/dlls/vbscript/interp.c: static HRESULT variant_call(exec_ctx_t *ctx, VARIANT *v, unsigned arg_cnt, VARIANT *res) { SAFEARRAY *array = NULL; DISPPARAMS dp; HRESULT hres; TRACE("%s\n", debugstr_variant(v)); if(V_VT(v) == (VT_VARIANT|VT_BYREF)) v = V_VARIANTREF(v); switch(V_VT(v)) { case VT_ARRAY|VT_BYREF|VT_VARIANT: array = *V_ARRAYREF(v); break; case VT_ARRAY|VT_VARIANT: array = V_ARRAY(v); break; #ifdef __SAFEARRAYFIX__ case VT_SAFEARRAY: array = V_ARRAY(v); break; #endif case VT_DISPATCH: vbstack_to_dp(ctx, arg_cnt, FALSE, &dp); hres = disp_call(ctx->script, V_DISPATCH(v), DISPID_VALUE, &dp, res); break; default: FIXME("unsupported on %s\n", debugstr_variant(v)); return E_NOTIMPL; } if(array) { if(!res) { FIXME("no res\n"); return E_NOTIMPL; } vbstack_to_dp(ctx, arg_cnt, FALSE, &dp); hres = array_access(ctx, array, &dp, &v); if(FAILED(hres)) return hres; V_VT(res) = VT_BYREF|VT_VARIANT; V_BYREF(res) = v; } stack_popn(ctx, arg_cnt); return S_OK; } inc/wine/dlls/vbscript/global.c: static HRESULT Global_UBound(BuiltinDisp *This, VARIANT *arg, unsigned args_cnt, VARIANT *res) { SAFEARRAY *sa; HRESULT hres; LONG ubound; int dim; assert(args_cnt == 1 || args_cnt == 2); TRACE("%s %s\n", debugstr_variant(arg), args_cnt == 2 ? debugstr_variant(arg + 1) : "1"); switch(V_VT(arg)) { case VT_VARIANT|VT_ARRAY: sa = V_ARRAY(arg); break; case VT_VARIANT|VT_ARRAY|VT_BYREF: sa = *V_ARRAYREF(arg); break; #ifdef __SAFEARRAYFIX__ case VT_SAFEARRAY: sa = V_ARRAY(arg); break; #endif default: FIXME("arg %s not supported\n", debugstr_variant(arg)); return E_NOTIMPL; } static HRESULT Global_LBound(BuiltinDisp *This, VARIANT *arg, unsigned args_cnt, VARIANT *res) { SAFEARRAY *sa; HRESULT hres; LONG ubound; int dim; assert(args_cnt == 1 || args_cnt == 2); TRACE("%s %s\n", debugstr_variant(arg), args_cnt == 2 ? debugstr_variant(arg + 1) : "1"); switch(V_VT(arg)) { case VT_VARIANT|VT_ARRAY: sa = V_ARRAY(arg); break; case VT_VARIANT|VT_ARRAY|VT_BYREF: sa = *V_ARRAYREF(arg); break; #ifdef __SAFEARRAYFIX__ case VT_SAFEARRAY: sa = V_ARRAY(arg); break; #endif default: FIXME("arg %s not supported\n", debugstr_variant(arg)); return E_NOTIMPL; } -- 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=53766 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=53766 --- Comment #1 from Jason Millard <jsm174(a)gmail.com> --- btw, all the code for this project is at: https://github.com/jsm174/vpvr/tree/feature/standalone/inc/wine It can be compiled and debugged under VSCode on mac with CMaketools (and hopefully soon linux). brew install sdl2 sdl2_ttf freeimage bison git clone -b feature/standalone https://github.com/jsm174/vpvr cd vpvr cp cmake/CMakeLists_osx-x64.txt CMakeLists.txt cmake -B build cmake --build build cd build ./VPinballX_GL -play ../res/exampleTable.vpx -- 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=53766 Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |leslie_alistair(a)hotmail.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=53766 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=53766 --- Comment #2 from Nikolay Sivov <bunglehead(a)gmail.com> --- As far as I can tell, VariantCopy() fails on Windows for SAFEARRAY(VT_VARIANT). I'm not sure if VT_SAFEARRAY should even be a thing within vbscript. I did confirm that using a global method that returns SAFEARRAY(VARIANT) does return VT_SAFEARRAY from getVT() test function. I believe Pinball code is using typelib API, so maybe our ITypeInfo::Invoke() should produce VT_ARRAY|VT_DISPATCH output. -- 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=53766 --- Comment #3 from Jason Millard <jsm174(a)gmail.com> --- The port of Visual Pinball that I am working on, does not use the typelib. I've made a tool that generates GetIDsOfNames and Invoke methods. https://github.com/vpinball/vpinball/blob/1639b9923a41d0c1ccd03010131e970768... https://github.com/vpinball/vpinball/blob/1639b9923a41d0c1ccd03010131e970768... I just tried switching VT_SAFEARRAY to VT_VARIANT | VT_ARRAY and it seems to be working! V_VT(pVarResult) = VT_SAFEARRAY; hr = GetBalls((SAFEARRAY**)&V_ARRAY(pVarResult)); to V_VT(pVarResult) = VT_VARIANT | VT_ARRAY; hr = GetBalls((SAFEARRAY**)&V_ARRAY(pVarResult)); I apologize as this is not a bug after all. -- 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=53766 --- Comment #4 from Nikolay Sivov <bunglehead(a)gmail.com> --- I don't know if it isn't a bug in vbscript. But does the same code using GetBalls(SAFEARRAY**) work on Windows for you? In exact same situation, preferably using the same binary. -- 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=53766 --- Comment #5 from Jason Millard <jsm174(a)gmail.com> --- This is the method that ultimately gets called: (which has worked for the past 20 years) STDMETHODIMP ScriptGlobalTable::GetBalls(LPSAFEARRAY *pVal) { if (!pVal || !g_pplayer) return E_POINTER; CComSafeArray<VARIANT> balls((ULONG)g_pplayer->m_vball.size()); for (size_t i = 0; i < g_pplayer->m_vball.size(); ++i) { BallEx *pballex = g_pplayer->m_vball[i]->m_pballex; if (!pballex) return E_POINTER; CComVariant v = static_cast<IDispatch*>(pballex); v.Detach(&balls[(int)i]); } *pVal = balls.Detach(); return S_OK; } -- 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=53766 --- Comment #6 from Nikolay Sivov <bunglehead(a)gmail.com> --- The method, sure. Are you using same Invoke() implementation on Windows as well? -- 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=53766 --- Comment #7 from Jason Millard <jsm174(a)gmail.com> --- (In reply to Nikolay Sivov from comment #6)
The method, sure. Are you using same Invoke() implementation on Windows as well?
Let me try that. Will take a little bit for me to set up. -- 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=53766 --- Comment #8 from Jason Millard <jsm174(a)gmail.com> --- (In reply to Nikolay Sivov from comment #6)
The method, sure. Are you using same Invoke() implementation on Windows as well?
Okay, so I updated the Windows version and overrode GetIDsOfNames and Invoke methods. Sure enough vbscript error on the UBound call. I switched it to VT_VARIANT | VT_ARRAY and it's working fine. I think we can close this and https://bugs.winehq.org/show_bug.cgi?id=53866 as well. -- 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=53766 Nikolay Sivov <bunglehead(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|UNCONFIRMED |RESOLVED Resolution|--- |INVALID --- Comment #9 from Nikolay Sivov <bunglehead(a)gmail.com> --- Great, thanks for testing. -- 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=53766 --- Comment #10 from Jason Millard <jsm174(a)gmail.com> --- Just a follow up on this. Even though solved, this path is releasing an extra reference for me. Given this code: Sub Table1_init drain.CreateSizedBallWithMass 50/2, 100 Dim BOT BOT = GetBalls Dim Balls(20) Balls(5) = BOT(0) End Sub Prior to Table1_init, GetBalls Array contains 1 BallEx object with a m_dwRef count of 2 After Table1_init, that BallEx object has a m_dwRef count of 1. Tricky stuff to track down... -- 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=53766 --- Comment #11 from Nikolay Sivov <bunglehead(a)gmail.com> --- (In reply to Jason Millard from comment #10)
Just a follow up on this. Even though solved, this path is releasing an extra reference for me.
Given this code:
Sub Table1_init drain.CreateSizedBallWithMass 50/2, 100
Dim BOT BOT = GetBalls
Dim Balls(20) Balls(5) = BOT(0) End Sub
Prior to Table1_init, GetBalls Array contains 1 BallEx object with a m_dwRef count of 2 After Table1_init, that BallEx object has a m_dwRef count of 1.
Tricky stuff to track down...
This sounds unrelated, so please open another report for it. After trying the same thing on Windows, maybe GetBalls() has to addref on elements, and it doesn't in your custom implementation, then Balls(5) getting out of scope releases VT_DISPATCH element and you get 2 -> 1 change. I'm only speculating. -- 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=53766 Gijs Vermeulen <gijsvrm(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED --- Comment #12 from Gijs Vermeulen <gijsvrm(a)gmail.com> --- Closing INVALID. -- 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