Re: [RFC] Add support of software single step to process record

[Michael Snyder <msnyder@vmware.com>]

Hui Zhu wrote:
> Hi guys,
> 
> This patch to make prec support software single step.
> I just try it with i386.  Shuchang, please help me try it on mips.  :)
> 
> BTW, There are still some other patches for record_resume and
> record_wait. And other arch patch is still not begin to be review.  So
> this patch is not very urgency.  It just help the guys that are
> working on prec arch porting.

Seems OK in principle.  Thanks for adding a nice clean API to breakpoint.


> 2009-12-18  Hui Zhu  <teawater@gmail.com>
> 
> 	* breakpoint.c (inserted_single_step_breakpoint_p): New
> 	function.
> 	* breakpoint.h (inserted_single_step_breakpoint_p): Extern.
> 	* record.c (record_resume): Add code for software single step.
> 	(record_wait): Ditto.
> 
> ---
>  breakpoint.c |   12 ++++++++++++
>  breakpoint.h |    1 +
>  record.c     |   35 ++++++++++++++++++++++++++++++-----
>  3 files changed, 43 insertions(+), 5 deletions(-)
> 
> --- a/breakpoint.c
> +++ b/breakpoint.c
> @@ -9624,6 +9624,18 @@ insert_single_step_breakpoint (struct gd
>  	     paddress (gdbarch, next_pc));
>  }
> 
> +/* Check if the breakpoints used for software single stepping
> inserted or not.  */
> +
> +int
> +inserted_single_step_breakpoint_p (void)
> +{
> +  if (single_step_breakpoints[0] != NULL
> +      || single_step_breakpoints[1] != NULL)
> +    return 1;
> +
> +  return 1;
> +}
> +
>  /* Remove and delete any breakpoints used for software single step.  */
> 
>  void
> --- a/breakpoint.h
> +++ b/breakpoint.h
> @@ -944,6 +944,7 @@ extern int remove_hw_watchpoints (void);
>     twice before remove is called.  */
>  extern void insert_single_step_breakpoint (struct gdbarch *,
>  					   struct address_space *, CORE_ADDR);
> +extern int inserted_single_step_breakpoint_p (void);
>  extern void remove_single_step_breakpoints (void);
> 
>  /* Manage manual breakpoints, separate from the normal chain of
> --- a/record.c
> +++ b/record.c
> @@ -995,6 +995,8 @@ record_resume (struct target_ops *ops, p
> 
>    if (!RECORD_IS_REPLAY)
>      {
> +      struct gdbarch *gdbarch = get_current_arch ();
> +
>        if (do_record_message (get_current_regcache (), signal))
>          {
>            record_resume_error = 0;
> @@ -1004,8 +1006,18 @@ record_resume (struct target_ops *ops, p
>            record_resume_error = 1;
>            return;
>          }
> -      record_beneath_to_resume (record_beneath_to_resume_ops, ptid, 1,
> -                                signal);
> +
> +      if (gdbarch_software_single_step_p (gdbarch))
> +        {
> +          if (!inserted_single_step_breakpoint_p ())
> +            gdbarch_software_single_step (gdbarch, get_current_frame ());
> +          record_beneath_to_resume (record_beneath_to_resume_ops,
> +                                    ptid, step, signal);
> +          record_resume_step = 0;
> +        }
> +      else
> +        record_beneath_to_resume (record_beneath_to_resume_ops, ptid, 1,
> +                                  signal);
>      }
>  }
> 
> @@ -1086,6 +1098,7 @@ record_wait (struct target_ops *ops,
>  	  /* This is not a single step.  */
>  	  ptid_t ret;
>  	  CORE_ADDR tmp_pc;
> +          struct gdbarch *gdbarch = get_current_arch ();
> 
>  	  while (1)
>  	    {
> @@ -1108,6 +1121,9 @@ record_wait (struct target_ops *ops,
>  		  tmp_pc = regcache_read_pc (regcache);
>  		  aspace = get_regcache_aspace (regcache);
> 
> +                  if (gdbarch_software_single_step_p (gdbarch))
> +                    remove_single_step_breakpoints ();
> +
>  		  if (target_stopped_by_watchpoint ())
>  		    {
>  		      /* Always interested in watchpoints.  */
> @@ -1133,9 +1149,18 @@ record_wait (struct target_ops *ops,
>  		      if (!do_record_message (regcache, TARGET_SIGNAL_0))
>  			break;
> 
> -		      record_beneath_to_resume (record_beneath_to_resume_ops,
> -						ptid, 1,
> -						TARGET_SIGNAL_0);
> +                      if (gdbarch_software_single_step_p (gdbarch))
> +                        {
> +                          gdbarch_software_single_step (gdbarch,
> +                                                        get_current_frame ());
> +                          record_beneath_to_resume
> (record_beneath_to_resume_ops,
> +                                                    ptid, 0,
> +                                                    TARGET_SIGNAL_0);
> +                        }
> +                      else
> +		        record_beneath_to_resume (record_beneath_to_resume_ops,
> +						  ptid, 1,
> +						  TARGET_SIGNAL_0);
>  		      continue;
>  		    }
>  		}