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] |
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] |