[PATCH 0/2] MR10853: oleaut32: Zero ITypeInfo::AddressOfMember out-pointer on failure.
For a TKIND_DISPATCH typeinfo built via ICreateTypeLib2, AddressOfMember has no DLL backing and returns TYPE_E_BADMODULEKIND. Native sets the out-pointer to NULL on this path; Wine currently leaves it untouched. Match native behavior: clear *ppv at entry so callers see NULL when AddressOfMember returns an error path (e.g. TYPE_E_BADMODULEKIND for typeinfos with no DLL backing). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10853
From: Francis De Brabandere <francisdb@gmail.com> For a TKIND_DISPATCH typeinfo built via ICreateTypeLib2, AddressOfMember has no DLL backing and returns TYPE_E_BADMODULEKIND. Native sets the out-pointer to NULL on this path; Wine currently leaves it untouched. --- dlls/oleaut32/tests/typelib.c | 54 +++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/dlls/oleaut32/tests/typelib.c b/dlls/oleaut32/tests/typelib.c index 153964e17d5..87ed918dba4 100644 --- a/dlls/oleaut32/tests/typelib.c +++ b/dlls/oleaut32/tests/typelib.c @@ -8763,6 +8763,59 @@ static void test_DeleteFuncDesc(void) DeleteFileW(filenameW); } +/* AddressOfMember on a TKIND_DISPATCH (built via ICreateTypeLib2) has + * no DLL backing, so it should fail with TYPE_E_BADMODULEKIND and + * leave the out-pointer NULL. */ +static void test_AddressOfMember(void) +{ + WCHAR filenameW[MAX_PATH], temp_path[MAX_PATH]; + static OLECHAR tinameW[] = L"dispiface"; + static OLECHAR methodW[] = L"meth"; + LPOLESTR pmethodW = methodW; + ICreateTypeLib2 *createtl; + ICreateTypeInfo *createti; + FUNCDESC funcdesc; + ITypeInfo *ti; + void *addr; + HRESULT hr; + + GetTempPathW(ARRAY_SIZE(temp_path), temp_path); + GetTempFileNameW(temp_path, L"tlb", 0, filenameW); + + hr = CreateTypeLib2(SYS_WIN64, filenameW, &createtl); + ok(hr == S_OK, "CreateTypeLib2 hr=%#lx\n", hr); + + hr = ICreateTypeLib2_CreateTypeInfo(createtl, tinameW, TKIND_DISPATCH, &createti); + ok(hr == S_OK, "CreateTypeInfo hr=%#lx\n", hr); + + memset(&funcdesc, 0, sizeof(funcdesc)); + funcdesc.memid = 0x10000000; + funcdesc.funckind = FUNC_DISPATCH; + funcdesc.invkind = INVOKE_FUNC; + funcdesc.callconv = CC_STDCALL; + funcdesc.elemdescFunc.tdesc.vt = VT_VOID; + hr = ICreateTypeInfo_AddFuncDesc(createti, 0, &funcdesc); + ok(hr == S_OK, "AddFuncDesc hr=%#lx\n", hr); + hr = ICreateTypeInfo_SetFuncAndParamNames(createti, 0, &pmethodW, 1); + ok(hr == S_OK, "SetFuncAndParamNames hr=%#lx\n", hr); + + hr = ICreateTypeInfo_LayOut(createti); + ok(hr == S_OK, "LayOut hr=%#lx\n", hr); + + hr = ICreateTypeInfo_QueryInterface(createti, &IID_ITypeInfo, (void**)&ti); + ok(hr == S_OK, "QI(ITypeInfo) hr=%#lx\n", hr); + + addr = (void*)0xdeadbeef; + hr = ITypeInfo_AddressOfMember(ti, 0x10000000, INVOKE_FUNC, &addr); + ok(hr == TYPE_E_BADMODULEKIND, "AddressOfMember hr=%#lx\n", hr); + todo_wine ok(!addr, "AddressOfMember left addr=%p, expected NULL\n", addr); + + ITypeInfo_Release(ti); + ICreateTypeInfo_Release(createti); + ICreateTypeLib2_Release(createtl); + DeleteFileW(filenameW); +} + START_TEST(typelib) { const WCHAR *filename; @@ -8806,4 +8859,5 @@ START_TEST(typelib) test_stub(); test_DeleteImplType(); test_DeleteFuncDesc(); + test_AddressOfMember(); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10853
From: Francis De Brabandere <francisdb@gmail.com> Match native behavior: clear *ppv at entry so callers see NULL when AddressOfMember returns an error path (e.g. TYPE_E_BADMODULEKIND for typeinfos with no DLL backing). --- dlls/oleaut32/tests/typelib.c | 2 +- dlls/oleaut32/typelib.c | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/dlls/oleaut32/tests/typelib.c b/dlls/oleaut32/tests/typelib.c index 87ed918dba4..8207e0cf179 100644 --- a/dlls/oleaut32/tests/typelib.c +++ b/dlls/oleaut32/tests/typelib.c @@ -8808,7 +8808,7 @@ static void test_AddressOfMember(void) addr = (void*)0xdeadbeef; hr = ITypeInfo_AddressOfMember(ti, 0x10000000, INVOKE_FUNC, &addr); ok(hr == TYPE_E_BADMODULEKIND, "AddressOfMember hr=%#lx\n", hr); - todo_wine ok(!addr, "AddressOfMember left addr=%p, expected NULL\n", addr); + ok(!addr, "AddressOfMember left addr=%p, expected NULL\n", addr); ITypeInfo_Release(ti); ICreateTypeInfo_Release(createti); diff --git a/dlls/oleaut32/typelib.c b/dlls/oleaut32/typelib.c index e9997a86d24..30c03efbac9 100644 --- a/dlls/oleaut32/typelib.c +++ b/dlls/oleaut32/typelib.c @@ -7998,6 +7998,10 @@ static HRESULT WINAPI ITypeInfo_fnAddressOfMember( ITypeInfo2 *iface, TRACE("%p, %lx, %#x, %p.\n", iface, memid, invKind, ppv); + if (!ppv) + return E_INVALIDARG; + *ppv = NULL; + hr = ITypeInfo2_GetDllEntry(iface, memid, invKind, &dll, &entry, &ordinal); if (FAILED(hr)) return hr; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10853
Do you have an app that depends on this behaviour? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10853#note_139532
These mr's are all related to !10851 which is an effort to upstream libwinevbs patches to wine. Libwinevbs is in use by Visual Pinball to run 1000s of virtual pinball tables. While comparing with native behavior issues like this one popped up. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10853#note_139533
Right, but is this (and are the other MRs) required to make this work, or were they just observed differences? We're unlikely to take patches like this unless there's an app that depends on that specific behaviour. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10853#note_139549
To be seen if these actually affect vpinball. The four small oleaut32 MRs (!10853 .. !10856) all surfaced from !10851's new test exercising Wine's own ICreateTypeLib2 path; matching native here is what lets the corresponding todo_wine markers come out of that test. If the bar is app-driven evidence before taking native-matching fixes in oleaut32, I can leave the todos in !10851 and close these four. However what's the harm in native parity? These patches are all very small and come with tests proving the native behavior. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10853#note_139553
However what's the harm in native parity? These patches are all very small and come with tests proving the native behavior.
Adding any non-essential change takes time and risks regressions. If no app depends on this, there's no reason for a future version of Windows to behave in the same way, so then we have to go back and change the tests, again taking more unnecessary time. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10853#note_139558
This merge request was closed by Francis De Brabandere. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10853
Ok, will close the ones that are not required -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10853#note_139673
participants (3)
-
Francis De Brabandere -
Francis De Brabandere (＠francisdb)
-
Huw Davies (＠huw)