[binutils-gdb] Fix gdb.threads/multiple-step-overs.exp fails on arm

[Yao Qi <qiyao at sourceware dot org>]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=1946c4ccca04e374acc040fc30c8b44d2c9ca0a8

commit 1946c4ccca04e374acc040fc30c8b44d2c9ca0a8
Author: Yao Qi <yao.qi@linaro.org>
Date:   Tue Nov 17 15:40:29 2015 +0000

    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 because 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.
    
    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-17  Yao Qi  <yao.qi@linaro.org>
    
    	* infrun.c (resume): Check control.trap_expected only
    	when deciding the set of threads to resume.

Diff:
---
 gdb/ChangeLog |  5 +++++
 gdb/infrun.c  | 15 +++++++++------
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 1dc2ab0..1f7aed0 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2015-11-17  Yao Qi  <yao.qi@linaro.org>
+
+	* infrun.c (resume): Check control.trap_expected only
+	when deciding the set of threads to resume.
+
 2015-11-17  Pedro Alves  <palves@redhat.com>
 
 	* cp-namespace.c (cp_lookup_bare_symbol)
diff --git a/gdb/infrun.c b/gdb/infrun.c
index d1b0e12..b14f34c 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -2656,14 +2656,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