This is the mail archive of the
mailing list for the GDB project.
Re: [patch 3/3] Fix stale tp->step_resume_breakpoint
- From: Pedro Alves <pedro at codesourcery dot com>
- To: Jan Kratochvil <jan dot kratochvil at redhat dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Wed, 24 Nov 2010 12:42:53 +0000
- Subject: Re: [patch 3/3] Fix stale tp->step_resume_breakpoint
- References: <20101124005034.GC7263@host0.dyn.jankratochvil.net>
On Wednesday 24 November 2010 00:50:34, Jan Kratochvil wrote:
> this is a follow-up to:
> Re: [patch] Fix stale tp->step_resume_breakpoint
> On Tue, 02 Nov 2010 02:05:18 +0100, Pedro Alves wrote:
> > I think this raises an obvious question, and hints at
> > a larger issue: if you find you you need to tuck away step_resume_breakpoint,
> > then, how come you don't need to do the same for all the other execution
> > command state? (step_range_start, step_range_end, step_frame_id,
> > continuations, etc.).
> Some of them were already saved, just not into `struct inferior_thread_state'
> but into `struct inferior_status'. So extended now further the latter.
> I find these two redundant, they could be merged? I did not try to.
> This could be a different patch.
Yeah, there's a bunch of duplication going around here, but these
two do serve a slightly different purpose, as mentioned in the
comment above each. But I guess they could be merged if the
logic of what-to-restore-and-when is preserved. No opinion.
There's a lot of duplication between inferior_status and
thread_info as well. I've though before about a new structure
that stores execution command state (what gdb is doing with
the thread, infcmd + handle_inferior_event state machine status),
e.g. (chosen names are only for illustration):
struct frame_id step_frame_id;
struct frame_id step_stack_frame_id;
and then making struct thread_info embed a field of those,
instead of all the the field duplication.
So when doing an infcall, you'd save/restore this
object, which should make it a little harder to forget
> +++ b/gdb/testsuite/gdb.base/step-resume-infcall.c
> @@ -0,0 +1,45 @@
> +#include <stdio.h>
> +cond (void)
> + puts ("cond-hit");
Can you make the test not rely on stdio, please? Say, instead increment a
global whenever this function is called, and check its value later? This
is because not all remote targets support remote file io, and gdbserver
does not. That is, this puts appears on gdbserver's terminal, not gdb's.
This is the reason several tests are skipped if
"target_info exists gdb,noinferiorio" is set. It's always better if we
can avoid io in the first place, as it gives broader coverage.
Otherwise looks good. Thanks again.