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

[Hui Zhu <teawater@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1284 bytes --]

Cool.  I think your idea is more better than I did.  Thanks.

This is the new patch.

Best regards,
Hui

On Wed, Dec 23, 2009 at 14:51, Joel Brobecker <brobecker@adacore.com> wrote:
>> Joel, I remove the current_gdbarch() from this patch, but for
>> get_current_frame, I cannot find any function that fit for replace it.
>>  Could you help me with it?
>
> Upon further investigation, I believe that this is the only way to do
> this.  I don't think you can get the frame from the arguments provided
> to "resume". I did a quick research and infrun.c:resume does the same
> thing (actually, it's maybe_software_singlestep).
>
>> +  if (single_step_breakpoints[0] != NULL
>> +      || single_step_breakpoints[1] != NULL)
>> +    return 1;
>> +
>> +  return 0;
>
> Style nit: You really don't have to fix this, as this is a matter of
> style more than correctness, but we usually write the above code as
>
>  return (single_step_breakpoints[0] != NULL
>          || single_step_breakpoints[1] != NULL)
>
> This avoids the type of mistake you made.  I tend to like the style
> you used when there are several consecutive if conditions that allow
> me to return early (a type of soft assert), or to avoid unreasonable
> nesting levels.
>
> --
> Joel
>

[-- Attachment #2: prec_software_single_step.txt --]
[-- Type: text/plain, Size: 3426 bytes --]

---
 breakpoint.c |   10 ++++++++++
 breakpoint.h |    1 +
 record.c     |   33 ++++++++++++++++++++++++++++++---
 3 files changed, 41 insertions(+), 3 deletions(-)

--- a/breakpoint.c
+++ b/breakpoint.c
@@ -9624,6 +9624,16 @@ insert_single_step_breakpoint (struct gd
 	     paddress (gdbarch, next_pc));
 }
 
+/* Check if the breakpoints used for software single stepping
+   were inserted or not.  */
+
+int
+single_step_breakpoints_inserted (void)
+{
+  return (single_step_breakpoints[0] != NULL
+          || single_step_breakpoints[1] != NULL);
+}
+
 /* 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 single_step_breakpoints_inserted (void);
 extern void remove_single_step_breakpoints (void);
 
 /* Manage manual breakpoints, separate from the normal chain of
--- a/record.c
+++ b/record.c
@@ -1002,9 +1002,23 @@ record_resume (struct target_ops *ops, p
 
   if (!RECORD_IS_REPLAY)
     {
+      struct gdbarch *gdbarch = target_thread_architecture (ptid);
+
       record_message (get_current_regcache (), signal);
       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);
     }
 }
 
@@ -1077,6 +1091,7 @@ record_wait (struct target_ops *ops,
 	  /* This is not a single step.  */
 	  ptid_t ret;
 	  CORE_ADDR tmp_pc;
+          struct gdbarch *gdbarch = target_thread_architecture (inferior_ptid);
 
 	  while (1)
 	    {
@@ -1099,6 +1114,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.  */
@@ -1129,9 +1147,18 @@ record_wait (struct target_ops *ops,
                            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;
 		    }
 		}