[RFA-v2] windows-nat.c Cygwin saved_context fix

["Pierre Muller" <muller@ics.u-strasbg.fr>]

After testing,
setting suspended field to -1
seems to work equally well for me.

  Here is an updated patch:


2009-09-22  Pierre Muller  <muller@ics.u-strasbg.fr>

	* windows-nat.c (saved_threadid): New variable.
	(do_windows_fetch_inferior_registers): Check for correct thread id
	and set suspended field to -1 if it is zero.
	(handle_output_debug_string): Set saved_threadid.

Index: src/gdb/windows-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/windows-nat.c,v
retrieving revision 1.197
diff -u -p -r1.197 windows-nat.c
--- src/gdb/windows-nat.c	21 Sep 2009 22:37:59 -0000	1.197
+++ src/gdb/windows-nat.c	21 Sep 2009 22:53:11 -0000
@@ -97,6 +97,7 @@ static CORE_ADDR cygwin_load_start;
 static CORE_ADDR cygwin_load_end;
 #endif
 
+static int saved_threadid;
 static int have_saved_context;	/* True if we've saved context from a cygwin
signal. */
 static CONTEXT saved_context;	/* Containes the saved context from a cygwin
signal. */
 
@@ -381,11 +382,15 @@ do_windows_fetch_inferior_registers (str
   if (current_thread->reload_context)
     {
 #ifdef __COPY_CONTEXT_SIZE
-      if (have_saved_context)
+      if (have_saved_context && current_thread->id == saved_threadid)
 	{
 	  /* Lie about where the program actually is stopped since cygwin
has informed us that
 	     we should consider the signal to have occurred at another
location which is stored
 	     in "saved_context. */
+	  if (!current_thread->suspended)
+	  /* Set suspended to -1 to avoid resetting reload_context
+	     later in get_thread.  */ 
+	    current_thread->suspended = -1;
 	  memcpy (&current_thread->context, &saved_context,
__COPY_CONTEXT_SIZE);
 	  have_saved_context = 0;
 	}
@@ -863,6 +868,7 @@ handle_output_debug_string (struct targe
 					 &saved_context,
__COPY_CONTEXT_SIZE, &n)
 		   && n == __COPY_CONTEXT_SIZE)
 	    have_saved_context = 1;
+	  saved_threadid = retval;
 	  current_event.dwThreadId = retval;
 	}
     }

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Pierre Muller
> Envoyé : Tuesday, September 22, 2009 12:36 AM
> À : gdb-patches@sourceware.org
> Objet : RE: [RFA] windows-nat.c Cygwin save_context fix
> 
> 
> 
> > -----Message d'origine-----
> > De : Christopher Faylor [mailto:cgf-use-the-mailinglist-
> > please@sourceware.org]
> > Envoyé : Tuesday, September 22, 2009 12:27 AM
> > À : gdb-patches@sourceware.org; Pierre Muller
> > Objet : Re: [RFA] windows-nat.c Cygwin save_context fix
> >
> > On Mon, Sep 21, 2009 at 05:14:57PM +0200, Pierre Muller wrote:
> > >  Cygwin has a way to give a "fake" context for
> > >an exception using a OUTPUT_DEBUG_STRING_EVENT.
> > >
> > >  While trying to debug some crashes inside cygwin dll,
> > >I realized the the saved_context code has a problem.
> > >
> > >  The saved context was correctly written to the thread_info
> > >struct, but later overwritten by a call to GetThreadContext.
> > >
> > >  After the cygwin special output_debug_string was correctly
> > >converted into a context stored in saved_context.
> > >  The next call to do_windows_fetch_inferior_registers
> > >then copied this context to the thread_info struct,
> > >set the reload_context field to zero.
> > >
> > >  But a later call to thread_rec() with get_context=1
> > >can reset reload_context to one, if suspended field
> > >is zero (this only happens if it is not the main thread).
> > >
> > >  My patch fixes the problem by explicitly calling
> > >SuspendThread for the threaded of the saved_context
> > >if suspended is still zero at that point.
> > >
> > >
> > >Pierre Muller
> > >Pascal language support maintainer for GDB
> > >
> > >
> > >
> > >2009-09-21  Pierre Muller  <muller@ics.u-strasbg.fr>
> > >
> > >	* src/gdb/windows-nat.c (saved_threadid): New variable.
> > >	(do_windows_fetch_inferior_registers): Check for correct thread
> > id
> > >	and force call to SuspendThread if needed.
> > >	(handle_output_debug_string): Set saved_threadid.
> >
> > I REALLY hate SuspendThread and am not likely to accept a patch which
> > uses it as a solution.
> 
>   I don't think that we really need to call SuspendThread,
> in fact, I suspect the best would be to set suspended
> to -1 as for main_threadid. I will try to check out if this also works.
> 
> > I don't really understand the scenario that you are talking about
> since
> > I (obviously?) have no problems debugging SEGVs in cygwin.  However,
> 
>   I was trying to debug a home compiled expect
> linked to 8.5.7 tcl library, that was call abort,
> and the subsequent cygwin stackdump was segfaulting.
> 
> > would just removing the "have_saved_context = 0;" from
> > do_windows_fetch_inferior_registers fix the problem?
> 
> But them we need to be really careful about to where
> we should reset it to zero, otherwise we might end up
> by still copying saved_context, even after having reran the debuggee,
> no?
> 
> Pierre
>