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