[PATCH v3 0/1] MR6175: ntdll: Raise exception on failed CS wait.

[Paul Gofman (＠gofman)]

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=57011

--
  v3: ntdll: Raise exception on failed CS wait.

https://gitlab.winehq.org/wine/wine/-/merge_requests/6175

[PATCH v3 1/1] ntdll: Raise exception on failed CS wait.

[Paul Gofman]

From: Paul Gofman

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=57011
---
 dlls/kernel32/tests/sync.c | 38 ++++++++++++++++++++++++++++++++++++++
 dlls/ntdll/sync.c          |  5 ++++-
 2 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/dlls/kernel32/tests/sync.c b/dlls/kernel32/tests/sync.c
index c06ced47298..cd6980b46a8 100644
--- a/dlls/kernel32/tests/sync.c
+++ b/dlls/kernel32/tests/sync.c
@@ -27,6 +27,7 @@
 #include <windef.h>
 #include <winbase.h>
 #include <winternl.h>
+#include <rtlsupportapi.h>

#include "wine/test.h"

@@ -2732,9 +2733,27 @@ static void test_apc_deadlock(void)
     CloseHandle(pi.hProcess);
 }

+static CONTEXT bad_cs_context;
+
+static LONG WINAPI bad_cs_handler( EXCEPTION_POINTERS *eptr )
+{
+    EXCEPTION_RECORD *rec = eptr->ExceptionRecord;
+
+    ok(!rec->NumberParameters, "got %lu.\n", rec->NumberParameters);
+    ok(rec->ExceptionCode == STATUS_INVALID_HANDLE, "got %#lx.\n", rec->ExceptionCode);
+    ok(rec->ExceptionFlags == EXCEPTION_NONCONTINUABLE
+            || rec->ExceptionFlags == (EXCEPTION_NONCONTINUABLE | EXCEPTION_SOFTWARE_ORIGINATE),
+            "got %#lx.\n", rec->ExceptionFlags);
+    NtContinue(&bad_cs_context, FALSE);
+    return EXCEPTION_CONTINUE_SEARCH;
+}
+
 static void test_crit_section(void)
 {
+    void *vectored_handler;
     CRITICAL_SECTION cs;
+    HANDLE old;
+    LONG pass;
     BOOL ret;

/* Win8+ does not initialize debug info, one has to use RTL_CRITICAL_SECTION_FLAG_FORCE_DEBUG_INFO
@@ -2798,6 +2817,25 @@ static void test_crit_section(void)

DeleteCriticalSection(&cs);
     ok(cs.DebugInfo == NULL, "Unexpected debug info pointer %p.\n", cs.DebugInfo);
+
+    ret = pInitializeCriticalSectionEx(&cs, 0, 0);
+    ok(ret, "got error %lu.\n", GetLastError());
+    old = cs.LockSemaphore;
+    cs.LockSemaphore = (HANDLE)0xdeadbeef;
+
+    cs.LockCount = 0;
+    vectored_handler = AddVectoredExceptionHandler(TRUE, bad_cs_handler);
+    pass = 0;
+    InterlockedIncrement(&pass);
+    RtlCaptureContext(&bad_cs_context);
+    InterlockedIncrement(&pass);
+    if (pass == 2)
+        EnterCriticalSection(&cs);
+    ok(pass == 3, "got %ld.\n", pass);
+    ok(cs.LockCount, "got %ld.\n", cs.LockCount);
+    RemoveVectoredExceptionHandler(vectored_handler);
+    cs.LockSemaphore = old;
+    DeleteCriticalSection(&cs);
 }

static DWORD WINAPI thread_proc(LPVOID unused)
diff --git a/dlls/ntdll/sync.c b/dlls/ntdll/sync.c
index 139931a595e..522ce0a2142 100644
--- a/dlls/ntdll/sync.c
+++ b/dlls/ntdll/sync.c
@@ -313,6 +313,7 @@ NTSTATUS WINAPI RtlpWaitForCriticalSection( RTL_CRITICAL_SECTION *crit )
         NTSTATUS status = wait_semaphore( crit, timeout );

if (status == STATUS_WAIT_0) break;
+        if (status != WAIT_TIMEOUT) return status;

timeout = (TRACE_ON(relay) ? 300 : 60);

@@ -368,6 +369,8 @@ NTSTATUS WINAPI RtlEnterCriticalSection( RTL_CRITICAL_SECTION *crit )

if (InterlockedIncrement( &crit->LockCount ))
     {
+        NTSTATUS status;
+
         if (crit->OwningThread == ULongToHandle(GetCurrentThreadId()))
         {
             crit->RecursionCount++;
@@ -375,7 +378,7 @@ NTSTATUS WINAPI RtlEnterCriticalSection( RTL_CRITICAL_SECTION *crit )
         }

/* Now wait for it */
-        RtlpWaitForCriticalSection( crit );
+        if ((status = RtlpWaitForCriticalSection( crit ))) RtlRaiseStatus( status );
     }
 done:
     crit->OwningThread   = ULongToHandle(GetCurrentThreadId());

[Paul Gofman (＠gofman)]

v3:
 - Don't use setjmp() in tests to avoid issues with clang compilation.

[PATCH v3 0/1] MR6175: Draft: ntdll: Raise exception on failed CS wait.

[Paul Gofman (＠gofman)]

Eh, sorry, RtlCaptureContext / NtContinue can't be used this way on i386 as it captures previous frame, thus crash loop in tests on i386. I feel like I am running out of sane options to catch an exception and continue without making ugly arch specific things, I am probably going to find out if setjmp is supposed to work on clang / i386 first.