[PATCH 0/1] MR7243: msvcp140: Add padding to _Cnd_t.
'unknown' is zero at 14.38 and later, non-zero before -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7243
From: Daniel Lehman <dlehman25(a)gmail.com> --- dlls/msvcp140/tests/msvcp140.c | 21 ++++++++++++++++++++- dlls/msvcp90/misc.c | 6 ++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/dlls/msvcp140/tests/msvcp140.c b/dlls/msvcp140/tests/msvcp140.c index 0688239a1fc..a2ca538c245 100644 --- a/dlls/msvcp140/tests/msvcp140.c +++ b/dlls/msvcp140/tests/msvcp140.c @@ -179,7 +179,10 @@ typedef struct { DWORD count; } *_Mtx_t; -typedef void *_Cnd_t; +typedef struct { + ULONG_PTR unknown; + CONDITION_VARIABLE cv; +} *_Cnd_t; typedef struct { __time64_t sec; @@ -1556,15 +1559,21 @@ static void test_cnd(void) cm.cnd = cnd; cm.mtx = mtx; cm.timed_wait = FALSE; + ok(cnd->cv.Ptr == 0, "cnd.cv = %p\n", cnd->cv.Ptr); p__Thrd_create(&threads[0], cnd_wait_thread, (void*)&cm); WaitForSingleObject(cm.initialized, INFINITE); p__Mtx_lock(mtx); p__Mtx_unlock(mtx); + todo_wine + ok(cnd->cv.Ptr != 0, "cnd.cv = %p\n", cnd->cv.Ptr); + r = p__Cnd_signal(cm.cnd); ok(!r, "failed to signal\n"); p__Thrd_join(threads[0], NULL); + todo_wine + ok(cnd->cv.Ptr == 0, "cnd.cv = %p\n", cnd->cv.Ptr); /* test _Cnd_timedwait time out */ p__Mtx_lock(mtx); @@ -1584,20 +1593,27 @@ static void test_cnd(void) /* test _Cnd_timedwait */ cm.started = 0; cm.timed_wait = TRUE; + todo_wine + ok(cnd->cv.Ptr == 0, "cnd.cv = %p\n", cnd->cv.Ptr); p__Thrd_create(&threads[0], cnd_wait_thread, (void*)&cm); WaitForSingleObject(cm.initialized, INFINITE); p__Mtx_lock(mtx); p__Mtx_unlock(mtx); + ok(cnd->cv.Ptr != 0, "cnd.cv = %p\n", cnd->cv.Ptr); r = p__Cnd_signal(cm.cnd); ok(!r, "failed to signal\n"); p__Thrd_join(threads[0], NULL); + todo_wine + ok(cnd->cv.Ptr == 0, "cnd.cv = %p\n", cnd->cv.Ptr); /* test _Cnd_broadcast */ cm.started = 0; cm.thread_no = NUM_THREADS; + todo_wine + ok(cnd->cv.Ptr == 0, "cnd.cv = %p\n", cnd->cv.Ptr); for(i = 0; i < cm.thread_no; i++) p__Thrd_create(&threads[i], cnd_wait_thread, (void*)&cm); @@ -1605,10 +1621,13 @@ static void test_cnd(void) p__Mtx_lock(mtx); p__Mtx_unlock(mtx); + ok(cnd->cv.Ptr != 0, "cnd.cv = %p\n", cnd->cv.Ptr); r = p__Cnd_broadcast(cnd); ok(!r, "failed to broadcast\n"); for(i = 0; i < cm.thread_no; i++) p__Thrd_join(threads[i], NULL); + todo_wine + ok(cnd->cv.Ptr == 0, "cnd.cv = %p\n", cnd->cv.Ptr); p__Cnd_destroy(cnd); p__Mtx_destroy(mtx); diff --git a/dlls/msvcp90/misc.c b/dlls/msvcp90/misc.c index 088ecc9976c..80256c1f357 100644 --- a/dlls/msvcp90/misc.c +++ b/dlls/msvcp90/misc.c @@ -832,6 +832,9 @@ void __cdecl _Mtx_reset_owner(_Mtx_arg_t mtx) typedef struct { +#if _MSVCP_VER >= 140 + ULONG_PTR unknown; +#endif cv cv; } *_Cnd_t; @@ -847,6 +850,9 @@ typedef _Cnd_t *_Cnd_arg_t; void __cdecl _Cnd_init_in_situ(_Cnd_t cnd) { +#if _MSVCP_VER >= 140 + cnd->unknown = 0; +#endif cv_init(&cnd->cv); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7243
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=151195 Your paranoid android. === debian11b (64 bit WoW report) === mf: mf.c:6785: Test failed: Unexpected hr 0. user32: input.c:4306: Test succeeded inside todo block: button_down_hwnd_todo 1: got MSG_TEST_WIN hwnd 0000000000A800F2, msg WM_LBUTTONDOWN, wparam 0x1, lparam 0x320032 msg.c:7536: Test failed: BM_CLICK on auto-radio button: 3: the msg 0x00f3 was expected, but got hook 0x0009 instead msg.c:7536: Test failed: BM_CLICK on auto-radio button: 3: the msg 0x00f3 was expected, but got hook 0x0005 instead msg.c:7536: Test failed: BM_CLICK on auto-radio button: 3: the msg 0x00f3 was expected, but got winevent_hook 0x0003 instead msg.c:7536: Test failed: BM_CLICK on auto-radio button: 3: the msg 0x00f3 was expected, but got msg 0x030f instead msg.c:7536: Test failed: BM_CLICK on auto-radio button: 3: the msg 0x00f3 was expected, but got msg 0x001c instead msg.c:7536: Test failed: BM_CLICK on auto-radio button: 3: the msg 0x00f3 was expected, but got msg 0x0086 instead msg.c:7536: Test failed: BM_CLICK on auto-radio button: 3: the msg 0x00f3 was expected, but got msg 0x0006 instead msg.c:7536: Test failed: BM_CLICK on auto-radio button: 3: the msg 0x00f3 was expected, but got hook 0x0009 instead msg.c:7536: Test failed: BM_CLICK on auto-radio button: 3: the msg 0x00f3 was expected, but got winevent_hook 0x8005 instead msg.c:7536: Test failed: BM_CLICK on auto-radio button: 3: the msg 0x00f3 was expected, but got msg 0x0007 instead msg.c:7536: Test failed: BM_CLICK on auto-radio button: 3: the msg 0x00f3 was expected, but got msg 0x0008 instead msg.c:7536: Test failed: BM_CLICK on auto-radio button: 3: the msg 0x00f3 was expected, but got winevent_hook 0x8005 instead msg.c:7536: Test failed: BM_CLICK on auto-radio button: 3: the msg 0x00f3 was expected, but got msg 0x0007 instead msg.c:7536: Test failed: BM_CLICK on auto-radio button: 3: the msg 0x00f3 was expected, but got msg 0x0138 instead msg.c:7536: Test failed: BM_CLICK on auto-radio button: 5: the winevent_hook 0x800a was expected, but got msg 0x0111 instead msg.c:7536: Test failed: BM_CLICK on auto-radio button: 6: the msg 0x0202 was expected, but got msg 0x0111 instead msg.c:7536: Test failed: BM_CLICK on auto-radio button: 7: the msg 0x00f3 was expected, but got msg 0x0111 instead msg.c:7536: Test failed: BM_CLICK on auto-radio button: 8: the msg 0x0138 was expected, but got msg 0x0111 instead msg.c:7536: Test failed: BM_CLICK on auto-radio button: 9: the winevent_hook 0x800a was expected, but got msg 0x0111 instead msg.c:7536: Test failed: BM_CLICK on auto-radio button: 10: the msg 0x0087 was expected, but got msg 0x0111 instead msg.c:7536: Test failed: BM_CLICK on auto-radio button: 11: the msg 0x00f1 was expected, but got msg 0x0111 instead msg.c:7536: Test failed: BM_CLICK on auto-radio button: 12: the msg 0x0087 was expected, but got msg 0x0111 instead msg.c:7536: Test failed: BM_CLICK on auto-radio button: 13: the msg 0x00f1 was expected, but got msg 0x0111 instead msg.c:7536: Test failed: BM_CLICK on auto-radio button: 14: the msg 0x0138 was expected, but got msg 0x0111 instead msg.c:7536: Test failed: BM_CLICK on auto-radio button: 15: the winevent_hook 0x800a was expected, but got msg 0x0111 instead msg.c:7536: Test failed: BM_CLICK on auto-radio button: 16: the msg 0x0087 was expected, but got msg 0x0111 instead msg.c:7536: Test failed: BM_CLICK on auto-radio button: 17: the msg 0x00f1 was expected, but got msg 0x0111 instead msg.c:7536: Test failed: BM_CLICK on auto-radio button: 18: the msg 0x0138 was expected, but got msg 0x0111 instead msg.c:7536: Test failed: BM_CLICK on auto-radio button: 19: the winevent_hook 0x800a was expected, but got msg 0x0111 instead msg.c:7536: Test failed: BM_CLICK on auto-radio button: 20: the msg 0x0087 was expected, but got msg 0x0111 instead msg.c:7536: Test failed: BM_CLICK on auto-radio button: 21: the winevent_hook 0x0009 was expected, but got msg 0x0111 instead msg.c:7536: Test failed: BM_CLICK on auto-radio button: 22: the msg 0x0215 was expected, but got msg 0x0111 instead msg.c:7536: Test failed: BM_CLICK on auto-radio button: 23: in msg 0x0111 expecting wParam 0x1f6 got 0x601f6 msg.c:7536: Test failed: BM_CLICK on auto-radio button: 27: the msg sequence is not complete: expected msg 0000 - actual msg 00f3
Why do you need this change? AFAIU native is always using pointer to this data (so size of exported structures is not affected by it). This way internal structure can be different in different versions of OS. It also makes it possible to change internal representation without breaking things. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7243#note_93434
On Mon Feb 3 14:17:08 2025 +0000, Piotr Caban wrote:
Why do you need this change? AFAIU native is always using pointer to this data (so size of exported structures is not affected by it). This way internal structure can be different in different versions of OS. It also makes it possible to change internal representation without breaking things. I think we need it in case object is modified by statically linked code.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7243#note_93436
Piotr Caban (@piotr) commented about dlls/msvcp140/tests/msvcp140.c:
cm.cnd = cnd; cm.mtx = mtx; cm.timed_wait = FALSE; + ok(cnd->cv.Ptr == 0, "cnd.cv = %p\n", cnd->cv.Ptr); p__Thrd_create(&threads[0], cnd_wait_thread, (void*)&cm);
WaitForSingleObject(cm.initialized, INFINITE); p__Mtx_lock(mtx); p__Mtx_unlock(mtx);
+ todo_wine + ok(cnd->cv.Ptr != 0, "cnd.cv = %p\n", cnd->cv.Ptr);
I don't think the tests should be based on internal `CONDITION_VARIABLE` structure. If it really needs to be tested it's probably better to mix `_Cnd_*` calls with `SleepConditionVariableSRW`/`WakeConditionVariable`. You can also try to limit the tests to checking size of memory block allocated by `_Cnd_init` only. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7243#note_93441
On Mon Feb 3 17:25:01 2025 +0000, Piotr Caban wrote:
I think we need it in case object is modified by statically linked code. yeah, a recent compiler upgrade bypasses the _Cnd_init (and _Mtx_init) functions
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7243#note_93464
participants (4)
-
Daniel Lehman -
Daniel Lehman (＠dlehman25)
-
Marvin -
Piotr Caban (＠piotr)