Re: [Patch] Use resume instead of target_resume when stepping over watchpoint.

[Joel Brobecker <brobecker at adacore dot com>]

Pedro, Others,

What do you think of this patch? Personally, I have pretty much
convinced myself that it shouldn't do any harm, but I really
wished that "resume" would take a ptid as an argument. Except
that this is not trivial to do, and I think that the current
"resume" would need to be split a bit, to remove the code that
determines what to resume.

Anyway, I don't see anything wrong with this patch, but I'd love
for someone to take a look as well. This is a pretty delicate
part of the debugger.  Do we really need the gdb_assert thought?

On Tue, Sep 09, 2008 at 05:31:33PM -0700, David Daney wrote:
> In handle_inferior_event() when stepping over a watch point currently we
> issue target_resume().  This only works on architectures that have
> hardware single step support.  For gdbarch_software_single_step_p()
> systems (like MIPS), we need to insert a single step breakpoint instead.
> 
> The fix is to call resume() as it does the right thing already.  I also
> added an assert that inferior_ptid == ecs->ptid to be sure that resume()
> was stepping the proper thread.
> 
> This is essentially the change requested by Daniel in:
> http://sourceware.org/ml/gdb-patches/2008-04/msg00443.html
> 
> This change is a prerequisite for my forthcoming MIPS hardware watch patch.
> 
> Tested on x86_64-pc-linux-gnu as well as mipsel-linux (in conjunction
> with the MIPS hardware watch patch).
> 
> OK to commit?
> 
> 2008-09-09  David Daney  <ddaney@avtrex.com>
> 
> 	* infrun.c (handle_inferior_event): Call resume instead of
> 	target_resume when stepping over watchpoint.
> 
> 

> Index: infrun.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/infrun.c,v
> retrieving revision 1.316
> diff -u -p -r1.316 infrun.c
> --- infrun.c	8 Sep 2008 22:10:20 -0000	1.316
> +++ infrun.c	9 Sep 2008 23:37:09 -0000
> @@ -2472,7 +2472,8 @@ targets should add new threads to the th
>        if (!HAVE_STEPPABLE_WATCHPOINT)
>  	remove_breakpoints ();
>        registers_changed ();
> -      target_resume (ecs->ptid, 1, TARGET_SIGNAL_0);	/* Single step */
> +      gdb_assert (ptid_equal (inferior_ptid, ecs->ptid));
> +      resume (1, TARGET_SIGNAL_0);	/* Single step */
>        waiton_ptid = ecs->ptid;
>        if (HAVE_STEPPABLE_WATCHPOINT)
>  	infwait_state = infwait_step_watch_state;


-- 
Joel