[PATCH 0/1] MR8016: msvcp: Use _beginthreadex() in _Thrd_start().
From: Paul Gofman <pgofman(a)codeweavers.com> Test code is based in ucrtbase/thread.c tests. --- dlls/msvcp140/tests/Makefile.in | 4 +- dlls/msvcp140/tests/msvcp140.c | 87 ++++++++++++++++++++++++++++++ dlls/msvcp140/tests/threaddll.c | 54 +++++++++++++++++++ dlls/msvcp140/tests/threaddll.h | 23 ++++++++ dlls/msvcp140/tests/threaddll.spec | 2 + dlls/msvcp90/misc.c | 4 +- 6 files changed, 172 insertions(+), 2 deletions(-) create mode 100644 dlls/msvcp140/tests/threaddll.c create mode 100644 dlls/msvcp140/tests/threaddll.h create mode 100644 dlls/msvcp140/tests/threaddll.spec diff --git a/dlls/msvcp140/tests/Makefile.in b/dlls/msvcp140/tests/Makefile.in index 130ba6ef93a..044b170bd77 100644 --- a/dlls/msvcp140/tests/Makefile.in +++ b/dlls/msvcp140/tests/Makefile.in @@ -1,4 +1,6 @@ TESTDLL = msvcp140.dll SOURCES = \ - msvcp140.c + msvcp140.c \ + threaddll.c \ + threaddll.spec diff --git a/dlls/msvcp140/tests/msvcp140.c b/dlls/msvcp140/tests/msvcp140.c index 7dc0ab1ef24..1d9d9a33f3c 100644 --- a/dlls/msvcp140/tests/msvcp140.c +++ b/dlls/msvcp140/tests/msvcp140.c @@ -29,6 +29,8 @@ #include "wine/test.h" #include "winbase.h" +#include "threaddll.h" + #define SECSPERDAY 86400 /* 1601 to 1970 is 369 years plus 89 leap days */ #define SECS_1601_TO_1970 ((369 * 365 + 89) * (ULONGLONG)SECSPERDAY) @@ -247,6 +249,7 @@ static int (__cdecl *p__Cnd_timedwait)(_Cnd_t, _Mtx_t, const xtime*); static int (__cdecl *p__Cnd_broadcast)(_Cnd_t); static int (__cdecl *p__Cnd_signal)(_Cnd_t); static int (__cdecl *p__Thrd_create)(_Thrd_t*, _Thrd_start_t, void*); +static int (__cdecl *p__Thrd_start)(_Thrd_t*, LPTHREAD_START_ROUTINE, void *); static int (__cdecl *p__Thrd_join)(_Thrd_t, int*); static int (__cdecl *p__Xtime_diff_to_millis2)(const xtime*, const xtime*); static int (__cdecl *p_xtime_get)(xtime*, int); @@ -455,6 +458,7 @@ static BOOL init(void) SET(p__Cnd_broadcast, "_Cnd_broadcast"); SET(p__Cnd_signal, "_Cnd_signal"); SET(p__Thrd_create, "_Thrd_create"); + SET(p__Thrd_start, "_Thrd_start"); SET(p__Thrd_join, "_Thrd_join"); SET(p__Xtime_diff_to_millis2, "_Xtime_diff_to_millis2"); SET(p_xtime_get, "xtime_get"); @@ -2375,6 +2379,88 @@ void test_codecvt_char16(void) } } +static char *get_thread_dll_path(void) +{ + static char path[MAX_PATH]; + const char dll_name[] = "threaddll.dll"; + DWORD written; + HANDLE file; + HRSRC res; + void *ptr; + + GetTempPathA(ARRAY_SIZE(path), path); + strcat(path, dll_name); + + file = CreateFileA(path, GENERIC_READ|GENERIC_WRITE, 0, NULL, CREATE_ALWAYS, 0, 0); + ok(file != INVALID_HANDLE_VALUE, "Failed to create file %s: %lu.\n", + debugstr_a(path), GetLastError()); + + res = FindResourceA(NULL, dll_name, "TESTDLL"); + ok(!!res, "Failed to load resource: %lu\n", GetLastError()); + ptr = LockResource(LoadResource(GetModuleHandleA(NULL), res)); + WriteFile(file, ptr, SizeofResource( GetModuleHandleA(NULL), res), &written, NULL); + ok(written == SizeofResource(GetModuleHandleA(NULL), res), "Failed to write resource\n"); + CloseHandle(file); + + return path; +} + +static void set_thead_dll_detach_event(HANDLE dll, HANDLE event) +{ + void (WINAPI *_set_detach_event)(HANDLE event); + _set_detach_event = (void*) GetProcAddress(dll, "set_detach_event"); + ok(_set_detach_event != NULL, "Failed to get set_detach_event: %lu\n", GetLastError()); + _set_detach_event(event); +} + +static void test_thread_library_reference(void) +{ + LPTHREAD_START_ROUTINE thread_proc; + HANDLE detach_event; + _Thrd_t thread; + HMODULE dll; + DWORD ret; + + struct threaddll_args args; + + detach_event = CreateEventA(NULL, FALSE, FALSE, NULL); + ok(detach_event != NULL, "Failed to create an event: %lu\n", GetLastError()); + args.confirm_running = CreateEventA(NULL, FALSE, FALSE, NULL); + ok(args.confirm_running != NULL, "Failed to create an event: %lu\n", GetLastError()); + args.past_free = CreateEventA(NULL, FALSE, FALSE, NULL); + ok(args.past_free != NULL, "Failed to create an event: %lu\n", GetLastError()); + + dll = LoadLibraryA(get_thread_dll_path()); + ok(!!dll, "Failed to load the test dll: %lu\n", GetLastError()); + + set_thead_dll_detach_event(dll, detach_event); + + thread_proc = (void *)GetProcAddress(dll, "thread_proc"); + ok(!!thread_proc, "Failed to get thread_proc: %lu\n", GetLastError()); + p__Thrd_start(&thread, thread_proc, &args); + + ret = WaitForSingleObject(args.confirm_running, 200); + ok(ret == WAIT_OBJECT_0, "Event was not signaled, ret: %lu, err: %lu\n", ret, GetLastError()); + + ret = FreeLibrary(dll); + ok(ret, "Failed to free the library: %lu\n", GetLastError()); + + ret = WaitForSingleObject(detach_event, 0); + ok(ret == WAIT_TIMEOUT, "Thread detach happened unexpectedly signaling an event, ret: %ld, err: %lu\n", ret, GetLastError()); + + ret = SetEvent(args.past_free); + ok(ret, "Failed to signal event: %ld\n", GetLastError()); + + ret = WaitForSingleObject(detach_event, 1000); + ok(ret == WAIT_OBJECT_0, "Detach event was not signaled, ret: %ld, err: %lu\n", ret, GetLastError()); + + p__Thrd_join(thread, NULL); + + CloseHandle(args.past_free); + CloseHandle(args.confirm_running); + CloseHandle(detach_event); +} + START_TEST(msvcp140) { if(!init()) return; @@ -2405,5 +2491,6 @@ START_TEST(msvcp140) test__Mtx(); test__Fiopen(); test_codecvt_char16(); + test_thread_library_reference(); FreeLibrary(msvcp); } diff --git a/dlls/msvcp140/tests/threaddll.c b/dlls/msvcp140/tests/threaddll.c new file mode 100644 index 00000000000..aec8e6d81e4 --- /dev/null +++ b/dlls/msvcp140/tests/threaddll.c @@ -0,0 +1,54 @@ +/* + * Copyright 2025 Paul Gofman for CodeWeavers + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#if 0 +#pragma makedep testdll +#endif + +#include <windows.h> + +#include "wine/test.h" + +#include "threaddll.h" + +static HANDLE detach_event; + +void WINAPI set_detach_event(HANDLE event) +{ + detach_event = event; +} + +unsigned WINAPI thread_proc(void *param) +{ + struct threaddll_args *args = param; + SetEvent(args->confirm_running); + WaitForSingleObject(args->past_free, INFINITE); + return 0; +} + +BOOL WINAPI DllMain(HINSTANCE instance_new, DWORD reason, LPVOID reserved) +{ + switch (reason) + { + case DLL_PROCESS_DETACH: + if (detach_event) SetEvent(detach_event); + break; + } + + return TRUE; +} diff --git a/dlls/msvcp140/tests/threaddll.h b/dlls/msvcp140/tests/threaddll.h new file mode 100644 index 00000000000..8e60756ea60 --- /dev/null +++ b/dlls/msvcp140/tests/threaddll.h @@ -0,0 +1,23 @@ +/* + * Copyright 2025 Paul Gofman for CodeWeavers + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +struct threaddll_args +{ + HANDLE confirm_running; + HANDLE past_free; +}; diff --git a/dlls/msvcp140/tests/threaddll.spec b/dlls/msvcp140/tests/threaddll.spec new file mode 100644 index 00000000000..2bc9c51fa22 --- /dev/null +++ b/dlls/msvcp140/tests/threaddll.spec @@ -0,0 +1,2 @@ +@ stdcall set_detach_event(ptr) +@ stdcall thread_proc(ptr) diff --git a/dlls/msvcp90/misc.c b/dlls/msvcp90/misc.c index 80256c1f357..47dc44fbecb 100644 --- a/dlls/msvcp90/misc.c +++ b/dlls/msvcp90/misc.c @@ -19,6 +19,7 @@ #include <stdarg.h> #include <limits.h> #include <errno.h> +#include <process.h> #include "msvcp90.h" @@ -1320,7 +1321,8 @@ int __cdecl _Thrd_join(_Thrd_t thr, int *code) int __cdecl _Thrd_start(_Thrd_t *thr, LPTHREAD_START_ROUTINE proc, void *arg) { TRACE("(%p %p %p)\n", thr, proc, arg); - thr->hnd = CreateThread(NULL, 0, proc, arg, 0, &thr->id); + + thr->hnd = (HANDLE)_beginthreadex(NULL, 0, (_beginthreadex_start_routine_t)proc, arg, 0, (unsigned int *)&thr->id); return thr->hnd ? 0 : _THRD_ERROR; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8016
Fixes Atelier Yumia: The Alchemist of Memories & the Envisioned Land crashing on start with builtin msvcp140.dll. The relevant deference introduced by this change is that _beginthreadex() takes the reference to the dll containing thread procedure (which turns out to be the case as well for msvcp140._Thrd_start). A puzzling bit is that msvcp140._Thrd_create doesn't take the reference to the dll on Windows (the same test as included crashes when trying to use that instead of _Thrd_start. With this change in mscp140.dll that is also the case with Wine. From the Wine code that happens so because _Thrd_create passes wrapper function to _Thrd_create (which function is in msvcp140 and not in the user dll). Absence of the similar module reference on user function's dll from _Thrd_create doesn't make much sense to me but that's what I've got with the tests. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8016#note_103079
How about changing `_Thrd_start` function signature (it was guessed anyway)? See attached [patch](/uploads/8ae75bf373c54c2895f7deaa24379ac6/0001-msvcp-Use-_beginthreadex-in-_Thrd_start.patch) for details. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8016#note_103311
participants (3)
-
Paul Gofman -
Paul Gofman (＠gofman)
-
Piotr Caban (＠piotr)