[PATCH v2 0/1] MR9236: kernel32: Avoid RBP-based frame in BaseThreadInitThunk() on x64.

[Paul Gofman (＠gofman)]

--
  v2: kernel32: Avoid RBP-based frame in BaseThreadInitThunk() on x64.
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/9236

[PATCH v2 1/1] kernel32: Avoid RBP-based frame in BaseThreadInitThunk() on x64.

[Paul Gofman]

From: Paul Gofman <pgofman(a)codeweavers.com>

---
 dlls/kernel32/thread.c       |  9 +++++++++
 dlls/ntdll/tests/exception.c | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/dlls/kernel32/thread.c b/dlls/kernel32/thread.c
index 7f1a3f5b2fb..2ecc3bb5ed8 100644
--- a/dlls/kernel32/thread.c
+++ b/dlls/kernel32/thread.c
@@ -55,6 +55,15 @@ __ASM_FASTCALL_FUNC( BaseThreadInitThunk, 12,
                     "call *%edx\n\t"
                     "movl %eax,(%esp)\n\t"
                     "call " __ASM_STDCALL( "RtlExitUserThread", 4 ))
+#elif defined(__x86_64__) && defined(__WINE_PE_BUILD) && !defined(__arm64ec__)
+__ASM_GLOBAL_FUNC( BaseThreadInitThunk,
+                    "subq $0x28,%rsp\n\t"
+                   ".seh_stackalloc 0x28\n\t"
+                   ".seh_endprologue\n\t"
+                    "movq %r8,%rcx\n\t"
+                    "call *%rdx\n\t"
+                    "movl %eax,%ecx\n\t"
+                    "call " __ASM_NAME( "RtlExitUserThread" ))
 #else
 void __fastcall BaseThreadInitThunk( DWORD unknown, LPTHREAD_START_ROUTINE entry, void *arg )
 {
diff --git a/dlls/ntdll/tests/exception.c b/dlls/ntdll/tests/exception.c
index c071aa36f65..6d1b29860a1 100644
--- a/dlls/ntdll/tests/exception.c
+++ b/dlls/ntdll/tests/exception.c
@@ -6343,6 +6343,37 @@ static void test_user_callback_context(void)
     UnregisterClassA(cls.lpszClassName, GetModuleHandleA(0));
 }
 
+static void test_base_init_thunk_unwind(void)
+{
+    static const BYTE thread_proc_code[] =
+    {
+        /* buffer offset 2 */
+        0x49, 0xb8, 0, 0, 0, 0, 0, 0, 0, 0,         /* movabs imm64,%r8 */
+        0x48, 0x31, 0xc9,                           /* xor %rcx,%rcx */
+        0x48, 0xc7, 0xc2, 0x00, 0x01, 0x00, 0x00,   /* mov $256,%rdx */
+        0x4d, 0x31, 0xc9,                           /* xor %r9,%r9 */
+        0x48, 0x31, 0xed,                           /* xor %rbp,%rbp */
+        /* RtlCaptureStackBackTrace offset 28  */
+        0x48, 0xb8, 0, 0, 0, 0, 0, 0, 0, 0,         /* movabs RtlCaptureStackBackTrace, %rax */
+        0xff, 0xe0,                                 /* jmpq *%rax */
+    };
+    static void *addrs[256];
+    HANDLE thread;
+    BOOL bret;
+    DWORD count;
+
+    memset( addrs, 0xcc, sizeof(addrs) );
+    memcpy( code_mem, thread_proc_code, sizeof(thread_proc_code) );
+    *(void **)((char *)code_mem + 2) = addrs;
+    *(void **)((char *)code_mem + 28) = RtlCaptureStackBackTrace;
+    thread = CreateThread( NULL, 0, code_mem, NULL, 0, NULL );
+    WaitForSingleObject( thread, INFINITE );
+    bret = GetExitCodeThread( thread, &count );
+    CloseHandle( thread );
+    ok( bret, "got error %lu.\n", GetLastError() );
+    ok( count == 2, "got count %lu.\n", count );
+}
+
 #elif defined(__arm__)
 
 static void test_thread_context(void)
@@ -12587,6 +12618,7 @@ START_TEST(exception)
     test_instrumentation_callback();
     test_direct_syscalls();
     test_single_step_address();
+    test_base_init_thunk_unwind();
 
 #elif defined(__aarch64__)
 
-- 
GitLab

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

[Dylan Donnell (＠dy-tea)]

This seems to improve performance as well which is good. Also the access violations are hit less frequently. This MR seems like exactly what Blue Protocol: Star Resonance needs; !9218 just being a side effect of the current behavior, although potentially still useful in other cases.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/9236#note_119228

[Paul Gofman (＠gofman)]

If access violations are still hit, could you please attach PROTON_LOG=1 %command% if running with Proton? Or, if it is with upstream Wine, WINEDEBUG=+pid,+timestamp,+debugstr,+threadname,+seh,+unwind,+loaddll,+module ?

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/9236#note_119230

[Dylan Donnell (＠dy-tea)]

It appears unrelated to this specifically. The anticheat has an feature that generates large amounts of unrelated write to null pointer access violations. I think your MR is still correct and doesn't have any relation to the infrequent access violations.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/9236#note_119231

[Dylan Donnell (＠dy-tea)]

The anticheat seems to do intentional access violations, so I would not worry about it too much.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/9236#note_119232