[PATCH] server: copy_context(): Copy only valid information from source context.

[Joachim Priesner]

A thread's context may be incomplete. For example, the i386
RtlCaptureContext() function does not save floating-point registers,
setting the context's flags accordingly.

Signed-off-by: Joachim Priesner <joachim.priesner(a)web.de>
---
I'd like some feedback on whether this patch goes in the right
direction. In particular, it breaks the functionality introduced in
03d31ea8 ("Only update the modified parts of the context on thread
suspend") because the flags field is now used both to state what part of
the context is valid and what part has changed.

I found the issue when a program that manually raised an exception
using RaiseException() was run under winedbg. The copy_context()
invocation copied the zeroed-out floating point status from the
suspended thread's context and set the SERVER_CTX_FLOATING_POINT
flag. When winedbg wrote back the context after handling the exception,
it overwrote the floating-point registers with zeroes.
---
 server/thread.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/server/thread.c b/server/thread.c
index 2cf5054738..dfd2f61104 100644
--- a/server/thread.c
+++ b/server/thread.c
@@ -1129,6 +1129,7 @@ void kill_thread( struct thread *thread, int violent_death )
 static void copy_context( context_t *to, const context_t *from, unsigned int flags )
 {
     assert( to->cpu == from->cpu );
+    flags &= from->flags;
     to->flags |= flags;
     if (flags & SERVER_CTX_CONTROL) to->ctl = from->ctl;
     if (flags & SERVER_CTX_INTEGER) to->integer = from->integer;
@@ -1754,7 +1755,6 @@ DECL_HANDLER(set_suspend_context)
     else if ((current->suspend_context = mem_alloc( sizeof(context_t) )))
     {
         memcpy( current->suspend_context, get_req_data(), sizeof(context_t) );
-        current->suspend_context->flags = 0;  /* to keep track of what is modified */
         current->context = current->suspend_context;
         if (current->debug_break) break_thread( current );
     }
-- 
2.19.0

[Alexandre Julliard]

Joachim Priesner <joachim.priesner(a)web.de> writes:
> A thread's context may be incomplete. For example, the i386
> RtlCaptureContext() function does not save floating-point registers,
> setting the context's flags accordingly.
>
> Signed-off-by: Joachim Priesner <joachim.priesner(a)web.de>
> ---
> I'd like some feedback on whether this patch goes in the right
> direction. In particular, it breaks the functionality introduced in
> 03d31ea8 ("Only update the modified parts of the context on thread
> suspend") because the flags field is now used both to state what part of
> the context is valid and what part has changed.

I think it's the right direction, it shouldn't be hard to use a separate
field to store which flags have been changed instead of abusing the
context flags.

-- 
Alexandre Julliard
julliard(a)winehq.org