This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
[RFA-v2] windows-nat.c Cygwin saved_context fix
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 (¤t_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
>