Patchwork [5/5] windows-nat: Don't change current_event.dwThreadId in handle_output_debug_string()

login
register
mail settings
Submitter Jon TURNEY
Date April 16, 2015, 7:23 p.m.
Message ID <1429212209-20548-6-git-send-email-jon.turney@dronecode.org.uk>
Download mbox | patch
Permalink /patch/6268/
State New
Headers show

Comments

Jon TURNEY - April 16, 2015, 7:23 p.m.
Since a Cygwin signal may be reported by a different thread to the thread the
signal is to be delivered to, use the signal target thread id by returning it,
rather than re-writing the thread id in current_event.

Altering current_event.dwThreadId() will cause ContinueDebugEvent() to be
applied to the wrong thread and fail, leaving the actual thread which reported
the debug event stuck in the suspended state.

gdb/ChangeLog:

2015-04-16  Jon Turney  <jon.turney@dronecode.org.uk>

	* windows-nat.c (handle_output_debug_string): Don't change
	current_event.dwThreadId.
	(get_windows_debug_event): Use thread_id, rather than relying on
	current_event.dwThreadId being changed.
---
 gdb/ChangeLog     | 7 +++++++
 gdb/windows-nat.c | 3 +--
 2 files changed, 8 insertions(+), 2 deletions(-)
Joel Brobecker - April 22, 2015, 2:18 p.m.
> Since a Cygwin signal may be reported by a different thread to the thread the
> signal is to be delivered to, use the signal target thread id by returning it,
> rather than re-writing the thread id in current_event.
> 
> Altering current_event.dwThreadId() will cause ContinueDebugEvent() to be
> applied to the wrong thread and fail, leaving the actual thread which reported
> the debug event stuck in the suspended state.
> 
> gdb/ChangeLog:
> 
> 2015-04-16  Jon Turney  <jon.turney@dronecode.org.uk>
> 
> 	* windows-nat.c (handle_output_debug_string): Don't change
> 	current_event.dwThreadId.
> 	(get_windows_debug_event): Use thread_id, rather than relying on
> 	current_event.dwThreadId being changed.

I think this one makes better sense to me. So OK, with the same comments
as for patch 4/5: I'd like the revision history to be self-sufficient,
to allow anyone looking at this change years from now to be able to
understand what it was that this patch was trying to do.  So can you
amend it so that it gives a more complete description of the problem,
making it less abstract? In particular, a copy of the GDB session would
show what the symptoms are, and a copy of the behavior afterwards would
help.

Thank you,

Patch

diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 94d295e..9a4276e 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -850,7 +850,6 @@  handle_output_debug_string (struct target_waitstatus *ourstatus)
 					 __COPY_CONTEXT_SIZE, &n)
 		   && n == __COPY_CONTEXT_SIZE)
 	    have_saved_context = 1;
-	  current_event.dwThreadId = retval;
 	}
     }
 #endif
@@ -1508,7 +1507,7 @@  get_windows_debug_event (struct target_ops *ops,
 				  thread_id);
       current_thread = th;
       if (!current_thread)
-	current_thread = thread_rec (current_event.dwThreadId, TRUE);
+	current_thread = thread_rec (thread_id, TRUE);
     }
 
 out: