[PATCH v6 0/2] MR3566: server: Store both contexts in pending context object.

[Jacek Caban (＠jacek)]

And discard irrelevant parts in select request. We currently store something in CTX_PENDING and ignore it later in select request if we're currently running in wow context.

--
  v6: ntdll/tests: Add tests for setting context on unsuspended thread.
      server: Store both contexts in pending context object.
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3566

[PATCH v6 1/2] server: Store both contexts in pending context object.

[Jacek Caban]

From: Jacek Caban <jacek(a)codeweavers.com>

And discard irrelevant parts in select request.
---
 server/thread.c | 67 +++++++++++++++++++++++--------------------------
 1 file changed, 32 insertions(+), 35 deletions(-)

diff --git a/server/thread.c b/server/thread.c
index 3e35b27f694..83ae381d5c5 100644
--- a/server/thread.c
+++ b/server/thread.c
@@ -119,11 +119,10 @@ struct context
 {
     struct object   obj;        /* object header */
     unsigned int    status;     /* status of the context */
-    context_t       regs[3];    /* context data */
+    context_t       regs[2];    /* context data */
 };
 #define CTX_NATIVE  0  /* context for native machine */
 #define CTX_WOW     1  /* context if thread is inside WoW */
-#define CTX_PENDING 2  /* pending native context when we don't know whether thread is inside WoW */
 
 /* flags for registers that always need to be set from the server side */
 static const unsigned int system_flags = SERVER_CTX_DEBUG_REGISTERS;
@@ -292,7 +291,6 @@ static struct context *create_thread_context( struct thread *thread )
     context->status = STATUS_PENDING;
     memset( &context->regs, 0, sizeof(context->regs) );
     context->regs[CTX_NATIVE].machine = native_machine;
-    context->regs[CTX_PENDING].machine = native_machine;
     return context;
 }
 
@@ -1591,8 +1589,6 @@ DECL_HANDLER(select)
         const context_t *native_context = (const context_t *)((const char *)(result + 1) + req->size);
         const context_t *wow_context = (ctx_count > 1) ? native_context + 1 : NULL;
 
-        if (current->context && current->context->status != STATUS_PENDING) goto invalid_param;
-
         if (native_context->machine == native_machine)
         {
             if (wow_context && wow_context->machine != current->process->machine) goto invalid_param;
@@ -1605,7 +1601,14 @@ DECL_HANDLER(select)
         }
         else goto invalid_param;
 
-        if (!current->context && !(current->context = create_thread_context( current ))) return;
+        if ((ctx = current->context))
+        {
+            if (ctx->status != STATUS_PENDING) goto invalid_param;
+            /* if context was modified in pending state, discard irrelevant changes */
+            if (wow_context) ctx->regs[CTX_NATIVE].flags &= ~ctx->regs[CTX_WOW].flags;
+            else ctx->regs[CTX_WOW].flags = ctx->regs[CTX_WOW].machine = 0;
+        }
+        else if (!(current->context = create_thread_context( current ))) return;
 
         ctx = current->context;
         if (native_context)
@@ -1618,13 +1621,6 @@ DECL_HANDLER(select)
             ctx->regs[CTX_WOW].machine = current->process->machine;
             copy_context( &ctx->regs[CTX_WOW], wow_context, wow_context->flags & ~ctx->regs[CTX_WOW].flags );
         }
-        else if (ctx->regs[CTX_PENDING].flags)
-        {
-            unsigned int flags = ctx->regs[CTX_PENDING].flags & ~ctx->regs[CTX_NATIVE].flags;
-            copy_context( &ctx->regs[CTX_NATIVE], &ctx->regs[CTX_PENDING], flags );
-            ctx->regs[CTX_NATIVE].flags |= flags;
-        }
-        ctx->regs[CTX_PENDING].flags = 0;
         ctx->status = STATUS_SUCCESS;
         current->suspend_cookie = req->cookie;
         wake_up( &ctx->obj, 0 );
