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


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.

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]