[PATCH 0/1] MR7243: msvcp140: Add padding to _Cnd_t.

[Daniel Lehman (＠dlehman25)]

'unknown' is zero at 14.38 and later, non-zero before

[PATCH 1/1] msvcp140: Add padding to _Cnd_t.

[Daniel Lehman]

From: Daniel Lehman

---
 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);
 }

[PATCH 1/1] msvcp140: Add padding to _Cnd_t.

[Marvin]

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

[Piotr Caban (＠piotr)]

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.

[Piotr Caban (＠piotr)]

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.

[Piotr Caban (＠piotr)]

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.

[Daniel Lehman (＠dlehman25)]

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