@@ -1931,36 +1927,37 @@ DECL_HANDLER(set_thread_context)
         set_error( STATUS_INVALID_PARAMETER );
     else if (thread->state != TERMINATED)
     {
-        unsigned int ctx = CTX_NATIVE;
-        const context_t *context = &contexts[CTX_NATIVE];
-        unsigned int flags = system_flags & context->flags;
-        unsigned int native_flags = context->flags & req->native_flags;
+        unsigned int flags = system_flags & contexts[CTX_NATIVE].flags;
 
         if (thread != current) stop_thread( thread );
-        else if (flags) set_thread_context( thread, context, flags );
+        else if (flags) set_thread_context( thread, &contexts[CTX_NATIVE], flags );
+
         if (thread->context && !get_error())
         {
-            if (ctx_count == 2)
+            /* If context is in a pending state, we don't know if we will use WoW or native
+             * context, so store both and discard irrevelant one in select request. */
+            const int is_pending = thread->context->status == STATUS_PENDING;
+            unsigned int native_flags = contexts[CTX_NATIVE].flags;
+
+            if (ctx_count == 2 && (is_pending || thread->context->regs[CTX_WOW].machine))
             {
-                /* If the target thread doesn't have a WoW context, set native instead.
-                 * If we don't know yet whether we have a WoW context, store native context
-                 * in CTX_PENDING and update when the target thread sends its context(s). */
-                if (thread->context->status != STATUS_PENDING)
-                {
-                    ctx = thread->context->regs[CTX_WOW].machine ? CTX_WOW : CTX_NATIVE;
-                    context = &contexts[ctx];
-                }
-                else ctx = CTX_PENDING;
+                context_t *ctx = &thread->context->regs[CTX_WOW];
+
+                /* some regs are always set from the native context */
+                flags = contexts[CTX_WOW].flags & ~req->native_flags;
+                if (is_pending) ctx->machine = contexts[CTX_WOW].machine;
+                else native_flags &= req->native_flags;
+
+                copy_context( ctx, &contexts[CTX_WOW], flags );
+                ctx->flags |= flags;
             }
-            flags = context->flags;
-            if (native_flags && ctx != CTX_NATIVE) /* some regs are always set from the native context */
+
+            if (native_flags)
             {
-                copy_context( &thread->context->regs[CTX_NATIVE], &contexts[CTX_NATIVE], native_flags );
-                thread->context->regs[CTX_NATIVE].flags |= native_flags;
-                flags &= ~native_flags;
+                context_t *ctx = &thread->context->regs[CTX_NATIVE];
+                copy_context( ctx, &contexts[CTX_NATIVE], native_flags );
+                ctx->flags |= native_flags;
             }
-            copy_context( &thread->context->regs[ctx], context, flags );
-            thread->context->regs[ctx].flags |= flags;
         }
     }
     else set_error( STATUS_UNSUCCESSFUL );
-- 
GitLab


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

[PATCH v6 2/2] ntdll/tests: Add tests for setting context on unsuspended thread.

[Jacek Caban]

From: Jacek Caban <jacek(a)codeweavers.com>

Based on patch by Evan Tang.
---
 dlls/ntdll/tests/exception.c | 82 ++++++++++++++++++++++++++++++++++++
 1 file changed, 82 insertions(+)

diff --git a/dlls/ntdll/tests/exception.c b/dlls/ntdll/tests/exception.c
index ef3ccb68f17..b8f8f7774cb 100644
--- a/dlls/ntdll/tests/exception.c
+++ b/dlls/ntdll/tests/exception.c
@@ -11262,6 +11262,86 @@ static void test_copy_context(void)
                 dst_xs->Mask == 4, sizeof(dst_xs->YmmContext));
     }
 }
