Yuxuan Shui : ntdll: Tweak the binary representation of SRWLOCK.

[Alexandre Julliard]

Module: wine
Branch: master
Commit: d1e02148b6f2ed7ffe3c57452aba9900f0434c9a
URL:    https://gitlab.winehq.org/wine/wine/-/commit/d1e02148b6f2ed7ffe3c57452aba990...

Author: Yuxuan Shui 
Date:   Mon Nov  6 16:05:38 2023 +0000

ntdll: Tweak the binary representation of SRWLOCK.

There are applications that uses SRWLOCK in an invalid way and then checks its binary
representation. Tweak our representation a bit so they are happy.

---

dlls/kernel32/tests/sync.c | 17 +++++++++++++++++
 dlls/ntdll/sync.c          | 24 ++++++++++++++----------
 2 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/dlls/kernel32/tests/sync.c b/dlls/kernel32/tests/sync.c
index 10765765bc5..56a9d6e4859 100644
--- a/dlls/kernel32/tests/sync.c
+++ b/dlls/kernel32/tests/sync.c
@@ -2535,6 +2535,22 @@ static void test_srwlock_example(void)
     trace("number of total exclusive accesses is %ld\n", srwlock_protected_value);
 }

+static void test_srwlock_quirk(void)
+{
+    union { SRWLOCK *s; LONG *l; } u = { &srwlock_example };
+
+    if (!pInitializeSRWLock) {
+        /* function is not yet in XP, only in newer Windows */
+        win_skip("no srw lock support.\n");
+        return;
+    }
+
+    /* WeCom 4.x checks releasing a lock with value 0x1 results in it becoming 0x0. */
+    *u.l = 1;
+    pReleaseSRWLockExclusive(&srwlock_example);
+    ok(*u.l == 0, "expected 0x0, got %lx\n", *u.l);
+}
+
 static DWORD WINAPI alertable_wait_thread(void *param)
 {
     HANDLE *semaphores = param;
@@ -2887,6 +2903,7 @@ START_TEST(sync)
     test_condvars_base(&unaligned_cv.cv);
     test_condvars_consumer_producer();
     test_srwlock_base(&aligned_srwlock);
+    test_srwlock_quirk();
 #if defined(__i386__) || defined(__x86_64__)
     /* unaligned locks only work on x86 platforms */
     test_srwlock_base(&unaligned_srwlock.lock);
diff --git a/dlls/ntdll/sync.c b/dlls/ntdll/sync.c
index fa64917029a..bb5dcbb66e6 100644
--- a/dlls/ntdll/sync.c
+++ b/dlls/ntdll/sync.c
@@ -473,9 +473,10 @@ DWORD WINAPI RtlRunOnceExecuteOnce( RTL_RUN_ONCE *once, PRTL_RUN_ONCE_INIT_FN fu

struct srw_lock
 {
+    /* bit 0 - if the lock is held exclusive. bit 1.. - number of exclusive waiters. */
     short exclusive_waiters;

-    /* Number of shared owners, or -1 if owned exclusive.
+    /* Number of owners.
      *
      * Sadly Windows has no equivalent to FUTEX_WAIT_BITSET, so in order to wake
      * up *only* exclusive or *only* shared waiters (and thus avoid spurious
@@ -486,7 +487,7 @@ struct srw_lock
      * the latter waits only on the "owners" member. Note then that "owners"
      * must not be the first element in the structure.
      */
-    short owners;
+    unsigned short owners;
 };
 C_ASSERT( sizeof(struct srw_lock) == 4 );

@@ -517,7 +518,7 @@ void WINAPI RtlAcquireSRWLockExclusive( RTL_SRWLOCK *lock )
 {
     union { RTL_SRWLOCK *rtl; struct srw_lock *s; LONG *l; } u = { lock };

-    InterlockedIncrement16( &u.s->exclusive_waiters );
+    InterlockedExchangeAdd16( &u.s->exclusive_waiters, 2 );

for (;;)
     {
@@ -532,8 +533,9 @@ void WINAPI RtlAcquireSRWLockExclusive( RTL_SRWLOCK *lock )
             if (!old.s.owners)
             {
                 /* Not locked exclusive or shared. We can try to grab it. */
-                new.s.owners = -1;
-                --new.s.exclusive_waiters;
+                new.s.owners = 1;
+                new.s.exclusive_waiters -= 2;
+                new.s.exclusive_waiters |= 1;
                 wait = FALSE;
             }
             else
@@ -568,7 +570,7 @@ void WINAPI RtlAcquireSRWLockShared( RTL_SRWLOCK *lock )
             old.s = *u.s;
             new = old;

-            if (old.s.owners != -1 && !old.s.exclusive_waiters)
+            if (!old.s.exclusive_waiters)
             {
                 /* Not locked exclusive, and no exclusive waiters.
                  * We can try to grab it. */
@@ -599,9 +601,10 @@ void WINAPI RtlReleaseSRWLockExclusive( RTL_SRWLOCK *lock )
         old.s = *u.s;
         new = old;

-        if (old.s.owners != -1) ERR("Lock %p is not owned exclusive!\n", lock);
+        if (!(old.s.exclusive_waiters & 1)) ERR("Lock %p is not owned exclusive!\n", lock);

new.s.owners = 0;
+        new.s.exclusive_waiters &= ~1;
     } while (InterlockedCompareExchange( u.l, new.l, old.l ) != old.l);

if (new.s.exclusive_waiters)
@@ -623,7 +626,7 @@ void WINAPI RtlReleaseSRWLockShared( RTL_SRWLOCK *lock )
         old.s = *u.s;
         new = old;

-        if (old.s.owners == -1) ERR("Lock %p is owned exclusive!\n", lock);
+        if (old.s.exclusive_waiters & 1) ERR("Lock %p is owned exclusive!\n", lock);
         else if (!old.s.owners) ERR("Lock %p is not owned shared!\n", lock);

--new.s.owners;
@@ -654,7 +657,8 @@ BOOLEAN WINAPI RtlTryAcquireSRWLockExclusive( RTL_SRWLOCK *lock )
         if (!old.s.owners)
         {
             /* Not locked exclusive or shared. We can try to grab it. */
-            new.s.owners = -1;
+            new.s.owners = 1;
+            new.s.exclusive_waiters |= 1;
             ret = TRUE;
         }
         else
@@ -680,7 +684,7 @@ BOOLEAN WINAPI RtlTryAcquireSRWLockShared( RTL_SRWLOCK *lock )
         old.s = *u.s;
         new.s = old.s;

-        if (old.s.owners != -1 && !old.s.exclusive_waiters)
+        if (!old.s.exclusive_waiters)
         {
             /* Not locked exclusive, and no exclusive waiters.
              * We can try to grab it. */