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

[Hui Zhu <teawater at gmail dot com>]

On Tue, Jun 22, 2010 at 18:12, Pedro Alves <pedro@codesourcery.com> wrote:
> Hi Hui,
>
> On Sunday 20 June 2010 08:28:40, Hui Zhu wrote:
>> On Fri, Jun 11, 2010 at 21:55, Pedro Alves <pedro@codesourcery.com> wrote:
>> > I'm felling a bit dense, and I don't see what is that actually
>> > catching. ?If going backwards, the assertion always ends up
>> > evaled as true, nomatter if sofware single-steps are inserted
>> > or not, or whether `step' is set. ?Did you mean to assert
>> > that when going backwards, there shouldn't ever be software
>> > single-step breakpoints inserted?
>> >
>> > This patch is okay otherwise. ?Thanks.
>>
>> Thanks Pedro.
>> I was also confused by this issue too. ?I thought it will never happen
>> too. ?But Ping said he got this issue. ?And I didn't have the risc
>> board to test. ?So I gived up and put this patch to him.
>>
>> So I think this patch is not very hurry to checked in until some one
>> post a risc prec support patch. ?At that time, I will make this issue
>> clear.
>
> I'd be fine with putting the patch in now, but without the change to
> that gdb_assert. ?It looked like a step in the right direction,
> and we can fix any left issues later.
>
> --
> Pedro Alves
>

Agree with you.
I delay this patch some days because I want make it check in after 7.2.

Now, following patch checked in.

Thanks,
Hui

2010-07-19  Hui Zhu  <teawater@gmail.com>

	* breakpoint.c (single_step_breakpoints_inserted): New
	function.
	* breakpoint.h (single_step_breakpoints_inserted): Extern.
	* infrun.c (maybe_software_singlestep): Add check code.
	* record.c (record_resume): Add code for software single step.
	(record_wait): Ditto.


---
 breakpoint.c |   10 +++++++++
 breakpoint.h |    1
 infrun.c     |    3 +-
 record.c     |   64 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 4 files changed, 72 insertions(+), 6 deletions(-)

--- a/breakpoint.c
+++ b/breakpoint.c
@@ -10468,6 +10468,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
@@ -984,6 +984,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/infrun.c
+++ b/infrun.c
@@ -1515,7 +1515,8 @@ maybe_software_singlestep (struct gdbarc
 {
   int hw_step = 1;

-  if (gdbarch_software_single_step_p (gdbarch)
+  if (execution_direction == EXEC_FORWARD
+      && gdbarch_software_single_step_p (gdbarch)
       && gdbarch_software_single_step (gdbarch, get_current_frame ()))
     {
       hw_step = 0;
--- a/record.c
+++ b/record.c
@@ -1011,9 +1011,43 @@ 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 (!step)
+        {
+          /* This is not hard single step.  */
+          if (!gdbarch_software_single_step_p (gdbarch))
+            {
+              /* This is a normal continue.  */
+              step = 1;
+            }
+          else
+            {
+              /* This arch support soft sigle step.  */
+              if (single_step_breakpoints_inserted ())
+                {
+                  /* This is a soft single step.  */
+                  record_resume_step = 1;
+                }
+              else
+                {
+                  /* This is a continue.
+                     Try to insert a soft single step breakpoint.  */
+                  if (!gdbarch_software_single_step (gdbarch,
+                                                     get_current_frame ()))
+                    {
+                      /* This system don't want use soft single step.
+                         Use hard sigle step.  */
+                      step = 1;
+                    }
+                }
+            }
+        }
+
+      record_beneath_to_resume (record_beneath_to_resume_ops,
+                                ptid, step, signal);
     }
 }

@@ -1089,12 +1123,16 @@ 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)
 	    {
 	      ret = record_beneath_to_wait (record_beneath_to_wait_ops,
 					    ptid, status, options);

+              if (single_step_breakpoints_inserted ())
+                remove_single_step_breakpoints ();
+
 	      if (record_resume_step)
 	        return ret;

@@ -1134,8 +1172,12 @@ record_wait (struct target_ops *ops,
 		    }
 		  else
 		    {
-		      /* This must be a single-step trap.  Record the
-		         insn and issue another step.  */
+		      /* This is a single-step trap.  Record the
+		         insn and issue another step.
+                         FIXME: this part can be a random SIGTRAP too.
+                         But GDB cannot handle it.  */
+                      int step = 1;
+
 		      if (!record_message_wrapper_safe (regcache,
                                                         TARGET_SIGNAL_0))
   			{
@@ -1144,8 +1186,20 @@ record_wait (struct target_ops *ops,
                            break;
   			}

+                      if (gdbarch_software_single_step_p (gdbarch))
+			{
+			  /* Try to insert the software single step breakpoint.
+			     If insert success, set step to 0.  */
+			  set_executing (inferior_ptid, 0);
+			  reinit_frame_cache ();
+			  if (gdbarch_software_single_step (gdbarch,
+
get_current_frame ()))
+			    step = 0;
+			  set_executing (inferior_ptid, 1);
+			}
+
 		      record_beneath_to_resume (record_beneath_to_resume_ops,
-						ptid, 1,
+						ptid, step,
 						TARGET_SIGNAL_0);
 		      continue;
 		    }