This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


On Tue, May 25, 2010 at 11:04, Hui Zhu <teawater@gmail.com> wrote:
> Thanks Pedro.
>
> On Sat, Jan 9, 2010 at 00:24, Pedro Alves <pedro@codesourcery.com> wrote:
>> On Monday 04 January 2010 14:23:21, Hui Zhu wrote:
>>> Sorry guys, the prev patch is so ugly.
>>
>> :-)
>>
>>> Thanks for teach me clear about the gdbarch_software_single_step, Pedro.
>>> I did some extend with your idea. ?Because record_wait need
>>> record_resume_step point out this resume is signal step or continue.
>>>
>>> ? ? ? 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;
>>> ? ? ? ? ? ? ? ? ? ? }
>>> ? ? ? ? ? ? ? ? }
>>> ? ? ? ? ? ? }
>>> ? ? ? ? }
>>
>> Cool, this looks pretty clear to me now. ?Thanks.
>>
>>
>>
>>> @@ -1077,6 +1111,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 +1134,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 ();
>>
>> This will gdb_assert inside remove_single_step_breakpoints
>> if SSS bkpts are not inserted, but gdbarch_software_single_step_p
>> returns true. ?This instead is safer:
>>
>> ? ? ? ? ? ? ? ? ?if (single_step_breakpoints_inserted ())
>> ? ? ? ? ? ? ? ? ? ?remove_single_step_breakpoints ();
>
> OK. ?I will fix it.
>
>>
>> But, what if it was infrun that had inserted the single-step
>> breakpoints, for a "next" or "step", etc.? ?Shouldn't you check
>> for record_resume_step too?
>>
>> ? ? ? ? ? ? ? if (!record_resume_step && single_step_breakpoints_inserted ())
>> ? ? ? ? ? ? ? ? remove_single_step_breakpoints ();
>>
>> Otherwise, the check below for
>>
>> ? ? ? ? ? ? ? ? ?else if (breakpoint_inserted_here_p (aspace, tmp_pc))
>> ? ? ? ? ? ? ? ? ? ?{
>> ? ? ? ? ? ? ? ? ? ? ?/* There is a breakpoint here. ?Let the core
>> ? ? ? ? ? ? ? ? ? ? ? ? handle it. ?*/
>> ? ? ? ? ? ? ? ? ? ? ?if (software_breakpoint_inserted_here_p (aspace, tmp_pc))
>> ? ? ? ? ? ? ? ? ? ? ? ?{
>>
>> would fail, and the finished single-step wouldn't be reported to the
>> core, right?
>
> I think this single step will be handle by line:
> ? ? ?if (record_resume_step)
> ? ? ? ?{
> ? ? ? ? ?/* This is a single step. ?*/
> ? ? ? ? ?return record_beneath_to_wait (record_beneath_to_wait_ops,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ptid, status, options);
> ? ? ? ?}
>
>>
>>
>> Lastly, you may also want to confirm that the SSS bkpt managed by record.d itself explains the SIGTRAP before removing before issueing another
>> single-step. ?If any unexplainable SIGTRAP happens for any reason while
>> single-stepping, you should report it to infrun instead. ?In other words:
>>
>> With software single-stepping, we can distinguish most random
>> SIGTRAPs from SSS SIGTRAPs, so:
>>
>> ? ? ? ? ? ? ? ? ? ? ?/* This must be a single-step trap. ?Record the
>> ? ? ? ? ? ? ? ? ? ? ? ? insn and issue another step. ?*/
>>
>> ... the "must" here ends up being a bit too strong. ?I'd certainly
>> understand ignoring this for simplicity or performance reasons though.
>
> Ah. ?Looks we didn't have good way to handle it. ?I change this comment to:
> ? ? ? ? ? ? ? ? ? ? ?/* 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. ?*/
>
>
> Shuchang, ?could you try your code just use command si and
> reverse-xxx. ?If that part OK. ?Please help me try this patch.
>
> Ping, please help me test this patch. ?And about hellogcc, you can find us in:
> https://groups.google.com/group/hellogcc
> https://webchat.freenode.net/ #hellogcc
>
> Thanks,
> Hui
>
> 2010-05-25 ?Hui Zhu ?<teawater@gmail.com>
>
> ? ? ? ?* breakpoint.c (single_step_breakpoints_inserted): New
> ? ? ? ?function.
> ? ? ? ?* breakpoint.h (single_step_breakpoints_inserted): Extern.
> ? ? ? ?* record.c (record_resume): Add code for software single step.
> ? ? ? ?(record_wait): Ditto.
>

Hello,

After do some test with Ping, I found some trouble and fixed them.

1.  Add following:
@@ -1134,8 +1176,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);
+			}

This is because in record_wait, we cannot call get_current_frame ()
directly.  And the frame message need refresh each exec cycle.

2.  Ping found that reverse-exec cannot single step in RISC board.
That is because "gdbarch_software_single_step" just can insert the
breakpoint to the next addr.  So I add following:
@@ -1436,7 +1436,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 ()))

If reverse, gdb will not user sss breakpoint to single step.

3.  Ping got some gdb_assert in sometime.  And I am not close to his
board.  So I didn't know what happen.  So I add following:
@@ -1534,7 +1535,8 @@ a command like `return' or `jump' to con
       /* If STEP is set, it's a request to use hardware stepping
 	 facilities.  But in that case, we should never
 	 use singlestep breakpoint.  */
-      gdb_assert (!(singlestep_breakpoints_inserted_p && step));
+      gdb_assert (!(execution_direction == EXEC_FORWARD
+                    && singlestep_breakpoints_inserted_p && step));

The lost one still need be test.

Thanks,
Hui

Attachment: prec_software_single_step.txt
Description: Text document


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]