* [PATCH 0/2] Fix gdb.threads/multiple-step-overs.exp fails on arm @ 2015-10-29 17:10 Yao Qi 2015-10-29 17:10 ` [PATCH 1/2] New function displaced_step_in_progress_thread Yao Qi 2015-10-29 17:10 ` [PATCH 2/2] Fix gdb.threads/multiple-step-overs.exp fails on arm Yao Qi 0 siblings, 2 replies; 8+ messages in thread From: Yao Qi @ 2015-10-29 17:10 UTC (permalink / raw) To: gdb-patches Hi, This patch series fixes fails in gdb.threads/multiple-step-overs.exp on arm-linux. Patch 1 adds a new function, and patch 2 is the main part. See details in patch 2. Whole series is regression tested on x86_64-linux and arm-linux. *** BLURB HERE *** Yao Qi (2): New function displaced_step_in_progress_thread Fix gdb.threads/multiple-step-overs.exp fails on arm gdb/infrun.c | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) -- 1.9.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] New function displaced_step_in_progress_thread 2015-10-29 17:10 [PATCH 0/2] Fix gdb.threads/multiple-step-overs.exp fails on arm Yao Qi @ 2015-10-29 17:10 ` Yao Qi 2015-11-04 17:20 ` Pedro Alves 2015-10-29 17:10 ` [PATCH 2/2] Fix gdb.threads/multiple-step-overs.exp fails on arm Yao Qi 1 sibling, 1 reply; 8+ messages in thread From: Yao Qi @ 2015-10-29 17:10 UTC (permalink / raw) To: gdb-patches This patch adds a new function displaced_step_in_progress_thread, which returns whether the thread is in progress of displaced stepping. gdb: 2015-10-28 Yao Qi <yao.qi@linaro.org> * infrun.c (displaced_step_in_progress_thread): New function. (handle_inferior_event_1): Call it. --- gdb/infrun.c | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/gdb/infrun.c b/gdb/infrun.c index 917f9be..0265d35 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -1536,6 +1536,21 @@ displaced_step_in_progress_any_inferior (void) return 0; } +/* Return true if thread represented by PTID is doing a displaced + step. */ + +static int +displaced_step_in_progress_thread (ptid_t ptid) +{ + struct displaced_step_inferior_state *displaced; + + gdb_assert (!ptid_equal (ptid, null_ptid)); + + displaced = get_displaced_stepping_state (ptid_get_pid (ptid)); + + return (displaced != NULL && ptid_equal (displaced->step_ptid, ptid)); +} + /* Return true if process PID has a thread doing a displaced step. */ static int @@ -4923,12 +4938,10 @@ Cannot fill $_exitsignal with the correct signal number.\n")); { struct regcache *regcache = get_thread_regcache (ecs->ptid); struct gdbarch *gdbarch = get_regcache_arch (regcache); - struct displaced_step_inferior_state *displaced - = get_displaced_stepping_state (ptid_get_pid (ecs->ptid)); /* If checking displaced stepping is supported, and thread ecs->ptid is displaced stepping. */ - if (displaced && ptid_equal (displaced->step_ptid, ecs->ptid)) + if (displaced_step_in_progress_thread (ecs->ptid)) { struct inferior *parent_inf = find_inferior_ptid (ecs->ptid); @@ -4947,6 +4960,9 @@ Cannot fill $_exitsignal with the correct signal number.\n")); if (ecs->ws.kind == TARGET_WAITKIND_FORKED) { + struct displaced_step_inferior_state *displaced + = get_displaced_stepping_state (ptid_get_pid (ecs->ptid)); + /* Restore scratch pad for child process. */ displaced_step_restore (displaced, ecs->ws.value.related_pid); } -- 1.9.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] New function displaced_step_in_progress_thread 2015-10-29 17:10 ` [PATCH 1/2] New function displaced_step_in_progress_thread Yao Qi @ 2015-11-04 17:20 ` Pedro Alves 0 siblings, 0 replies; 8+ messages in thread From: Pedro Alves @ 2015-11-04 17:20 UTC (permalink / raw) To: Yao Qi, gdb-patches On 10/29/2015 03:01 PM, Yao Qi wrote: > This patch adds a new function displaced_step_in_progress_thread, > which returns whether the thread is in progress of displaced > stepping. > > gdb: > > 2015-10-28 Yao Qi <yao.qi@linaro.org> > > * infrun.c (displaced_step_in_progress_thread): New function. > (handle_inferior_event_1): Call it. LGTM. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] Fix gdb.threads/multiple-step-overs.exp fails on arm 2015-10-29 17:10 [PATCH 0/2] Fix gdb.threads/multiple-step-overs.exp fails on arm Yao Qi 2015-10-29 17:10 ` [PATCH 1/2] New function displaced_step_in_progress_thread Yao Qi @ 2015-10-29 17:10 ` Yao Qi 2015-11-04 17:20 ` Pedro Alves 1 sibling, 1 reply; 8+ messages in thread From: Yao Qi @ 2015-10-29 17:10 UTC (permalink / raw) To: gdb-patches Hi, Some tests in gdb.threads/multiple-step-overs.exp fail on arm target when the displaced stepping on, but they pass when displaced stepping is off. FAIL: gdb.threads/multiple-step-overs.exp: displaced=on: step: step FAIL: gdb.threads/multiple-step-overs.exp: displaced=on: next: next FAIL: gdb.threads/multiple-step-overs.exp: displaced=on: continue: continue FAIL: gdb.threads/multiple-step-overs.exp: displaced=on: signal thr1: continue to sigusr1_handler when displaced stepping is on, Sending packet: $vCont;c#a8...infrun: infrun_async(1)^M <--- [1] infrun: prepare_to_wait^M infrun: target_wait (-1.0.0, status) =^M infrun: -1.0.0 [Thread 0],^M infrun: status->kind = ignore^M infrun: TARGET_WAITKIND_IGNORE^M infrun: prepare_to_wait^M Packet received: T05swbreak:;0b:f8faffbe;0d:409ee7b6;0f:d0880000;thread:p635.636;core:0;^M infrun: target_wait (-1.0.0, status) =^M infrun: 1589.1590.0 [Thread 1590],^M infrun: status->kind = stopped, signal = GDB_SIGNAL_TRAP^M infrun: TARGET_WAITKIND_STOPPED^M infrun: stop_pc = 0x88d0^M infrun: context switch^M infrun: Switching context from Thread 1591 to Thread 1590^ GDB resumes the whole process (all threads) rather than the specific thread it wants to single step (as shown in [1]). That is wrong. when displaced stepping is off, GDB behaves correctly, only resumes the specific thread (as shown in [2]). Sending packet: $vCont;c:p611.613#b2...infrun: infrun_async(1)^M <-- [2] infrun: prepare_to_wait^M infrun: target_wait (-1.0.0, status) =^M infrun: -1.0.0 [Thread 0],^M infrun: status->kind = ignore^M infrun: TARGET_WAITKIND_IGNORE^M infrun: prepare_to_wait^M Packet received: T05swbreak:;0b:f8faffbe;0d:409e67b6;0f:48880000;thread:p611.613;core:1;^M infrun: target_wait (-1.0.0, status) =^M infrun: 1553.1555.0 [Thread 1555],^M infrun: status->kind = stopped, signal = GDB_SIGNAL_TRAP^M infrun: TARGET_WAITKIND_STOPPED^M infrun: clear_step_over_info^M infrun: stop_pc = 0x8848 The current logic in GDB on deciding the set of threads to resume is: /* Decide the set of threads to ask the target to resume. */ if ((step || thread_has_single_step_breakpoints_set (tp)) && tp->control.trap_expected) { /* We're allowing a thread to run past a breakpoint it has hit, by single-stepping the thread with the breakpoint removed. In which case, we need to single-step only this thread, and keep others stopped, as they can miss this breakpoint if allowed to run. */ resume_ptid = inferior_ptid; } else resume_ptid = internal_resume_ptid (user_step); it doesn't handle the case correctly that GDB continue (instead of single step) the thread for displaced stepping. I also update the comment below to reflect the code. I remove the "with the breakpoint removed" comment, because GDB doesn't remove breakpoints in displaced stepping, so we don't have to worry that other threads may miss the breakpoint. Patch is regression tested on both x86_64-linux and arm-linux. gdb: 2015-10-28 Yao Qi <yao.qi@linaro.org> * infrun.c (resume): Check displaced_step_in_progress_thread when deciding the set of threads to resume. --- gdb/infrun.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/gdb/infrun.c b/gdb/infrun.c index 0265d35..c619b61 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -2631,14 +2631,12 @@ resume (enum gdb_signal sig) gdb_assert (!(thread_has_single_step_breakpoints_set (tp) && step)); /* Decide the set of threads to ask the target to resume. */ - if ((step || thread_has_single_step_breakpoints_set (tp)) + if ((step || thread_has_single_step_breakpoints_set (tp) + || displaced_step_in_progress_thread (tp->ptid)) && tp->control.trap_expected) { /* We're allowing a thread to run past a breakpoint it has - hit, by single-stepping the thread with the breakpoint - removed. In which case, we need to single-step only this - thread, and keep others stopped, as they can miss this - breakpoint if allowed to run. */ + hit, by single-stepping (in-line or out-of-line) the thread. */ resume_ptid = inferior_ptid; } else -- 1.9.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] Fix gdb.threads/multiple-step-overs.exp fails on arm 2015-10-29 17:10 ` [PATCH 2/2] Fix gdb.threads/multiple-step-overs.exp fails on arm Yao Qi @ 2015-11-04 17:20 ` Pedro Alves 2015-11-09 11:24 ` Yao Qi 0 siblings, 1 reply; 8+ messages in thread From: Pedro Alves @ 2015-11-04 17:20 UTC (permalink / raw) To: Yao Qi, gdb-patches On 10/29/2015 03:01 PM, Yao Qi wrote: > Hi, > Some tests in gdb.threads/multiple-step-overs.exp fail on arm target > when the displaced stepping on, but they pass when displaced stepping > is off. > > FAIL: gdb.threads/multiple-step-overs.exp: displaced=on: step: step > FAIL: gdb.threads/multiple-step-overs.exp: displaced=on: next: next > FAIL: gdb.threads/multiple-step-overs.exp: displaced=on: continue: continue > FAIL: gdb.threads/multiple-step-overs.exp: displaced=on: signal thr1: continue to sigusr1_handler > > when displaced stepping is on, > > Sending packet: $vCont;c#a8...infrun: infrun_async(1)^M <--- [1] > infrun: prepare_to_wait^M > infrun: target_wait (-1.0.0, status) =^M > infrun: -1.0.0 [Thread 0],^M > infrun: status->kind = ignore^M > infrun: TARGET_WAITKIND_IGNORE^M > infrun: prepare_to_wait^M > Packet received: T05swbreak:;0b:f8faffbe;0d:409ee7b6;0f:d0880000;thread:p635.636;core:0;^M > infrun: target_wait (-1.0.0, status) =^M > infrun: 1589.1590.0 [Thread 1590],^M > infrun: status->kind = stopped, signal = GDB_SIGNAL_TRAP^M > infrun: TARGET_WAITKIND_STOPPED^M > infrun: stop_pc = 0x88d0^M > infrun: context switch^M > infrun: Switching context from Thread 1591 to Thread 1590^ > > GDB resumes the whole process (all threads) rather than the specific > thread it wants to single step (as shown in [1]). That is wrong. (I understand this, but I think it'd make it clearer to explicitly state _why_ that is wrong.) > > when displaced stepping is off, GDB behaves correctly, only resumes > the specific thread (as shown in [2]). > > Sending packet: $vCont;c:p611.613#b2...infrun: infrun_async(1)^M <-- [2] > infrun: prepare_to_wait^M > infrun: target_wait (-1.0.0, status) =^M > infrun: -1.0.0 [Thread 0],^M > infrun: status->kind = ignore^M > infrun: TARGET_WAITKIND_IGNORE^M > infrun: prepare_to_wait^M > Packet received: T05swbreak:;0b:f8faffbe;0d:409e67b6;0f:48880000;thread:p611.613;core:1;^M > infrun: target_wait (-1.0.0, status) =^M > infrun: 1553.1555.0 [Thread 1555],^M > infrun: status->kind = stopped, signal = GDB_SIGNAL_TRAP^M > infrun: TARGET_WAITKIND_STOPPED^M > infrun: clear_step_over_info^M > infrun: stop_pc = 0x8848 > > The current logic in GDB on deciding the set of threads to resume is: > > /* Decide the set of threads to ask the target to resume. */ > if ((step || thread_has_single_step_breakpoints_set (tp)) > && tp->control.trap_expected) > { > /* We're allowing a thread to run past a breakpoint it has > hit, by single-stepping the thread with the breakpoint > removed. In which case, we need to single-step only this > thread, and keep others stopped, as they can miss this > breakpoint if allowed to run. */ > resume_ptid = inferior_ptid; > } > else > resume_ptid = internal_resume_ptid (user_step); > > it doesn't handle the case correctly that GDB continue (instead of > single step) the thread for displaced stepping. > > I also update the comment below to reflect the code. I remove the > "with the breakpoint removed" comment, because GDB doesn't remove > breakpoints in displaced stepping, so we don't have to worry that > other threads may miss the breakpoint. > > Patch is regression tested on both x86_64-linux and arm-linux. > > gdb: > > 2015-10-28 Yao Qi <yao.qi@linaro.org> > > * infrun.c (resume): Check displaced_step_in_progress_thread > when deciding the set of threads to resume. > --- > gdb/infrun.c | 8 +++----- > 1 file changed, 3 insertions(+), 5 deletions(-) > > diff --git a/gdb/infrun.c b/gdb/infrun.c > index 0265d35..c619b61 100644 > --- a/gdb/infrun.c > +++ b/gdb/infrun.c > @@ -2631,14 +2631,12 @@ resume (enum gdb_signal sig) > gdb_assert (!(thread_has_single_step_breakpoints_set (tp) && step)); > > /* Decide the set of threads to ask the target to resume. */ > - if ((step || thread_has_single_step_breakpoints_set (tp)) > + if ((step || thread_has_single_step_breakpoints_set (tp) > + || displaced_step_in_progress_thread (tp->ptid)) > && tp->control.trap_expected) I wonder, can't we just remove the "step" check, like: if (tp->control.trap_expected) { > { > /* We're allowing a thread to run past a breakpoint it has > - hit, by single-stepping the thread with the breakpoint > - removed. In which case, we need to single-step only this > - thread, and keep others stopped, as they can miss this > - breakpoint if allowed to run. */ > + hit, by single-stepping (in-line or out-of-line) the thread. */ The change looks good to me, though I think we should clarify the comment here. How about: /* We're allowing a thread to run past a breakpoint it has hit, either by single-stepping the thread with the breakpoint removed, or by displaced stepping, with the breakpoint inserted. In the former case, we need to single-step only this thread, and keep others stopped, as they can miss this breakpoint if allowed to run. That's not really a problem for displaced stepping, but, we still keep other threads stopped, in case another thread is also stopped for a breakpoint waiting for its turn in the displaced stepping queue. */ I think we could optimize this by checking for thread_step_over_chain_next (tp) == NULL, because if no other thread is waiting for a step-over, then we could resume all threads, but that's maybe not worth it. > resume_ptid = inferior_ptid; > } > else Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] Fix gdb.threads/multiple-step-overs.exp fails on arm 2015-11-04 17:20 ` Pedro Alves @ 2015-11-09 11:24 ` Yao Qi 2015-11-17 12:33 ` Pedro Alves 0 siblings, 1 reply; 8+ messages in thread From: Yao Qi @ 2015-11-09 11:24 UTC (permalink / raw) To: Pedro Alves; +Cc: Yao Qi, gdb-patches Pedro Alves <palves@redhat.com> writes: Hi Pedro, >> GDB resumes the whole process (all threads) rather than the specific >> thread it wants to single step (as shown in [1]). That is wrong. > > (I understand this, but I think it'd make it clearer to explicitly > state _why_ that is wrong.) I am not sure how to make the description clearer, how about this? GDB resumes the whole process (all threads) rather than the specific thread for which GDB wants to step over the breakpoint (as shown in [1]). That is wrong. >> diff --git a/gdb/infrun.c b/gdb/infrun.c >> index 0265d35..c619b61 100644 >> --- a/gdb/infrun.c >> +++ b/gdb/infrun.c >> @@ -2631,14 +2631,12 @@ resume (enum gdb_signal sig) >> gdb_assert (!(thread_has_single_step_breakpoints_set (tp) && step)); >> >> /* Decide the set of threads to ask the target to resume. */ >> - if ((step || thread_has_single_step_breakpoints_set (tp)) >> + if ((step || thread_has_single_step_breakpoints_set (tp) >> + || displaced_step_in_progress_thread (tp->ptid)) >> && tp->control.trap_expected) > > > I wonder, can't we just remove the "step" check, like: > > if (tp->control.trap_expected) > { > Yes, it works! as the patch below, > > >> { >> /* We're allowing a thread to run past a breakpoint it has >> - hit, by single-stepping the thread with the breakpoint >> - removed. In which case, we need to single-step only this >> - thread, and keep others stopped, as they can miss this >> - breakpoint if allowed to run. */ >> + hit, by single-stepping (in-line or out-of-line) the thread. */ > > The change looks good to me, though I think we should clarify > the comment here. How about: > > /* We're allowing a thread to run past a breakpoint it has > hit, either by single-stepping the thread with the breakpoint > removed, or by displaced stepping, with the breakpoint inserted. > In the former case, we need to single-step only this thread, and keep > others stopped, as they can miss this breakpoint if allowed to run. > That's not really a problem for displaced stepping, but, we still keep > other threads stopped, in case another thread is also stopped for a > breakpoint waiting for its turn in the displaced stepping queue. */ > They are copied into the patch. > I think we could optimize this by checking for > thread_step_over_chain_next (tp) == NULL, because if no other thread > is waiting for a step-over, then we could resume all threads, but > that's maybe not worth it. 'thread_step_over_chain_next (tp) == NULL' doesn't work on arm-linux, however, I didn't investigate why it doesn't. Patch below is regression tested on x86_64-linux and arm-linux. -- Yao (齐尧) From 9f1915b1d183b14286b1e785dec84d5c95f8ff5c Mon Sep 17 00:00:00 2001 From: Yao Qi <yao.qi@linaro.org> Date: Wed, 28 Oct 2015 11:58:34 +0000 Subject: [PATCH] Fix gdb.threads/multiple-step-overs.exp fails on arm Hi, Some tests in gdb.threads/multiple-step-overs.exp fail on arm target when the displaced stepping on, but they pass when displaced stepping is off. FAIL: gdb.threads/multiple-step-overs.exp: displaced=on: step: step FAIL: gdb.threads/multiple-step-overs.exp: displaced=on: next: next FAIL: gdb.threads/multiple-step-overs.exp: displaced=on: continue: continue FAIL: gdb.threads/multiple-step-overs.exp: displaced=on: signal thr1: continue to sigusr1_handler when displaced stepping is on, Sending packet: $vCont;c#a8...infrun: infrun_async(1)^M <--- [1] infrun: prepare_to_wait^M infrun: target_wait (-1.0.0, status) =^M infrun: -1.0.0 [Thread 0],^M infrun: status->kind = ignore^M infrun: TARGET_WAITKIND_IGNORE^M infrun: prepare_to_wait^M Packet received: T05swbreak:;0b:f8faffbe;0d:409ee7b6;0f:d0880000;thread:p635.636;core:0;^M infrun: target_wait (-1.0.0, status) =^M infrun: 1589.1590.0 [Thread 1590],^M infrun: status->kind = stopped, signal = GDB_SIGNAL_TRAP^M infrun: TARGET_WAITKIND_STOPPED^M infrun: stop_pc = 0x88d0^M infrun: context switch^M infrun: Switching context from Thread 1591 to Thread 1590^ GDB resumes the whole process (all threads) rather than the specific thread for which GDB wants to step over the breakpoint (as shown in [1]). That is wrong. when displaced stepping is off, GDB behaves correctly, only resumes the specific thread (as shown in [2]). Sending packet: $vCont;c:p611.613#b2...infrun: infrun_async(1)^M <-- [2] infrun: prepare_to_wait^M infrun: target_wait (-1.0.0, status) =^M infrun: -1.0.0 [Thread 0],^M infrun: status->kind = ignore^M infrun: TARGET_WAITKIND_IGNORE^M infrun: prepare_to_wait^M Packet received: T05swbreak:;0b:f8faffbe;0d:409e67b6;0f:48880000;thread:p611.613;core:1;^M infrun: target_wait (-1.0.0, status) =^M infrun: 1553.1555.0 [Thread 1555],^M infrun: status->kind = stopped, signal = GDB_SIGNAL_TRAP^M infrun: TARGET_WAITKIND_STOPPED^M infrun: clear_step_over_info^M infrun: stop_pc = 0x8848 The current logic in GDB on deciding the set of threads to resume is: /* Decide the set of threads to ask the target to resume. */ if ((step || thread_has_single_step_breakpoints_set (tp)) && tp->control.trap_expected) { /* We're allowing a thread to run past a breakpoint it has hit, by single-stepping the thread with the breakpoint removed. In which case, we need to single-step only this thread, and keep others stopped, as they can miss this breakpoint if allowed to run. */ resume_ptid = inferior_ptid; } else resume_ptid = internal_resume_ptid (user_step); it doesn't handle the case correctly that GDB continue (instead of single step) the thread for displaced stepping. I also update the comment below to reflect the code. I remove the "with the breakpoint removed" comment, because GDB doesn't remove breakpoints in displaced stepping, so we don't have to worry that other threads may miss the breakpoint. Patch is regression tested on both x86_64-linux and arm-linux. gdb: 2015-11-09 Yao Qi <yao.qi@linaro.org> * infrun.c (resume): Check control.trap_expected only when deciding the set of threads to resume. diff --git a/gdb/infrun.c b/gdb/infrun.c index 185b79b..4a66d17 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -2654,14 +2654,17 @@ resume (enum gdb_signal sig) gdb_assert (!(thread_has_single_step_breakpoints_set (tp) && step)); /* Decide the set of threads to ask the target to resume. */ - if ((step || thread_has_single_step_breakpoints_set (tp)) - && tp->control.trap_expected) + if (tp->control.trap_expected) { /* We're allowing a thread to run past a breakpoint it has - hit, by single-stepping the thread with the breakpoint - removed. In which case, we need to single-step only this - thread, and keep others stopped, as they can miss this - breakpoint if allowed to run. */ + hit, either by single-stepping the thread with the breakpoint + removed, or by displaced stepping, with the breakpoint inserted. + In the former case, we need to single-step only this thread, + and keep others stopped, as they can miss this breakpoint if + allowed to run. That's not really a problem for displaced + stepping, but, we still keep other threads stopped, in case + another thread is also stopped for a breakpoint waiting for + its turn in the displaced stepping queue. */ resume_ptid = inferior_ptid; } else ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] Fix gdb.threads/multiple-step-overs.exp fails on arm 2015-11-09 11:24 ` Yao Qi @ 2015-11-17 12:33 ` Pedro Alves 2015-11-17 15:41 ` Yao Qi 0 siblings, 1 reply; 8+ messages in thread From: Pedro Alves @ 2015-11-17 12:33 UTC (permalink / raw) To: Yao Qi; +Cc: gdb-patches On 11/09/2015 11:23 AM, Yao Qi wrote: > Pedro Alves <palves@redhat.com> writes: > > Hi Pedro, > >>> GDB resumes the whole process (all threads) rather than the specific >>> thread it wants to single step (as shown in [1]). That is wrong. >> >> (I understand this, but I think it'd make it clearer to explicitly >> state _why_ that is wrong.) > > I am not sure how to make the description clearer, how about this? > > GDB resumes the whole process (all threads) rather than the specific > thread for which GDB wants to step over the breakpoint (as shown in [1]). > That is wrong. I think the _why_ is that wrong is still missing. I was thinking of something around this: The reason we resume a single thread and leave others stopped when doing a normal step over where we temporarily remove the breakpoint, single-step, reinsert the breakpoint, is that if we let other threads run in the period while the breakpoint is removed, then these other threads could miss the breakpoint. Since with displaced stepping, we don't ever remove the breakpoint, it should be fine to let other threads run. However, there's another reason that we should not let other threads run: that is the case where some of those threads are also stopped for a breakpoint that itself needs to be stepped over. If we just let those threads run, then they immediately re-trap their breakpoint again. > > Patch below is regression tested on x86_64-linux and arm-linux. > LGTM. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] Fix gdb.threads/multiple-step-overs.exp fails on arm 2015-11-17 12:33 ` Pedro Alves @ 2015-11-17 15:41 ` Yao Qi 0 siblings, 0 replies; 8+ messages in thread From: Yao Qi @ 2015-11-17 15:41 UTC (permalink / raw) To: Pedro Alves; +Cc: Yao Qi, gdb-patches Pedro Alves <palves@redhat.com> writes: > The reason we resume a single thread and leave others stopped when doing a > normal step over where we temporarily remove the breakpoint, single-step, > reinsert the breakpoint, is that if we let other threads run in the period > while the breakpoint is removed, then these other threads could miss > the breakpoint. Since with displaced stepping, we don't ever remove the > breakpoint, it should be fine to let other threads run. However, > there's another > reason that we should not let other threads run: that is the case where some of > those threads are also stopped for a breakpoint that itself needs to be > stepped over. If we just let those threads run, then they immediately re-trap > their breakpoint again. > OK, copy them into the commit log. >> >> Patch below is regression tested on x86_64-linux and arm-linux. >> > > LGTM. Patch is pushed in. -- Yao (齐尧) ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2015-11-17 15:41 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-10-29 17:10 [PATCH 0/2] Fix gdb.threads/multiple-step-overs.exp fails on arm Yao Qi 2015-10-29 17:10 ` [PATCH 1/2] New function displaced_step_in_progress_thread Yao Qi 2015-11-04 17:20 ` Pedro Alves 2015-10-29 17:10 ` [PATCH 2/2] Fix gdb.threads/multiple-step-overs.exp fails on arm Yao Qi 2015-11-04 17:20 ` Pedro Alves 2015-11-09 11:24 ` Yao Qi 2015-11-17 12:33 ` Pedro Alves 2015-11-17 15:41 ` 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).