+
+#if defined(__i386__)
+# define IP_REG(ctx) ctx.Eip
+#else
+# define IP_REG(ctx) ctx.Rip
+#endif
+
+static volatile int exit_ip_test;
+static WINAPI DWORD ip_test_thread_proc( void *param )
+{
+    SetEvent( param );
+    while (!exit_ip_test);
+    return ERROR_SUCCESS;
+}
+
+static void test_set_live_context(void)
+{
+    UINT_PTR old_ip, target;
+    HANDLE thread, event;
+    char *target_ptr;
+    CONTEXT ctx;
+    DWORD res;
+    int i;
+
+    /* jmp to self at offset 0 and 4 */
+    static const char target_code[] = {0xeb, 0xfe, 0x90, 0x90, 0xeb, 0xfe};
+
+    target_ptr = VirtualAlloc( NULL, 65536, MEM_RESERVE | MEM_COMMIT, PAGE_EXECUTE_READWRITE );
+    memcpy( target_ptr, target_code, sizeof(target_code) );
+    target = (UINT_PTR)target_ptr;
+
+    event = CreateEventW( NULL, TRUE, FALSE, NULL );
+    thread = CreateThread( NULL, 65536, ip_test_thread_proc, event, 0, NULL );
+    ok( thread != NULL, "Failed to create thread: %lx\n", GetLastError() );
+    res = WaitForSingleObject( event, 1000 );
+    ok( !res, "wait returned: %ld\n", res );
+    CloseHandle( event );
+
+    memset( &ctx, 0, sizeof(ctx) );
+    ctx.ContextFlags = CONTEXT_ALL;
+    res = GetThreadContext( thread, &ctx );
+    ok( res, "Failed to get thread context: %lx\n", GetLastError() );
+    old_ip = IP_REG(ctx);
+
+    IP_REG(ctx) = target;
+    res = SetThreadContext( thread, &ctx );
+    ok( res, "Failed to set thread context: %lx\n", GetLastError() );
+
+    for (i = 0; i < 10; i++)
+    {
+        IP_REG(ctx) = target;
+        res = SetThreadContext( thread, &ctx );
+        ok( res, "Failed to set thread context: %lx\n", GetLastError() );
+
+        ctx.ContextFlags = CONTEXT_ALL;
+        res = GetThreadContext( thread, &ctx );
+        ok( res, "Failed to get thread context: %lx\n", GetLastError() );
+        ok( IP_REG(ctx) == target, "IP = %p, expected %p\n", (void *)IP_REG(ctx), target_ptr );
+
+        IP_REG(ctx) = target + 4;
+        res = SetThreadContext( thread, &ctx );
+        ok( res, "Failed to set thread context: %lx\n", GetLastError()) ;
+
+        ctx.ContextFlags = CONTEXT_ALL;
+        res = GetThreadContext( thread, &ctx );
+        ok( res, "Failed to get thread context: %lx\n", GetLastError() );
+        ok( IP_REG(ctx) == target + 4, "IP = %p, expected %p\n", (void *)IP_REG(ctx), target_ptr + 4 );
+    }
+
+    exit_ip_test = 1;
+    ctx.ContextFlags = CONTEXT_ALL;
+    IP_REG(ctx) = old_ip;
+    res = SetThreadContext( thread, &ctx );
+    ok( res, "Failed to restore thread context: %lx\n", GetLastError() );
+
+    res = WaitForSingleObject( thread, 1000 );
+    ok( !res, "wait returned: %ld\n", res );
+
+    VirtualFree( target_ptr, 0, MEM_RELEASE );
+}
 #endif
 
 START_TEST(exception)
@@ -11445,6 +11525,7 @@ START_TEST(exception)
     test_kiuserexceptiondispatcher();
     test_extended_context();
     test_copy_context();
+    test_set_live_context();
 
 #elif defined(__x86_64__)
 
@@ -11487,6 +11568,7 @@ START_TEST(exception)
       win_skip( "Dynamic unwind functions not found\n" );
     test_extended_context();
     test_copy_context();
+    test_set_live_context();
     test_unwind_from_apc();
     test_syscall_clobbered_regs();
     test_raiseexception_regs();
-- 
GitLab

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

[Jacek Caban (＠jacek)]

Moved tests to exception.c, where other similar tests are implemented.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/3566#note_45183