[PATCH v18 0/2] MR10393: Draft: vbscript: Check named item dispatch for Dim variable names.
When declaring variables with Dim in a visible named item context, check the named item's IDispatch for each variable name via GetIDsOfNames. This matches Windows VBScript behavior where Dim probes the host object before creating the variable. -- v18: vbscript/tests: Probe Dim host-dispatch interaction. https://gitlab.winehq.org/wine/wine/-/merge_requests/10393
From: Francis De Brabandere <francisdb@gmail.com> When declaring variables with Dim in a visible named item context, check the named item's IDispatch for each variable name via GetIDsOfNames. This matches Windows VBScript behavior where Dim probes the host object before creating the variable. --- dlls/vbscript/tests/vbscript.c | 4 ++-- dlls/vbscript/vbscript.c | 38 ++++++++++++++++++++++++++++++++++ dlls/vbscript/vbscript.h | 1 + 3 files changed, 41 insertions(+), 2 deletions(-) diff --git a/dlls/vbscript/tests/vbscript.c b/dlls/vbscript/tests/vbscript.c index 7500ec30a8d..2eb366fe1c0 100644 --- a/dlls/vbscript/tests/vbscript.c +++ b/dlls/vbscript/tests/vbscript.c @@ -2088,8 +2088,8 @@ static void test_named_items(void) hres = IActiveScriptParse_ParseScriptText(parse, L"dim abc\n", L"visibleItem", NULL, NULL, 0, 0, 0, NULL, NULL); ok(hres == S_OK, "ParseScriptText failed: %08lx\n", hres); CHECK_CALLED(OnEnterScript); - todo_wine CHECK_CALLED(GetItemInfo_visible); - todo_wine CHECK_CALLED(GetIDsOfNames_visible); + CHECK_CALLED(GetItemInfo_visible); + CHECK_CALLED(GetIDsOfNames_visible); CHECK_CALLED(OnLeaveScript); SET_EXPECT(OnEnterScript); SET_EXPECT(OnLeaveScript); diff --git a/dlls/vbscript/vbscript.c b/dlls/vbscript/vbscript.c index ff28b3b17ef..6bdc692d82d 100644 --- a/dlls/vbscript/vbscript.c +++ b/dlls/vbscript/vbscript.c @@ -93,6 +93,8 @@ static inline BOOL is_started(VBScript *This) || This->state == SCRIPTSTATE_DISCONNECTED; } +static HRESULT retrieve_named_item_disp(IActiveScriptSite *site, named_item_t *item); + HRESULT exec_global_code(script_ctx_t *ctx, vbscode_t *code, VARIANT *res, BOOL extern_caller) { ScriptDisp *obj = ctx->script_obj; @@ -138,8 +140,43 @@ HRESULT exec_global_code(script_ctx_t *ctx, vbscode_t *code, VARIANT *res, BOOL obj->global_funcs_size = cnt; } + /* For visible named items, the first exec_global_code with Dim + declarations release-and-refetches the host dispatch, then probes + each variable name via GetIDsOfNames. Subsequent parses reuse the + cache; the variable is always created regardless of whether the + name exists on the host dispatch. + TODO: It is unclear why Windows refetches the host dispatch on the + first Dim when the result is not used to skip variable creation. + Perhaps it is used for some side effect or diagnostic purpose that + we have not yet identified. */ + if(code->main_code.var_cnt && code->named_item + && !(code->named_item->flags & SCRIPTITEM_CODEONLY) + && !code->named_item->dim_disp_probed) + { + if(code->named_item->disp) { + IDispatch_Release(code->named_item->disp); + code->named_item->disp = NULL; + } + retrieve_named_item_disp(ctx->site, code->named_item); + code->named_item->dim_disp_probed = TRUE; + } + for (i = 0; i < code->main_code.var_cnt; i++) { + /* Probe the named item's dispatch for each variable name, matching + Windows behavior. The variable is always created regardless. */ + if(code->named_item && code->named_item->disp + && !(code->named_item->flags & SCRIPTITEM_CODEONLY)) + { + BSTR name = SysAllocString(code->main_code.vars[i].name); + if(name) + { + DISPID id; + disp_get_id(code->named_item->disp, name, VBDISP_CALLGET, TRUE, &id); + SysFreeString(name); + } + } + if (script_disp_find_var(obj, code->main_code.vars[i].name)) continue; @@ -869,6 +906,7 @@ static HRESULT WINAPI VBScript_AddNamedItem(IActiveScript *iface, LPCOLESTR pstr item->disp = disp; item->flags = dwFlags; item->script_obj = NULL; + item->dim_disp_probed = FALSE; item->name = wcsdup(pstrName); if(!item->name) { if(disp) diff --git a/dlls/vbscript/vbscript.h b/dlls/vbscript/vbscript.h index 1420e2da0d1..b0a674cc67f 100644 --- a/dlls/vbscript/vbscript.h +++ b/dlls/vbscript/vbscript.h @@ -163,6 +163,7 @@ typedef struct named_item_t { unsigned ref; DWORD flags; LPWSTR name; + BOOL dim_disp_probed; struct list entry; } named_item_t; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10393
From: Francis De Brabandere <francisdb@gmail.com> Add coverage that exercises specific shapes of script source feeding exec_global_code on a visible named item, to map which forms cause native to call GetItemInfo / GetIDsOfNames on the host: - Multiple names in a single Dim statement (one GetIDsOfNames per name). - Dim with explicit array bounds. - Dim inside a Sub (top-level only, sub never called). - Class declaration with property. - Bare Sub declaration at script scope. - A second top-level Dim parse after the disp is already cached. The current Wine implementation in exec_global_code releases and re-fetches the named item dispatch on every parse with Dim declarations. These tests will reveal on Windows CI whether native matches that behavior or only fetches once per item lifetime, and will also pin down which top-level constructs the host probe applies to beyond the simple bare-Dim case. --- dlls/vbscript/tests/vbscript.c | 65 ++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/dlls/vbscript/tests/vbscript.c b/dlls/vbscript/tests/vbscript.c index 2eb366fe1c0..161c1404f69 100644 --- a/dlls/vbscript/tests/vbscript.c +++ b/dlls/vbscript/tests/vbscript.c @@ -2113,6 +2113,71 @@ static void test_named_items(void) CHECK_CALLED(OnEnterScript); CHECK_CALLED(OnLeaveScript); + /* Probe: multiple names in a single Dim statement. Each name should + * be probed against the host dispatch via GetIDsOfNames. The host + * dispatch is already cached from the earlier "dim abc" parse, so + * GetItemInfo is not refetched. */ + SET_EXPECT(OnEnterScript); + SET_EXPECT_MULTI(GetIDsOfNames_visible, 3); + SET_EXPECT(OnLeaveScript); + hres = IActiveScriptParse_ParseScriptText(parse, L"dim probe_a, probe_b, probe_c\n", L"visibleItem", NULL, NULL, 0, 0, 0, NULL, NULL); + ok(hres == S_OK, "ParseScriptText failed: %08lx\n", hres); + CHECK_CALLED(OnEnterScript); + CHECK_CALLED_MULTI(GetIDsOfNames_visible, 3); + CHECK_CALLED(OnLeaveScript); + + /* Probe: Dim with explicit array bounds. */ + SET_EXPECT(OnEnterScript); + SET_EXPECT(GetIDsOfNames_visible); + SET_EXPECT(OnLeaveScript); + hres = IActiveScriptParse_ParseScriptText(parse, L"dim probe_arr(5)\n", L"visibleItem", NULL, NULL, 0, 0, 0, NULL, NULL); + ok(hres == S_OK, "ParseScriptText failed: %08lx\n", hres); + CHECK_CALLED(OnEnterScript); + CHECK_CALLED(GetIDsOfNames_visible); + CHECK_CALLED(OnLeaveScript); + + /* Probes 3-5: top-level constructs that aren't bare Dim. We don't yet + * know whether native probes the host on these. Each probe absorbs + * any GetIDsOfNames_visible calls and then deliberately reports the + * actual count via a failing ok(), so the count appears in the CI + * failure message and reveals which constructs native probes. Once + * the native counts are known, the implementation can be updated and + * these probes either replaced with exact-count assertions or + * removed. */ +#define PROBE_DIM_HOST(label, src) \ + do { \ + int probe_count; \ + SET_EXPECT(OnEnterScript); \ + SET_EXPECT_MULTI(GetIDsOfNames_visible, 64); \ + SET_EXPECT(OnLeaveScript); \ + hres = IActiveScriptParse_ParseScriptText(parse, src, L"visibleItem", NULL, NULL, 0, 0, 0, NULL, NULL); \ + ok(hres == S_OK, "ParseScriptText failed: %08lx\n", hres); \ + CHECK_CALLED(OnEnterScript); \ + probe_count = called_GetIDsOfNames_visible; \ + expect_GetIDsOfNames_visible = called_GetIDsOfNames_visible = 0; \ + CHECK_CALLED(OnLeaveScript); \ + ok(0, "probe " label ": GetIDsOfNames_visible fired %d times\n", probe_count); \ + } while (0) + + PROBE_DIM_HOST("sub-with-dim", L"sub probe_sub\ndim probe_local\nend sub\n"); + PROBE_DIM_HOST("class-with-prop", L"class probe_cls\npublic probe_prop\nend class\n"); + PROBE_DIM_HOST("bare-sub", L"sub probe_named_sub\nend sub\n"); + PROBE_DIM_HOST("function-decl", L"function probe_fn\nend function\n"); + PROBE_DIM_HOST("dim-and-sub", L"dim probe_top\nsub probe_s3\nend sub\n"); +#undef PROBE_DIM_HOST + + /* Probe: a second top-level Dim parse on the same named item. The + * host dispatch is already cached, so GetItemInfo should not fire + * a second time. */ + SET_EXPECT(OnEnterScript); + SET_EXPECT(GetIDsOfNames_visible); + SET_EXPECT(OnLeaveScript); + hres = IActiveScriptParse_ParseScriptText(parse, L"dim probe_second\n", L"visibleItem", NULL, NULL, 0, 0, 0, NULL, NULL); + ok(hres == S_OK, "ParseScriptText failed: %08lx\n", hres); + CHECK_CALLED(OnEnterScript); + CHECK_CALLED(GetIDsOfNames_visible); + CHECK_CALLED(OnLeaveScript); + SET_EXPECT(OnEnterScript); SET_EXPECT(OnLeaveScript); hres = IActiveScriptParse_ParseScriptText(parse, L"set global_me = me\n", L"globalItem", NULL, NULL, 0, 0, SCRIPTTEXT_ISPERSISTENT, NULL, NULL); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10393
participants (2)
-
Francis De Brabandere -
Francis De Brabandere (@francisdb)