* [PATCH 0/2] Honour software single step in fallback of displaced stepping @ 2015-04-07 15:52 Yao Qi 2015-04-07 15:52 ` [PATCH 1/2] [gdbserver] assert on step if !can_hardware_single_step Yao Qi ` (2 more replies) 0 siblings, 3 replies; 9+ messages in thread From: Yao Qi @ 2015-04-07 15:52 UTC (permalink / raw) To: gdb-patches From: Yao Qi <yao.qi@linaro.org> Hi, This patch set fixes many fails I've seen in gdb.threads/non-stop-fair-events.exp on arm-linux target. I don't see they are covered by Pedro's "All-stop on top of non-stop" V2, so I post them out for the review. Patch 2 is the real fix and the patch 1 is to make the debugging log clear to identify the problem. The two patches are tested on native and gdbserver {x86_64,arm}-linux. *** BLURB HERE *** Yao Qi (2): [gdbserver] assert on step if !can_hardware_single_step Honour software single step in fallback of displaced stepping gdb/gdbserver/linux-low.c | 3 +++ gdb/infrun.c | 8 ++++++-- 2 files changed, 9 insertions(+), 2 deletions(-) -- 1.9.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] [gdbserver] assert on step if !can_hardware_single_step 2015-04-07 15:52 [PATCH 0/2] Honour software single step in fallback of displaced stepping Yao Qi @ 2015-04-07 15:52 ` Yao Qi 2015-04-07 17:09 ` Pedro Alves 2015-04-07 15:52 ` [PATCH 2/2] Honour software single step in fallback of displaced stepping Yao Qi 2015-04-07 16:59 ` [PATCH 0/2] " Pedro Alves 2 siblings, 1 reply; 9+ messages in thread From: Yao Qi @ 2015-04-07 15:52 UTC (permalink / raw) To: gdb-patches From: Yao Qi <yao.qi@linaro.org> GDB sends vCont;s by mistake to GDBserver on arm target which doesn't have single step at all. However, it is hard to find the problem from the debugging log. With this patch applied, the problem is easy to identify, like: (gdb) PASS: gdb.threads/non-stop-fair-events.exp: signal_thread=2: switch to thread 6 to step it step&^M (gdb) PASS: gdb.threads/non-stop-fair-events.exp: signal_thread=2: set 6 thread stepping thread /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/linux-low.c:3686: A problem internal to GDBserver has been detected.^M linux_resume_one_lwp_throw: Assertion `step == 0' failed. gdb/gdbserver: 2015-04-02 Yao Qi <yao.qi@linaro.org> * linux-low.c (linux_resume_one_lwp_throw): Assert on step. --- gdb/gdbserver/linux-low.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c index e4c5420..bc6ab1ae 100644 --- a/gdb/gdbserver/linux-low.c +++ b/gdb/gdbserver/linux-low.c @@ -3682,6 +3682,9 @@ linux_resume_one_lwp_throw (struct lwp_info *lwp, if (the_low_target.prepare_to_resume != NULL) the_low_target.prepare_to_resume (lwp); + if (!can_hardware_single_step ()) + gdb_assert (step == 0); + regcache_invalidate_thread (thread); errno = 0; lwp->stepping = step; -- 1.9.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] [gdbserver] assert on step if !can_hardware_single_step 2015-04-07 15:52 ` [PATCH 1/2] [gdbserver] assert on step if !can_hardware_single_step Yao Qi @ 2015-04-07 17:09 ` Pedro Alves 0 siblings, 0 replies; 9+ messages in thread From: Pedro Alves @ 2015-04-07 17:09 UTC (permalink / raw) To: Yao Qi, gdb-patches On 04/07/2015 04:52 PM, Yao Qi wrote: > From: Yao Qi <yao.qi@linaro.org> > > GDB sends vCont;s by mistake to GDBserver on arm target which doesn't > have single step at all. However, it is hard to find the problem from > the debugging log. With this patch applied, the problem is easy to > identify, like: > > (gdb) PASS: gdb.threads/non-stop-fair-events.exp: signal_thread=2: switch to thread 6 to step it > step&^M > (gdb) PASS: gdb.threads/non-stop-fair-events.exp: signal_thread=2: set 6 thread stepping > thread /home/yao/SourceCode/gnu/gdb/git/gdb/gdbserver/linux-low.c:3686: A problem internal to GDBserver has been detected.^M > linux_resume_one_lwp_throw: Assertion `step == 0' failed. > > gdb/gdbserver: > > 2015-04-02 Yao Qi <yao.qi@linaro.org> > > * linux-low.c (linux_resume_one_lwp_throw): Assert on step. > --- > gdb/gdbserver/linux-low.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c > index e4c5420..bc6ab1ae 100644 > --- a/gdb/gdbserver/linux-low.c > +++ b/gdb/gdbserver/linux-low.c > @@ -3682,6 +3682,9 @@ linux_resume_one_lwp_throw (struct lwp_info *lwp, > if (the_low_target.prepare_to_resume != NULL) > the_low_target.prepare_to_resume (lwp); > > + if (!can_hardware_single_step ()) > + gdb_assert (step == 0); Yeah, I have something like that on my x86 software single-step branch, on the native side, and also in infrun.c. See: https://github.com/palves/gdb/commits/palves/x86_software_single_step https://github.com/palves/gdb/commit/52940835548c124a80bd6f381f1a463eda9bab4c ( I just realized/recalled the top commit fixes the exact same as your patch #2 :-) ) So I think your patch is a good idea. :-) But as you're suggesting it for inclusion, I have to raise the bar a little. I think gdbserver crashing/exiting due to bad input from gdb isn't ideal. This isn't gdbserver's fault after all. I think this should be an error instead. Or perhaps even better, this could stay as an assert in the backend, if server.c errors out earlier, even, while parsing the vCont;s / s packets. ( One nit, as step is a boolean, I think: gdb_assert (!step); would read more naturally. ) Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] Honour software single step in fallback of displaced stepping 2015-04-07 15:52 [PATCH 0/2] Honour software single step in fallback of displaced stepping Yao Qi 2015-04-07 15:52 ` [PATCH 1/2] [gdbserver] assert on step if !can_hardware_single_step Yao Qi @ 2015-04-07 15:52 ` Yao Qi 2015-04-07 16:59 ` [PATCH 0/2] " Pedro Alves 2 siblings, 0 replies; 9+ messages in thread From: Yao Qi @ 2015-04-07 15:52 UTC (permalink / raw) To: gdb-patches From: Yao Qi <yao.qi@linaro.org> Hi, When I run gdb.threads/non-stop-fair-events.exp on arm-linux target, I see the following error in the log, displaced: breakpoint is gone: Thread 22518, step(1)^M Sending packet: $vCont;s:p57f3.57f6#9d... gdb/gdbserver/linux-low.c:3686: A problem internal to GDBserver has been detected.^M linux_resume_one_lwp_throw: Assertion `step == 0' failed. GDB sends vCont;s by mistake, and GDBserver fails on assert. GDB doesn't consider software single step in infrun.c:displaced_step_fixup, /* Go back to what we were trying to do. */ step = currently_stepping (tp); if (debug_displaced) fprintf_unfiltered (gdb_stdlog, "displaced: breakpoint is gone: %s, step(%d)\n", target_pid_to_str (tp->ptid), step); target_resume (ptid, step, GDB_SIGNAL_0); The patch is to let GDB consider software single step here. It fixes fails in gdb.threads/non-stop-fair-events.exp on arm. gdb: 2015-04-02 Yao Qi <yao.qi@linaro.org> * infrun.c (maybe_software_singlestep): Declare. (displaced_step_fixup): Call maybe_software_singlestep. --- gdb/infrun.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/gdb/infrun.c b/gdb/infrun.c index f5faa0a..f4bbf67 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -100,6 +100,8 @@ static void insert_step_resume_breakpoint_at_caller (struct frame_info *); static void insert_longjmp_resume_breakpoint (struct gdbarch *, CORE_ADDR); +static int maybe_software_singlestep (struct gdbarch *gdbarch, CORE_ADDR pc); + /* When set, stop the 'step' command if we enter a function which has no line number information. The normal behavior is that we step over such function. */ @@ -1847,6 +1849,7 @@ displaced_step_fixup (ptid_t event_ptid, enum gdb_signal signal) regcache = get_thread_regcache (ptid); actual_pc = regcache_read_pc (regcache); aspace = get_regcache_aspace (regcache); + gdbarch = get_regcache_arch (regcache); if (breakpoint_here_p (aspace, actual_pc)) { @@ -1857,8 +1860,6 @@ displaced_step_fixup (ptid_t event_ptid, enum gdb_signal signal) displaced_step_prepare (ptid); - gdbarch = get_regcache_arch (regcache); - if (debug_displaced) { CORE_ADDR actual_pc = regcache_read_pc (regcache); @@ -1891,6 +1892,9 @@ displaced_step_fixup (ptid_t event_ptid, enum gdb_signal signal) /* Go back to what we were trying to do. */ step = currently_stepping (tp); + if (step) + step = maybe_software_singlestep (gdbarch, actual_pc); + if (debug_displaced) fprintf_unfiltered (gdb_stdlog, "displaced: breakpoint is gone: %s, step(%d)\n", -- 1.9.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] Honour software single step in fallback of displaced stepping 2015-04-07 15:52 [PATCH 0/2] Honour software single step in fallback of displaced stepping Yao Qi 2015-04-07 15:52 ` [PATCH 1/2] [gdbserver] assert on step if !can_hardware_single_step Yao Qi 2015-04-07 15:52 ` [PATCH 2/2] Honour software single step in fallback of displaced stepping Yao Qi @ 2015-04-07 16:59 ` Pedro Alves 2015-04-08 9:51 ` Yao Qi 2 siblings, 1 reply; 9+ messages in thread From: Pedro Alves @ 2015-04-07 16:59 UTC (permalink / raw) To: Yao Qi, gdb-patches On 04/07/2015 04:52 PM, Yao Qi wrote: > From: Yao Qi <yao.qi@linaro.org> > > Hi, > This patch set fixes many fails I've seen in > gdb.threads/non-stop-fair-events.exp on arm-linux target. I don't see > they are covered by Pedro's "All-stop on top of non-stop" V2, so I > post them out for the review. They actually are. :-) I tested v2 against x86 software single-step, and it caught issues like that. This patch: [PATCH v2 07/23] Embed the pending step-over chain in thread_info objects https://sourceware.org/ml/gdb-patches/2015-04/msg00218.html splits that code you're touching to a separate "start_step_over_inferior" function. And then this patch: [PATCH v2 11/23] Use keep_going in proceed and start_step_over too https://sourceware.org/ml/gdb-patches/2015-04/msg00203.html rewrites that whole function to defer to keep_going instead. keep_going already handles the case of the breakpoint disappearing (thread_still_needs_step_over returns false). And in case the breakpoint is still around, it ends in 'resume' again, which is then the only place that knows how to start a displaced step. I don't mind if you push your patch in first. I'll just end up deleting that code again when I rebase it. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] Honour software single step in fallback of displaced stepping 2015-04-07 16:59 ` [PATCH 0/2] " Pedro Alves @ 2015-04-08 9:51 ` Yao Qi 2015-04-16 12:00 ` Yao Qi 0 siblings, 1 reply; 9+ messages in thread From: Yao Qi @ 2015-04-08 9:51 UTC (permalink / raw) To: Pedro Alves; +Cc: Yao Qi, gdb-patches Pedro Alves <palves@redhat.com> writes: > They actually are. :-) I tested v2 against x86 software single-step, > and it caught issues like that. Yeah, I realised that when I apply your V2 on top of my patches later yesterday for the testing. I tested your V2 only, and fails in gdb.threads/non-stop-fair-events.exp go away! > This patch: > > [PATCH v2 07/23] Embed the pending step-over chain in thread_info objects > https://sourceware.org/ml/gdb-patches/2015-04/msg00218.html > > splits that code you're touching to a separate "start_step_over_inferior" > function. > > And then this patch: > > [PATCH v2 11/23] Use keep_going in proceed and start_step_over too > https://sourceware.org/ml/gdb-patches/2015-04/msg00203.html > > rewrites that whole function to defer to keep_going instead. keep_going > already handles the case of the breakpoint disappearing > (thread_still_needs_step_over > returns false). And in case the breakpoint is still around, it ends > in 'resume' again, which is then the only place that knows > how to start a displaced step. OK, I'll read the corresponding patches then. > > I don't mind if you push your patch in first. I'll just > end up deleting that code again when I rebase it. I'll let your patches go in. -- Yao (齐尧) ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] Honour software single step in fallback of displaced stepping 2015-04-08 9:51 ` Yao Qi @ 2015-04-16 12:00 ` Yao Qi 2015-04-16 12:02 ` Pedro Alves 0 siblings, 1 reply; 9+ messages in thread From: Yao Qi @ 2015-04-16 12:00 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches On 08/04/15 10:51, Yao Qi wrote: >> I don't mind if you push your patch in first. I'll just >> >end up deleting that code again when I rebase it. > I'll let your patches go in. Hi Pedro, Since this bug is still in FSF trunk, most of the tests in gdb.threads/non-stop-fair-events.exp are timeout on arm-linux, and takes much more time to run the whole testsuite. Can I push patch 1/2 in to fix this problem? assuming your V3 "all stop on top of non-stop" series can't go in in a a couple of days. -- Yao (é½å°§) ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] Honour software single step in fallback of displaced stepping 2015-04-16 12:00 ` Yao Qi @ 2015-04-16 12:02 ` Pedro Alves 2015-04-16 13:01 ` Yao Qi 0 siblings, 1 reply; 9+ messages in thread From: Pedro Alves @ 2015-04-16 12:02 UTC (permalink / raw) To: Yao Qi; +Cc: gdb-patches On 04/16/2015 01:00 PM, Yao Qi wrote: > On 08/04/15 10:51, Yao Qi wrote: >>> I don't mind if you push your patch in first. I'll just >>>> end up deleting that code again when I rebase it. >> I'll let your patches go in. > > Hi Pedro, > Since this bug is still in FSF trunk, most of the tests in > gdb.threads/non-stop-fair-events.exp are timeout on arm-linux, > and takes much more time to run the whole testsuite. > > Can I push patch 1/2 in to fix this problem? assuming > your V3 "all stop on top of non-stop" series can't go in in a > a couple of days. Sure, go ahead. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/2] Honour software single step in fallback of displaced stepping 2015-04-16 12:02 ` Pedro Alves @ 2015-04-16 13:01 ` Yao Qi 0 siblings, 0 replies; 9+ messages in thread From: Yao Qi @ 2015-04-16 13:01 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches On 16/04/15 13:02, Pedro Alves wrote: > Sure, go ahead. Thanks, Pedro. Patch is pushed in. -- Yao (é½å°§) ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2015-04-16 13:01 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-04-07 15:52 [PATCH 0/2] Honour software single step in fallback of displaced stepping Yao Qi 2015-04-07 15:52 ` [PATCH 1/2] [gdbserver] assert on step if !can_hardware_single_step Yao Qi 2015-04-07 17:09 ` Pedro Alves 2015-04-07 15:52 ` [PATCH 2/2] Honour software single step in fallback of displaced stepping Yao Qi 2015-04-07 16:59 ` [PATCH 0/2] " Pedro Alves 2015-04-08 9:51 ` Yao Qi 2015-04-16 12:00 ` Yao Qi 2015-04-16 12:02 ` Pedro Alves 2015-04-16 13:01 ` Yao Qi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).