public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 10/17] Factor out code to re-resume stepped thread
  2015-04-01 22:14 [PATCH 00/17] All-stop on top of non-stop Pedro Alves
@ 2015-04-01 22:14 ` Pedro Alves
  2015-04-01 22:14 ` [PATCH 13/17] Fix signal-while-stepping-over-bp-other-thread.exp on targets always in non-stop Pedro Alves
                   ` (16 subsequent siblings)
  17 siblings, 0 replies; 26+ messages in thread
From: Pedro Alves @ 2015-04-01 22:14 UTC (permalink / raw)
  To: gdb-patches

Just a code refactor, no funcionality change intended.

gdb/ChangeLog:
2015-04-01  Pedro Alves  <palves@redhat.com>

	* infrun.c (keep_going_stepped_thread): New function, factored out
	from ...
	(switch_back_to_stepped_thread): ... here.
---
 gdb/infrun.c | 198 ++++++++++++++++++++++++++++++++---------------------------
 1 file changed, 106 insertions(+), 92 deletions(-)

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 9c8d253..9fd4001 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -1873,6 +1873,7 @@ reset_ecs (struct execution_control_state *ecs, struct thread_info *tp)
 
 static void keep_going_pass (struct execution_control_state *ecs);
 static void prepare_to_wait (struct execution_control_state *ecs);
+static int keep_going_stepped_thread (struct thread_info *tp);
 static int thread_still_needs_step_over (struct thread_info *tp);
 
 /* Are there any pending step-over requests for INF?  If so, run one
@@ -5711,110 +5712,123 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs)
 
       if (stepping_thread != NULL)
 	{
-	  struct frame_info *frame;
-	  struct gdbarch *gdbarch;
-
-	  tp = stepping_thread;
-
-	  /* If the stepping thread exited, then don't try to switch
-	     back and resume it, which could fail in several different
-	     ways depending on the target.  Instead, just keep going.
-
-	     We can find a stepping dead thread in the thread list in
-	     two cases:
-
-	     - The target supports thread exit events, and when the
-	     target tries to delete the thread from the thread list,
-	     inferior_ptid pointed at the exiting thread.  In such
-	     case, calling delete_thread does not really remove the
-	     thread from the list; instead, the thread is left listed,
-	     with 'exited' state.
-
-	     - The target's debug interface does not support thread
-	     exit events, and so we have no idea whatsoever if the
-	     previously stepping thread is still alive.  For that
-	     reason, we need to synchronously query the target
-	     now.  */
-	  if (is_exited (tp->ptid)
-	      || !target_thread_alive (tp->ptid))
-	    {
-	      if (debug_infrun)
-		fprintf_unfiltered (gdb_stdlog,
-				    "infrun: not switching back to "
-				    "stepped thread, it has vanished\n");
-
-	      delete_thread (tp->ptid);
-	      return 0;
-	    }
-
 	  if (debug_infrun)
 	    fprintf_unfiltered (gdb_stdlog,
 				"infrun: switching back to stepped thread\n");
 
-	  reset_ecs (ecs, tp);
-	  switch_to_thread (tp->ptid);
+	  if (keep_going_stepped_thread (stepping_thread))
+	    {
+	      prepare_to_wait (ecs);
+	      return 1;
+	    }
+	}
+    }
 
-	  stop_pc = regcache_read_pc (get_thread_regcache (tp->ptid));
-	  frame = get_current_frame ();
-	  gdbarch = get_frame_arch (frame);
+  return 0;
+}
 
-	  /* If the PC of the thread we were trying to single-step has
-	     changed, then that thread has trapped or been signaled,
-	     but the event has not been reported to GDB yet.  Re-poll
-	     the target looking for this particular thread's event
-	     (i.e. temporarily enable schedlock) by:
+/* Set a previously stepped thread back to stepping.  Returns true on
+   success, false if the resume is not possible (e.g., the thread
+   vanished).  */
+
+static int
+keep_going_stepped_thread (struct thread_info *tp)
+{
+  struct frame_info *frame;
+  struct gdbarch *gdbarch;
+  struct execution_control_state ecss;
+  struct execution_control_state *ecs = &ecss;
 
-	       - setting a break at the current PC
-	       - resuming that particular thread, only (by setting
-		 trap expected)
+  /* If the stepping thread exited, then don't try to switch back and
+     resume it, which could fail in several different ways depending
+     on the target.  Instead, just keep going.
 
-	     This prevents us continuously moving the single-step
-	     breakpoint forward, one instruction at a time,
-	     overstepping.  */
+     We can find a stepping dead thread in the thread list in two
+     cases:
 
-	  if (stop_pc != tp->prev_pc)
-	    {
-	      ptid_t resume_ptid;
+     - The target supports thread exit events, and when the target
+       tries to delete the thread from the thread list, inferior_ptid
+       pointed at the exiting thread.  In such case, calling
+       delete_thread does not really remove the thread from the list;
+       instead, the thread is left listed, with 'exited' state.
 
-	      if (debug_infrun)
-		fprintf_unfiltered (gdb_stdlog,
-				    "infrun: expected thread advanced also "
-				    "(%s -> %s)\n",
-				    paddress (target_gdbarch (), tp->prev_pc),
-				    paddress (target_gdbarch (), stop_pc));
-
-	      /* Clear the info of the previous step-over, as it's no
-		 longer valid (if the thread was trying to step over a
-		 breakpoint, it has already succeeded).  It's what
-		 keep_going would do too, if we called it.  Do this
-		 before trying to insert the sss breakpoint, otherwise
-		 if we were previously trying to step over this exact
-		 address in another thread, the breakpoint is
-		 skipped.  */
-	      clear_step_over_info ();
-	      tp->control.trap_expected = 0;
-
-	      insert_single_step_breakpoint (get_frame_arch (frame),
-					     get_frame_address_space (frame),
-					     stop_pc);
-
-	      resume_ptid = user_visible_resume_ptid (tp->control.stepping_command);
-	      do_target_resume (resume_ptid, 0, GDB_SIGNAL_0);
-	      prepare_to_wait (ecs);
-	    }
-	  else
-	    {
-	      if (debug_infrun)
-		fprintf_unfiltered (gdb_stdlog,
-				    "infrun: expected thread still "
-				    "hasn't advanced\n");
-	      keep_going_pass (ecs);
-	    }
+       - The target's debug interface does not support thread exit
+         events, and so we have no idea whatsoever if the previously
+         stepping thread is still alive.  For that reason, we need to
+         synchronously query the target now.  */
 
-	  return 1;
-	}
+  if (is_exited (tp->ptid)
+      || !target_thread_alive (tp->ptid))
+    {
+      if (debug_infrun)
+	fprintf_unfiltered (gdb_stdlog,
+			    "infrun: not resuming previously  "
+			    "stepped thread, it has vanished\n");
+
+      delete_thread (tp->ptid);
+      return 0;
     }
-  return 0;
+
+  if (debug_infrun)
+    fprintf_unfiltered (gdb_stdlog,
+			"infrun: resuming previously stepped thread\n");
+
+  reset_ecs (ecs, tp);
+  switch_to_thread (tp->ptid);
+
+  stop_pc = regcache_read_pc (get_thread_regcache (tp->ptid));
+  frame = get_current_frame ();
+  gdbarch = get_frame_arch (frame);
+
+  /* If the PC of the thread we were trying to single-step has
+     changed, then that thread has trapped or been signaled, but the
+     event has not been reported to GDB yet.  Re-poll the target
+     looking for this particular thread's event (i.e. temporarily
+     enable schedlock) by:
+
+     - setting a break at the current PC
+     - resuming that particular thread, only (by setting trap
+     expected)
+
+     This prevents us continuously moving the single-step breakpoint
+     forward, one instruction at a time, overstepping.  */
+
+  if (stop_pc != tp->prev_pc)
+    {
+      ptid_t resume_ptid;
+
+      if (debug_infrun)
+	fprintf_unfiltered (gdb_stdlog,
+			    "infrun: expected thread advanced also (%s -> %s)\n",
+			    paddress (target_gdbarch (), tp->prev_pc),
+			    paddress (target_gdbarch (), stop_pc));
+
+      /* Clear the info of the previous step-over, as it's no longer
+	 valid (if the thread was trying to step over a breakpoint, it
+	 has already succeeded).  It's what keep_going would do too,
+	 if we called it.  Do this before trying to insert the sss
+	 breakpoint, otherwise if we were previously trying to step
+	 over this exact address in another thread, the breakpoint is
+	 skipped.  */
+      clear_step_over_info ();
+      tp->control.trap_expected = 0;
+
+      insert_single_step_breakpoint (get_frame_arch (frame),
+				     get_frame_address_space (frame),
+				     stop_pc);
+
+      resume_ptid = user_visible_resume_ptid (tp->control.stepping_command);
+      do_target_resume (resume_ptid, 0, GDB_SIGNAL_0);
+    }
+  else
+    {
+      if (debug_infrun)
+	fprintf_unfiltered (gdb_stdlog,
+			    "infrun: expected thread still hasn't advanced\n");
+
+      keep_going_pass (ecs);
+    }
+  return 1;
 }
 
 /* Is thread TP in the middle of single-stepping?  */
-- 
1.9.3

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 03/17] Displaced stepping debug: fetch the right regcache
  2015-04-01 22:14 [PATCH 00/17] All-stop on top of non-stop Pedro Alves
  2015-04-01 22:14 ` [PATCH 10/17] Factor out code to re-resume stepped thread Pedro Alves
  2015-04-01 22:14 ` [PATCH 13/17] Fix signal-while-stepping-over-bp-other-thread.exp on targets always in non-stop Pedro Alves
@ 2015-04-01 22:14 ` Pedro Alves
  2015-04-07 10:47   ` Pedro Alves
  2015-04-07 13:55   ` Yao Qi
  2015-04-01 22:14 ` [PATCH 17/17] native Linux: enable always non-stop by default Pedro Alves
                   ` (14 subsequent siblings)
  17 siblings, 2 replies; 26+ messages in thread
From: Pedro Alves @ 2015-04-01 22:14 UTC (permalink / raw)
  To: gdb-patches

Although not currently possible in practice when we get here,
'resume_ptid' can also be a wildcard throughout this function.  It's
clearer to fetch the regcache using the thread's ptid.

gdb/ChangeLog:
2015-04-01  Pedro Alves  <pedro@codesourcery.com>

	* infrun.c (resume) <displaced stepping debug output>: Get the
	leader thread's regcache, not resume_ptid's.
---
 gdb/infrun.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/infrun.c b/gdb/infrun.c
index cf0ef32..f23e76e 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -2374,7 +2374,7 @@ resume (enum gdb_signal sig)
       && tp->control.trap_expected
       && use_displaced_stepping_now_p (gdbarch, sig))
     {
-      struct regcache *resume_regcache = get_thread_regcache (resume_ptid);
+      struct regcache *resume_regcache = get_thread_regcache (tp->ptid);
       struct gdbarch *resume_gdbarch = get_regcache_arch (resume_regcache);
       CORE_ADDR actual_pc = regcache_read_pc (resume_regcache);
       gdb_byte buf[4];
-- 
1.9.3

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 02/17] PR13858 - Can't do displaced stepping with no symbols
  2015-04-01 22:14 [PATCH 00/17] All-stop on top of non-stop Pedro Alves
                   ` (5 preceding siblings ...)
  2015-04-01 22:14 ` [PATCH 08/17] Use keep_going in proceed and start_step_over too Pedro Alves
@ 2015-04-01 22:14 ` Pedro Alves
  2015-04-01 22:14 ` [PATCH 04/17] Change adjust_pc_after_break's prototype Pedro Alves
                   ` (10 subsequent siblings)
  17 siblings, 0 replies; 26+ messages in thread
From: Pedro Alves @ 2015-04-01 22:14 UTC (permalink / raw)
  To: gdb-patches

Running break-interp.exp with the target always in non-stop mode trips
on PR13858, as enabling non-stop also enables displaced stepping.

The problem is that when GDB doesn't know where the entry point is, it
doesn't know where to put the displaced stepping scratch pad.  The
test added by this commit exercises this.  Without the fix, we get:

 (gdb) PASS: gdb.base/step-over-no-symbols.exp: displaced=on: break *$pc
 set displaced-stepping on
 (gdb) PASS: gdb.base/step-over-no-symbols.exp: displaced=on: set displaced-stepping on
 stepi
 0x00000000004005be in ?? ()
 Entry point address is not known.
 (gdb) PASS: gdb.base/step-over-no-symbols.exp: displaced=on: stepi
 p /x $pc
 $2 = 0x4005be
 (gdb) PASS: gdb.base/step-over-no-symbols.exp: displaced=on: get after PC
 FAIL: gdb.base/step-over-no-symbols.exp: displaced=on: advanced

The fix is to fall back to stepping over the breakpoint in-line if we
don't know where the entry point address is.

This is enough to fix all-stop + "set displaced on".  For non-stop,
we'll need to teach core gdb to pause all threads to be able to start
the in-line step-over (because then we need to remove the breakpoint
from the target temporarily).

gdb/ChangeLog:
2015-04-01  Pedro Alves  <palves@redhat.com>

	PR gdb/13858
	* infrun.c (use_displaced_stepping): Rename to ...
	(can_use_displaced_stepping_p): ... this.
	(use_displaced_stepping_now_p): New function.
	(resume): Clear trap_expected if waiting for vfork-done.  Use
	use_displaced_stepping_now_p.
	(keep_going): Use use_displaced_stepping_now_p now.

gdb/testsuite/
2015-04-01  Pedro Alves  <palves@redhat.com>

	PR gdb/13858
	* gdb.base/step-over-no-symbols.exp: New file.
---
 gdb/infrun.c                                    | 83 +++++++++++++----------
 gdb/testsuite/gdb.base/step-over-no-symbols.exp | 88 +++++++++++++++++++++++++
 2 files changed, 138 insertions(+), 33 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/step-over-no-symbols.exp

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 01d5a34..cf0ef32 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -1567,7 +1567,7 @@ show_can_use_displaced_stepping (struct ui_file *file, int from_tty,
    over breakpoints.  */
 
 static int
-use_displaced_stepping (struct gdbarch *gdbarch)
+can_use_displaced_stepping_p (struct gdbarch *gdbarch)
 {
   return (((can_use_displaced_stepping == AUTO_BOOLEAN_AUTO && non_stop)
 	   || can_use_displaced_stepping == AUTO_BOOLEAN_TRUE)
@@ -1575,6 +1575,24 @@ use_displaced_stepping (struct gdbarch *gdbarch)
 	  && find_record_target () == NULL);
 }
 
+/* Return non-zero if displaced stepping should be used to step
+   over a breakpoint in the current thread.  */
+
+static int
+use_displaced_stepping_now_p (struct gdbarch *gdbarch,
+			      enum gdb_signal sig)
+{
+  CORE_ADDR retval;
+
+  /* We can't use displaced stepping when we have a signal to deliver;
+     the comments for displaced_step_prepare explain why.  The
+     comments in the handle_inferior event for dealing with 'random
+     signals' explain what we do instead.  */
+  return  (sig == GDB_SIGNAL_0
+	   && can_use_displaced_stepping_p (gdbarch)
+	   && entry_point_address_query (&retval));
+}
+
 /* Clean out any stray displaced stepping state.  */
 static void
 displaced_step_clear (struct displaced_step_inferior_state *displaced)
@@ -2110,6 +2128,10 @@ resume (enum gdb_signal sig)
 	fprintf_unfiltered (gdb_stdlog,
 			    "infrun: resume : clear step\n");
       step = 0;
+
+      /* Likewise, make sure we don't use displaced stepping, which
+	 would poke the scratch buffer in the child.  */
+      tp->control.trap_expected = 0;
     }
 
   if (debug_infrun)
@@ -2205,20 +2227,8 @@ resume (enum gdb_signal sig)
     tp->control.may_range_step = 0;
 
   /* If enabled, step over breakpoints by executing a copy of the
-     instruction at a different address.
-
-     We can't use displaced stepping when we have a signal to deliver;
-     the comments for displaced_step_prepare explain why.  The
-     comments in the handle_inferior event for dealing with 'random
-     signals' explain what we do instead.
-
-     We can't use displaced stepping when we are waiting for vfork_done
-     event, displaced stepping breaks the vfork child similarly as single
-     step software breakpoint.  */
-  if (use_displaced_stepping (gdbarch)
-      && tp->control.trap_expected
-      && sig == GDB_SIGNAL_0
-      && !current_inferior ()->waiting_for_vfork_done)
+     instruction at a different address.  */
+  if (tp->control.trap_expected && use_displaced_stepping_now_p (gdbarch, sig))
     {
       struct displaced_step_inferior_state *displaced;
 
@@ -2361,8 +2371,8 @@ resume (enum gdb_signal sig)
     }
 
   if (debug_displaced
-      && use_displaced_stepping (gdbarch)
-      && tp->control.trap_expected)
+      && tp->control.trap_expected
+      && use_displaced_stepping_now_p (gdbarch, sig))
     {
       struct regcache *resume_regcache = get_thread_regcache (resume_ptid);
       struct gdbarch *resume_gdbarch = get_regcache_arch (resume_regcache);
@@ -2677,7 +2687,9 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal)
      displaced stepping to do so, insert all breakpoints (watchpoints,
      etc.) but the one we're stepping over, step one instruction, and
      then re-insert the breakpoint when that step is finished.  */
-  if (tp->stepping_over_breakpoint && !use_displaced_stepping (gdbarch))
+  if (tp->stepping_over_breakpoint
+      && !use_displaced_stepping_now_p (gdbarch,
+					tp->suspend.stop_signal))
     {
       struct regcache *regcache = get_current_regcache ();
 
@@ -6225,6 +6237,7 @@ keep_going (struct execution_control_state *ecs)
       struct regcache *regcache = get_current_regcache ();
       int remove_bp;
       int remove_wps;
+      enum gdb_signal signo;
 
       /* Either the trap was not expected, but we are continuing
 	 anyway (if we got a signal, the user asked it be passed to
@@ -6250,7 +6263,25 @@ keep_going (struct execution_control_state *ecs)
       remove_wps = (ecs->event_thread->stepping_over_watchpoint
 		    && !target_have_steppable_watchpoint);
 
-      if (remove_bp && !use_displaced_stepping (get_regcache_arch (regcache)))
+      /* Do not deliver GDB_SIGNAL_TRAP (except when the user
+	 explicitly specifies that such a signal should be delivered
+	 to the target program).  Typically, that would occur when a
+	 user is debugging a target monitor on a simulator: the target
+	 monitor sets a breakpoint; the simulator encounters this
+	 breakpoint and halts the simulation handing control to GDB;
+	 GDB, noting that the stop address doesn't map to any known
+	 breakpoint, returns control back to the simulator; the
+	 simulator then delivers the hardware equivalent of a
+	 GDB_SIGNAL_TRAP to the program being debugged.	 */
+      if (ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP
+	  && !signal_program[ecs->event_thread->suspend.stop_signal])
+	ecs->event_thread->suspend.stop_signal = GDB_SIGNAL_0;
+
+      signo = ecs->event_thread->suspend.stop_signal;
+
+      if (remove_bp
+	  && !use_displaced_stepping_now_p (get_regcache_arch (regcache),
+					    signo))
 	{
 	  set_step_over_info (get_regcache_aspace (regcache),
 			      regcache_read_pc (regcache), remove_wps);
@@ -6276,20 +6307,6 @@ keep_going (struct execution_control_state *ecs)
 
       ecs->event_thread->control.trap_expected = (remove_bp || remove_wps);
 
-      /* Do not deliver GDB_SIGNAL_TRAP (except when the user
-	 explicitly specifies that such a signal should be delivered
-	 to the target program).  Typically, that would occur when a
-	 user is debugging a target monitor on a simulator: the target
-	 monitor sets a breakpoint; the simulator encounters this
-	 breakpoint and halts the simulation handing control to GDB;
-	 GDB, noting that the stop address doesn't map to any known
-	 breakpoint, returns control back to the simulator; the
-	 simulator then delivers the hardware equivalent of a
-	 GDB_SIGNAL_TRAP to the program being debugged.	 */
-      if (ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP
-	  && !signal_program[ecs->event_thread->suspend.stop_signal])
-	ecs->event_thread->suspend.stop_signal = GDB_SIGNAL_0;
-
       discard_cleanups (old_cleanups);
       resume (ecs->event_thread->suspend.stop_signal);
     }
diff --git a/gdb/testsuite/gdb.base/step-over-no-symbols.exp b/gdb/testsuite/gdb.base/step-over-no-symbols.exp
new file mode 100644
index 0000000..d7bfd64
--- /dev/null
+++ b/gdb/testsuite/gdb.base/step-over-no-symbols.exp
@@ -0,0 +1,88 @@
+# Copyright (C) 2015 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test that GDB can step past a breakpoint even if GDB doesn't have
+# symbols for the main binary.
+
+standard_testfile start.c
+
+if { ![support_displaced_stepping] } {
+    unsupported "displaced stepping"
+    return -1
+}
+
+if { [build_executable "failed to build" ${testfile} $srcfile] } {
+    return -1
+}
+
+# Get the current PC.  MSG is used as test message.
+
+proc get_pc { msg } {
+    global hex gdb_prompt
+
+    set addr ""
+    gdb_test_multiple "p /x \$pc" "$msg" {
+	-re " = ($hex).*$gdb_prompt $" {
+	    set addr $expect_out(1,string)
+	    pass "$msg"
+	}
+    }
+
+    return $addr
+}
+
+# Test stepping past a breakpoint with no symbols.  DISPLACED is one
+# of the "set displaced-stepping" options.  GDB should be able to fall
+# back to stepping past the breakpoint using an in-line step-over.
+
+proc test_step_over {displaced} {
+    global hex
+    global binfile
+
+    clean_restart $binfile
+
+    if ![runto_main] {
+	fail "couldn't run to main"
+	untested "stepping over breakpoint with displaced=$displaced"
+	return -1
+    }
+
+    delete_breakpoints
+
+    set msg "purging symbols"
+    gdb_test_multiple "symbol-file" "$msg" {
+	-re "Discard symbol table.*y or n. $" {
+	    gdb_test "y" "No symbol file now." "$msg"
+	}
+    }
+
+    set before_addr [get_pc "get before PC"]
+
+    gdb_test "break *\$pc" "Breakpoint .* at $hex"
+
+    gdb_test_no_output "set displaced-stepping $displaced"
+
+    gdb_test "stepi" "$hex in \?\? .*"
+
+    set after_addr [get_pc "get after PC"]
+
+    gdb_assert {$before_addr != $after_addr} "advanced"
+}
+
+foreach displaced { "off" "on" "auto" } {
+    with_test_prefix "displaced=$displaced" {
+	test_step_over $displaced
+    }
+}
-- 
1.9.3

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 08/17] Use keep_going in proceed and start_step_over too
  2015-04-01 22:14 [PATCH 00/17] All-stop on top of non-stop Pedro Alves
                   ` (4 preceding siblings ...)
  2015-04-01 22:14 ` [PATCH 01/17] Fix and test "checkpoint" in non-stop mode Pedro Alves
@ 2015-04-01 22:14 ` Pedro Alves
  2015-04-01 22:14 ` [PATCH 02/17] PR13858 - Can't do displaced stepping with no symbols Pedro Alves
                   ` (11 subsequent siblings)
  17 siblings, 0 replies; 26+ messages in thread
From: Pedro Alves @ 2015-04-01 22:14 UTC (permalink / raw)
  To: gdb-patches

The main motivation of this patch is sharing more code between the
proceed (starting the inferior for the first time) and keep_going
(restarting the inferior after handling an event) paths and using the
step_over_chain queue now embedded in the thread_info object for
in-line step-overs too (instead of just for displaced stepping).

So this commit:

 - splits out a new keep_going_pass function out of keep_going that is
   just like keep_going except for the bits that clear the signal to
   pass if the signal is set to "handle nopass".

 - makes proceed use keep_going too.

 - Makes start_step_over use keep_going_pass instead of lower level
   displaced stepping things.

One user visible change.  If inserting breakpoints while trying to
proceed fails, we now get:

  (gdb) si
  Warning:
  Could not insert hardware watchpoint 7.
  Could not insert hardware breakpoints:
  You may have requested too many hardware breakpoints/watchpoints.

  Command aborted.
  (gdb)

while before we only saw warnings with no indication that the command
was cancelled:

  (gdb) si
  Warning:
  Could not insert hardware watchpoint 7.
  Could not insert hardware breakpoints:
  You may have requested too many hardware breakpoints/watchpoints.

  (gdb)

Tested on x86_64 Fedora 20.

gdb/ChangeLog:
2015-04-01  Pedro Alves  <palves@redhat.com>

	* gdbthread.h (struct thread_info) <prev_pc>: Extend comment.
	* infrun.c (struct execution_control_state): Move higher up in the
	file.
	(reset_ecs): New function.
	(start_step_over_inferior): Now returns int.  Rewrite to use
	keep_going_pass instead of manually starting a displaced step.
	(start_step_over): Now returns int.
	(do_target_resume): Assert the thread isn't THREAD_EXITED.
	(resume): Assert the thread isn't in the step-over chain.  Don't
	call set_running here.  If displaced stepping can't start, clear
	trap_expected.
	(find_thread_needs_step_over): Delete function.
	(proceed): Set up finish_thread_state_cleanup.  Call set_running.
	If the current thread needs a step over, push it in the step-over
	chain.  Don't set prev_pc here, nor insert breakpoints nor call
	resume directly.  Instead rewrite to use start_step_over and
	keep_going_pass.
	(finish_step_over): New function.
	(handle_signal_stop): Call finish_step_over instead of
	start_step_over.
	(handle_signal_stop) <random signal>: No need to clear the signal
	if calling keep_going.  Clear step-over info before inserting a
	step-resume breakpoint.
	(switch_back_to_stepped_thread): If the event thread needs another
	step-over do that first.  Use start_step_over.
	(keep_going_pass): New function, factored out from ...
	(keep_going): ... here.
	(_initialize_infrun): Comment moved here.
	* thread.c (set_running_thread): New function.
	(set_running, finish_thread_state): Use set_running_thread.
---
 gdb/gdbthread.h |   6 +-
 gdb/infrun.c    | 577 +++++++++++++++++++++++++++-----------------------------
 gdb/thread.c    |  52 +++--
 3 files changed, 315 insertions(+), 320 deletions(-)

diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index e654432..f15cbc4 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -208,8 +208,10 @@ struct thread_info
 
   /* Internal stepping state.  */
 
-  /* Record the pc of the thread the last time it stopped.  This is
-     maintained by proceed and keep_going, and used in
+  /* Record the pc of the thread the last time it was resumed.  (It
+     can't be done on stop as the PC may change since the last stop,
+     e.g., "return" command, or "p $pc = 0xf000").  This is maintained
+     by proceed and keep_going, and among other things, it's used in
      adjust_pc_after_break to distinguish a hardware single-step
      SIGTRAP from a breakpoint SIGTRAP.  */
   CORE_ADDR prev_pc;
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 2a6f422..45ebdaf 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -1838,112 +1838,127 @@ displaced_step_fixup (ptid_t event_ptid, enum gdb_signal signal)
   displaced->step_ptid = null_ptid;
 }
 
-/* Are there any pending step-over requests for INF?  */
+/* Data to be passed around while handling an event.  This data is
+   discarded between events.  */
+struct execution_control_state
+{
+  ptid_t ptid;
+  /* The thread that got the event, if this was a thread event; NULL
+     otherwise.  */
+  struct thread_info *event_thread;
+
+  struct target_waitstatus ws;
+  int stop_func_filled_in;
+  CORE_ADDR stop_func_start;
+  CORE_ADDR stop_func_end;
+  const char *stop_func_name;
+  int wait_some_more;
+
+  /* True if the event thread hit the single-step breakpoint of
+     another thread.  Thus the event doesn't cause a stop, the thread
+     needs to be single-stepped past the single-step breakpoint before
+     we can switch back to the original stepping thread.  */
+  int hit_singlestep_breakpoint;
+};
+
+/* Clear ECS and set it to point at TP.  */
 
 static void
+reset_ecs (struct execution_control_state *ecs, struct thread_info *tp)
+{
+  memset (ecs, 0, sizeof (*ecs));
+  ecs->event_thread = tp;
+  ecs->ptid = tp->ptid;
+}
+
+static void keep_going_pass (struct execution_control_state *ecs);
+static void prepare_to_wait (struct execution_control_state *ecs);
+static int thread_still_needs_step_over (struct thread_info *tp);
+
+/* Are there any pending step-over requests for INF?  If so, run one
+   now and return true.  Otherwise, return false.  */
+
+static int
 start_step_over_inferior (struct inferior *inf)
 {
   /* Don't start a new step-over if we already have a displaced step
      operation ongoing.  */
   if (displaced_step_in_progress (inf->pid))
-    return;
+    return 0;
 
   while (inf->step_over_queue_head != NULL)
     {
-      ptid_t ptid;
-      struct displaced_step_inferior_state *displaced;
-      struct regcache *regcache;
-      struct gdbarch *gdbarch;
-      CORE_ADDR actual_pc;
-      struct address_space *aspace;
       struct thread_info *tp;
+      struct execution_control_state ecss;
+      struct execution_control_state *ecs = &ecss;
 
       tp = inf->step_over_queue_head;
-      displaced = get_displaced_stepping_state (inf->pid);
 
       step_over_chain_dequeue (&inf->step_over_queue_head);
 
-      ptid = tp->ptid;
-      context_switch (ptid);
-
-      regcache = get_thread_regcache (ptid);
-      actual_pc = regcache_read_pc (regcache);
-      aspace = get_regcache_aspace (regcache);
-
-      if (breakpoint_here_p (aspace, actual_pc))
+      if (tp->control.trap_expected || tp->executing)
 	{
-	  if (debug_displaced)
-	    fprintf_unfiltered (gdb_stdlog,
-				"displaced: stepping queued %s now\n",
-				target_pid_to_str (ptid));
+	  internal_error (__FILE__, __LINE__,
+			  "[%s] has inconsistent state: "
+			  "trap_expected=%d, executing=%d\n",
+			  target_pid_to_str (tp->ptid),
+			  tp->control.trap_expected,
+			  tp->executing);
+	}
 
-	  displaced_step_prepare (ptid);
+      if (debug_infrun)
+	fprintf_unfiltered (gdb_stdlog,
+			    "infrun: resuming [%s] for step-over\n",
+			    target_pid_to_str (tp->ptid));
+
+      /* keep_going_pass skips the step-over if the breakpoint is no
+	 longer inserted.  In all-stop, we want to keep looking for a
+	 that needs a step-over instead of resuming TP, because we
+	 wouldn't be able to resume anything else until the target
+	 stops again.  In non-stop, the resume always resumes only TP,
+	 so it's OK to let the thread resume freely.  */
+      if (!non_stop && !thread_still_needs_step_over (tp))
+	continue;
 
-	  gdbarch = get_regcache_arch (regcache);
+      switch_to_thread (tp->ptid);
+      reset_ecs (ecs, tp);
+      keep_going_pass (ecs);
 
-	  if (debug_displaced)
-	    {
-	      CORE_ADDR actual_pc = regcache_read_pc (regcache);
-	      gdb_byte buf[4];
+      if (!ecs->wait_some_more)
+	error (_("Command aborted."));
 
-	      fprintf_unfiltered (gdb_stdlog, "displaced: run %s: ",
-				  paddress (gdbarch, actual_pc));
-	      read_memory (actual_pc, buf, sizeof (buf));
-	      displaced_step_dump_bytes (gdb_stdlog, buf, sizeof (buf));
+      if (tp->control.trap_expected)
+	{
+	  if (inf->step_over_queue_head == NULL)
+	    {
+	      if (debug_infrun)
+		fprintf_unfiltered (gdb_stdlog,
+				    "infrun: step-over queue of process %d "
+				    "now empty\n", inf->pid);
 	    }
-
-	  if (gdbarch_displaced_step_hw_singlestep (gdbarch,
-						    displaced->step_closure))
-	    target_resume (ptid, 1, GDB_SIGNAL_0);
-	  else
-	    target_resume (ptid, 0, GDB_SIGNAL_0);
-
-	  /* Done, we're stepping a thread.  */
-	  break;
+	  return 1;
 	}
       else
 	{
-	  int step;
-	  struct thread_info *tp = inferior_thread ();
-
-	  /* The breakpoint we were sitting under has since been
-	     removed.  */
-	  tp->control.trap_expected = 0;
-
-	  /* 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);
-	  tp->suspend.stop_signal = GDB_SIGNAL_0;
-
-	  /* This request was discarded.  See if there's any other
-	     thread waiting for its turn.  */
+	  /* In all-stop, we shouldn't have resumed unless we needed a
+	     step over.  */
+	  gdb_assert (non_stop);
 	}
     }
 
-  if (inf->step_over_queue_head == NULL)
-    {
-      if (debug_infrun)
-	fprintf_unfiltered (gdb_stdlog,
-			    "infrun: step-over queue of process %d now empty\n",
-			    inf->pid);
-    }
+  return 0;
 }
 
 /* Are there any pending step-over requests for the inferior of
-   EVENT_PTID?  */
+   EVENT_PTID?  If so, run one now and return true.  Otherwise return
+   false.  */
 
-static void
+static int
 start_step_over (ptid_t event_ptid)
 {
   struct inferior *inf = find_inferior_ptid (event_ptid);
 
-  start_step_over_inferior (inf);
+  return start_step_over_inferior (inf);
 }
 
 /* Update global variables holding ptids to hold NEW_PTID if they were
@@ -2125,6 +2140,8 @@ resume (enum gdb_signal sig)
 
   tp->stepped_breakpoint = 0;
 
+  gdb_assert (tp->step_over_next == NULL);
+
   QUIT;
 
   /* Depends on stepped_breakpoint.  */
@@ -2253,16 +2270,11 @@ resume (enum gdb_signal sig)
 
       if (!displaced_step_prepare (inferior_ptid))
 	{
-	  /* Got placed in displaced stepping queue.  Will be resumed
-	     later when all the currently queued displaced stepping
-	     requests finish.  The thread is not executing at this
-	     point, and the call to set_executing will be made later.
-	     But we need to call set_running here, since from the
-	     user/frontend's point of view, threads were set running.
-	     Unless we're calling an inferior function, as in that
-	     case we pretend the inferior doesn't run at all.  */
-	  if (!tp->control.in_infcall)
-	    set_running (user_visible_resume_ptid (user_step), 1);
+	  if (debug_infrun)
+	    fprintf_unfiltered (gdb_stdlog,
+				"Got placed in displaced stepping queue\n");
+
+	  tp->control.trap_expected = 0;
 	  discard_cleanups (old_cleanups);
 	  return;
 	}
@@ -2337,14 +2349,6 @@ resume (enum gdb_signal sig)
      by applying increasingly restricting conditions.  */
   resume_ptid = user_visible_resume_ptid (user_step);
 
-  /* Even if RESUME_PTID is a wildcard, and we end up resuming less
-     (e.g., we might need to step over a breakpoint), from the
-     user/frontend's point of view, all threads in RESUME_PTID are now
-     running.  Unless we're calling an inferior function, as in that
-     case pretend we inferior doesn't run at all.  */
-  if (!tp->control.in_infcall)
-    set_running (resume_ptid, 1);
-
   /* Maybe resume a single thread after all.  */
   if ((step || thread_has_single_step_breakpoints_set (tp))
       && tp->control.trap_expected)
@@ -2566,48 +2570,6 @@ schedlock_applies (struct thread_info *tp)
 	      && tp->control.stepping_command));
 }
 
-/* Look a thread other than EXCEPT that has previously reported a
-   breakpoint event, and thus needs a step-over in order to make
-   progress.  Returns NULL is none is found.  */
-
-static struct thread_info *
-find_thread_needs_step_over (struct thread_info *except)
-{
-  struct thread_info *tp, *current;
-
-  /* With non-stop mode on, threads are always handled individually.  */
-  gdb_assert (! non_stop);
-
-  current = inferior_thread ();
-
-  /* If scheduler locking applies, we can avoid iterating over all
-     threads.  */
-  if (schedlock_applies (except))
-    {
-      if (except != current
-	  && thread_still_needs_step_over (current))
-	return current;
-
-      return NULL;
-    }
-
-  ALL_NON_EXITED_THREADS (tp)
-    {
-      /* Ignore the EXCEPT thread.  */
-      if (tp == except)
-	continue;
-      /* Ignore threads of processes we're not resuming.  */
-      if (!sched_multi
-	  && ptid_get_pid (tp->ptid) != ptid_get_pid (inferior_ptid))
-	continue;
-
-      if (thread_still_needs_step_over (tp))
-	return tp;
-    }
-
-  return NULL;
-}
-
 /* Basic routine for continuing the program in various fashions.
 
    ADDR is the address to resume at, or -1 for resume where stopped.
@@ -2628,6 +2590,12 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal)
   struct thread_info *tp;
   CORE_ADDR pc;
   struct address_space *aspace;
+  ptid_t resume_ptid;
+  struct execution_control_state ecss;
+  struct execution_control_state *ecs = &ecss;
+  struct cleanup *old_chain;
+  struct thread_info *current;
+  int started;
 
   /* If we're stopped at a fork/vfork, follow the branch set by the
      "set follow-fork-mode" command; otherwise, we'll just proceed
@@ -2649,6 +2617,7 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal)
   aspace = get_regcache_aspace (regcache);
   pc = regcache_read_pc (regcache);
   tp = inferior_thread ();
+  current = tp;
 
   /* Fill in with reasonable starting values.  */
   init_thread_stepping_state (tp);
@@ -2691,7 +2660,23 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal)
      (next/step/etc.), we'll want to print stop event output to the MI
      console channel (the stepped-to line, etc.), as if the user
      entered the execution command on a real GDB console.  */
-  inferior_thread ()->control.command_interp = command_interp ();
+  tp->control.command_interp = command_interp ();
+
+  resume_ptid = user_visible_resume_ptid (tp->control.stepping_command);
+
+  /* If an exception is thrown from this point on, make sure to
+     propagate GDB's knowledge of the executing state to the
+     frontend/user running state.  */
+  old_chain = make_cleanup (finish_thread_state_cleanup, &resume_ptid);
+
+  /* Even if RESUME_PTID is a wildcard, and we end up resuming fewer
+     threads (e.g., we might need to set threads stepping over
+     breakpoints first), from the user/frontend's point of view, all
+     threads in RESUME_PTID are now running.  Unless we're calling an
+     inferior function, as in that case we pretend the inferior
+     doesn't run at all.  */
+  if (!tp->control.in_infcall)
+   set_running (resume_ptid, 1);
 
   if (debug_infrun)
     fprintf_unfiltered (gdb_stdlog,
@@ -2699,93 +2684,74 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal)
 			paddress (gdbarch, addr),
 			gdb_signal_to_symbol_string (siggnal));
 
-  if (non_stop)
-    /* In non-stop, each thread is handled individually.  The context
-       must already be set to the right thread here.  */
-    ;
-  else
+  annotate_starting ();
+
+  /* Make sure that output from GDB appears before output from the
+     inferior.  */
+  gdb_flush (gdb_stdout);
+
+  /* In a multi-threaded task we may select another thread and
+     then continue or step.
+
+     But if a thread that we're resuming had stopped at a breakpoint,
+     it will immediately cause another breakpoint stop without any
+     execution (i.e. it will report a breakpoint hit incorrectly).  So
+     we must step over it first.
+
+     Look for threads other than the current (TP) that reported a
+     breakpoint hit and haven't been resumed yet since.  */
+
+  /* If scheduler locking applies, we can avoid iterating over all
+     threads.  */
+  if (!non_stop && !schedlock_applies (tp))
     {
-      struct thread_info *step_over;
+      ALL_NON_EXITED_THREADS (tp)
+        {
+	  /* Ignore the current thread here.  It's handled
+	     afterwards.  */
+	  if (tp == current)
+	    continue;
+
+	  /* Ignore threads of processes we're not resuming.  */
+	  if (!ptid_match (tp->ptid, resume_ptid))
+	    continue;
 
-      /* In a multi-threaded task we may select another thread and
-	 then continue or step.
+	  if (!thread_still_needs_step_over (tp))
+	    continue;
 
-	 But if the old thread was stopped at a breakpoint, it will
-	 immediately cause another breakpoint stop without any
-	 execution (i.e. it will report a breakpoint hit incorrectly).
-	 So we must step over it first.
+	  gdb_assert (tp->step_over_next == NULL);
+	  gdb_assert (tp->step_over_prev == NULL);
 
-	 Look for a thread other than the current (TP) that reported a
-	 breakpoint hit and hasn't been resumed yet since.  */
-      step_over = find_thread_needs_step_over (tp);
-      if (step_over != NULL)
-	{
 	  if (debug_infrun)
 	    fprintf_unfiltered (gdb_stdlog,
 				"infrun: need to step-over [%s] first\n",
-				target_pid_to_str (step_over->ptid));
+				target_pid_to_str (tp->ptid));
 
-	  /* Store the prev_pc for the stepping thread too, needed by
-	     switch_back_to_stepped_thread.  */
-	  tp->prev_pc = regcache_read_pc (get_current_regcache ());
-	  switch_to_thread (step_over->ptid);
-	  tp = step_over;
+	  thread_step_over_chain_enqueue (tp);
 	}
-    }
-
-  /* If we need to step over a breakpoint, and we're not using
-     displaced stepping to do so, insert all breakpoints (watchpoints,
-     etc.) but the one we're stepping over, step one instruction, and
-     then re-insert the breakpoint when that step is finished.  */
-  if (tp->stepping_over_breakpoint
-      && !use_displaced_stepping_now_p (gdbarch,
-					tp->suspend.stop_signal))
-    {
-      struct regcache *regcache = get_current_regcache ();
 
-      set_step_over_info (get_regcache_aspace (regcache),
-			  regcache_read_pc (regcache), 0);
+      tp = current;
     }
-  else
-    clear_step_over_info ();
-
-  insert_breakpoints ();
-
-  tp->control.trap_expected = tp->stepping_over_breakpoint;
 
-  annotate_starting ();
+  /* Enqueue the current thread last, so that we move all other
+     threads over their breakpoints first.  */
+  if (tp->stepping_over_breakpoint)
+    thread_step_over_chain_enqueue (tp);
 
-  /* Make sure that output from GDB appears before output from the
-     inferior.  */
-  gdb_flush (gdb_stdout);
+  started = start_step_over (tp->ptid);
+  if (started)
+    {
+      /* A new displaced stepping sequence was started.  */
+    }
+  else if (tp->step_over_next == NULL)
+    {
+      reset_ecs (ecs, tp);
+      keep_going_pass (ecs);
+      if (!ecs->wait_some_more)
+	error ("Command aborted.");
+    }
 
-  /* Refresh prev_pc value just prior to resuming.  This used to be
-     done in stop_waiting, however, setting prev_pc there did not handle
-     scenarios such as inferior function calls or returning from
-     a function via the return command.  In those cases, the prev_pc
-     value was not set properly for subsequent commands.  The prev_pc value 
-     is used to initialize the starting line number in the ecs.  With an 
-     invalid value, the gdb next command ends up stopping at the position
-     represented by the next line table entry past our start position.
-     On platforms that generate one line table entry per line, this
-     is not a problem.  However, on the ia64, the compiler generates
-     extraneous line table entries that do not increase the line number.
-     When we issue the gdb next command on the ia64 after an inferior call
-     or a return command, we often end up a few instructions forward, still 
-     within the original line we started.
-
-     An attempt was made to refresh the prev_pc at the same time the
-     execution_control_state is initialized (for instance, just before
-     waiting for an inferior event).  But this approach did not work
-     because of platforms that use ptrace, where the pc register cannot
-     be read unless the inferior is stopped.  At that point, we are not
-     guaranteed the inferior is stopped and so the regcache_read_pc() call
-     can fail.  Setting the prev_pc value here ensures the value is updated
-     correctly when the inferior is stopped.  */
-  tp->prev_pc = regcache_read_pc (get_current_regcache ());
-
-  /* Resume inferior.  */
-  resume (tp->suspend.stop_signal);
+  discard_cleanups (old_chain);
 
   /* Wait for it to stop (if not standalone)
      and in any case decode why it stopped, and act accordingly.  */
@@ -2853,28 +2819,6 @@ init_wait_for_inferior (void)
 }
 
 \f
-/* Data to be passed around while handling an event.  This data is
-   discarded between events.  */
-struct execution_control_state
-{
-  ptid_t ptid;
-  /* The thread that got the event, if this was a thread event; NULL
-     otherwise.  */
-  struct thread_info *event_thread;
-
-  struct target_waitstatus ws;
-  int stop_func_filled_in;
-  CORE_ADDR stop_func_start;
-  CORE_ADDR stop_func_end;
-  const char *stop_func_name;
-  int wait_some_more;
-
-  /* True if the event thread hit the single-step breakpoint of
-     another thread.  Thus the event doesn't cause a stop, the thread
-     needs to be single-stepped past the single-step breakpoint before
-     we can switch back to the original stepping thread.  */
-  int hit_singlestep_breakpoint;
-};
 
 static void handle_inferior_event (struct execution_control_state *ecs);
 
@@ -2888,7 +2832,6 @@ static void check_exception_resume (struct execution_control_state *,
 
 static void end_stepping_range (struct execution_control_state *ecs);
 static void stop_waiting (struct execution_control_state *ecs);
-static void prepare_to_wait (struct execution_control_state *ecs);
 static void keep_going (struct execution_control_state *ecs);
 static void process_event_stop_test (struct execution_control_state *ecs);
 static int switch_back_to_stepped_thread (struct execution_control_state *ecs);
@@ -4255,6 +4198,34 @@ Cannot fill $_exitsignal with the correct signal number.\n"));
     }
 }
 
+/* Called when we get an event that may finish an in-line or
+   out-of-line (displaced stepping) step-over started previously.  */
+
+static void
+finish_step_over (struct execution_control_state *ecs)
+{
+  displaced_step_fixup (ecs->ptid,
+			ecs->event_thread->suspend.stop_signal);
+
+  if (step_over_info_valid_p ())
+    {
+      /* If we're stepping over a breakpoint with all threads locked,
+	 then only the thread that was stepped should be reporting
+	 back an event.  */
+      gdb_assert (ecs->event_thread->control.trap_expected);
+
+      if (ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP)
+	clear_step_over_info ();
+    }
+
+  if (!non_stop)
+    return;
+
+  /* Start a new step-over in another thread if there's one that
+     needs it.  */
+  start_step_over (ecs->ptid);
+}
+
 /* Come here when the program has stopped with a signal.  */
 
 static void
@@ -4271,9 +4242,7 @@ handle_signal_stop (struct execution_control_state *ecs)
   /* Do we need to clean up the state of a thread that has
      completed a displaced single-step?  (Doing so usually affects
      the PC, so do it here, before we set stop_pc.)  */
-  displaced_step_fixup (ecs->ptid,
-			ecs->event_thread->suspend.stop_signal);
-  start_step_over (ecs->ptid);
+  finish_step_over (ecs);
 
   /* If we either finished a single-step or hit a breakpoint, but
      the user wanted this thread to be stopped, pretend we got a
@@ -5622,7 +5591,6 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs)
     {
       struct thread_info *tp;
       struct thread_info *stepping_thread;
-      struct thread_info *step_over;
 
       /* If any thread is blocked on some internal breakpoint, and we
 	 simply need to step over that breakpoint to get it going
@@ -5665,14 +5633,20 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs)
 	 return 1;
        }
 
-      /* Otherwise, we no longer expect a trap in the current thread.
-	 Clear the trap_expected flag before switching back -- this is
-	 what keep_going does as well, if we call it.  */
-      ecs->event_thread->control.trap_expected = 0;
-
-      /* Likewise, clear the signal if it should not be passed.  */
-      if (!signal_program[ecs->event_thread->suspend.stop_signal])
-	ecs->event_thread->suspend.stop_signal = GDB_SIGNAL_0;
+      /* If this thread needs yet another step-over (e.g., stepping
+	 through a delay slot), do it first before moving on to
+	 another thread.  */
+      if (thread_still_needs_step_over (ecs->event_thread))
+	{
+	  if (debug_infrun)
+	    {
+	      fprintf_unfiltered (gdb_stdlog,
+				  "infrun: thread [%s] still needs step-over\n",
+				  target_pid_to_str (ecs->event_thread->ptid));
+	    }
+	  keep_going (ecs);
+	  return 1;
+	}
 
       /* If scheduler locking applies even if not stepping, there's no
 	 need to walk over threads.  Above we've checked whether the
@@ -5682,12 +5656,26 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs)
       if (schedlock_applies (ecs->event_thread))
 	return 0;
 
-      /* Look for the stepping/nexting thread, and check if any other
-	 thread other than the stepping thread needs to start a
-	 step-over.  Do all step-overs before actually proceeding with
+      /* Otherwise, we no longer expect a trap in the current thread.
+	 Clear the trap_expected flag before switching back -- this is
+	 what keep_going does as well, if we call it.  */
+      ecs->event_thread->control.trap_expected = 0;
+
+      /* Likewise, clear the signal if it should not be passed.  */
+      if (!signal_program[ecs->event_thread->suspend.stop_signal])
+	ecs->event_thread->suspend.stop_signal = GDB_SIGNAL_0;
+
+      /* Do all pending step-overs before actually proceeding with
 	 step/next/etc.  */
+      if (start_step_over (ecs->ptid))
+	{
+	  prepare_to_wait (ecs);
+	  return 1;
+	}
+
+      /* Look for the stepping/nexting thread.  */
       stepping_thread = NULL;
-      step_over = NULL;
+
       ALL_NON_EXITED_THREADS (tp)
         {
 	  /* Ignore threads of processes we're not resuming.  */
@@ -5719,37 +5707,6 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs)
 
 	      stepping_thread = tp;
 	    }
-	  else if (thread_still_needs_step_over (tp))
-	    {
-	      step_over = tp;
-
-	      /* At the top we've returned early if the event thread
-		 is stepping.  If some other thread not the event
-		 thread is stepping, then scheduler locking can't be
-		 in effect, and we can resume this thread.  No need to
-		 keep looking for the stepping thread then.  */
-	      break;
-	    }
-	}
-
-      if (step_over != NULL)
-	{
-	  tp = step_over;
-	  if (debug_infrun)
-	    {
-	      fprintf_unfiltered (gdb_stdlog,
-				  "infrun: need to step-over [%s]\n",
-				  target_pid_to_str (tp->ptid));
-	    }
-
-	  /* Only the stepping thread should have this set.  */
-	  gdb_assert (tp->control.step_range_end == 0);
-
-	  ecs->ptid = tp->ptid;
-	  ecs->event_thread = tp;
-	  switch_to_thread (ecs->ptid);
-	  keep_going (ecs);
-	  return 1;
 	}
 
       if (stepping_thread != NULL)
@@ -5848,7 +5805,7 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs)
 		fprintf_unfiltered (gdb_stdlog,
 				    "infrun: expected thread still "
 				    "hasn't advanced\n");
-	      keep_going (ecs);
+	      keep_going_pass (ecs);
 	    }
 
 	  return 1;
@@ -6263,24 +6220,32 @@ stop_waiting (struct execution_control_state *ecs)
   ecs->wait_some_more = 0;
 }
 
-/* Called when we should continue running the inferior, because the
-   current event doesn't cause a user visible stop.  This does the
-   resuming part; waiting for the next event is done elsewhere.  */
+/* Like keep_going, but passes the signal to the inferior, even if the
+   signal is set to nopass.  */
 
 static void
-keep_going (struct execution_control_state *ecs)
+keep_going_pass (struct execution_control_state *ecs)
 {
   /* Make sure normal_stop is called if we get a QUIT handled before
      reaching resume.  */
   struct cleanup *old_cleanups = make_cleanup (resume_cleanups, 0);
 
+  gdb_assert (ptid_equal (ecs->event_thread->ptid, inferior_ptid));
+
   /* Save the pc before execution, to compare with pc after stop.  */
   ecs->event_thread->prev_pc
     = regcache_read_pc (get_thread_regcache (ecs->ptid));
 
-  if (ecs->event_thread->control.trap_expected
-      && ecs->event_thread->suspend.stop_signal != GDB_SIGNAL_TRAP)
+  if (ecs->event_thread->control.trap_expected)
     {
+      struct thread_info *tp = ecs->event_thread;
+
+      if (debug_infrun)
+	fprintf_unfiltered (gdb_stdlog,
+			    "infrun: %s has trap_expected set, "
+			    "resuming to collect trap\n",
+			    target_pid_to_str (tp->ptid));
+
       /* We haven't yet gotten our trap, and either: intercepted a
 	 non-signal event (e.g., a fork); or took a signal which we
 	 are supposed to pass through to the inferior.  Simply
@@ -6293,7 +6258,7 @@ keep_going (struct execution_control_state *ecs)
       struct regcache *regcache = get_current_regcache ();
       int remove_bp;
       int remove_wps;
-      enum gdb_signal signo;
+      enum gdb_signal signo = ecs->event_thread->suspend.stop_signal;
       enum step_over_what step_what;
 
       /* Either the trap was not expected, but we are continuing
@@ -6315,22 +6280,6 @@ keep_going (struct execution_control_state *ecs)
 	 instruction, and then re-insert the breakpoint when that step
 	 is finished.  */
 
-      /* Do not deliver GDB_SIGNAL_TRAP (except when the user
-	 explicitly specifies that such a signal should be delivered
-	 to the target program).  Typically, that would occur when a
-	 user is debugging a target monitor on a simulator: the target
-	 monitor sets a breakpoint; the simulator encounters this
-	 breakpoint and halts the simulation handing control to GDB;
-	 GDB, noting that the stop address doesn't map to any known
-	 breakpoint, returns control back to the simulator; the
-	 simulator then delivers the hardware equivalent of a
-	 GDB_SIGNAL_TRAP to the program being debugged.	 */
-      if (ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP
-	  && !signal_program[ecs->event_thread->suspend.stop_signal])
-	ecs->event_thread->suspend.stop_signal = GDB_SIGNAL_0;
-
-      signo = ecs->event_thread->suspend.stop_signal;
-
       step_what = thread_still_needs_step_over (ecs->event_thread);
 
       remove_bp = (ecs->hit_singlestep_breakpoint
@@ -6372,6 +6321,22 @@ keep_going (struct execution_control_state *ecs)
   prepare_to_wait (ecs);
 }
 
+/* Called when we should continue running the inferior, because the
+   current event doesn't cause a user visible stop.  This does the
+   resuming part; waiting for the next event is done elsewhere.  */
+
+static void
+keep_going (struct execution_control_state *ecs)
+{
+  if (ecs->event_thread->control.trap_expected
+      && ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP)
+    ecs->event_thread->control.trap_expected = 0;
+
+  if (!signal_program[ecs->event_thread->suspend.stop_signal])
+    ecs->event_thread->suspend.stop_signal = GDB_SIGNAL_0;
+  keep_going_pass (ecs);
+}
+
 /* This function normally comes after a resume, before
    handle_inferior_event exits.  It takes care of any last bits of
    housekeeping, and sets the all-important wait_some_more flag.  */
@@ -7888,9 +7853,21 @@ leave it stopped or free to run as needed."),
       signal_catch[i] = 0;
     }
 
-  /* Signals caused by debugger's own actions
-     should not be given to the program afterwards.  */
+  /* Signals caused by debugger's own actions should not be given to
+     the program afterwards.
+
+     Do not deliver GDB_SIGNAL_TRAP by default, except when the user
+     explicitly specifies that it should be delivered to the target
+     program.  Typically, that would occur when a user is debugging a
+     target monitor on a simulator: the target monitor sets a
+     breakpoint; the simulator encounters this breakpoint and halts
+     the simulation handing control to GDB; GDB, noting that the stop
+     address doesn't map to any known breakpoint, returns control back
+     to the simulator; the simulator then delivers the hardware
+     equivalent of a GDB_SIGNAL_TRAP to the program being
+     debugged.  */
   signal_program[GDB_SIGNAL_TRAP] = 0;
+
   signal_program[GDB_SIGNAL_INT] = 0;
 
   /* Signals that are not errors should not normally enter the debugger.  */
diff --git a/gdb/thread.c b/gdb/thread.c
index d5c9896..a01f5d8 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -848,44 +848,62 @@ thread_change_ptid (ptid_t old_ptid, ptid_t new_ptid)
   observer_notify_thread_ptid_changed (old_ptid, new_ptid);
 }
 
+/* Helper for set_running, that marks one thread either running or
+   stopped.  */
+
+static int
+set_running_thread (struct thread_info *tp, int running)
+{
+  int started = 0;
+
+  if (running && tp->state == THREAD_STOPPED)
+    started = 1;
+  tp->state = running ? THREAD_RUNNING : THREAD_STOPPED;
+
+  if (!running)
+    {
+      /* If the thread is now marked stopped, remove it from
+	 the step-over queue, so that we don't try to resume
+	 it until the user wants it to.  */
+      if (tp->step_over_next != NULL)
+	thread_step_over_chain_remove (tp);
+    }
+
+  return started;
+}
+
 void
 set_running (ptid_t ptid, int running)
 {
   struct thread_info *tp;
   int all = ptid_equal (ptid, minus_one_ptid);
+  int any_started = 0;
 
   /* We try not to notify the observer if no thread has actually changed 
      the running state -- merely to reduce the number of messages to 
      frontend.  Frontend is supposed to handle multiple *running just fine.  */
   if (all || ptid_is_pid (ptid))
     {
-      int any_started = 0;
-
       for (tp = thread_list; tp; tp = tp->next)
 	if (all || ptid_get_pid (tp->ptid) == ptid_get_pid (ptid))
 	  {
 	    if (tp->state == THREAD_EXITED)
 	      continue;
-	    if (running && tp->state == THREAD_STOPPED)
+
+	    if (set_running_thread (tp, running))
 	      any_started = 1;
-	    tp->state = running ? THREAD_RUNNING : THREAD_STOPPED;
 	  }
-      if (any_started)
-	observer_notify_target_resumed (ptid);
     }
   else
     {
-      int started = 0;
-
       tp = find_thread_ptid (ptid);
-      gdb_assert (tp);
+      gdb_assert (tp != NULL);
       gdb_assert (tp->state != THREAD_EXITED);
-      if (running && tp->state == THREAD_STOPPED)
- 	started = 1;
-      tp->state = running ? THREAD_RUNNING : THREAD_STOPPED;
-      if (started)
-  	observer_notify_target_resumed (ptid);
+      if (set_running_thread (tp, running))
+	any_started = 1;
     }
+  if (any_started)
+    observer_notify_target_resumed (ptid);
 }
 
 static int
@@ -1004,9 +1022,8 @@ finish_thread_state (ptid_t ptid)
   	    continue;
 	  if (all || ptid_get_pid (ptid) == ptid_get_pid (tp->ptid))
 	    {
-	      if (tp->executing && tp->state == THREAD_STOPPED)
+	      if (set_running_thread (tp, tp->executing))
 		any_started = 1;
-	      tp->state = tp->executing ? THREAD_RUNNING : THREAD_STOPPED;
 	    }
 	}
     }
@@ -1016,9 +1033,8 @@ finish_thread_state (ptid_t ptid)
       gdb_assert (tp);
       if (tp->state != THREAD_EXITED)
 	{
-	  if (tp->executing && tp->state == THREAD_STOPPED)
+	  if (set_running_thread (tp, tp->executing))
 	    any_started = 1;
-	  tp->state = tp->executing ? THREAD_RUNNING : THREAD_STOPPED;
 	}
     }
 
-- 
1.9.3

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 13/17] Fix signal-while-stepping-over-bp-other-thread.exp on targets always in non-stop
  2015-04-01 22:14 [PATCH 00/17] All-stop on top of non-stop Pedro Alves
  2015-04-01 22:14 ` [PATCH 10/17] Factor out code to re-resume stepped thread Pedro Alves
@ 2015-04-01 22:14 ` Pedro Alves
  2015-04-01 22:14 ` [PATCH 03/17] Displaced stepping debug: fetch the right regcache Pedro Alves
                   ` (15 subsequent siblings)
  17 siblings, 0 replies; 26+ messages in thread
From: Pedro Alves @ 2015-04-01 22:14 UTC (permalink / raw)
  To: gdb-patches

With "maint set target-non-stop on" we get:

 -PASS: gdb.threads/signal-while-stepping-over-bp-other-thread.exp: step
 +FAIL: gdb.threads/signal-while-stepping-over-bp-other-thread.exp: step

The issue is simply that switch_back_to_stepped_thread is not used in
non-stop mode, thus infrun doesn't output the expected "signal arrived
while stepping over breakpoint" log.

gdb/testsuite/ChangeLog:
2015-04-01  Pedro Alves  <palves@redhat.com>

	* signal-while-stepping-over-bp-other-thread.exp: Use
	gdb_test_sequence.  Expect "restart threads" as alternative to
	"switching back to stepped thread".
---
 .../signal-while-stepping-over-bp-other-thread.exp   | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/gdb/testsuite/gdb.threads/signal-while-stepping-over-bp-other-thread.exp b/gdb/testsuite/gdb.threads/signal-while-stepping-over-bp-other-thread.exp
index bb00c50..1d5eabd 100644
--- a/gdb/testsuite/gdb.threads/signal-while-stepping-over-bp-other-thread.exp
+++ b/gdb/testsuite/gdb.threads/signal-while-stepping-over-bp-other-thread.exp
@@ -95,19 +95,19 @@ gdb_test "set scheduler-locking off"
 # Make sure we're exercising the paths we want to.
 gdb_test "set debug infrun 1"
 
-gdb_test \
-    "step" \
-    ".*need to step-over.*resume \\(step=1.*signal arrived while stepping over breakpoint.*switching back to stepped thread.*stepped to a different line.*callme.*" \
-    "step"
+set test "step"
+gdb_test_sequence $test $test {
+    "need to step-over \[^\r\n\]* first"
+    "resume \\(step=1"
+    "Program received signal SIGUSR1"
+    "(restart threads|signal arrived while stepping over breakpoint)"
+    "stepped to a different line"
+    "callme \\(\\);"
+}
 
 set cnt_after [get_value "args\[$my_number\]" "get count after step"]
 
 # Test that GDB doesn't inadvertently resume the stepped thread when a
 # signal arrives while stepping over a breakpoint in another thread.
 
-set test "stepped thread under control"
-if { $cnt_before + 1 == $cnt_after } {
-    pass $test
-} else {
-    fail $test
-}
+gdb_assert { $cnt_before + 1 == $cnt_after } "stepped thread under control"
-- 
1.9.3

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 01/17] Fix and test "checkpoint" in non-stop mode
  2015-04-01 22:14 [PATCH 00/17] All-stop on top of non-stop Pedro Alves
                   ` (3 preceding siblings ...)
  2015-04-01 22:14 ` [PATCH 17/17] native Linux: enable always non-stop by default Pedro Alves
@ 2015-04-01 22:14 ` Pedro Alves
  2015-04-01 22:14 ` [PATCH 08/17] Use keep_going in proceed and start_step_over too Pedro Alves
                   ` (12 subsequent siblings)
  17 siblings, 0 replies; 26+ messages in thread
From: Pedro Alves @ 2015-04-01 22:14 UTC (permalink / raw)
  To: gdb-patches

Letting a "checkpoint" run to exit with "set non-stop on" behaves
differently in compared to the default all-stop mode.

Currently in non-stop mode:

  (gdb) start
  Temporary breakpoint 1 at 0x40086b: file src/gdb/testsuite/gdb.base/checkpoint.c, line 28.
  Starting program: build/gdb/testsuite/gdb.base/checkpoint

  Temporary breakpoint 1, main () at src/gdb/testsuite/gdb.base/checkpoint.c:28
  28        char *tmp = &linebuf[0];
  (gdb) checkpoint
  checkpoint 1: fork returned pid 24948.
  (gdb) c
  Continuing.
  Copy complete.
  Deleting copy.
  [Inferior 1 (process 24944) exited normally]
  [Switching to process 24948]
  (gdb) info threads
    Id   Target Id         Frame
    1    process 24948 "checkpoint" (running)

  No selected thread.  See `help thread'.
  (gdb) c
  The program is not being run.
  (gdb)

Two issues above:

 1. Thread 1 got stuck in "(running)" state (it isn't really running)

 2. While checkpoints try to preserve the illusion that the thread is
    still the same, GDB switched to "No thread selected." instead of
    staying with thread 1 selected.

Problem 1. is caused by handle_inferior_event and normal_stop not
considering that when a
TARGET_WAITKIND_SIGNALLED/TARGET_WAITKIND_EXITED event is reported,
and the inferior is mourned, the target may still have execution.

Problem 2. is caused by the make_cleanup_restore_current_thread
cleanup installed by fetch_inferior_event not being able to find the
original thread 1's ptid in the thread list, thus not being able to
restore thread 1 as selected thread.  The fix is to make the cleanup
installed by make_cleanup_restore_current_thread aware of thread ptid
changes, by installing a thread_ptid_changed observer that adjusts the
cleanup's data.

After the patch, we get the same in all-stop and non-stop modes:

  (gdb) c
  Continuing.
  Copy complete.
  Deleting copy.
  [Inferior 1 (process 25109) exited normally]
  [Switching to process 25113]
  (gdb) info threads
    Id   Target Id         Frame
  * 1    process 25113 "checkpoint" main () at src/gdb/testsuite/gdb.base/checkpoint.c:28
  (gdb)

Turns out the whole checkpoints.exp file can run in non-stop mode
unmodified.  I thought of moving most of the test file's contents to a
procedure that can be called twice, once in non-stop mode and another
in all-stop mode.  But then, the test already takes over 30 seconds to
run on my machine, so I thought it'd be nicer to run all-stop and
non-stop mode in parallel.  Thus I added a new checkpoint-ns.exp file
that just sources checkpoint.exp, and sets a knob that
checkpoint-ns.exp reads to know it should test non-stop mode.  No
other test in the tree currently uses this mechanism, but I can't see
a reason we shouldn't do this.

gdb/ChangeLog:
2015-03-26  Pedro Alves  <palves@redhat.com>

	* infrun.c (handle_inferior_event): If we get
	TARGET_WAITKIND_SIGNALLED or TARGET_WAITKIND_EXITED in non-stop
	mode, mark all threads of the exiting process as not-executing.
	(normal_stop): If we get TARGET_WAITKIND_SIGNALLED or
	TARGET_WAITKIND_EXITED in non-stop mode, finish all threads of the
	exiting process, if inferior_ptid still points at a process.
	* thread.c (struct current_thread_cleanup) <next>: New field.
	(current_thread_cleanup_chain): New global.
	(restore_current_thread_ptid_changed): New function.
	(restore_current_thread_cleanup_dtor): Remove the cleanup from the
	current_thread_cleanup_chain list.
	(make_cleanup_restore_current_thread): Add the cleanup data to the
	current_thread_cleanup_chain list.
	(_initialize_thread): Install restore_current_thread_ptid_changed
	as thread_ptid_changed observer.

gdb/testsuite/ChangeLog:
2015-03-26  Pedro Alves  <palves@redhat.com>

	* gdb.base/checkpoint-ns.exp: New file.
	* gdb.base/checkpoint.exp (checkpoint_non_stop): New input
	variable.  If set, test in non-stop mode.  Pass explicit
	"checkpoint.c" to standard_testfile.
---
 gdb/infrun.c                             | 31 +++++++++++++++++++++-----
 gdb/testsuite/gdb.base/checkpoint-ns.exp | 27 +++++++++++++++++++++++
 gdb/testsuite/gdb.base/checkpoint.exp    | 31 +++++++++++++++++++++-----
 gdb/thread.c                             | 38 ++++++++++++++++++++++++++++++++
 4 files changed, 117 insertions(+), 10 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/checkpoint-ns.exp

diff --git a/gdb/infrun.c b/gdb/infrun.c
index f5faa0a..01d5a34 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -3766,8 +3766,18 @@ handle_inferior_event (struct execution_control_state *ecs)
      any other process were left running.  */
   if (!non_stop)
     set_executing (minus_one_ptid, 0);
-  else if (ecs->ws.kind != TARGET_WAITKIND_SIGNALLED
-	   && ecs->ws.kind != TARGET_WAITKIND_EXITED)
+  else if (ecs->ws.kind == TARGET_WAITKIND_SIGNALLED
+	   && ecs->ws.kind == TARGET_WAITKIND_EXITED)
+    {
+      ptid_t pid_ptid;
+
+      /* Some targets still have execution when a process exits.
+	 E.g., for "checkpoint", when when a fork exits and is
+	 mourned, linux-fork.c switches to another fork.  */
+      pid_ptid = pid_to_ptid (ptid_get_pid (ecs->ptid));
+      set_executing (pid_ptid, 0);
+    }
+  else
     set_executing (ecs->ptid, 0);
 
   switch (ecs->ws.kind)
@@ -6506,6 +6516,7 @@ normal_stop (void)
   struct target_waitstatus last;
   ptid_t last_ptid;
   struct cleanup *old_chain = make_cleanup (null_cleanup, NULL);
+  ptid_t pid_ptid;
 
   get_last_target_status (&last_ptid, &last);
 
@@ -6515,9 +6526,19 @@ normal_stop (void)
      here, so do this before any filtered output.  */
   if (!non_stop)
     make_cleanup (finish_thread_state_cleanup, &minus_one_ptid);
-  else if (last.kind != TARGET_WAITKIND_SIGNALLED
-	   && last.kind != TARGET_WAITKIND_EXITED
-	   && last.kind != TARGET_WAITKIND_NO_RESUMED)
+  else if (last.kind == TARGET_WAITKIND_SIGNALLED
+	   || last.kind == TARGET_WAITKIND_EXITED)
+    {
+      /* Some targets still have execution when a process exits.
+	 E.g., for "checkpoint", when when a fork exits and is
+	 mourned, linux-fork.c switches to another fork.  */
+      if (!ptid_equal (inferior_ptid, null_ptid))
+	{
+	  pid_ptid = pid_to_ptid (ptid_get_pid (inferior_ptid));
+	  make_cleanup (finish_thread_state_cleanup, &pid_ptid);
+	}
+    }
+  else if (last.kind != TARGET_WAITKIND_NO_RESUMED)
     make_cleanup (finish_thread_state_cleanup, &inferior_ptid);
 
   /* As we're presenting a stop, and potentially removing breakpoints,
diff --git a/gdb/testsuite/gdb.base/checkpoint-ns.exp b/gdb/testsuite/gdb.base/checkpoint-ns.exp
new file mode 100644
index 0000000..03a1747
--- /dev/null
+++ b/gdb/testsuite/gdb.base/checkpoint-ns.exp
@@ -0,0 +1,27 @@
+# Copyright 2015 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+# Test gdb checkpoint and restart in non-stop mode.
+
+# We drive non-stop mode from a separate file because the whole test
+# takes a while to run.  This way, we can test both modes in parallel.
+
+# checkpoint.exp reads this variable.
+set checkpoint_non_stop 1
+
+source $srcdir/$subdir/checkpoint.exp
+
+# Unset to avoid problems with non-parallel mode.
+unset checkpoint_non_stop
diff --git a/gdb/testsuite/gdb.base/checkpoint.exp b/gdb/testsuite/gdb.base/checkpoint.exp
index 6d94ab6..c297b50 100644
--- a/gdb/testsuite/gdb.base/checkpoint.exp
+++ b/gdb/testsuite/gdb.base/checkpoint.exp
@@ -13,6 +13,20 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
+#
+# This tests gdb checkpoint and restart.
+#
+
+# Note this file is also sourced by checkpoint-ns.exp to run all the
+# tests below in non-stop mode.  That's driven by a separate file so
+# testing both all-stop and non-stop can be done in parallel.
+
+# This is the variable checkpoint-ns.exp sets to request non-stop
+# mode.  When not set, default to all-stop mode.
+if {![info exists checkpoint_non_stop]} {
+    set checkpoint_non_stop 0
+}
+
 if { [is_remote target] || ![isnative] } then {
     continue
 }
@@ -24,8 +38,10 @@ if {![istarget "*-*-linux*"]} then {
     continue
 }
 
-
-standard_testfile .c
+# Must name the source file explicitly, otherwise when driven by
+# checkpoints-ns.exp, we'd try compiling checkpoints-ns.c, which
+# doesn't exist.
+standard_testfile checkpoint.c
 
 set pi_txt [gdb_remote_download host ${srcdir}/${subdir}/pi.txt]
 if {[is_remote host]} {
@@ -42,9 +58,9 @@ if {[prepare_for_testing ${testfile}.exp $testfile $srcfile \
 
 global gdb_prompt
 
-#
-# This tests gdb checkpoint and restart.
-#
+if {$checkpoint_non_stop} {
+    gdb_test_no_output "set non-stop on"
+}
 
 runto_main
 set break1_loc [gdb_get_line_number "breakpoint 1"]
@@ -327,6 +343,11 @@ gdb_start
 gdb_reinitialize_dir $srcdir/$subdir
 gdb_load ${binfile}
 
+if {$checkpoint_non_stop} {
+    gdb_test_no_output "set non-stop on" \
+	"set non-stop on, for many checkpoints"
+}
+
 runto_main
 gdb_breakpoint $break1_loc
 
diff --git a/gdb/thread.c b/gdb/thread.c
index ec398f5..db631c9 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -1266,8 +1266,16 @@ restore_selected_frame (struct frame_id a_frame_id, int frame_level)
     }
 }
 
+/* Data used by the cleanup installed by
+   'make_cleanup_restore_current_thread'.  */
+
 struct current_thread_cleanup
 {
+  /* Next in list of currently installed 'struct
+     current_thread_cleanup' cleanups.  See
+     'current_thread_cleanup_chain' below.  */
+  struct current_thread_cleanup *next;
+
   ptid_t inferior_ptid;
   struct frame_id selected_frame_id;
   int selected_frame_level;
@@ -1276,6 +1284,29 @@ struct current_thread_cleanup
   int was_removable;
 };
 
+/* A chain of currently installed 'struct current_thread_cleanup'
+   cleanups.  Restoring the previously selected thread looks up the
+   old thread in the thread list by ptid.  If the thread changes ptid,
+   we need to update the cleanup's thread structure so the look up
+   succeeds.  */
+static struct current_thread_cleanup *current_thread_cleanup_chain;
+
+/* A thread_ptid_changed observer.  Update all currently installed
+   current_thread_cleanup cleanups that want to switch back to
+   OLD_PTID to switch back to NEW_PTID instead.  */
+
+static void
+restore_current_thread_ptid_changed (ptid_t old_ptid, ptid_t new_ptid)
+{
+  struct current_thread_cleanup *it;
+
+  for (it = current_thread_cleanup_chain; it != NULL; it = it->next)
+    {
+      if (ptid_equal (it->inferior_ptid, old_ptid))
+	it->inferior_ptid = new_ptid;
+    }
+}
+
 static void
 do_restore_current_thread_cleanup (void *arg)
 {
@@ -1316,6 +1347,8 @@ restore_current_thread_cleanup_dtor (void *arg)
   struct thread_info *tp;
   struct inferior *inf;
 
+  current_thread_cleanup_chain = current_thread_cleanup_chain->next;
+
   tp = find_thread_ptid (old->inferior_ptid);
   if (tp)
     tp->refcount--;
@@ -1349,6 +1382,9 @@ make_cleanup_restore_current_thread (void)
   old->inf_id = current_inferior ()->num;
   old->was_removable = current_inferior ()->removable;
 
+  old->next = current_thread_cleanup_chain;
+  current_thread_cleanup_chain = old;
+
   if (!ptid_equal (inferior_ptid, null_ptid))
     {
       old->was_stopped = is_stopped (inferior_ptid);
@@ -1803,4 +1839,6 @@ Show printing of thread events (such as thread start and exit)."), NULL,
          &setprintlist, &showprintlist);
 
   create_internalvar_type_lazy ("_thread", &thread_funcs, NULL);
+
+  observer_attach_thread_ptid_changed (restore_current_thread_ptid_changed);
 }
-- 
1.9.3

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 09/17] Misc switch_back_to_stepped_thread cleanups
  2015-04-01 22:14 [PATCH 00/17] All-stop on top of non-stop Pedro Alves
                   ` (7 preceding siblings ...)
  2015-04-01 22:14 ` [PATCH 04/17] Change adjust_pc_after_break's prototype Pedro Alves
@ 2015-04-01 22:14 ` Pedro Alves
  2015-04-01 22:24 ` [PATCH 07/17] Embed the pending step-over chain in thread_info objects Pedro Alves
                   ` (8 subsequent siblings)
  17 siblings, 0 replies; 26+ messages in thread
From: Pedro Alves @ 2015-04-01 22:14 UTC (permalink / raw)
  To: gdb-patches

Several misc cleanups that prepare the tail end of this function, the
part that actually re-resumes the stepped thread.  The most
non-obvious would be the currently_stepping change, I guess.  That's
because it isn't ever correct to pass step=1 to target_resume on
software single-step targets, and currently_stepping works at a
conceptual higher level, it returns step=true even on software step
targets.  It doesn't really matter on hardware step targets, as the
breakpoint will be hit immediately, but it's just wrong on software
step.  I wouldn't be surprised at all if this fixes problems on e.g.,
ARM today, but I haven't tested it yet (I'd need to find a box to test
on).  Help would be most welcome.

gdb/ChangeLog:
2015-04-01  Pedro Alves  <palves@redhat.com>

	* infrun.c (switch_back_to_stepped_thread): Use ecs->ptid instead
	pf inferior_ptid.  If the stepped thread vanished, return 0
	instead of resuming here.  Use reset_ecs.  Print the prev_pc and
	the current stop_pc in log message.  Clear trap_expected if the
	thread advanced.  Don't pass currently_stepping to
	do_target_resume.
---
 gdb/infrun.c | 33 ++++++++++++++++++---------------
 1 file changed, 18 insertions(+), 15 deletions(-)

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 45ebdaf..9c8d253 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -5680,7 +5680,7 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs)
         {
 	  /* Ignore threads of processes we're not resuming.  */
 	  if (!sched_multi
-	      && ptid_get_pid (tp->ptid) != ptid_get_pid (inferior_ptid))
+	      && ptid_get_pid (tp->ptid) != ptid_get_pid (ecs->ptid))
 	    continue;
 
 	  /* When stepping over a breakpoint, we lock all threads
@@ -5744,19 +5744,17 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs)
 				    "stepped thread, it has vanished\n");
 
 	      delete_thread (tp->ptid);
-	      keep_going (ecs);
-	      return 1;
+	      return 0;
 	    }
 
 	  if (debug_infrun)
 	    fprintf_unfiltered (gdb_stdlog,
 				"infrun: switching back to stepped thread\n");
 
-	  ecs->event_thread = tp;
-	  ecs->ptid = tp->ptid;
-	  context_switch (ecs->ptid);
+	  reset_ecs (ecs, tp);
+	  switch_to_thread (tp->ptid);
 
-	  stop_pc = regcache_read_pc (get_thread_regcache (ecs->ptid));
+	  stop_pc = regcache_read_pc (get_thread_regcache (tp->ptid));
 	  frame = get_current_frame ();
 	  gdbarch = get_frame_arch (frame);
 
@@ -5780,23 +5778,28 @@ switch_back_to_stepped_thread (struct execution_control_state *ecs)
 
 	      if (debug_infrun)
 		fprintf_unfiltered (gdb_stdlog,
-				    "infrun: expected thread advanced also\n");
+				    "infrun: expected thread advanced also "
+				    "(%s -> %s)\n",
+				    paddress (target_gdbarch (), tp->prev_pc),
+				    paddress (target_gdbarch (), stop_pc));
 
 	      /* Clear the info of the previous step-over, as it's no
-		 longer valid.  It's what keep_going would do too, if
-		 we called it.  Must do this before trying to insert
-		 the sss breakpoint, otherwise if we were previously
-		 trying to step over this exact address in another
-		 thread, the breakpoint ends up not installed.  */
+		 longer valid (if the thread was trying to step over a
+		 breakpoint, it has already succeeded).  It's what
+		 keep_going would do too, if we called it.  Do this
+		 before trying to insert the sss breakpoint, otherwise
+		 if we were previously trying to step over this exact
+		 address in another thread, the breakpoint is
+		 skipped.  */
 	      clear_step_over_info ();
+	      tp->control.trap_expected = 0;
 
 	      insert_single_step_breakpoint (get_frame_arch (frame),
 					     get_frame_address_space (frame),
 					     stop_pc);
 
 	      resume_ptid = user_visible_resume_ptid (tp->control.stepping_command);
-	      do_target_resume (resume_ptid,
-				currently_stepping (tp), GDB_SIGNAL_0);
+	      do_target_resume (resume_ptid, 0, GDB_SIGNAL_0);
 	      prepare_to_wait (ecs);
 	    }
 	  else
-- 
1.9.3

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 17/17] native Linux: enable always non-stop by default
  2015-04-01 22:14 [PATCH 00/17] All-stop on top of non-stop Pedro Alves
                   ` (2 preceding siblings ...)
  2015-04-01 22:14 ` [PATCH 03/17] Displaced stepping debug: fetch the right regcache Pedro Alves
@ 2015-04-01 22:14 ` Pedro Alves
  2015-04-01 22:14 ` [PATCH 01/17] Fix and test "checkpoint" in non-stop mode Pedro Alves
                   ` (13 subsequent siblings)
  17 siblings, 0 replies; 26+ messages in thread
From: Pedro Alves @ 2015-04-01 22:14 UTC (permalink / raw)
  To: gdb-patches

The testsuite shows no regressions with this forced on, so let's try
making it the default.

Tested on x86_64 Fedora 20, with and output "set displaced off"
forced.

gdb/ChangeLog:
2015-04-01  Pedro Alves  <palves@redhat.com>

	* linux-nat.c (linux_nat_always_non_stop_p): New function.
---
 gdb/linux-nat.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index d8001f9..4b08b31 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -4556,7 +4556,7 @@ linux_nat_supports_non_stop (struct target_ops *self)
 static int
 linux_nat_always_non_stop_p (struct target_ops *self)
 {
-  return 0;
+  return 1;
 }
 
 /* True if we want to support multi-process.  To be removed when GDB
-- 
1.9.3

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 04/17] Change adjust_pc_after_break's prototype
  2015-04-01 22:14 [PATCH 00/17] All-stop on top of non-stop Pedro Alves
                   ` (6 preceding siblings ...)
  2015-04-01 22:14 ` [PATCH 02/17] PR13858 - Can't do displaced stepping with no symbols Pedro Alves
@ 2015-04-01 22:14 ` Pedro Alves
  2015-04-01 22:14 ` [PATCH 09/17] Misc switch_back_to_stepped_thread cleanups Pedro Alves
                   ` (9 subsequent siblings)
  17 siblings, 0 replies; 26+ messages in thread
From: Pedro Alves @ 2015-04-01 22:14 UTC (permalink / raw)
  To: gdb-patches

Prepare to use it in contexts without an ecs handy.  Follow up patches
will make use of this.

gdb/ChangeLog:
2015-04-01  Pedro Alves  <pedro@codesourcery.com>

	* infrun.c (adjust_pc_after_break): Now takes thread_info and
	waitstatus pointers instead of an ecs.  Adjust.
	(handle_inferior_event): Adjust caller.
---
 gdb/infrun.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/gdb/infrun.c b/gdb/infrun.c
index f23e76e..1e3a3db 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -3420,8 +3420,14 @@ context_switch (ptid_t ptid)
   switch_to_thread (ptid);
 }
 
+/* If the target can't tell whether we've hit breakpoints
+   (target_supports_stopped_by_sw_breakpoint), and we got a SIGTRAP,
+   check whether that could have been caused by a breakpoint.  If so,
+   adjust the PC, per gdbarch_decr_pc_after_break.  */
+
 static void
-adjust_pc_after_break (struct execution_control_state *ecs)
+adjust_pc_after_break (struct thread_info *thread,
+		       struct target_waitstatus *ws)
 {
   struct regcache *regcache;
   struct gdbarch *gdbarch;
@@ -3449,10 +3455,10 @@ adjust_pc_after_break (struct execution_control_state *ecs)
      target with both of these set in GDB history, and it seems unlikely to be
      correct, so gdbarch_have_nonsteppable_watchpoint is not checked here.  */
 
-  if (ecs->ws.kind != TARGET_WAITKIND_STOPPED)
+  if (ws->kind != TARGET_WAITKIND_STOPPED)
     return;
 
-  if (ecs->ws.value.sig != GDB_SIGNAL_TRAP)
+  if (ws->value.sig != GDB_SIGNAL_TRAP)
     return;
 
   /* In reverse execution, when a breakpoint is hit, the instruction
@@ -3498,7 +3504,7 @@ adjust_pc_after_break (struct execution_control_state *ecs)
 
   /* If this target does not decrement the PC after breakpoints, then
      we have nothing to do.  */
-  regcache = get_thread_regcache (ecs->ptid);
+  regcache = get_thread_regcache (thread->ptid);
   gdbarch = get_regcache_arch (regcache);
 
   decr_pc = gdbarch_decr_pc_after_break (gdbarch);
@@ -3552,10 +3558,10 @@ adjust_pc_after_break (struct execution_control_state *ecs)
 	 software breakpoint.  In this case (prev_pc == breakpoint_pc),
 	 we also need to back up to the breakpoint address.  */
 
-      if (thread_has_single_step_breakpoints_set (ecs->event_thread)
-	  || !currently_stepping (ecs->event_thread)
-	  || (ecs->event_thread->stepped_breakpoint
-	      && ecs->event_thread->prev_pc == breakpoint_pc))
+      if (thread_has_single_step_breakpoints_set (thread)
+	  || !currently_stepping (thread)
+	  || (thread->stepped_breakpoint
+	      && thread->prev_pc == breakpoint_pc))
 	regcache_write_pc (regcache, breakpoint_pc);
 
       do_cleanups (old_cleanups);
@@ -3736,7 +3742,7 @@ handle_inferior_event (struct execution_control_state *ecs)
     }
 
   /* Dependent on valid ECS->EVENT_THREAD.  */
-  adjust_pc_after_break (ecs);
+  adjust_pc_after_break (ecs->event_thread, &ecs->ws);
 
   /* Dependent on the current PC value modified by adjust_pc_after_break.  */
   reinit_frame_cache ();
-- 
1.9.3

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 00/17] All-stop on top of non-stop
@ 2015-04-01 22:14 Pedro Alves
  2015-04-01 22:14 ` [PATCH 10/17] Factor out code to re-resume stepped thread Pedro Alves
                   ` (17 more replies)
  0 siblings, 18 replies; 26+ messages in thread
From: Pedro Alves @ 2015-04-01 22:14 UTC (permalink / raw)
  To: gdb-patches

(Eli, documentation changes in patch #12.)

This series implements user-visible all-stop mode running with the
target_ops backend always in non-stop mode.  This is a stepping stone
towards finer-grained control of threads, being able to do interesting
things like inferior and thread groups/sets (itsets), associating
groups with breakpoints (which threads cause a stop, which threads are
implicitly paused when a breakpoint triggers), etc.  From the user's
perspective, all-stop mode is really just a special case of being able
to stop and resume specific sets of threads, so it makes sense to do
this step first.

After this series, even in all-stop mode ("set non-stop off", the
default) the target is no longer in charge of stopping all threads
before reporting an event to the core -- the core takes care of it
when it sees fit.  For example, when "next"- or "step"-ing, we can
avoid stopping and resuming all threads at each internal single-step,
and instead only stop all threads when we're about to present the stop
to the user.

In order to get there, the series teaches GDB to do non-stop mode even
without displaced stepping (stop all threads, step over breakpoint,
restart threads), as displaced stepping isn't implemented everywhere,
and doesn't work in some scenarios (as in, we can't use it, not that
it's buggy).

And then it fixes all testsuite regressions (on native x86-64
GNU/Linux) this new mode causes, compared to all-stop ("set non-stop
off") with the target backend in all-stop mode too (i.e., the current
default).  Making "target remote" work in always non-stop mode is
deferred for later (this will be even more useful for remote as the
all-stop mode RSP variant can't really do asynchronous debugging).

Tested on x86_64 Fedora 20, native, with and without "set displaced
off", and with and without "maint set target-non-stop on"; and also
against gdbserver.

I've pushed this to users/palves/all-stop-non-stop for convenience.

Pedro Alves (17):
  Fix and test "checkpoint" in non-stop mode
  PR13858 - Can't do displaced stepping with no symbols
  Displaced stepping debug: fetch the right regcache
  Change adjust_pc_after_break's prototype
  remote.c/all-stop: Implement TARGET_WAITKIND_NO_RESUMED and
    TARGET_WNOHANG
  Make thread_still_needs_step_over consider stepping_over_watchpoint
    too
  Embed the pending step-over chain in thread_info objects
  Use keep_going in proceed and start_step_over too
  Misc switch_back_to_stepped_thread cleanups
  Factor out code to re-resume stepped thread
  Teach non-stop to do in-line step-overs (stop all, step, restart)
  Implement all-stop on top of a target running non-stop mode
  Fix signal-while-stepping-over-bp-other-thread.exp on targets always
    in non-stop
  Fix interrupt-noterm.exp on targets always in non-stop
  Fix
    step-over-trips-on-watchpoint.exp/step-over-lands-on-breakpoint.exp
    race
  Disable displaced stepping if trying it fails
  native Linux: enable always non-stop by default

 gdb/breakpoint.c                                   |   11 +-
 gdb/darwin-nat.c                                   |    6 +-
 gdb/doc/gdb.texinfo                                |   23 +
 gdb/gdbthread.h                                    |   57 +-
 gdb/gnu-nat.c                                      |    7 -
 gdb/inf-ptrace.c                                   |    6 +-
 gdb/infcmd.c                                       |    6 +-
 gdb/inferior.h                                     |    8 +
 gdb/infrun.c                                       | 2122 +++++++++++++++-----
 gdb/infrun.h                                       |    3 +
 gdb/linux-nat.c                                    |   33 +-
 gdb/monitor.c                                      |    4 +-
 gdb/nto-procfs.c                                   |   14 +-
 gdb/procfs.c                                       |    4 +-
 gdb/remote-m32r-sdi.c                              |    6 +-
 gdb/remote-sim.c                                   |   20 +-
 gdb/remote.c                                       |   57 +-
 gdb/target-delegates.c                             |   59 +
 gdb/target.c                                       |   83 +
 gdb/target.h                                       |   24 +-
 gdb/target/waitstatus.h                            |    5 +-
 gdb/testsuite/gdb.base/checkpoint-ns.exp           |   27 +
 gdb/testsuite/gdb.base/checkpoint.exp              |   31 +-
 gdb/testsuite/gdb.base/step-over-no-symbols.exp    |   88 +
 gdb/testsuite/gdb.base/valgrind-disp-step.c        |   32 +
 gdb/testsuite/gdb.base/valgrind-disp-step.exp      |  116 ++
 .../signal-while-stepping-over-bp-other-thread.exp |   20 +-
 .../gdb.threads/step-over-lands-on-breakpoint.c    |   17 +-
 .../gdb.threads/step-over-lands-on-breakpoint.exp  |    6 +-
 .../gdb.threads/step-over-trips-on-watchpoint.c    |   17 +-
 .../gdb.threads/step-over-trips-on-watchpoint.exp  |    8 +-
 gdb/thread.c                                       |  206 +-
 gdb/windows-nat.c                                  |    6 +-
 33 files changed, 2451 insertions(+), 681 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/checkpoint-ns.exp
 create mode 100644 gdb/testsuite/gdb.base/step-over-no-symbols.exp
 create mode 100644 gdb/testsuite/gdb.base/valgrind-disp-step.c
 create mode 100644 gdb/testsuite/gdb.base/valgrind-disp-step.exp

-- 
1.9.3

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 07/17] Embed the pending step-over chain in thread_info objects
  2015-04-01 22:14 [PATCH 00/17] All-stop on top of non-stop Pedro Alves
                   ` (8 preceding siblings ...)
  2015-04-01 22:14 ` [PATCH 09/17] Misc switch_back_to_stepped_thread cleanups Pedro Alves
@ 2015-04-01 22:24 ` Pedro Alves
  2015-04-01 22:40 ` [PATCH 06/17] Make thread_still_needs_step_over consider stepping_over_watchpoint too Pedro Alves
                   ` (7 subsequent siblings)
  17 siblings, 0 replies; 26+ messages in thread
From: Pedro Alves @ 2015-04-01 22:24 UTC (permalink / raw)
  To: gdb-patches

In order to teach non-stop mode to do in-line step-overs (pause all
threads, remove breakpoint, single-step, reinsert breakpoint, restart
threads), we'll need to be able to queue in-line step over requests,
much like we queue displaced stepping (out-of-line) requests.
Actually, the queue should be the same -- threads wait for their turn
to step past something (breakpoint, watchpoint), doesn't matter what
technique we end up using when the step over actually starts.

I found that the queue management ends up simpler and more efficient
if embedded in the thread objects themselves.  This commit converts
the existing displaced stepping queue to that.  Later patches will
make the in-line step-overs code paths use it too.

gdb/ChangeLog:
2015-04-01  Pedro Alves  <palves@redhat.com>

	* gdbthread.h (struct thread_info) <step_over_prev,
	step_over_next>: New fields.
	(thread_step_over_chain_enqueue, thread_step_over_chain_remove)
	(inferior_step_over_chain_remove_all, step_over_chain_dequeue):
	New declarations.
	* inferior.h (struct inferior) <step_over_queue_head>: New field.
	* infrun.c (struct displaced_step_request): Delete.
	(struct displaced_step_inferior_state) <step_request_queue>:
	Delete field.
	(displaced_step_in_progress): New function.
	(displaced_step_prepare): Assert that trap_expected is set.  Use
	thread_step_over_chain_enqueue.  Split starting a new displaced
	stepping to ...
	(start_step_over_inferior): ... this new function.
	(start_step_over): New function.
	(infrun_thread_ptid_changed): Delete references to the old
	displaced step request queue.
	(proceed): Assert the thread isn't waiting for a step over
	already.
	(infrun_thread_stop_requested): Adjust to remove threads from the
	embedded step-over chain.
	(handle_inferior_event) <fork/vfork>: Call start_step_over after
	displaced_step_fixup.
	(handle_signal_stop): Call start_step_over after
	displaced_step_fixup.
	* thread.c (step_over_chain_enqueue, step_over_chain_remove)
	(step_over_chain_dequeue, thread_step_over_chain_enqueue)
	(thread_step_over_chain_remove)
	(inferior_step_over_chain_remove_all): New function.
	(delete_thread_1): Remove thread from the step-over chain.
---
 gdb/gdbthread.h |  20 ++++++++
 gdb/inferior.h  |   4 ++
 gdb/infrun.c    | 144 ++++++++++++++++++++++++++++++++------------------------
 gdb/thread.c    |  93 ++++++++++++++++++++++++++++++++++++
 4 files changed, 200 insertions(+), 61 deletions(-)

diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index bb15717..e654432 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -285,6 +285,10 @@ struct thread_info
   /* Values that are stored as temporaries on stack while evaluating
      expressions.  */
   value_vec *stack_temporaries;
+
+  /* Step-over chain.  */
+  struct thread_info *step_over_prev;
+  struct thread_info *step_over_next;
 };
 
 /* Create an empty thread list, or empty the existing one.  */
@@ -498,6 +502,22 @@ extern struct value *get_last_thread_stack_temporary (ptid_t);
 
 extern int value_in_thread_stack_temporaries (struct value *, ptid_t);
 
+/* Add TP to the end of its inferior's pending step-over chain.  */
+
+extern void thread_step_over_chain_enqueue (struct thread_info *tp);
+
+/* Remove TP from its inferior's pending step-over chain.  */
+
+extern void thread_step_over_chain_remove (struct thread_info *tp);
+
+/* Remove all threads from inferior INF's pending step-over chain.  */
+
+extern void inferior_step_over_chain_remove_all (struct inferior *inf);
+
+/* Remove the head of LIST_P, a pending step-over chain.  */
+
+extern void step_over_chain_dequeue (struct thread_info **list_p);
+
 extern struct thread_info *thread_list;
 
 #endif /* GDBTHREAD_H */
diff --git a/gdb/inferior.h b/gdb/inferior.h
index 2530777..3cec101 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -410,6 +410,10 @@ struct inferior
      this gdbarch.  */
   struct gdbarch *gdbarch;
 
+  /* The queue of this inferior's threads that need to do a step-over
+     operation to get past e.g., a breakpoint.  */
+  struct thread_info *step_over_queue_head;
+
   /* Per inferior data-pointers required by other GDB modules.  */
   REGISTRY_FIELDS;
 };
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 5034261..2a6f422 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -1406,12 +1406,6 @@ step_over_info_valid_p (void)
    displaced step operation on it.  See displaced_step_prepare and
    displaced_step_fixup for details.  */
 
-struct displaced_step_request
-{
-  ptid_t ptid;
-  struct displaced_step_request *next;
-};
-
 /* Per-inferior displaced stepping state.  */
 struct displaced_step_inferior_state
 {
@@ -1421,10 +1415,6 @@ struct displaced_step_inferior_state
   /* The process this displaced step state refers to.  */
   int pid;
 
-  /* A queue of pending displaced stepping requests.  One entry per
-     thread that needs to do a displaced step.  */
-  struct displaced_step_request *step_request_queue;
-
   /* If this is not null_ptid, this is the thread carrying out a
      displaced single-step in process PID.  This thread's state will
      require fixing up once it has completed its step.  */
@@ -1465,6 +1455,22 @@ get_displaced_stepping_state (int pid)
   return NULL;
 }
 
+/* Return true if process PID has a thread doing a displaced step.  */
+
+static int
+displaced_step_in_progress (int pid)
+{
+  struct displaced_step_inferior_state *displaced;
+
+  /* Don't start a new step-over if we already have a displaced step
+     operation ongoing.  */
+  displaced = get_displaced_stepping_state (pid);
+  if (displaced != NULL && !ptid_equal (displaced->step_ptid, null_ptid))
+    return 1;
+
+  return 0;
+}
+
 /* Add a new displaced stepping state for process PID to the displaced
    stepping state list, or return a pointer to an already existing
    entry, if it already exists.  Never returns NULL.  */
@@ -1660,6 +1666,9 @@ displaced_step_prepare (ptid_t ptid)
      support displaced stepping.  */
   gdb_assert (gdbarch_displaced_step_copy_insn_p (gdbarch));
 
+  /* Nor if the thread isn't meant to step over a breakpoint.  */
+  gdb_assert (tp->control.trap_expected);
+
   /* Disable range stepping while executing in the scratch pad.  We
      want a single-step even if executing the displaced instruction in
      the scratch buffer lands within the stepping range (e.g., a
@@ -1675,28 +1684,13 @@ displaced_step_prepare (ptid_t ptid)
     {
       /* Already waiting for a displaced step to finish.  Defer this
 	 request and place in queue.  */
-      struct displaced_step_request *req, *new_req;
 
       if (debug_displaced)
 	fprintf_unfiltered (gdb_stdlog,
-			    "displaced: defering step of %s\n",
+			    "displaced: deferring step of %s\n",
 			    target_pid_to_str (ptid));
 
-      new_req = xmalloc (sizeof (*new_req));
-      new_req->ptid = ptid;
-      new_req->next = NULL;
-
-      if (displaced->step_request_queue)
-	{
-	  for (req = displaced->step_request_queue;
-	       req && req->next;
-	       req = req->next)
-	    ;
-	  req->next = new_req;
-	}
-      else
-	displaced->step_request_queue = new_req;
-
+      thread_step_over_chain_enqueue (tp);
       return 0;
     }
   else
@@ -1842,24 +1836,34 @@ displaced_step_fixup (ptid_t event_ptid, enum gdb_signal signal)
   do_cleanups (old_cleanups);
 
   displaced->step_ptid = null_ptid;
+}
 
-  /* Are there any pending displaced stepping requests?  If so, run
-     one now.  Leave the state object around, since we're likely to
-     need it again soon.  */
-  while (displaced->step_request_queue)
+/* Are there any pending step-over requests for INF?  */
+
+static void
+start_step_over_inferior (struct inferior *inf)
+{
+  /* Don't start a new step-over if we already have a displaced step
+     operation ongoing.  */
+  if (displaced_step_in_progress (inf->pid))
+    return;
+
+  while (inf->step_over_queue_head != NULL)
     {
-      struct displaced_step_request *head;
       ptid_t ptid;
+      struct displaced_step_inferior_state *displaced;
       struct regcache *regcache;
       struct gdbarch *gdbarch;
       CORE_ADDR actual_pc;
       struct address_space *aspace;
+      struct thread_info *tp;
+
+      tp = inf->step_over_queue_head;
+      displaced = get_displaced_stepping_state (inf->pid);
 
-      head = displaced->step_request_queue;
-      ptid = head->ptid;
-      displaced->step_request_queue = head->next;
-      xfree (head);
+      step_over_chain_dequeue (&inf->step_over_queue_head);
 
+      ptid = tp->ptid;
       context_switch (ptid);
 
       regcache = get_thread_regcache (ptid);
@@ -1921,6 +1925,25 @@ displaced_step_fixup (ptid_t event_ptid, enum gdb_signal signal)
 	     thread waiting for its turn.  */
 	}
     }
+
+  if (inf->step_over_queue_head == NULL)
+    {
+      if (debug_infrun)
+	fprintf_unfiltered (gdb_stdlog,
+			    "infrun: step-over queue of process %d now empty\n",
+			    inf->pid);
+    }
+}
+
+/* Are there any pending step-over requests for the inferior of
+   EVENT_PTID?  */
+
+static void
+start_step_over (ptid_t event_ptid)
+{
+  struct inferior *inf = find_inferior_ptid (event_ptid);
+
+  start_step_over_inferior (inf);
 }
 
 /* Update global variables holding ptids to hold NEW_PTID if they were
@@ -1940,10 +1963,6 @@ infrun_thread_ptid_changed (ptid_t old_ptid, ptid_t new_ptid)
     {
       if (ptid_equal (displaced->step_ptid, old_ptid))
 	displaced->step_ptid = new_ptid;
-
-      for (it = displaced->step_request_queue; it; it = it->next)
-	if (ptid_equal (it->ptid, old_ptid))
-	  it->ptid = new_ptid;
     }
 }
 
@@ -2634,6 +2653,9 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal)
   /* Fill in with reasonable starting values.  */
   init_thread_stepping_state (tp);
 
+  gdb_assert (tp->step_over_next == NULL);
+  gdb_assert (tp->step_over_prev == NULL);
+
   if (addr == (CORE_ADDR) -1)
     {
       if (pc == stop_pc
@@ -2941,35 +2963,31 @@ infrun_thread_stop_requested_callback (struct thread_info *info, void *arg)
 static void
 infrun_thread_stop_requested (ptid_t ptid)
 {
-  struct displaced_step_inferior_state *displaced;
-
   /* PTID was requested to stop.  Remove it from the displaced
      stepping queue, so we don't try to resume it automatically.  */
 
-  for (displaced = displaced_step_inferior_states;
-       displaced;
-       displaced = displaced->next)
+  if (ptid_equal (minus_one_ptid, ptid))
     {
-      struct displaced_step_request *it, **prev_next_p;
+      struct inferior *inf;
 
-      it = displaced->step_request_queue;
-      prev_next_p = &displaced->step_request_queue;
-      while (it)
+      ALL_INFERIORS (inf)
 	{
-	  if (ptid_match (it->ptid, ptid))
-	    {
-	      *prev_next_p = it->next;
-	      it->next = NULL;
-	      xfree (it);
-	    }
-	  else
-	    {
-	      prev_next_p = &it->next;
-	    }
-
-	  it = *prev_next_p;
+	  inferior_step_over_chain_remove_all (inf);
 	}
     }
+  else if (ptid_is_pid (ptid))
+    {
+      struct inferior *inf = find_inferior_ptid (ptid);
+
+      inferior_step_over_chain_remove_all (inf);
+    }
+  else
+    {
+      struct thread_info *tp = find_thread_ptid (ptid);
+
+      if (tp->step_over_next != NULL)
+	thread_step_over_chain_remove (tp);
+    }
 
   iterate_over_threads (infrun_thread_stop_requested_callback, &ptid);
 }
@@ -4020,6 +4038,9 @@ Cannot fill $_exitsignal with the correct signal number.\n"));
 	       that this operation also cleans up the child process for vfork,
 	       because their pages are shared.  */
 	    displaced_step_fixup (ecs->ptid, GDB_SIGNAL_TRAP);
+	    /* Start a new step-over in another thread if there's one
+	       that needs it.  */
+	    start_step_over (ecs->ptid);
 
 	    if (ecs->ws.kind == TARGET_WAITKIND_FORKED)
 	      {
@@ -4252,6 +4273,7 @@ handle_signal_stop (struct execution_control_state *ecs)
      the PC, so do it here, before we set stop_pc.)  */
   displaced_step_fixup (ecs->ptid,
 			ecs->event_thread->suspend.stop_signal);
+  start_step_over (ecs->ptid);
 
   /* If we either finished a single-step or hit a breakpoint, but
      the user wanted this thread to be stopped, pretend we got a
diff --git a/gdb/thread.c b/gdb/thread.c
index db631c9..d5c9896 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -307,6 +307,95 @@ add_thread (ptid_t ptid)
   return add_thread_with_info (ptid, NULL);
 }
 
+/* Add TP to the end of the step-over chain LIST_P.  */
+
+static void
+step_over_chain_enqueue (struct thread_info **list_p, struct thread_info *tp)
+{
+  gdb_assert (tp->step_over_next == NULL);
+  gdb_assert (tp->step_over_prev == NULL);
+
+  if (*list_p == NULL)
+    {
+      *list_p = tp;
+      tp->step_over_prev = tp->step_over_next = tp;
+    }
+  else
+    {
+      struct thread_info *head = *list_p;
+      struct thread_info *tail = head->step_over_prev;
+
+      tp->step_over_prev = tail;
+      tp->step_over_next = head;
+      head->step_over_prev = tp;
+      tail->step_over_next = tp;
+    }
+}
+
+/* Remove TP from step-over chain LIST_P.  */
+
+static void
+step_over_chain_remove (struct thread_info **list_p, struct thread_info *tp)
+{
+  gdb_assert (tp->step_over_next != NULL);
+  gdb_assert (tp->step_over_prev != NULL);
+
+  if (*list_p == tp)
+    {
+      if (tp == tp->step_over_next)
+	*list_p = NULL;
+      else
+	*list_p = tp->step_over_next;
+    }
+
+  tp->step_over_prev->step_over_next = tp->step_over_next;
+  tp->step_over_next->step_over_prev = tp->step_over_prev;
+  tp->step_over_prev = tp->step_over_next = NULL;
+}
+
+/* See gdbthread.h.  */
+
+void
+step_over_chain_dequeue (struct thread_info **list_p)
+{
+  step_over_chain_remove (list_p, *list_p);
+}
+
+/* See gdbthread.h.  */
+
+void
+thread_step_over_chain_enqueue (struct thread_info *tp)
+{
+  struct inferior *inf;
+
+  inf = find_inferior_ptid (tp->ptid);
+  gdb_assert (inf != NULL);
+
+  step_over_chain_enqueue (&inf->step_over_queue_head, tp);
+}
+
+/* See gdbthread.h.  */
+
+void
+thread_step_over_chain_remove (struct thread_info *tp)
+{
+  struct inferior *inf;
+
+  inf = find_inferior_ptid (tp->ptid);
+  gdb_assert (inf != NULL);
+
+  step_over_chain_remove (&inf->step_over_queue_head, tp);
+}
+
+/* See gdbthread.h.  */
+
+void
+inferior_step_over_chain_remove_all (struct inferior *inf)
+{
+  while (inf->step_over_queue_head != NULL)
+    step_over_chain_dequeue (&inf->step_over_queue_head);
+}
+
 /* Delete thread PTID.  If SILENT, don't notify the observer of this
    exit.  */
 static void
@@ -323,6 +412,10 @@ delete_thread_1 (ptid_t ptid, int silent)
   if (!tp)
     return;
 
+  /* Dead threads don't need to step-over.  Remove from queue.  */
+  if (tp->step_over_next != NULL)
+    thread_step_over_chain_remove (tp);
+
   /* If this is the current thread, or there's code out there that
      relies on it existing (refcount > 0) we can't delete yet.  Mark
      it as exited, and notify it.  */
-- 
1.9.3

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 15/17] Fix step-over-trips-on-watchpoint.exp/step-over-lands-on-breakpoint.exp race
  2015-04-01 22:14 [PATCH 00/17] All-stop on top of non-stop Pedro Alves
                   ` (10 preceding siblings ...)
  2015-04-01 22:40 ` [PATCH 06/17] Make thread_still_needs_step_over consider stepping_over_watchpoint too Pedro Alves
@ 2015-04-01 22:40 ` Pedro Alves
  2015-04-01 22:41 ` [PATCH 12/17] Implement all-stop on top of a target running non-stop mode Pedro Alves
                   ` (5 subsequent siblings)
  17 siblings, 0 replies; 26+ messages in thread
From: Pedro Alves @ 2015-04-01 22:40 UTC (permalink / raw)
  To: gdb-patches

On a target that is both always in non-stop mode and can do displaced
stepping (such as native GNU/Linux, with "maint set target-non-stop
on"), the step-over-trips-on-watchpoint.exp test sometimes fails like
this:

   (gdb) PASS: gdb.threads/step-over-trips-on-watchpoint.exp: no thread-specific bp: step: thread 1
   set scheduler-locking off
   (gdb) PASS: gdb.threads/step-over-trips-on-watchpoint.exp: no thread-specific bp: step: set scheduler-locking off
   step
  -[Switching to Thread 0x7ffff7fc0700 (LWP 11782)]
  -Hardware watchpoint 4: watch_me
  -
  -Old value = 0
  -New value = 1
  -child_function (arg=0x0) at /home/pedro/gdb/mygit/src/gdb/testsuite/gdb.threads/step-over-trips-on-watchpoint.c:39
  -39           other = 1; /* set thread-specific breakpoint here */
  -(gdb) PASS: gdb.threads/step-over-trips-on-watchpoint.exp: no thread-specific bp: step: step
  +wait_threads () at /home/pedro/gdb/mygit/src/gdb/testsuite/gdb.threads/step-over-trips-on-watchpoint.c:49
  +49       return 1; /* in wait_threads */
  +(gdb) FAIL: gdb.threads/step-over-trips-on-watchpoint.exp: no thread-specific bp: step: step

Note "scheduler-locking" was set off.  The problem is that on such
targets, the step-over of thread 2 and the "step" of thread 1 can be
set to run simultaneously (since with displaced stepping the
breakpoint isn't ever removed from the target), and sometimes, the
"step" of thread 1 finishes first, so it'd take another resume to see
the watchpoint trigger.  Fix this by replacing the wait_threads
function with a one-line infinite loop that doesn't call any function,
so that the "step" of thread 1 never finishes.

gdb/testsuite/ChangeLog:
2015-04-01  Pedro Alves  <palves@redhat.com>

	* gdb.threads/step-over-lands-on-breakpoint.c (wait_threads):
	Delete function.
	(main): Add alarm.  Run an infinite loop instead of calling
	wait_threads.
	* gdb.threads/step-over-lands-on-breakpoint.exp (do_test): Change
	comment.
	* gdb.threads/step-over-trips-on-watchpoint.c (wait_threads):
	Delete function.
	(main): Add alarm.  Run an infinite loop instead of calling
	wait_threads.
	* gdb.threads/step-over-trips-on-watchpoint.exp (do_test): Change
	comment.
---
 .../gdb.threads/step-over-lands-on-breakpoint.c         | 17 ++++++++++-------
 .../gdb.threads/step-over-lands-on-breakpoint.exp       |  6 +++---
 .../gdb.threads/step-over-trips-on-watchpoint.c         | 17 ++++++++++-------
 .../gdb.threads/step-over-trips-on-watchpoint.exp       |  8 ++++----
 4 files changed, 27 insertions(+), 21 deletions(-)

diff --git a/gdb/testsuite/gdb.threads/step-over-lands-on-breakpoint.c b/gdb/testsuite/gdb.threads/step-over-lands-on-breakpoint.c
index 0a6ed8f..2480164 100644
--- a/gdb/testsuite/gdb.threads/step-over-lands-on-breakpoint.c
+++ b/gdb/testsuite/gdb.threads/step-over-lands-on-breakpoint.c
@@ -41,23 +41,26 @@ child_function (void *arg)
   pthread_exit (NULL);
 }
 
-static int
-wait_threads (void)
-{
-  return 1; /* in wait_threads */
-}
-
 int
 main ()
 {
   int res;
   long i;
 
+  alarm (300);
+
   pthread_barrier_init (&barrier, NULL, 2);
 
   res = pthread_create (&child_thread, NULL, child_function, NULL);
   pthread_barrier_wait (&barrier);
-  wait_threads (); /* set wait-thread breakpoint here */
+
+  /* Use an infinite loop with no function calls so that "step" over
+     this line never finishes before the breakpoint in the other
+     thread triggers.  That can happen if the step-over of thread 2 is
+     done with displaced stepping on a target that is always in
+     non-stop mode, as in that case GDB runs both threads
+     simultaneously.  */
+  while (1); /* set wait-thread breakpoint here */
 
   pthread_join (child_thread, NULL);
 
diff --git a/gdb/testsuite/gdb.threads/step-over-lands-on-breakpoint.exp b/gdb/testsuite/gdb.threads/step-over-lands-on-breakpoint.exp
index 1e12314..8ac741f 100644
--- a/gdb/testsuite/gdb.threads/step-over-lands-on-breakpoint.exp
+++ b/gdb/testsuite/gdb.threads/step-over-lands-on-breakpoint.exp
@@ -54,9 +54,9 @@ foreach command {"step" "next" "continue" } {
 	gdb_test_no_output "set scheduler-locking off"
 
 	# Thread 2 is still stopped at a breakpoint that needs to be
-	# stepped over before proceeding thread 1.  However, right
-	# where the step-over lands there's another breakpoint
-	# installed, which should trap and be reported to the user.
+	# stepped over.  However, right where the step-over lands
+	# there's another breakpoint installed, which should trap and
+	# be reported to the user.
 	gdb_test "$command" "step-over here.*"
     }
 }
diff --git a/gdb/testsuite/gdb.threads/step-over-trips-on-watchpoint.c b/gdb/testsuite/gdb.threads/step-over-trips-on-watchpoint.c
index 13404da..d6d59de 100644
--- a/gdb/testsuite/gdb.threads/step-over-trips-on-watchpoint.c
+++ b/gdb/testsuite/gdb.threads/step-over-trips-on-watchpoint.c
@@ -43,23 +43,26 @@ child_function (void *arg)
   pthread_exit (NULL);
 }
 
-static int
-wait_threads (void)
-{
-  return 1; /* in wait_threads */
-}
-
 int
 main ()
 {
   int res;
   long i;
 
+  alarm (300);
+
   pthread_barrier_init (&barrier, NULL, 2);
 
   res = pthread_create (&child_thread, NULL, child_function, NULL);
   pthread_barrier_wait (&barrier);
-  wait_threads (); /* set wait-thread breakpoint here */
+
+  /* Use an infinite loop with no function calls so that "step" over
+     this line never finishes before the watchpoint in the other
+     thread triggers.  That can happen if the step-over of thread 2 is
+     done with displaced stepping on a target that is always in
+     non-stop mode, as in that case GDB runs both threads
+     simultaneously.  */
+  while (1); /* set wait-thread breakpoint here */
 
   pthread_join (child_thread, NULL);
 
diff --git a/gdb/testsuite/gdb.threads/step-over-trips-on-watchpoint.exp b/gdb/testsuite/gdb.threads/step-over-trips-on-watchpoint.exp
index b8fc6a8..f40ef0a 100644
--- a/gdb/testsuite/gdb.threads/step-over-trips-on-watchpoint.exp
+++ b/gdb/testsuite/gdb.threads/step-over-trips-on-watchpoint.exp
@@ -76,10 +76,10 @@ proc do_test { with_bp } {
 		gdb_test "thread 1" "Switching to .*"
 		gdb_test_no_output "set scheduler-locking off"
 
-		# Thread 2 is still stopped at a breakpoint that needs to be
-		# stepped over before proceeding thread 1.  However, right
-		# where the step-over lands there's another breakpoint
-		# installed, which should trap and be reported to the user.
+		# Thread 2 is still stopped at a breakpoint that needs
+		# to be stepped over.  However, the instruction that
+		# is under the breakpoint triggers a watchpoint, which
+		# should trap and be reported to the user.
 		gdb_test "$command" "Hardware watchpoint.*: watch_me.*New value = 1.*"
 	    }
 	}
-- 
1.9.3

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 06/17] Make thread_still_needs_step_over consider stepping_over_watchpoint too
  2015-04-01 22:14 [PATCH 00/17] All-stop on top of non-stop Pedro Alves
                   ` (9 preceding siblings ...)
  2015-04-01 22:24 ` [PATCH 07/17] Embed the pending step-over chain in thread_info objects Pedro Alves
@ 2015-04-01 22:40 ` Pedro Alves
  2015-04-01 22:40 ` [PATCH 15/17] Fix step-over-trips-on-watchpoint.exp/step-over-lands-on-breakpoint.exp race Pedro Alves
                   ` (6 subsequent siblings)
  17 siblings, 0 replies; 26+ messages in thread
From: Pedro Alves @ 2015-04-01 22:40 UTC (permalink / raw)
  To: gdb-patches

I noticed that even though keep_going knows to start a step over for a
watchpoint, thread_still_needs_step_over doesn't.

gdb/ChangeLog:
2015-04-01  Pedro Alves  <palves@redhat.com>

	* infrun.c (thread_still_needs_step_over): Rename to ...
	(thread_still_needs_step_over_bp): ... this.
	(enum step_over_what): New.
	(thread_still_needs_step_over): Reimplement.
---
 gdb/infrun.c | 46 ++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 38 insertions(+), 8 deletions(-)

diff --git a/gdb/infrun.c b/gdb/infrun.c
index 1e3a3db..5034261 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -2491,7 +2491,7 @@ clear_proceed_status (int step)
    meanwhile, we can skip the whole step-over dance.  */
 
 static int
-thread_still_needs_step_over (struct thread_info *tp)
+thread_still_needs_step_over_bp (struct thread_info *tp)
 {
   if (tp->stepping_over_breakpoint)
     {
@@ -2508,6 +2508,34 @@ thread_still_needs_step_over (struct thread_info *tp)
   return 0;
 }
 
+/* Bit flags indicating what the thread needs to step over.  */
+
+enum step_over_what
+  {
+    STEP_OVER_BREAKPOINT = 1,
+    STEP_OVER_WATCHPOINT = 2
+  };
+
+/* Check whether thread TP still needs to start a step-over in order
+   to make progress when resumed.  Returns a union of enum
+   step_over_what bits, indicating what needs to be stepped over.  */
+
+static int
+thread_still_needs_step_over (struct thread_info *tp)
+{
+  struct inferior *inf = find_inferior_ptid (tp->ptid);
+  int what = 0;
+
+  if (thread_still_needs_step_over_bp (tp))
+    what |= STEP_OVER_BREAKPOINT;
+
+  if (tp->stepping_over_watchpoint
+      && !target_have_steppable_watchpoint)
+    what |= STEP_OVER_WATCHPOINT;
+
+  return what;
+}
+
 /* Returns true if scheduler locking applies.  STEP indicates whether
    we're about to do a step/next-like command to a thread.  */
 
@@ -6244,6 +6272,7 @@ keep_going (struct execution_control_state *ecs)
       int remove_bp;
       int remove_wps;
       enum gdb_signal signo;
+      enum step_over_what step_what;
 
       /* Either the trap was not expected, but we are continuing
 	 anyway (if we got a signal, the user asked it be passed to
@@ -6264,11 +6293,6 @@ keep_going (struct execution_control_state *ecs)
 	 instruction, and then re-insert the breakpoint when that step
 	 is finished.  */
 
-      remove_bp = (ecs->hit_singlestep_breakpoint
-		   || thread_still_needs_step_over (ecs->event_thread));
-      remove_wps = (ecs->event_thread->stepping_over_watchpoint
-		    && !target_have_steppable_watchpoint);
-
       /* Do not deliver GDB_SIGNAL_TRAP (except when the user
 	 explicitly specifies that such a signal should be delivered
 	 to the target program).  Typically, that would occur when a
@@ -6285,6 +6309,12 @@ keep_going (struct execution_control_state *ecs)
 
       signo = ecs->event_thread->suspend.stop_signal;
 
+      step_what = thread_still_needs_step_over (ecs->event_thread);
+
+      remove_bp = (ecs->hit_singlestep_breakpoint
+		   || (step_what & STEP_OVER_BREAKPOINT));
+      remove_wps = (step_what & STEP_OVER_WATCHPOINT);
+
       if (remove_bp
 	  && !use_displaced_stepping_now_p (get_regcache_arch (regcache),
 					    signo))
@@ -6297,6 +6327,8 @@ keep_going (struct execution_control_state *ecs)
       else
 	clear_step_over_info ();
 
+      ecs->event_thread->control.trap_expected = (remove_bp || remove_wps);
+
       /* Stop stepping if inserting breakpoints fails.  */
       TRY
 	{
@@ -6311,8 +6343,6 @@ keep_going (struct execution_control_state *ecs)
 	}
       END_CATCH
 
-      ecs->event_thread->control.trap_expected = (remove_bp || remove_wps);
-
       discard_cleanups (old_cleanups);
       resume (ecs->event_thread->suspend.stop_signal);
     }
-- 
1.9.3

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 12/17] Implement all-stop on top of a target running non-stop mode
  2015-04-01 22:14 [PATCH 00/17] All-stop on top of non-stop Pedro Alves
                   ` (11 preceding siblings ...)
  2015-04-01 22:40 ` [PATCH 15/17] Fix step-over-trips-on-watchpoint.exp/step-over-lands-on-breakpoint.exp race Pedro Alves
@ 2015-04-01 22:41 ` Pedro Alves
  2015-04-02 14:58   ` Eli Zaretskii
  2015-04-01 23:06 ` [PATCH 16/17] Disable displaced stepping if trying it fails Pedro Alves
                   ` (4 subsequent siblings)
  17 siblings, 1 reply; 26+ messages in thread
From: Pedro Alves @ 2015-04-01 22:41 UTC (permalink / raw)
  To: gdb-patches

This finally implements user-visible all-stop mode running with the
target_ops backend always in non-stop mode.  This is a stepping stone
towards finer-grained control of threads, being able to do interesting
things like thread groups, associating groups with breakpoints, etc.
From the user's perspective, all-stop mode is really just a special
case of being able to stop and resume specific sets of threads, so it
makes sense to do this step first.

With this, even in all-stop, the target is no longer in charge of
stopping all threads before reporting an event to the core -- the core
takes care of it when it sees fit.  For example, when "next"- or
"step"-ing, we can avoid stopping and resuming all threads at each
internal single-step, and instead only stop all threads when we're
about to present the stop to the user.

The implementation is almost straight forward, as the heavy lifting
has been done already in previous patches.  Basically, we replace
checks for "set non-stop on/off" (the non_stop global), with calls to
a new target_is_non_stop_p function.  In a few places, if "set
non-stop off", we stop all threads explicitly, and in a few other
places we resume all threads explicitly, making use of existing
methods that were added for teaching non-stop to step over breakpoints
without displaced stepping.

This adds a new "maint set target-non-stop on/off/auto" knob that
allows both disabling the feature if we find problems, and
force-enable it for development (useful when teaching a target about
this.  The default is "auto", which means the feature is enabled if a
new target method says it should be enabled.  The patch implements the
method in linux-nat.c, just for illustration, because it still returns
false.  We'll need a few follow up fixes before turning it on by
default.  This is a separate target method from indicating regular
non-stop support, because e.g., while e.g., native linux-nat.c is
close to regression free with all-stop-non-stop (with following
patches will fixing the remaining regressions), remote.c+gdbserver
will still need more fixing, even though it supports "set non-stop
on".

Tested on x86_64 Fedora 20, native, with and without "set displaced
off", and with and without "maint set target-non-stop on"; and also
against gdbserver.

gdb/ChangeLog:
2015-04-01  Pedro Alves  <palves@redhat.com>

	* breakpoint.c (update_global_location_list): Check
	target_is_non_stop_p instead of non_stop.
	* infcmd.c (attach_command_post_wait, attach_command): Likewise.
	* infrun.c (show_can_use_displaced_stepping)
	(can_use_displaced_stepping_p, start_step_over_inferior):
	Likewise.
	(resume): Always resume a single thread if the target is in
	non-stop mode.
	(proceed): Check target_is_non_stop_p instead of non_stop.  If in
	all-mode but the target is always in non-stop mode, start all the
	other threads that are implicitly resumed too.
	(for_each_just_stopped_thread, fetch_inferior_event)
	(adjust_pc_after_break, stop_all_threads): Check
	target_is_non_stop_p instead of non_stop.
	(handle_inferior_event): Likewise.  Handle detach-fork in all-stop
	with the target always in non-stop mode.
	(handle_signal_stop) <random signal>: If we get a signal while
	stepping over a breakpoint, and the target is always in non-stop
	mode, restart all threads.
	(switch_back_to_stepped_thread): Check target_is_non_stop_p
	instead of non_stop.
	(keep_going_stepped_thread): Always resume a single thread if the
	target is in non-stop mode.
	(stop_waiting): If in all-stop mode, and the target is in non-stop
	mode, stop all threads.
	(keep_going_pass): Likewise, when starting a new in-line step-over
	sequence.
	* linux-nat.c (get_pending_status, select_event_lwp)
	(linux_nat_filter_event, linux_nat_wait_1, linux_nat_wait): Check
	target_is_non_stop_p instead of non_stop.
	(linux_nat_always_non_stop_p): New function.
	(linux_nat_stop): Check target_is_non_stop_p instead of non_stop.
	(linux_nat_add_target): Install linux_nat_always_non_stop_p.
	* target-delegates.c: Regenerate.
	* target.c (target_is_non_stop_p): New function.
	(target_non_stop_enabled, target_non_stop_enabled_1): New globals.
	(maint_set_target_non_stop_command)
	(maint_show_target_non_stop_command): New functions.
	(_initilize_target): Install "maint set/show target-non-stop"
	commands.
	* target.h (struct target_ops) <to_always_non_stop_p>: New field.
	(target_non_stop_enabled): New declaration.
	(target_is_non_stop_p): New declaration.

gdb/doc/ChangeLog:
2015-04-01  Pedro Alves  <palves@redhat.com>

	* gdb.texinfo (Maintenance Commands): Document "maint set/show
	target-non-stop".
---
 gdb/breakpoint.c       |   2 +-
 gdb/doc/gdb.texinfo    |  23 +++++++++++
 gdb/infcmd.c           |   4 +-
 gdb/infrun.c           | 109 ++++++++++++++++++++++++++++++++++++++++---------
 gdb/linux-nat.c        |  25 ++++++++----
 gdb/target-delegates.c |  31 ++++++++++++++
 gdb/target.c           |  62 ++++++++++++++++++++++++++++
 gdb/target.h           |  13 ++++++
 8 files changed, 239 insertions(+), 30 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 76c86f8..641d7f4 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -12346,7 +12346,7 @@ update_global_location_list (enum ugll_insert_mode insert_mode)
 
       if (!found_object)
 	{
-	  if (removed && non_stop
+	  if (removed && target_is_non_stop_p ()
 	      && need_moribund_for_location_type (old_loc))
 	    {
 	      /* This location was removed from the target.  In
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index c6e9b9b..f2b2fec 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -34103,6 +34103,29 @@ asynchronous mode (@pxref{Background Execution}).  Normally the
 default is asynchronous, if it is available; but this can be changed
 to more easily debug problems occurring only in synchronous mode.
 
+@kindex maint set target-non-stop @var{mode} [on|off|auto]
+@kindex maint show target-non-stop
+@item maint set target-non-stop
+@itemx maint show target-non-stop
+This controls whether @value{GDBN} targets always operate in non-mode
+even if @code{set non-stop} is @code{off} (@pxref{Non-Stop Mode}).
+This default is @code{auto}, meaning this is enabled if available; but
+this can be changed to more easily debug problems.
+
+@table @code
+@item maint set target-non-stop auto
+This is the default mode.  @value{GDBN} controls the target in
+non-stop mode if the target supports it.
+
+@item maint set target-non-stop on
+@value{GDBN} controls the target in non-stop mode even if the target
+does not indicate support.
+
+@item maint set target-non-stop off
+@value{GDBN} does not control the target in non-stop mode even if the
+target supports it.
+@end table
+
 @kindex maint set per-command
 @kindex maint show per-command
 @item maint set per-command
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 1c70675..94d87214 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -2565,7 +2565,7 @@ attach_command_post_wait (char *args, int from_tty, int async_exec)
 	 selected thread is stopped, others may still be executing.
 	 Be sure to explicitly stop all threads of the process.  This
 	 should have no effect on already stopped threads.  */
-      if (non_stop)
+      if (target_is_non_stop_p ())
 	target_stop (pid_to_ptid (inferior->pid));
 
       /* Tell the user/frontend where we're stopped.  */
@@ -2670,7 +2670,7 @@ attach_command (char *args, int from_tty)
   init_wait_for_inferior ();
   clear_proceed_status (0);
 
-  if (non_stop)
+  if (target_is_non_stop_p ())
     {
       /* If we find that the current thread isn't stopped, explicitly
 	 do so now, because we're going to install breakpoints and
diff --git a/gdb/infrun.c b/gdb/infrun.c
index d95a60a..184622a 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -1595,7 +1595,7 @@ show_can_use_displaced_stepping (struct ui_file *file, int from_tty,
     fprintf_filtered (file,
 		      _("Debugger's willingness to use displaced stepping "
 			"to step over breakpoints is %s (currently %s).\n"),
-		      value, non_stop ? "on" : "off");
+		      value, target_is_non_stop_p () ? "on" : "off");
   else
     fprintf_filtered (file,
 		      _("Debugger's willingness to use displaced stepping "
@@ -1608,7 +1608,8 @@ show_can_use_displaced_stepping (struct ui_file *file, int from_tty,
 static int
 can_use_displaced_stepping_p (struct gdbarch *gdbarch)
 {
-  return (((can_use_displaced_stepping == AUTO_BOOLEAN_AUTO && non_stop)
+  return (((can_use_displaced_stepping == AUTO_BOOLEAN_AUTO
+	    && target_is_non_stop_p ())
 	   || can_use_displaced_stepping == AUTO_BOOLEAN_TRUE)
 	  && gdbarch_displaced_step_copy_insn_p (gdbarch)
 	  && find_record_target () == NULL);
@@ -1966,7 +1967,7 @@ start_step_over_inferior (struct inferior *inf)
 	 wouldn't be able to resume anything else until the target
 	 stops again.  In non-stop, the resume always resumes only TP,
 	 so it's OK to let the thread resume freely.  */
-      if (!non_stop && !thread_still_needs_step_over (tp))
+      if (!target_is_non_stop_p () && !thread_still_needs_step_over (tp))
 	continue;
 
       switch_to_thread (tp->ptid);
@@ -1991,7 +1992,7 @@ start_step_over_inferior (struct inferior *inf)
 	{
 	  /* In all-stop, we shouldn't have resumed unless we needed a
 	     step over.  */
-	  gdb_assert (non_stop);
+	  gdb_assert (target_is_non_stop_p ());
 	}
     }
 
@@ -2330,7 +2331,10 @@ resume (enum gdb_signal sig)
 	      insert_single_step_breakpoint (gdbarch, aspace, pc);
 	      insert_breakpoints ();
 
-	      resume_ptid = user_visible_resume_ptid (user_step);
+	      if (target_is_non_stop_p ())
+		resume_ptid = inferior_ptid;
+	      else
+		resume_ptid = user_visible_resume_ptid (user_step);
 	      do_target_resume (resume_ptid, 0, GDB_SIGNAL_0);
 	      discard_cleanups (old_cleanups);
 	      return;
@@ -2432,8 +2436,14 @@ resume (enum gdb_signal sig)
   resume_ptid = user_visible_resume_ptid (user_step);
 
   /* Maybe resume a single thread after all.  */
-  if ((step || thread_has_single_step_breakpoints_set (tp))
-      && tp->control.trap_expected)
+  if (target_is_non_stop_p ())
+    {
+      /* If non-stop mode, threads are always controlled
+	 individually.  */
+      resume_ptid = inferior_ptid;
+    }
+  else 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
@@ -2864,7 +2874,7 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal)
 
   started = start_step_over (tp->ptid);
 
-  if (!non_stop)
+  if (!target_is_non_stop_p ())
     {
       if (started)
 	{
@@ -2886,7 +2896,7 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal)
     }
   else if (step_over_info_valid_p ())
     ;
-  else
+  else if (non_stop)
     {
       /* If the thread wasn't started, and isn't queued, run it.  */
       if (!tp->executing && tp->step_over_next == NULL)
@@ -2897,6 +2907,46 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal)
 	    error ("Command aborted.");
 	}
     }
+  else
+    {
+      /* Start all other threads that are implicitly resumed too.  */
+      ALL_NON_EXITED_THREADS (tp)
+        {
+	  /* Ignore threads of processes we're not resuming.  */
+	  if (!ptid_match (tp->ptid, resume_ptid))
+	    continue;
+
+	  if (tp->executing)
+	    {
+	      if (debug_infrun)
+		fprintf_unfiltered (gdb_stdlog,
+				    "infrun: proceed: [%s] executing\n",
+				    target_pid_to_str (tp->ptid));
+	      gdb_assert (tp->resumed);
+	      continue;
+	    }
+
+	  if (tp->step_over_next != NULL)
+	    {
+	      if (debug_infrun)
+		fprintf_unfiltered (gdb_stdlog,
+				    "infrun: proceed: [%s] needs step-over\n",
+				    target_pid_to_str (tp->ptid));
+	      continue;
+	    }
+
+	  if (debug_infrun)
+	    fprintf_unfiltered (gdb_stdlog,
+				"infrun: proceed: resuming %s\n",
+				target_pid_to_str (tp->ptid));
+
+	  reset_ecs (ecs, tp);
+	  switch_to_thread (tp->ptid);
+	  keep_going_pass (ecs);
+	  if (!ecs->wait_some_more)
+	    error ("Command aborted.");
+	}
+    }
 
   discard_cleanups (old_chain);
 
@@ -3113,7 +3163,7 @@ for_each_just_stopped_thread (for_each_just_stopped_thread_callback_func func)
   if (!target_has_execution || ptid_equal (inferior_ptid, null_ptid))
     return;
 
-  if (non_stop)
+  if (target_is_non_stop_p ())
     {
       /* If in non-stop mode, only the current thread stopped.  */
       func (inferior_thread ());
@@ -3589,7 +3639,7 @@ fetch_inferior_event (void *client_data)
   /* If an error happens while handling the event, propagate GDB's
      knowledge of the executing state to the frontend/user running
      state.  */
-  if (!non_stop)
+  if (!target_is_non_stop_p ())
     ts_old_chain = make_cleanup (finish_thread_state_cleanup, &minus_one_ptid);
   else
     ts_old_chain = make_cleanup (finish_thread_state_cleanup, &ecs->ptid);
@@ -3828,7 +3878,8 @@ adjust_pc_after_break (struct thread_info *thread,
      to get the "stopped by SW BP and needs adjustment" info out of
      the target/kernel (and thus never reach here; see above).  */
   if (software_breakpoint_inserted_here_p (aspace, breakpoint_pc)
-      || (non_stop && moribund_breakpoint_here_p (aspace, breakpoint_pc)))
+      || (target_is_non_stop_p ()
+	  && moribund_breakpoint_here_p (aspace, breakpoint_pc)))
     {
       struct cleanup *old_cleanups = make_cleanup (null_cleanup, NULL);
 
@@ -4033,7 +4084,7 @@ stop_all_threads (void)
   ptid_t entry_ptid;
   struct cleanup *old_chain;
 
-  gdb_assert (non_stop);
+  gdb_assert (target_is_non_stop_p ());
 
   if (debug_infrun)
     fprintf_unfiltered (gdb_stdlog, "infrun.c: stop_all_threads\n");
@@ -4393,7 +4444,7 @@ handle_inferior_event (struct execution_control_state *ecs)
   {
     ptid_t mark_ptid;
 
-    if (!non_stop)
+    if (!target_is_non_stop_p ())
       mark_ptid = minus_one_ptid;
     else if (ecs->ws.kind == TARGET_WAITKIND_SIGNALLED
 	     && ecs->ws.kind == TARGET_WAITKIND_EXITED)
@@ -4696,7 +4747,8 @@ Cannot fill $_exitsignal with the correct signal number.\n"));
 	  child = ecs->ws.value.related_pid;
 
 	  /* In non-stop mode, also resume the other branch.  */
-	  if (non_stop && !detach_fork)
+	  if (!detach_fork && (non_stop
+			       || (sched_multi && target_is_non_stop_p ())))
 	    {
 	      if (follow_child)
 		switch_to_thread (parent);
@@ -4932,7 +4984,7 @@ finish_step_over (struct execution_control_state *ecs)
 	clear_step_over_info ();
     }
 
-  if (!non_stop)
+  if (!target_is_non_stop_p ())
     return;
 
   /* Start a new step-over in another thread if there's one that
@@ -5432,6 +5484,17 @@ handle_signal_stop (struct execution_control_state *ecs)
 	  /* Reset trap_expected to ensure breakpoints are re-inserted.  */
 	  ecs->event_thread->control.trap_expected = 0;
 
+	  if (!non_stop && target_is_non_stop_p ())
+	    {
+	      keep_going (ecs);
+
+	      /* We've canceled the step-over temporarily while the
+		 signal handler executes.  Let other threads run,
+		 according to schedlock.  */
+	      restart_threads (ecs->event_thread);
+	      return;
+	    }
+
 	  /* If we were nexting/stepping some other thread, switch to
 	     it, so that we don't continue it, losing control.  */
 	  if (!switch_back_to_stepped_thread (ecs))
@@ -6315,7 +6378,7 @@ process_event_stop_test (struct execution_control_state *ecs)
 static int
 switch_back_to_stepped_thread (struct execution_control_state *ecs)
 {
-  if (!non_stop)
+  if (!target_is_non_stop_p ())
     {
       struct thread_info *tp;
       struct thread_info *stepping_thread;
@@ -6566,7 +6629,10 @@ keep_going_stepped_thread (struct thread_info *tp)
 				     stop_pc);
 
       tp->resumed = 1;
-      resume_ptid = user_visible_resume_ptid (tp->control.stepping_command);
+      if (target_is_non_stop_p ())
+	resume_ptid = inferior_ptid;
+      else
+	resume_ptid = user_visible_resume_ptid (tp->control.stepping_command);
       do_target_resume (resume_ptid, 0, GDB_SIGNAL_0);
     }
   else
@@ -6984,6 +7050,11 @@ stop_waiting (struct execution_control_state *ecs)
 
   /* Let callers know we don't want to wait for the inferior anymore.  */
   ecs->wait_some_more = 0;
+
+  /* If all-stop, but the target is always in non-stop mode, stop all
+     threads now that we're presenting the stop to the user.  */
+  if (!non_stop && target_is_non_stop_p ())
+    stop_all_threads ();
 }
 
 /* Like keep_going, but passes the signal to the inferior, even if the
@@ -7100,7 +7171,7 @@ keep_going_pass (struct execution_control_state *ecs)
 	 insert_breakpoints below, because that removes the breakpoint
 	 we're about to step over, otherwise other threads could miss
 	 it.  */
-      if (step_over_info_valid_p () && non_stop)
+      if (step_over_info_valid_p () && target_is_non_stop_p ())
 	stop_all_threads ();
 
       /* Stop stepping if inserting breakpoints fails.  */
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 04707dc..9de291e 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -1375,13 +1375,13 @@ get_pending_status (struct lwp_info *lp, int *status)
     signo = GDB_SIGNAL_0; /* a pending ptrace event, not a real signal.  */
   else if (lp->status)
     signo = gdb_signal_from_host (WSTOPSIG (lp->status));
-  else if (non_stop && !is_executing (lp->ptid))
+  else if (target_is_non_stop_p () && !is_executing (lp->ptid))
     {
       struct thread_info *tp = find_thread_ptid (lp->ptid);
 
       signo = tp->suspend.stop_signal;
     }
-  else if (!non_stop)
+  else if (!target_is_non_stop_p ())
     {
       struct target_waitstatus last;
       ptid_t last_ptid;
@@ -2931,7 +2931,7 @@ select_event_lwp (ptid_t filter, struct lwp_info **orig_lp, int *status)
      having stepped the thread, wouldn't understand what the trap was
      for, and therefore would report it to the user as a random
      signal.  */
-  if (!non_stop)
+  if (!target_is_non_stop_p ())
     {
       event_lp = iterate_over_lwps (filter,
 				    select_singlestep_lwp_callback, NULL);
@@ -3278,7 +3278,7 @@ linux_nat_filter_event (int lwpid, int status)
     {
       enum gdb_signal signo = gdb_signal_from_host (WSTOPSIG (status));
 
-      if (!non_stop)
+      if (!target_is_non_stop_p ())
 	{
 	  /* Only do the below in all-stop, as we currently use SIGSTOP
 	     to implement target_stop (see linux_nat_stop) in
@@ -3544,7 +3544,7 @@ linux_nat_wait_1 (struct target_ops *ops,
   status = lp->status;
   lp->status = 0;
 
-  if (!non_stop)
+  if (!target_is_non_stop_p ())
     {
       /* Now stop all other LWP's ...  */
       iterate_over_lwps (minus_one_ptid, stop_callback, NULL);
@@ -3586,7 +3586,7 @@ linux_nat_wait_1 (struct target_ops *ops,
      clears it.  */
   last_resume_kind = lp->last_resume_kind;
 
-  if (!non_stop)
+  if (!target_is_non_stop_p ())
     {
       /* In all-stop, from the core's perspective, all LWPs are now
 	 stopped until a new resume action is sent over.  */
@@ -3719,7 +3719,7 @@ linux_nat_wait (struct target_ops *ops,
      specific_process, for example, see linux_nat_wait_1), and
      meanwhile the event became uninteresting.  Don't bother resuming
      LWPs we're not going to wait for if they'd stop immediately.  */
-  if (non_stop)
+  if (target_is_non_stop_p ())
     iterate_over_lwps (minus_one_ptid, resume_stopped_resumed_lwps, &ptid);
 
   event_ptid = linux_nat_wait_1 (ops, ptid, ourstatus, target_options);
@@ -4551,6 +4551,14 @@ linux_nat_supports_non_stop (struct target_ops *self)
   return 1;
 }
 
+/* to_always_non_stop_p implementation.  */
+
+static int
+linux_nat_always_non_stop_p (struct target_ops *self)
+{
+  return 0;
+}
+
 /* True if we want to support multi-process.  To be removed when GDB
    supports multi-exec.  */
 
@@ -4770,7 +4778,7 @@ linux_nat_stop_lwp (struct lwp_info *lwp, void *data)
 static void
 linux_nat_stop (struct target_ops *self, ptid_t ptid)
 {
-  if (non_stop)
+  if (target_is_non_stop_p ())
     iterate_over_lwps (ptid, linux_nat_stop_lwp, NULL);
   else
     linux_ops->to_stop (linux_ops, ptid);
@@ -4868,6 +4876,7 @@ linux_nat_add_target (struct target_ops *t)
   t->to_can_async_p = linux_nat_can_async_p;
   t->to_is_async_p = linux_nat_is_async_p;
   t->to_supports_non_stop = linux_nat_supports_non_stop;
+  t->to_always_non_stop_p = linux_nat_always_non_stop_p;
   t->to_async = linux_nat_async;
   t->to_terminal_inferior = linux_nat_terminal_inferior;
   t->to_terminal_ours = linux_nat_terminal_ours;
diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
index b29abaf..a8e0b35 100644
--- a/gdb/target-delegates.c
+++ b/gdb/target-delegates.c
@@ -1744,6 +1744,33 @@ debug_supports_non_stop (struct target_ops *self)
 }
 
 static int
+delegate_always_non_stop_p (struct target_ops *self)
+{
+  self = self->beneath;
+  return self->to_always_non_stop_p (self);
+}
+
+static int
+tdefault_always_non_stop_p (struct target_ops *self)
+{
+  return 0;
+}
+
+static int
+debug_always_non_stop_p (struct target_ops *self)
+{
+  int result;
+  fprintf_unfiltered (gdb_stdlog, "-> %s->to_always_non_stop_p (...)\n", debug_target.to_shortname);
+  result = debug_target.to_always_non_stop_p (&debug_target);
+  fprintf_unfiltered (gdb_stdlog, "<- %s->to_always_non_stop_p (", debug_target.to_shortname);
+  target_debug_print_struct_target_ops_p (&debug_target);
+  fputs_unfiltered (") = ", gdb_stdlog);
+  target_debug_print_int (result);
+  fputs_unfiltered ("\n", gdb_stdlog);
+  return result;
+}
+
+static int
 delegate_find_memory_regions (struct target_ops *self, find_memory_region_ftype arg1, void *arg2)
 {
   self = self->beneath;
@@ -3978,6 +4005,8 @@ install_delegators (struct target_ops *ops)
     ops->to_async = delegate_async;
   if (ops->to_supports_non_stop == NULL)
     ops->to_supports_non_stop = delegate_supports_non_stop;
+  if (ops->to_always_non_stop_p == NULL)
+    ops->to_always_non_stop_p = delegate_always_non_stop_p;
   if (ops->to_find_memory_regions == NULL)
     ops->to_find_memory_regions = delegate_find_memory_regions;
   if (ops->to_make_corefile_notes == NULL)
@@ -4203,6 +4232,7 @@ install_dummy_methods (struct target_ops *ops)
   ops->to_is_async_p = tdefault_is_async_p;
   ops->to_async = tdefault_async;
   ops->to_supports_non_stop = tdefault_supports_non_stop;
+  ops->to_always_non_stop_p = tdefault_always_non_stop_p;
   ops->to_find_memory_regions = dummy_find_memory_regions;
   ops->to_make_corefile_notes = dummy_make_corefile_notes;
   ops->to_get_bookmark = tdefault_get_bookmark;
@@ -4350,6 +4380,7 @@ init_debug_target (struct target_ops *ops)
   ops->to_is_async_p = debug_is_async_p;
   ops->to_async = debug_async;
   ops->to_supports_non_stop = debug_supports_non_stop;
+  ops->to_always_non_stop_p = debug_always_non_stop_p;
   ops->to_find_memory_regions = debug_find_memory_regions;
   ops->to_make_corefile_notes = debug_make_corefile_notes;
   ops->to_get_bookmark = debug_get_bookmark;
diff --git a/gdb/target.c b/gdb/target.c
index 6d814c6..2dd9c89 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -3733,6 +3733,58 @@ maint_show_target_async_command (struct ui_file *file, int from_tty,
 		      "asynchronous mode is %s.\n"), value);
 }
 
+/* See target.h.  */
+
+int
+target_is_non_stop_p (void)
+{
+  return (non_stop
+	  || target_non_stop_enabled == AUTO_BOOLEAN_TRUE
+	  || (target_non_stop_enabled == AUTO_BOOLEAN_AUTO
+	      && current_target.to_always_non_stop_p (&current_target)));
+}
+
+/* Controls if targets can report that they always run in non-stop
+   mode.  This is just for maintainers to use when debugging gdb.  */
+enum auto_boolean target_non_stop_enabled = AUTO_BOOLEAN_AUTO;
+
+/* The set command writes to this variable.  If the inferior is
+   executing, target_async_permitted is *not* updated.  */
+static enum auto_boolean target_non_stop_enabled_1 = AUTO_BOOLEAN_AUTO;
+
+/* Implementation of "maint set target-non-stop".  */
+
+static void
+maint_set_target_non_stop_command (char *args, int from_tty,
+				   struct cmd_list_element *c)
+{
+  if (have_live_inferiors ())
+    {
+      target_non_stop_enabled_1 = target_non_stop_enabled;
+      error (_("Cannot change this setting while the inferior is running."));
+    }
+
+  target_non_stop_enabled = target_non_stop_enabled_1;
+}
+
+/* Implementation of "maint show target-non-stop".  */
+
+static void
+maint_show_target_non_stop_command (struct ui_file *file, int from_tty,
+				    struct cmd_list_element *c,
+				    const char *value)
+{
+  if (target_non_stop_enabled == AUTO_BOOLEAN_AUTO)
+    fprintf_filtered (file,
+		      _("Whether the target is always in non-stop mode "
+			"is %s (currently %s).\n"), value,
+		      target_is_non_stop_p () ? "on" : "off");
+  else
+    fprintf_filtered (file,
+		      _("Whether the target is always in non-stop mode "
+			"is %s.\n"), value);
+}
+
 /* Temporary copies of permission settings.  */
 
 static int may_write_registers_1 = 1;
@@ -3835,6 +3887,16 @@ Tells gdb whether to control the inferior in asynchronous mode."),
 			   &maintenance_set_cmdlist,
 			   &maintenance_show_cmdlist);
 
+  add_setshow_auto_boolean_cmd ("target-non-stop", no_class,
+				&target_non_stop_enabled_1, _("\
+Set whether gdb always controls the inferior in non-stop mode."), _("\
+Show whether gdb always controls the inferior in non-stop mode."), _("\
+Tells gdb whether to control the inferior in non-stop mode."),
+			   maint_set_target_non_stop_command,
+			   maint_show_target_non_stop_command,
+			   &maintenance_set_cmdlist,
+			   &maintenance_show_cmdlist);
+
   add_setshow_boolean_cmd ("may-write-registers", class_support,
 			   &may_write_registers_1, _("\
 Set permission to write into registers."), _("\
diff --git a/gdb/target.h b/gdb/target.h
index f400558..667e742 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -635,6 +635,10 @@ struct target_ops
        comment on 'to_can_run'.  */
     int (*to_supports_non_stop) (struct target_ops *)
       TARGET_DEFAULT_RETURN (0);
+    /* Return true if the target operates in non-stop mode even when
+       "set non-stop off".  */
+    int (*to_always_non_stop_p) (struct target_ops *)
+      TARGET_DEFAULT_RETURN (0);
     /* find_memory_regions support method for gcore */
     int (*to_find_memory_regions) (struct target_ops *,
 				   find_memory_region_ftype func, void *data)
@@ -1689,6 +1693,15 @@ extern int target_async_permitted;
 /* Enables/disabled async target events.  */
 extern void target_async (int enable);
 
+/* Whether support for controlling the target backends always in
+   non-stop mode is enabled.  */
+extern enum auto_boolean target_non_stop_enabled;
+
+/* Is the target in non-stop mode?  Some targets control the inferior
+   in non-stop mode even with "set non-stop off".  Always true if "set
+   non-stop" is on.  */
+extern int target_is_non_stop_p (void);
+
 #define target_execution_direction() \
   (current_target.to_execution_direction (&current_target))
 
-- 
1.9.3

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 14/17] Fix interrupt-noterm.exp on targets always in non-stop
  2015-04-01 22:14 [PATCH 00/17] All-stop on top of non-stop Pedro Alves
                   ` (13 preceding siblings ...)
  2015-04-01 23:06 ` [PATCH 16/17] Disable displaced stepping if trying it fails Pedro Alves
@ 2015-04-01 23:06 ` Pedro Alves
  2015-04-01 23:07 ` [PATCH 05/17] remote.c/all-stop: Implement TARGET_WAITKIND_NO_RESUMED and TARGET_WNOHANG Pedro Alves
                   ` (2 subsequent siblings)
  17 siblings, 0 replies; 26+ messages in thread
From: Pedro Alves @ 2015-04-01 23:06 UTC (permalink / raw)
  To: gdb-patches

With "maint set target-non-stop on" we get:

 @@ -66,13 +66,16 @@ Continuing.
  interrupt
  (gdb) PASS: gdb.base/interrupt-noterm.exp: interrupt

 -Program received signal SIGINT, Interrupt.
 -PASS: gdb.base/interrupt-noterm.exp: inferior received SIGINT
 -testcase src/gdb/testsuite/gdb.base/interrupt-noterm.exp completed in 0 seconds
 +[process 12119] #1 stopped.
 +0x0000003615ebc6d0 in __nanosleep_nocancel () at ../sysdeps/unix/syscall-template.S:81
 +81     T_PSEUDO (SYSCALL_SYMBOL, SYSCALL_NAME, SYSCALL_NARGS)
 +FAIL: gdb.base/interrupt-noterm.exp: inferior received SIGINT (timeout)
 +testcase src/gdb/testsuite/gdb.base/interrupt-noterm.exp completed in 10 seconds

That is, we get "[$thread] #1 stopped" instead of SIGINT.

The issue is that we don't currently distinguish send
"interrupt/ctrl-c" to target terminal vs "stop/pause" thread well;
both cases go through "target_stop".

And then, the native Linux backend (linux-nat.c) implements
target_stop with SIGSTOP in non-stop mode, and SIGINT in all-stop
mode.  Since "maint set target-non-stop on" forces the backend to be
always running in non-stop mode, even though the user-visible behavior
is "set non-stop" is "off", "interrupt" causes a SIGSTOP instead of
the SIGINT the test expects.

Fix this by introducing a target_interrupt method to use in the
"interrupt/ctrl-c" case, so "set non-stop off" can always work the
same irrespective of "maint set target-non-stop on/off".  I'm
explictly considering changing the "set non-stop on" behavior as out
of scope here.

Most of the patch is an across-the-board rename of to_stop hook
implementations to to_interrupt.  The only targets where something
more than a rename is being done are linux-nat.c and remote.c, which
are the only targets that support async, and thus are the only ones
the core side calls target_stop on.

gdb/ChangeLog:
2015-04-01  Pedro Alves  <palves@redhat.com>

	* darwin-nat.c (darwin_stop): Rename to ...
	(darwin_interrupt): ... this.
	(_initialize_darwin_inferior): Adjust.
	* gnu-nat.c (gnu_stop): Delete.
	(gnu_thread_alive): Don't install gnu_stop.
	* inf-ptrace.c (inf_ptrace_stop): Rename to ...
	(inf_ptrace_interrupt): ... this.
	(inf_ptrace_target): Adjust.
	* infcmd.c (interrupt_target_1): Use target_interrupt instead of
	target_stop.
	* linux-nat (linux_nat_stop): Rename to ...
	(linux_nat_interrupt): ... this.
	(linux_nat_stop): Reimplement.
	(linux_nat_add_target): Install linux_nat_interrupt.
	* monitor.c (monitor_stop): Rename to ...
	(monitor_interrupt): ... this.
	(init_base_monitor_ops): Adjust.
	* nto-procfs.c (nto_interrupt_twice): Rename to ...
	(nto_handle_sigint_twice): ... this.
	(nto_interrupt): Rename to ...
	(nto_handle_sigint): ... this.  Call target_interrupt instead of
	target_stop.
	(procfs_wait, init_procfs_targets): Adjust.
	* procfs.c (procfs_target): Adjust.
	(procfs_stop): Rename to ...
	(procfs_interrupt): ... this.
	* remote-m32r-sdi.c (m32r_stop): Rename to ...
	(m32r_interrupt): ... this.
	(init_m32r_ops): Adjust.
	* remote-sim.c (gdbsim_stop_inferior): Rename to ...
	(gdbsim_interrupt_inferior): ... this.
	(gdbsim_stop): Rename to ...
	(gdbsim_interrupt): ... this.
	(gdbsim_cntrl_c): Adjust.
	(init_gdbsim_ops): Adjust.
	* remote.c (sync_remote_interrupt): Adjust comments.
	(remote_stop_as): Rename to ...
	(remote_interrupt_as): ... this.
	(remote_stop): Adjust comment.
	(remote_interrupt): New function.
	(init_remote_ops): Install remote_interrupt.
	* target.c (target_interrupt): New function.
	* target.h (struct target_ops) <to_interrupt>: New field.
	(target_interrupt): New declaration.
	* windows-nat.c (windows_stop): Rename to ...
	(windows_interrupt): ... this.
	* target-delegates.c: Regenerate.
---
 gdb/darwin-nat.c       |  6 +++---
 gdb/gnu-nat.c          |  7 -------
 gdb/inf-ptrace.c       |  6 +++---
 gdb/infcmd.c           |  2 +-
 gdb/linux-nat.c        | 12 +++++++++---
 gdb/monitor.c          |  4 ++--
 gdb/nto-procfs.c       | 14 +++++++-------
 gdb/procfs.c           |  4 ++--
 gdb/remote-m32r-sdi.c  |  6 +++---
 gdb/remote-sim.c       | 20 ++++++++++----------
 gdb/remote.c           | 44 +++++++++++++++++++++++++++++++++-----------
 gdb/target-delegates.c | 28 ++++++++++++++++++++++++++++
 gdb/target.c           | 12 ++++++++++++
 gdb/target.h           |  8 ++++++++
 gdb/windows-nat.c      |  6 +++---
 15 files changed, 124 insertions(+), 55 deletions(-)

diff --git a/gdb/darwin-nat.c b/gdb/darwin-nat.c
index e1acc05..30e968f 100644
--- a/gdb/darwin-nat.c
+++ b/gdb/darwin-nat.c
@@ -83,7 +83,7 @@
 #define PTRACE(CMD, PID, ADDR, SIG) \
  darwin_ptrace(#CMD, CMD, (PID), (ADDR), (SIG))
 
-static void darwin_stop (struct target_ops *self, ptid_t);
+static void darwin_interrupt (struct target_ops *self, ptid_t);
 
 static void darwin_resume_to (struct target_ops *ops, ptid_t ptid, int step,
                               enum gdb_signal signal);
@@ -1198,7 +1198,7 @@ darwin_wait_to (struct target_ops *ops,
 }
 
 static void
-darwin_stop (struct target_ops *self, ptid_t t)
+darwin_interrupt (struct target_ops *self, ptid_t t)
 {
   struct inferior *inf = current_inferior ();
 
@@ -2156,7 +2156,7 @@ _initialize_darwin_inferior (void)
   darwin_ops->to_wait = darwin_wait_to;
   darwin_ops->to_mourn_inferior = darwin_mourn_inferior;
   darwin_ops->to_kill = darwin_kill_inferior;
-  darwin_ops->to_stop = darwin_stop;
+  darwin_ops->to_interrupt = darwin_interrupt;
   darwin_ops->to_resume = darwin_resume_to;
   darwin_ops->to_thread_alive = darwin_thread_alive;
   darwin_ops->to_pid_to_str = darwin_pid_to_str;
diff --git a/gdb/gnu-nat.c b/gdb/gnu-nat.c
index d830773..4753634 100644
--- a/gdb/gnu-nat.c
+++ b/gdb/gnu-nat.c
@@ -2271,12 +2271,6 @@ gnu_terminal_init (struct target_ops *self)
   child_terminal_init_with_pgrp (gnu_current_inf->pid);
 }
 
-static void
-gnu_stop (struct target_ops *self, ptid_t ptid)
-{
-  error (_("to_stop target function not implemented"));
-}
-
 static int
 gnu_thread_alive (struct target_ops *ops, ptid_t ptid)
 {
@@ -2686,7 +2680,6 @@ gnu_target (void)
   t->to_mourn_inferior = gnu_mourn_inferior;
   t->to_thread_alive = gnu_thread_alive;
   t->to_pid_to_str = gnu_pid_to_str;
-  t->to_stop = gnu_stop;
 
   return t;
 }
diff --git a/gdb/inf-ptrace.c b/gdb/inf-ptrace.c
index cd58dfb..17ba903 100644
--- a/gdb/inf-ptrace.c
+++ b/gdb/inf-ptrace.c
@@ -288,10 +288,10 @@ inf_ptrace_kill (struct target_ops *ops)
   target_mourn_inferior ();
 }
 
-/* Stop the inferior.  */
+/* Interrupt the inferior.  */
 
 static void
-inf_ptrace_stop (struct target_ops *self, ptid_t ptid)
+inf_ptrace_interrupt (struct target_ops *self, ptid_t ptid)
 {
   /* Send a SIGINT to the process group.  This acts just like the user
      typed a ^C on the controlling terminal.  Note that using a
@@ -686,7 +686,7 @@ inf_ptrace_target (void)
   t->to_mourn_inferior = inf_ptrace_mourn_inferior;
   t->to_thread_alive = inf_ptrace_thread_alive;
   t->to_pid_to_str = inf_ptrace_pid_to_str;
-  t->to_stop = inf_ptrace_stop;
+  t->to_interrupt = inf_ptrace_interrupt;
   t->to_xfer_partial = inf_ptrace_xfer_partial;
 #if defined (PT_IO) && defined (PIOD_READ_AUXV)
   t->to_auxv_parse = inf_ptrace_auxv_parse;
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 94d87214..6ad8494 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -2853,7 +2853,7 @@ interrupt_target_1 (int all_threads)
     ptid = minus_one_ptid;
   else
     ptid = inferior_ptid;
-  target_stop (ptid);
+  target_interrupt (ptid);
 
   /* Tag the thread as having been explicitly requested to stop, so
      other parts of gdb know not to resume this thread automatically,
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 9de291e..d8001f9 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -4778,10 +4778,16 @@ linux_nat_stop_lwp (struct lwp_info *lwp, void *data)
 static void
 linux_nat_stop (struct target_ops *self, ptid_t ptid)
 {
-  if (target_is_non_stop_p ())
+  iterate_over_lwps (ptid, linux_nat_stop_lwp, NULL);
+}
+
+static void
+linux_nat_interrupt (struct target_ops *self, ptid_t ptid)
+{
+  if (non_stop)
     iterate_over_lwps (ptid, linux_nat_stop_lwp, NULL);
   else
-    linux_ops->to_stop (linux_ops, ptid);
+    linux_ops->to_interrupt (linux_ops, ptid);
 }
 
 static void
@@ -4884,8 +4890,8 @@ linux_nat_add_target (struct target_ops *t)
   super_close = t->to_close;
   t->to_close = linux_nat_close;
 
-  /* Methods for non-stop support.  */
   t->to_stop = linux_nat_stop;
+  t->to_interrupt = linux_nat_interrupt;
 
   t->to_supports_multi_process = linux_nat_supports_multi_process;
 
diff --git a/gdb/monitor.c b/gdb/monitor.c
index 548dae3..e47d479 100644
--- a/gdb/monitor.c
+++ b/gdb/monitor.c
@@ -2267,7 +2267,7 @@ monitor_load (struct target_ops *self, const char *args, int from_tty)
 }
 
 static void
-monitor_stop (struct target_ops *self, ptid_t ptid)
+monitor_interrupt (struct target_ops *self, ptid_t ptid)
 {
   monitor_debug ("MON stop\n");
   if ((current_monitor->flags & MO_SEND_BREAK_ON_STOP) != 0)
@@ -2375,7 +2375,7 @@ init_base_monitor_ops (void)
   monitor_ops.to_load = monitor_load;
   monitor_ops.to_create_inferior = monitor_create_inferior;
   monitor_ops.to_mourn_inferior = monitor_mourn_inferior;
-  monitor_ops.to_stop = monitor_stop;
+  monitor_ops.to_interrupt = monitor_interrupt;
   monitor_ops.to_rcmd = monitor_rcmd;
   monitor_ops.to_log_command = serial_log_command;
   monitor_ops.to_thread_alive = monitor_thread_alive;
diff --git a/gdb/nto-procfs.c b/gdb/nto-procfs.c
index 7312060..c318f19 100644
--- a/gdb/nto-procfs.c
+++ b/gdb/nto-procfs.c
@@ -692,20 +692,20 @@ Give up (and stop debugging it)? ")))
 
 /* The user typed ^C twice.  */
 static void
-nto_interrupt_twice (int signo)
+nto_handle_sigint_twice (int signo)
 {
   signal (signo, ofunc);
   interrupt_query ();
-  signal (signo, nto_interrupt_twice);
+  signal (signo, nto_handle_sigint_twice);
 }
 
 static void
-nto_interrupt (int signo)
+nto_handle_sigint (int signo)
 {
   /* If this doesn't work, try more severe steps.  */
-  signal (signo, nto_interrupt_twice);
+  signal (signo, nto_handle_sigint_twice);
 
-  target_stop (inferior_ptid);
+  target_interrupt (inferior_ptid);
 }
 
 static ptid_t
@@ -733,7 +733,7 @@ procfs_wait (struct target_ops *ops,
   devctl (ctl_fd, DCMD_PROC_STATUS, &status, sizeof (status), 0);
   while (!(status.flags & _DEBUG_FLAG_ISTOP))
     {
-      ofunc = (void (*)()) signal (SIGINT, nto_interrupt);
+      ofunc = (void (*)()) signal (SIGINT, nto_handle_sigint);
       sigwaitinfo (&set, &info);
       signal (SIGINT, ofunc);
       devctl (ctl_fd, DCMD_PROC_STATUS, &status, sizeof (status), 0);
@@ -1444,7 +1444,7 @@ init_procfs_targets (void)
   t->to_thread_alive = procfs_thread_alive;
   t->to_update_thread_list = procfs_update_thread_list;
   t->to_pid_to_str = procfs_pid_to_str;
-  t->to_stop = procfs_stop;
+  t->to_interrupt = procfs_interrupt;
   t->to_have_continuable_watchpoint = 1;
   t->to_extra_thread_info = nto_extra_thread_info;
 
diff --git a/gdb/procfs.c b/gdb/procfs.c
index b62539f..e540e2a 100644
--- a/gdb/procfs.c
+++ b/gdb/procfs.c
@@ -194,7 +194,7 @@ procfs_target (void)
   t->to_xfer_partial = procfs_xfer_partial;
   t->to_pass_signals = procfs_pass_signals;
   t->to_files_info = procfs_files_info;
-  t->to_stop = procfs_stop;
+  t->to_interrupt = procfs_interrupt;
 
   t->to_update_thread_list = procfs_update_thread_list;
   t->to_thread_alive = procfs_thread_alive;
@@ -4204,7 +4204,7 @@ procfs_files_info (struct target_ops *ignore)
    kill(SIGINT) to the child's process group.  */
 
 static void
-procfs_stop (struct target_ops *self, ptid_t ptid)
+procfs_interrupt (struct target_ops *self, ptid_t ptid)
 {
   kill (-inferior_process_group (), SIGINT);
 }
diff --git a/gdb/remote-m32r-sdi.c b/gdb/remote-m32r-sdi.c
index 01cb5b6..b246cd9 100644
--- a/gdb/remote-m32r-sdi.c
+++ b/gdb/remote-m32r-sdi.c
@@ -1407,10 +1407,10 @@ m32r_load (struct target_ops *self, const char *args, int from_tty)
 }
 
 static void
-m32r_stop (struct target_ops *self, ptid_t ptid)
+m32r_interrupt (struct target_ops *self, ptid_t ptid)
 {
   if (remote_debug)
-    fprintf_unfiltered (gdb_stdlog, "m32r_stop()\n");
+    fprintf_unfiltered (gdb_stdlog, "m32r_interrupt()\n");
 
   send_cmd (SDI_STOP_CPU);
 
@@ -1664,7 +1664,7 @@ init_m32r_ops (void)
   m32r_ops.to_load = m32r_load;
   m32r_ops.to_create_inferior = m32r_create_inferior;
   m32r_ops.to_mourn_inferior = m32r_mourn_inferior;
-  m32r_ops.to_stop = m32r_stop;
+  m32r_ops.to_interrupt = m32r_interrupt;
   m32r_ops.to_log_command = serial_log_command;
   m32r_ops.to_thread_alive = m32r_thread_alive;
   m32r_ops.to_pid_to_str = m32r_pid_to_str;
diff --git a/gdb/remote-sim.c b/gdb/remote-sim.c
index fd2fd58..766356a 100644
--- a/gdb/remote-sim.c
+++ b/gdb/remote-sim.c
@@ -87,7 +87,7 @@ static void gdbsim_files_info (struct target_ops *target);
 
 static void gdbsim_mourn_inferior (struct target_ops *target);
 
-static void gdbsim_stop (struct target_ops *self, ptid_t ptid);
+static void gdbsim_interrupt (struct target_ops *self, ptid_t ptid);
 
 void simulator_command (char *args, int from_tty);
 
@@ -888,17 +888,17 @@ gdbsim_resume (struct target_ops *ops,
     error (_("The program is not being run."));
 }
 
-/* Notify the simulator of an asynchronous request to stop.
+/* Notify the simulator of an asynchronous request to interrupt.
 
-   The simulator shall ensure that the stop request is eventually
+   The simulator shall ensure that the interrupt request is eventually
    delivered to the simulator.  If the call is made while the
-   simulator is not running then the stop request is processed when
+   simulator is not running then the interrupt request is processed when
    the simulator is next resumed.
 
    For simulators that do not support this operation, just abort.  */
 
 static int
-gdbsim_stop_inferior (struct inferior *inf, void *arg)
+gdbsim_interrupt_inferior (struct inferior *inf, void *arg)
 {
   struct sim_inferior_data *sim_data
     = get_sim_inferior_data (inf, SIM_INSTANCE_NEEDED);
@@ -918,13 +918,13 @@ gdbsim_stop_inferior (struct inferior *inf, void *arg)
 }
 
 static void
-gdbsim_stop (struct target_ops *self, ptid_t ptid)
+gdbsim_interrupt (struct target_ops *self, ptid_t ptid)
 {
   struct sim_inferior_data *sim_data;
 
   if (ptid_equal (ptid, minus_one_ptid))
     {
-      iterate_over_inferiors (gdbsim_stop_inferior, NULL);
+      iterate_over_inferiors (gdbsim_interrupt_inferior, NULL);
     }
   else
     {
@@ -934,7 +934,7 @@ gdbsim_stop (struct target_ops *self, ptid_t ptid)
 	error (_("Can't stop pid %d.  No inferior found."),
 	       ptid_get_pid (ptid));
 
-      gdbsim_stop_inferior (inf, NULL);
+      gdbsim_interrupt_inferior (inf, NULL);
     }
 }
 
@@ -962,7 +962,7 @@ gdb_os_poll_quit (host_callback *p)
 static void
 gdbsim_cntrl_c (int signo)
 {
-  gdbsim_stop (NULL, minus_one_ptid);
+  gdbsim_interrupt (NULL, minus_one_ptid);
 }
 
 static ptid_t
@@ -1320,7 +1320,7 @@ init_gdbsim_ops (void)
   gdbsim_ops.to_load = gdbsim_load;
   gdbsim_ops.to_create_inferior = gdbsim_create_inferior;
   gdbsim_ops.to_mourn_inferior = gdbsim_mourn_inferior;
-  gdbsim_ops.to_stop = gdbsim_stop;
+  gdbsim_ops.to_interrupt = gdbsim_interrupt;
   gdbsim_ops.to_thread_alive = gdbsim_thread_alive;
   gdbsim_ops.to_pid_to_str = gdbsim_pid_to_str;
   gdbsim_ops.to_stratum = process_stratum;
diff --git a/gdb/remote.c b/gdb/remote.c
index f46dd89..0920b89 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -4991,11 +4991,12 @@ async_cleanup_sigint_signal_handler (void *dummy)
    packet.  */
 static void (*ofunc) (int);
 
-/* The command line interface's stop routine.  This function is installed
-   as a signal handler for SIGINT.  The first time a user requests a
-   stop, we call remote_stop to send a break or ^C.  If there is no
+/* The command line interface's interrupt routine.  This function is installed
+   as a signal handler for SIGINT.  The first time a user requests an
+   interrupt, we call remote_interrupt to send a break or ^C.  If there is no
    response from the target (it didn't stop when the user requested it),
    we ask the user if he'd like to detach from the target.  */
+
 static void
 sync_remote_interrupt (int signo)
 {
@@ -5066,12 +5067,12 @@ remote_stop_ns (ptid_t ptid)
     error (_("Stopping %s failed: %s"), target_pid_to_str (ptid), rs->buf);
 }
 
-/* All-stop version of target_stop.  Sends a break or a ^C to stop the
-   remote target.  It is undefined which thread of which process
-   reports the stop.  */
+/* All-stop version of target_interrupt.  Sends a break or a ^C to
+   interrupt the remote target.  It is undefined which thread of which
+   process reports the interrupt.  */
 
 static void
-remote_stop_as (ptid_t ptid)
+remote_interrupt_as (ptid_t ptid)
 {
   struct remote_state *rs = get_remote_state ();
 
@@ -5087,9 +5088,7 @@ remote_stop_as (ptid_t ptid)
   send_interrupt_sequence ();
 }
 
-/* This is the generic stop called via the target vector.  When a target
-   interrupt is requested, either by the command line or the GUI, we
-   will eventually end up here.  */
+/* Implement the to_stop function for the remote targets.  */
 
 static void
 remote_stop (struct target_ops *self, ptid_t ptid)
@@ -5100,7 +5099,29 @@ remote_stop (struct target_ops *self, ptid_t ptid)
   if (non_stop)
     remote_stop_ns (ptid);
   else
-    remote_stop_as (ptid);
+    {
+      /* We don't currently have a way to transparently pause the
+	 remote target in all-stop mode.  Interrupt it instead.  */
+      remote_interrupt_as (ptid);
+    }
+}
+
+/* Implement the to_interrupt function for the remote targets.  */
+
+static void
+remote_interrupt (struct target_ops *self, ptid_t ptid)
+{
+  if (remote_debug)
+    fprintf_unfiltered (gdb_stdlog, "remote_interrupt called\n");
+
+  if (non_stop)
+    {
+      /* We don't currently have a way to ^C the remote target in
+	 non-stop mode.  Stop it (with no signal) instead.  */
+      remote_stop_ns (ptid);
+    }
+  else
+    remote_interrupt_as (ptid);
 }
 
 /* Ask the user what to do when an interrupt is received.  */
@@ -11798,6 +11819,7 @@ Specify the serial device it is connected to\n\
   remote_ops.to_extra_thread_info = remote_threads_extra_info;
   remote_ops.to_get_ada_task_ptid = remote_get_ada_task_ptid;
   remote_ops.to_stop = remote_stop;
+  remote_ops.to_interrupt = remote_interrupt;
   remote_ops.to_xfer_partial = remote_xfer_partial;
   remote_ops.to_rcmd = remote_rcmd;
   remote_ops.to_log_command = serial_log_command;
diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
index a8e0b35..581b36a 100644
--- a/gdb/target-delegates.c
+++ b/gdb/target-delegates.c
@@ -1537,6 +1537,30 @@ debug_stop (struct target_ops *self, ptid_t arg1)
 }
 
 static void
+delegate_interrupt (struct target_ops *self, ptid_t arg1)
+{
+  self = self->beneath;
+  self->to_interrupt (self, arg1);
+}
+
+static void
+tdefault_interrupt (struct target_ops *self, ptid_t arg1)
+{
+}
+
+static void
+debug_interrupt (struct target_ops *self, ptid_t arg1)
+{
+  fprintf_unfiltered (gdb_stdlog, "-> %s->to_interrupt (...)\n", debug_target.to_shortname);
+  debug_target.to_interrupt (&debug_target, arg1);
+  fprintf_unfiltered (gdb_stdlog, "<- %s->to_interrupt (", debug_target.to_shortname);
+  target_debug_print_struct_target_ops_p (&debug_target);
+  fputs_unfiltered (", ", gdb_stdlog);
+  target_debug_print_ptid_t (arg1);
+  fputs_unfiltered (")\n", gdb_stdlog);
+}
+
+static void
 delegate_rcmd (struct target_ops *self, const char *arg1, struct ui_file *arg2)
 {
   self = self->beneath;
@@ -3989,6 +4013,8 @@ install_delegators (struct target_ops *ops)
     ops->to_thread_name = delegate_thread_name;
   if (ops->to_stop == NULL)
     ops->to_stop = delegate_stop;
+  if (ops->to_interrupt == NULL)
+    ops->to_interrupt = delegate_interrupt;
   if (ops->to_rcmd == NULL)
     ops->to_rcmd = delegate_rcmd;
   if (ops->to_pid_to_exec_file == NULL)
@@ -4224,6 +4250,7 @@ install_dummy_methods (struct target_ops *ops)
   ops->to_extra_thread_info = tdefault_extra_thread_info;
   ops->to_thread_name = tdefault_thread_name;
   ops->to_stop = tdefault_stop;
+  ops->to_interrupt = tdefault_interrupt;
   ops->to_rcmd = default_rcmd;
   ops->to_pid_to_exec_file = tdefault_pid_to_exec_file;
   ops->to_log_command = tdefault_log_command;
@@ -4372,6 +4399,7 @@ init_debug_target (struct target_ops *ops)
   ops->to_extra_thread_info = debug_extra_thread_info;
   ops->to_thread_name = debug_thread_name;
   ops->to_stop = debug_stop;
+  ops->to_interrupt = debug_interrupt;
   ops->to_rcmd = debug_rcmd;
   ops->to_pid_to_exec_file = debug_pid_to_exec_file;
   ops->to_log_command = debug_log_command;
diff --git a/gdb/target.c b/gdb/target.c
index 2dd9c89..be8c8c3 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -3225,6 +3225,18 @@ target_stop (ptid_t ptid)
   (*current_target.to_stop) (&current_target, ptid);
 }
 
+void
+target_interrupt (ptid_t ptid)
+{
+  if (!may_stop)
+    {
+      warning (_("May not interrupt or stop the target, ignoring attempt"));
+      return;
+    }
+
+  (*current_target.to_interrupt) (&current_target, ptid);
+}
+
 /* See target/target.h.  */
 
 void
diff --git a/gdb/target.h b/gdb/target.h
index 667e742..e6d2f3d 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -606,6 +606,8 @@ struct target_ops
       TARGET_DEFAULT_RETURN (NULL);
     void (*to_stop) (struct target_ops *, ptid_t)
       TARGET_DEFAULT_IGNORE ();
+    void (*to_interrupt) (struct target_ops *, ptid_t)
+      TARGET_DEFAULT_IGNORE ();
     void (*to_rcmd) (struct target_ops *,
 		     const char *command, struct ui_file *output)
       TARGET_DEFAULT_FUNC (default_rcmd);
@@ -1618,6 +1620,12 @@ extern void target_update_thread_list (void);
 
 extern void target_stop (ptid_t ptid);
 
+/* Interrupt the target just like the user typed a ^C on the
+   inferior's controlling terminal.  (For instance, under Unix, this
+   should act like SIGINT).  This function is asynchronous.  */
+
+extern void target_interrupt (ptid_t ptid);
+
 /* Send the specified COMMAND to the target's monitor
    (shell,interpreter) for execution.  The result of the query is
    placed in OUTBUF.  */
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index fd31083..46a378c 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -162,7 +162,7 @@ static int windows_initialization_done;
 #define DEBUG_MEM(x)	if (debug_memory)	printf_unfiltered x
 #define DEBUG_EXCEPT(x)	if (debug_exceptions)	printf_unfiltered x
 
-static void windows_stop (struct target_ops *self, ptid_t);
+static void windows_interrupt (struct target_ops *self, ptid_t);
 static int windows_thread_alive (struct target_ops *, ptid_t);
 static void windows_kill_inferior (struct target_ops *);
 
@@ -2294,7 +2294,7 @@ windows_mourn_inferior (struct target_ops *ops)
    ^C on the controlling terminal.  */
 
 static void
-windows_stop (struct target_ops *self, ptid_t ptid)
+windows_interrupt (struct target_ops *self, ptid_t ptid)
 {
   DEBUG_EVENTS (("gdb: GenerateConsoleCtrlEvent (CTRLC_EVENT, 0)\n"));
   CHECK (GenerateConsoleCtrlEvent (CTRL_C_EVENT, current_event.dwProcessId));
@@ -2488,7 +2488,7 @@ windows_target (void)
   t->to_mourn_inferior = windows_mourn_inferior;
   t->to_thread_alive = windows_thread_alive;
   t->to_pid_to_str = windows_pid_to_str;
-  t->to_stop = windows_stop;
+  t->to_interrupt = windows_interrupt;
   t->to_pid_to_exec_file = windows_pid_to_exec_file;
   t->to_get_ada_task_ptid = windows_get_ada_task_ptid;
   t->to_get_tib_address = windows_get_tib_address;
-- 
1.9.3

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 16/17] Disable displaced stepping if trying it fails
  2015-04-01 22:14 [PATCH 00/17] All-stop on top of non-stop Pedro Alves
                   ` (12 preceding siblings ...)
  2015-04-01 22:41 ` [PATCH 12/17] Implement all-stop on top of a target running non-stop mode Pedro Alves
@ 2015-04-01 23:06 ` Pedro Alves
  2015-04-01 23:06 ` [PATCH 14/17] Fix interrupt-noterm.exp on targets always in non-stop Pedro Alves
                   ` (3 subsequent siblings)
  17 siblings, 0 replies; 26+ messages in thread
From: Pedro Alves @ 2015-04-01 23:06 UTC (permalink / raw)
  To: gdb-patches

Running the testsuite with "maint set target-non-stop on" shows:

 (gdb) PASS: gdb.base/valgrind-infcall.exp: continue #98 (false warning)
 continue
 Continuing.
 dl_main (phdr=<optimized out>..., auxv=<optimized out>) at rtld.c:2302
 2302      LIBC_PROBE (init_complete, 2, LM_ID_BASE, r);
 Cannot access memory at address 0x400532
 (gdb) PASS: gdb.base/valgrind-infcall.exp: continue #99 (false warning)
 p gdb_test_infcall ()
 $1 = 1
 (gdb) FAIL: gdb.base/valgrind-infcall.exp: p gdb_test_infcall ()

Even though that was a native GNU/Linux test run, this test spawns
Valgrind and connects to it with "target remote".  The error above is
actually orthogonal to target-non-stop.  The real issue is that that
enables displaced stepping, and displaced stepping doesn't work with
Valgrind, because we can't write to the inferior memory (thus can't
copy the instruction to the scratch pad area).

I'm sure there will be other targets with the same issue, so trying to
identify Valgrind wouldn't be sufficient.  The fix is to try setting
up the displaced step anyway.  If we get a MEMORY_ERRORs, we disable
displaced stepping for that inferior, and fall back to doing an
in-line step-over.

Tested on x86_64 Fedora 20.

gdb/ChangeLog:
2015-04-01  Pedro Alves  <palves@redhat.com>

	* inferior.h (struct inferior) <displaced_stepping_failed>: New
	field.
	* infrun.c (use_displaced_stepping_now_p): New parameter 'inf'.
	Return false if dispaced stepping failed before.
	(resume): Pass the current inferior to
	use_displaced_stepping_now_p.  Wrap displaced_step_prepare in
	TRY/CATCH.  If we get a MEMORY_ERROR, set the inferior's
	displaced_stepping_failed flag, and fall back to an in-line
	step-over.

gdb/testsuite/ChangeLog:
2015-04-01  Pedro Alves  <palves@redhat.com>

	* gdb.base/valgrind-disp-step.c: New file.
	* gdb.base/valgrind-disp-step.exp: New file.
---
 gdb/inferior.h                                |   4 +
 gdb/infrun.c                                  |  66 ++++++++++++---
 gdb/testsuite/gdb.base/valgrind-disp-step.c   |  32 +++++++
 gdb/testsuite/gdb.base/valgrind-disp-step.exp | 116 ++++++++++++++++++++++++++
 4 files changed, 205 insertions(+), 13 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/valgrind-disp-step.c
 create mode 100644 gdb/testsuite/gdb.base/valgrind-disp-step.exp

diff --git a/gdb/inferior.h b/gdb/inferior.h
index 3cec101..d831c90 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -414,6 +414,10 @@ struct inferior
      operation to get past e.g., a breakpoint.  */
   struct thread_info *step_over_queue_head;
 
+  /* True if preparing a displaced step ever failed.  If so, we won't
+     try displaced stepping for this inferior again.  */
+  int displaced_stepping_failed;
+
   /* Per inferior data-pointers required by other GDB modules.  */
   REGISTRY_FIELDS;
 };
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 184622a..44c0fa6 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -1619,7 +1619,7 @@ can_use_displaced_stepping_p (struct gdbarch *gdbarch)
    over a breakpoint in the current thread.  */
 
 static int
-use_displaced_stepping_now_p (struct gdbarch *gdbarch,
+use_displaced_stepping_now_p (struct inferior *inf, struct gdbarch *gdbarch,
 			      enum gdb_signal sig)
 {
   CORE_ADDR retval;
@@ -1628,7 +1628,8 @@ use_displaced_stepping_now_p (struct gdbarch *gdbarch,
      the comments for displaced_step_prepare explain why.  The
      comments in the handle_inferior event for dealing with 'random
      signals' explain what we do instead.  */
-  return  (sig == GDB_SIGNAL_0
+  return  (!inf->displaced_stepping_failed
+	   && sig == GDB_SIGNAL_0
 	   && can_use_displaced_stepping_p (gdbarch)
 	   && entry_point_address_query (&retval));
 }
@@ -1920,6 +1921,7 @@ static void keep_going_pass (struct execution_control_state *ecs);
 static void prepare_to_wait (struct execution_control_state *ecs);
 static int keep_going_stepped_thread (struct thread_info *tp);
 static int thread_still_needs_step_over (struct thread_info *tp);
+static void stop_all_threads (void);
 
 /* Are there any pending step-over requests for INF?  If so, run one
    now and return true.  Otherwise, return false.  */
@@ -2349,11 +2351,33 @@ resume (enum gdb_signal sig)
 
   /* If enabled, step over breakpoints by executing a copy of the
      instruction at a different address.  */
-  if (tp->control.trap_expected && use_displaced_stepping_now_p (gdbarch, sig))
+  if (tp->control.trap_expected
+      && use_displaced_stepping_now_p (current_inferior (), gdbarch, sig))
     {
-      struct displaced_step_inferior_state *displaced;
+      int prepared = -1;
 
-      if (!displaced_step_prepare (inferior_ptid))
+      TRY
+	{
+	  prepared = displaced_step_prepare (inferior_ptid);
+	}
+      CATCH (ex, RETURN_MASK_ERROR)
+	{
+	  if (ex.error != MEMORY_ERROR)
+	    throw_exception (ex);
+
+	  if (debug_infrun)
+	    {
+	      fprintf_unfiltered (gdb_stdlog,
+				  "infrun: disabling displaced stepping: %s\n",
+				  ex.message);
+	    }
+
+	  /* Disable further displaced stepping attempts.  */
+	  current_inferior ()->displaced_stepping_failed = 1;
+	}
+      END_CATCH
+
+      if (prepared == 0)
 	{
 	  if (debug_infrun)
 	    fprintf_unfiltered (gdb_stdlog,
@@ -2364,14 +2388,29 @@ resume (enum gdb_signal sig)
 	  discard_cleanups (old_cleanups);
 	  return;
 	}
+      else if (prepared < 0)
+	{
+	  /* Fallback to stepping over the breakpoint in-line.  */
 
-      /* Update pc to reflect the new address from which we will execute
-	 instructions due to displaced stepping.  */
-      pc = regcache_read_pc (get_thread_regcache (inferior_ptid));
+	  if (target_is_non_stop_p ())
+	    stop_all_threads ();
 
-      displaced = get_displaced_stepping_state (ptid_get_pid (inferior_ptid));
-      step = gdbarch_displaced_step_hw_singlestep (gdbarch,
-						   displaced->step_closure);
+	  set_step_over_info (get_regcache_aspace (regcache),
+			      regcache_read_pc (regcache), 0);
+	  insert_breakpoints ();
+	}
+      else if (prepared > 0)
+	{
+	  struct displaced_step_inferior_state *displaced;
+
+	  /* Update pc to reflect the new address from which we will
+	     execute instructions due to displaced stepping.  */
+	  pc = regcache_read_pc (get_thread_regcache (inferior_ptid));
+
+	  displaced = get_displaced_stepping_state (ptid_get_pid (inferior_ptid));
+	  step = gdbarch_displaced_step_hw_singlestep (gdbarch,
+						       displaced->step_closure);
+	}
     }
 
   /* Do we need to do it the hard way, w/temp breakpoints?  */
@@ -2500,7 +2539,7 @@ resume (enum gdb_signal sig)
 
   if (debug_displaced
       && tp->control.trap_expected
-      && use_displaced_stepping_now_p (gdbarch, sig))
+      && use_displaced_stepping_now_p (current_inferior (), gdbarch, sig))
     {
       struct regcache *resume_regcache = get_thread_regcache (tp->ptid);
       struct gdbarch *resume_gdbarch = get_regcache_arch (resume_regcache);
@@ -7153,7 +7192,8 @@ keep_going_pass (struct execution_control_state *ecs)
       remove_wps = (step_what & STEP_OVER_WATCHPOINT);
 
       if (remove_bp
-	  && !use_displaced_stepping_now_p (get_regcache_arch (regcache),
+	  && !use_displaced_stepping_now_p (current_inferior (),
+					    get_regcache_arch (regcache),
 					    signo))
 	{
 	  set_step_over_info (get_regcache_aspace (regcache),
diff --git a/gdb/testsuite/gdb.base/valgrind-disp-step.c b/gdb/testsuite/gdb.base/valgrind-disp-step.c
new file mode 100644
index 0000000..baba74e
--- /dev/null
+++ b/gdb/testsuite/gdb.base/valgrind-disp-step.c
@@ -0,0 +1,32 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2015 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+static int
+foo (void)
+{
+}
+
+int
+main (void)
+{
+  foo (); /* stop 0 */
+  foo (); /* stop 1 */
+  foo (); /* stop 2 */
+  foo (); /* stop 3 */
+  foo (); /* stop 4 */
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/valgrind-disp-step.exp b/gdb/testsuite/gdb.base/valgrind-disp-step.exp
new file mode 100644
index 0000000..c46d331
--- /dev/null
+++ b/gdb/testsuite/gdb.base/valgrind-disp-step.exp
@@ -0,0 +1,116 @@
+# Copyright 2012-2015 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Step over breakpoints with displaced stepping off/on.  We can't
+# really use displaced stepping with Valgrind, so what this really
+# tests is that GDB falls back to in-line stepping automatically
+# instead of getting stuck or crashing.
+
+if [is_remote target] {
+    # The test always runs locally.
+    return 0
+}
+
+standard_testfile .c
+if {[build_executable "failed to build" $testfile $srcfile {debug}] == -1} {
+    return -1
+}
+
+set test "spawn valgrind"
+set cmd "valgrind --vgdb-error=0 $binfile"
+set res [remote_spawn host $cmd]
+if { $res < 0 || $res == "" } {
+    verbose -log "Spawning $cmd failed."
+    unsupported $test
+    return -1
+}
+pass $test
+# Declare GDB now as running.
+set gdb_spawn_id $res
+
+# GDB started by vgdb stops already after the startup is executed, like with
+# non-extended gdbserver.  It is also not correct to run/attach the inferior.
+set use_gdb_stub 1
+
+set test "valgrind started"
+# The trailing '.' differs for different memcheck versions.
+gdb_test_multiple "" $test {
+    -re "Memcheck, a memory error detector\\.?\r\n" {
+	pass $test
+    }
+    -re "valgrind: failed to start tool 'memcheck' for platform '.*': No such file or directory" {
+	unsupported $test
+	return -1
+    }
+    -re "valgrind: wrong ELF executable class" {
+	unsupported $test
+	return -1
+    }
+    -re "command not found" {
+	# The spawn succeeded, but then valgrind was not found - e.g. if
+	# we spawned SSH to a remote system.
+	unsupported $test
+	return -1
+    }
+    -re "valgrind: Bad option.*--vgdb-error=0" {
+	# valgrind is not >= 3.7.0.
+	unsupported $test
+	return -1
+    }
+}
+
+set test "vgdb prompt"
+# The trailing '.' differs for different memcheck versions.
+gdb_test_multiple "" $test {
+    -re "  (target remote | \[^\r\n\]*/vgdb \[^\r\n\]*)\r\n" {
+	set vgdbcmd $expect_out(1,string)
+	pass $test
+    }
+}
+
+# Do not kill valgrind.
+set valgrind_pid [exp_pid -i [board_info host fileid]]
+unset gdb_spawn_id
+set board [host_info name]
+unset_board_info fileid
+
+clean_restart $testfile
+
+gdb_test "$vgdbcmd" " in \\.?_start .*" "target remote for vgdb"
+
+gdb_test "monitor v.set gdb_output" "valgrind output will go to gdb.*"
+
+gdb_test_no_output "set displaced-stepping off"
+gdb_breakpoint "main" "breakpoint at main"
+gdb_test "continue" " stop 0 .*" "continue to main"
+delete_breakpoints
+
+set curr_stop 0
+foreach displaced { "off" "on" } {
+    with_test_prefix "displaced $displaced" {
+
+	gdb_test_no_output "set displaced-stepping $displaced"
+
+	foreach go { "once" "twice" } {
+	    with_test_prefix $go {
+		gdb_test "break" "Breakpoint .* at .*" "set breakpoint"
+		gdb_test "next" " stop [incr curr_stop] .*" "step over breakpoint"
+	    }
+	}
+    }
+}
+
+# Only if valgrind got stuck.
+remote_exec host "kill -9 ${valgrind_pid}"
-- 
1.9.3

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 05/17] remote.c/all-stop: Implement TARGET_WAITKIND_NO_RESUMED and TARGET_WNOHANG
  2015-04-01 22:14 [PATCH 00/17] All-stop on top of non-stop Pedro Alves
                   ` (14 preceding siblings ...)
  2015-04-01 23:06 ` [PATCH 14/17] Fix interrupt-noterm.exp on targets always in non-stop Pedro Alves
@ 2015-04-01 23:07 ` Pedro Alves
  2015-04-01 23:08 ` [PATCH 11/17] Teach non-stop to do in-line step-overs (stop all, step, restart) Pedro Alves
  2015-04-07 12:52 ` [cancel] Re: [PATCH 00/17] All-stop on top of non-stop Pedro Alves
  17 siblings, 0 replies; 26+ messages in thread
From: Pedro Alves @ 2015-04-01 23:07 UTC (permalink / raw)
  To: gdb-patches

Even though "target remote" supports target-async, the all-stop
target_wait implementation ignores TARGET_WNOHANG.  If the core
happens to poll for events and we've already read the stop reply out
of the serial/socket, remote_wait_as hangs forever instead of
returning an indication that there aren't events to process.  This
can't happen currently, but later changes will trigger this.

gdb/ChangeLog:
2015-04-01  Pedro Alves  <palves@redhat.com>

	* remote.c (remote_wait_as): If not waiting for a stop reply,
	return TARGET_WAITKIND_NO_RESUMED.  If TARGET_WNOHANG is
	requested, don't block waiting forever.
---
 gdb/remote.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/gdb/remote.c b/gdb/remote.c
index 131ebf1..f46dd89 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -5953,6 +5953,14 @@ remote_wait_as (ptid_t ptid, struct target_waitstatus *status, int options)
     {
       int ret;
       int is_notif;
+      int forever = ((options & TARGET_WNOHANG) == 0
+		     && wait_forever_enabled_p);
+
+      if (!rs->waiting_for_stop_reply)
+	{
+	  status->kind = TARGET_WAITKIND_NO_RESUMED;
+	  return minus_one_ptid;
+	}
 
       if (!target_is_async_p ())
 	{
@@ -5971,7 +5979,7 @@ remote_wait_as (ptid_t ptid, struct target_waitstatus *status, int options)
 	 However, before we do that we need to ensure that the caller
 	 knows how to take the target into/out of async mode.  */
       ret = getpkt_or_notif_sane (&rs->buf, &rs->buf_size,
-				  wait_forever_enabled_p, &is_notif);
+				  forever, &is_notif);
 
       if (!target_is_async_p ())
 	signal (SIGINT, ofunc);
@@ -5980,6 +5988,9 @@ remote_wait_as (ptid_t ptid, struct target_waitstatus *status, int options)
 	 not interesting.  */
       if (ret != -1 && is_notif)
 	return minus_one_ptid;
+
+      if (ret == -1 && (options & TARGET_WNOHANG) != 0)
+	return minus_one_ptid;
     }
 
   buf = rs->buf;
-- 
1.9.3

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 11/17] Teach non-stop to do in-line step-overs (stop all, step, restart)
  2015-04-01 22:14 [PATCH 00/17] All-stop on top of non-stop Pedro Alves
                   ` (15 preceding siblings ...)
  2015-04-01 23:07 ` [PATCH 05/17] remote.c/all-stop: Implement TARGET_WAITKIND_NO_RESUMED and TARGET_WNOHANG Pedro Alves
@ 2015-04-01 23:08 ` Pedro Alves
  2015-04-07 12:52 ` [cancel] Re: [PATCH 00/17] All-stop on top of non-stop Pedro Alves
  17 siblings, 0 replies; 26+ messages in thread
From: Pedro Alves @ 2015-04-01 23:08 UTC (permalink / raw)
  To: gdb-patches

That is, step past breakpoints by:

 - pausing all threads
 - removing breakpoint at PC
 - single-step
 - reinsert breakpoint
 - restart threads

similarly to all-stop (with displaced stepping disabled).  This allows
non-stop to work on targets/architectures without displaced stepping
support.  That is, it makes displaced stepping an optimization instead
of a requirement.  For example, in principle, all GNU/Linux ports
support non-stop mode at the target_ops level, but not all
corresponding gdbarch's implement displaced stepping.  This should
make non-stop work for all (albeit, not as efficiently).  And then
there are scenarios where even if the architecture supports displaced
stepping, we can't use it, because we e.g., don't find a usable
address to use as displaced step scratch pad.  It should also fix
stepping past watchpoints on targets that have non-continuable
watchpoints in non-stop mode (e.g., PPC, untested).  Running the
instruction out of line in the displaced stepping scratch pad doesn't
help that case, as the copied instruction reads/writes the same
watched memory...  We can fix that too by teaching GDB to only remove
the watchpoint from the thread that we want to move past the
watchpoint (currently, removing a watchpoint always removes it from
all threads), but again, that can be considered an optimization; not
all targets would support it.

For those familiar with the gdb and gdbserver Linux target_ops
backends, the implementation should look similar, except it is done on
the core side.  When we pause threads, we may find they stop with an
interesting event that should be handled later when the thread is
re-resumed, thus we store such events in the thread object, and mark
the event as pending.  We should only consume pending events if the
thread is indeed resumed, thus we add a new "resumed" flag to the
thread object. At a later stage, we might add new target methods to
accelerate some of this, like "pause all threads", with corresponding
RSP packets, but we'd still need a fallback method for remote targets
that don't support such packets, so, again, that can be deferred as
optimization.

My _real_ motivation here is making it possible to reimplement
all-stop mode on top of the target always working on non-stop mode, so
that e.g., we can send RSP packets to a remote target even while the
target is running -- can't do that in the all-stop RSP variant, by
design).

Tested on x86_64 Fedora 20, with and without "set displaced off"
forced.  The latter forces the new code paths whenever GDB needs to
step past a breakpoint.

gdb/ChangeLog:
2015-04-01  Pedro Alves  <pedro@codesourcery.com>

	* breakpoint.c (breakpoints_should_be_inserted_now): If any thread
	has a pending status, return true.
	* gdbthread.h: Include target/waitstatus.h.
	(struct thread_suspend_state) <stop_reason, waitstatus_pending_p,
	stop_pc>: New fields.
	(struct thread_info) <resumed>: New field.
	(set_resumed): Delare.
	* infrun.c: Include "event-loop.h".
	(infrun_async_inferior_event_token, infrun_is_async): New globals.
	(infrun_async): New function.
	(clear_step_over_info): Add debug output.
	(displaced_step_fixup): New returns int.
	(start_step_over_inferior): Also check step_over_info_valid_p.
	Also make sure the thread isn't resumed.
	(resume_cleanups): Clear the thread's resumed flag.
	(resume): Set the thread's resumed flag.  Return early if the
	thread has a pending status.  Allow stepping a breakpoint with no
	signal.
	(clear_proceed_status_thread): If the thread has a pending status,
	and that status is a finished step, discard the pending status.
	(clear_proceed_status): Don't clear step_over_info here.
	(proceed): If there's already an in-line step-over in progress
	return without resuming anything.
	(random_pending_event_thread, do_target_wait): New functions.
	(prepare_for_detach, wait_for_inferior, fetch_inferior_event): Use
	do_target_wait.
	(wait_one): New function.
	(THREAD_STOPPED_BY): New macro.
	(thread_stopped_by_watchpoint, thread_stopped_by_sw_breakpoint)
	(thread_stopped_by_hw_breakpoint): New functions.
	(switch_to_thread_cleanup, stop_all_threads): New functions.
	(handle_inferior_event): Also call set_resumed(false) on all
	threads implicitly stopped by the event.
	(restart_threads): New function.
	(finish_step_over): If we were doing an in-line step-over before,
	and no longer are after trying to start a new step-over, restart
	all threads.
	(handle_signal_stop) <random signal>: Clear step_over_info before
	delivering the signal.
	(keep_going_stepped_thread): Return early if the thread has a
	pending status.  Mark the thread as resumed.
	(keep_going_pass): Assert the thread isn't resumed.  If some other
	thread is doing an in-line step-over, defer the resume.  If we
	just started a new in-line step-over, stop all threads.
	(infrun_async_inferior_event_handler): New function.
	(_initialize_infrun): Create async event handler with
	infrun_async_inferior_event_handler as callback.
	(infrun_async): New declaration.
	* target.c (target_async): New function.
	* target.h (target_async): Declare macro and readd as function
	declaration.
	* target/waitstatus.h (enum target_stop_reason)
	<TARGET_STOPPED_BY_SINGLE_STEP>: New value.
	* thread.c (new_thread): Clear the new waitstatus field.
	(set_resumed): New function.
---
 gdb/breakpoint.c        |   9 +
 gdb/gdbthread.h         |  31 ++
 gdb/infrun.c            | 913 +++++++++++++++++++++++++++++++++++++++++++++---
 gdb/infrun.h            |   3 +
 gdb/target.c            |   9 +
 gdb/target.h            |   3 +-
 gdb/target/waitstatus.h |   5 +-
 gdb/thread.c            |  23 ++
 8 files changed, 940 insertions(+), 56 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 12955cb..76c86f8 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -468,6 +468,8 @@ breakpoints_should_be_inserted_now (void)
     }
   else if (target_has_execution)
     {
+      struct thread_info *tp;
+
       if (always_inserted_mode)
 	{
 	  /* The user wants breakpoints inserted even if all threads
@@ -477,6 +479,13 @@ breakpoints_should_be_inserted_now (void)
 
       if (threads_are_executing ())
 	return 1;
+
+      /* Don't remove breakpoints yet if, even though all threads are
+	 stopped, we still have events to process.  */
+      ALL_NON_EXITED_THREADS (tp)
+	if (tp->resumed
+	    && tp->suspend.waitstatus_pending_p)
+	  return 1;
     }
   return 0;
 }
diff --git a/gdb/gdbthread.h b/gdb/gdbthread.h
index f15cbc4..780d288 100644
--- a/gdb/gdbthread.h
+++ b/gdb/gdbthread.h
@@ -29,6 +29,7 @@ struct symtab;
 #include "inferior.h"
 #include "btrace.h"
 #include "common/vec.h"
+#include "target/waitstatus.h"
 
 /* Frontend view of the thread state.  Possible extensions: stepping,
    finishing, until(ling),...  */
@@ -159,6 +160,23 @@ struct thread_suspend_state
      should be suppressed, the core will take care of clearing this
      before the target is resumed.  */
   enum gdb_signal stop_signal;
+
+  /* The reason the thread last stopped, if we need to track it
+     (breakpoint, watchpoint, etc.)  */
+  enum target_stop_reason stop_reason;
+
+  /* The waitstatus for this thread's last event.  */
+  struct target_waitstatus waitstatus;
+  /* If true WAITSTATUS hasn't been handled yet.  */
+  int waitstatus_pending_p;
+
+  /* Record the pc of the thread the last time it stopped.  (This is
+     not the current thread's PC as that may have changed since the
+     last stop, e.g., "return" command, or "p $pc = 0xf000").  This
+     used in coordination with stop_reason and waitstatus_pending_p:
+     if the thread's PC is changed since it last stopped, a pending
+     breakpoint waitstatus is discarded.  */
+  CORE_ADDR stop_pc;
 };
 
 typedef struct value *value_ptr;
@@ -183,6 +201,14 @@ struct thread_info
      thread is off and running.  */
   int executing;
 
+  /* Non-zero if this thread will be/has been resumed.  Note that a
+     thread can be marked both as not-executing and resumed at the
+     same time.  This happens if we try to resume a thread that has a
+     wait status pending.  We shouldn't let the thread run until that
+     wait status has been processed, but we should not process that
+     wait status if we didn't try to let the thread run.  */
+  int resumed;
+
   /* Frontend view of the thread state.  Note that the THREAD_RUNNING/
      THREAD_STOPPED states are different from EXECUTING.  When the
      thread is stopped internally while handling an internal event,
@@ -399,6 +425,11 @@ extern int thread_count (void);
 /* Switch from one thread to another.  */
 extern void switch_to_thread (ptid_t ptid);
 
+/* Marks or clears thread(s) PTID as resumed.  If PTID is
+   MINUS_ONE_PTID, applies to all threads.  If ptid_is_pid(PTID) is
+   true, applies to all threads of the process pointed at by PTID.  */
+extern void set_resumed (ptid_t ptid, int resumed);
+
 /* Marks thread PTID is running, or stopped. 
    If PTID is minus_one_ptid, marks all threads.  */
 extern void set_running (ptid_t ptid, int running);
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 9fd4001..d95a60a 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -60,6 +60,36 @@
 #include "target-descriptions.h"
 #include "target-dcache.h"
 #include "terminal.h"
+#include "event-loop.h"
+
+/* Asynchronous signal handler registered as event loop source for
+   when we have pending events ready to be passed to the core.  */
+static struct async_event_handler *infrun_async_inferior_event_token;
+
+/* Stores whether infrun_async was previously enabled or disabled.
+   Starts off as -1, indicating "never enabled/disabled".  */
+static int infrun_is_async = -1;
+
+/* See infrun.h.  */
+
+void
+infrun_async (int enable)
+{
+  if (infrun_is_async != enable)
+    {
+      infrun_is_async = enable;
+
+      if (debug_infrun)
+	fprintf_unfiltered (gdb_stdlog,
+			    "infrun: infrun_async(%d)\n",
+			    enable);
+
+      if (enable)
+	mark_async_event_handler (infrun_async_inferior_event_token);
+      else
+	clear_async_event_handler (infrun_async_inferior_event_token);
+    }
+}
 
 /* Prototypes for local functions */
 
@@ -1283,6 +1313,9 @@ set_step_over_info (struct address_space *aspace, CORE_ADDR address,
 static void
 clear_step_over_info (void)
 {
+  if (debug_infrun)
+    fprintf_unfiltered (gdb_stdlog,
+			"infrun: clear_step_over_info\n");
   step_over_info.aspace = NULL;
   step_over_info.address = 0;
   step_over_info.nonsteppable_watchpoint_p = 0;
@@ -1788,21 +1821,28 @@ displaced_step_restore (struct displaced_step_inferior_state *displaced,
 				  displaced->step_copy));
 }
 
-static void
+/* If we displaced stepped an instruction successfully, adjust
+   registers and memory to yield the same effect the instruction would
+   have had if we had executed it at its original address, and return
+   1.  If the instruction didn't complete, relocate the PC and return
+   -1.  If the thread wasn't displaced stepping, return 0.  */
+
+static int
 displaced_step_fixup (ptid_t event_ptid, enum gdb_signal signal)
 {
   struct cleanup *old_cleanups;
   struct displaced_step_inferior_state *displaced
     = get_displaced_stepping_state (ptid_get_pid (event_ptid));
+  int ret;
 
   /* Was any thread of this process doing a displaced step?  */
   if (displaced == NULL)
-    return;
+    return 0;
 
   /* Was this event for the pid we displaced?  */
   if (ptid_equal (displaced->step_ptid, null_ptid)
       || ! ptid_equal (displaced->step_ptid, event_ptid))
-    return;
+    return 0;
 
   old_cleanups = make_cleanup (displaced_step_clear_cleanup, displaced);
 
@@ -1821,6 +1861,7 @@ displaced_step_fixup (ptid_t event_ptid, enum gdb_signal signal)
                                     displaced->step_original,
                                     displaced->step_copy,
                                     get_thread_regcache (displaced->step_ptid));
+      ret = 1;
     }
   else
     {
@@ -1831,11 +1872,14 @@ displaced_step_fixup (ptid_t event_ptid, enum gdb_signal signal)
 
       pc = displaced->step_original + (pc - displaced->step_copy);
       regcache_write_pc (regcache, pc);
+      ret = -1;
     }
 
   do_cleanups (old_cleanups);
 
   displaced->step_ptid = null_ptid;
+
+  return ret;
 }
 
 /* Data to be passed around while handling an event.  This data is
@@ -1882,9 +1926,10 @@ static int thread_still_needs_step_over (struct thread_info *tp);
 static int
 start_step_over_inferior (struct inferior *inf)
 {
-  /* Don't start a new step-over if we already have a displaced step
+  /* Don't start a new step-over if we already have a step-over
      operation ongoing.  */
-  if (displaced_step_in_progress (inf->pid))
+  if (step_over_info_valid_p ()
+      || displaced_step_in_progress (inf->pid))
     return 0;
 
   while (inf->step_over_queue_head != NULL)
@@ -1897,13 +1942,16 @@ start_step_over_inferior (struct inferior *inf)
 
       step_over_chain_dequeue (&inf->step_over_queue_head);
 
-      if (tp->control.trap_expected || tp->executing)
+      if (tp->control.trap_expected
+	  || tp->resumed
+	  || tp->executing)
 	{
 	  internal_error (__FILE__, __LINE__,
 			  "[%s] has inconsistent state: "
-			  "trap_expected=%d, executing=%d\n",
+			  "trap_expected=%d, resumed=%d, executing=%d\n",
 			  target_pid_to_str (tp->ptid),
 			  tp->control.trap_expected,
+			  tp->resumed,
 			  tp->executing);
 	}
 
@@ -1990,7 +2038,12 @@ static void
 resume_cleanups (void *ignore)
 {
   if (!ptid_equal (inferior_ptid, null_ptid))
-    delete_single_step_breakpoints (inferior_thread ());
+    {
+      struct thread_info *tp = inferior_thread ();
+
+      tp->resumed = 0;
+      delete_single_step_breakpoints (tp);
+    }
 
   normal_stop ();
 }
@@ -2139,12 +2192,39 @@ resume (enum gdb_signal sig)
      single-step).  */
   int step;
 
-  tp->stepped_breakpoint = 0;
+  tp->resumed = 1;
 
   gdb_assert (tp->step_over_next == NULL);
 
   QUIT;
 
+  if (tp->suspend.waitstatus_pending_p)
+    {
+      if (debug_infrun)
+	{
+	  char *statstr;
+
+	  statstr = target_waitstatus_to_string (&tp->suspend.waitstatus);
+	  fprintf_unfiltered (gdb_stdlog,
+			      "infrun: resume: thread %s has pending wait status %s "
+			      "(currently_stepping=%d).\n",
+			      target_pid_to_str (tp->ptid),  statstr,
+			      currently_stepping (tp));
+	  xfree (statstr);
+	}
+
+      /* Avoid confusing the next resume, if the next stop/resume
+	 happens to apply to another thread.  */
+      tp->suspend.stop_signal = GDB_SIGNAL_0;
+      discard_cleanups (old_cleanups);
+
+      if (target_can_async_p ())
+	target_async (1);
+      return;
+    }
+
+  tp->stepped_breakpoint = 0;
+
   /* Depends on stepped_breakpoint.  */
   step = currently_stepping (tp);
 
@@ -2276,6 +2356,7 @@ resume (enum gdb_signal sig)
 				"Got placed in displaced stepping queue\n");
 
 	  tp->control.trap_expected = 0;
+	  tp->resumed = 0;
 	  discard_cleanups (old_cleanups);
 	  return;
 	}
@@ -2365,11 +2446,12 @@ resume (enum gdb_signal sig)
   if (execution_direction != EXEC_REVERSE
       && step && breakpoint_inserted_here_p (aspace, pc))
     {
-      /* The only case we currently need to step a breakpoint
-	 instruction is when we have a signal to deliver.  See
-	 handle_signal_stop where we handle random signals that could
-	 take out us out of the stepping range.  Normally, in that
-	 case we end up continuing (instead of stepping) over the
+      /* There are two cases where we currently need to step a
+	 breakpoint instruction when we have a signal to deliver:
+
+	 - See handle_signal_stop where we handle random signals that
+	 could take out us out of the stepping range.  Normally, in
+	 that case we end up continuing (instead of stepping) over the
 	 signal handler with a breakpoint at PC, but there are cases
 	 where we should _always_ single-step, even if we have a
 	 step-resume breakpoint, like when a software watchpoint is
@@ -2382,8 +2464,20 @@ resume (enum gdb_signal sig)
 	 recurses and executes PC again, it'll miss the breakpoint.
 	 So we leave the breakpoint inserted anyway, but we need to
 	 record that we tried to step a breakpoint instruction, so
-	 that adjust_pc_after_break doesn't end up confused.  */
-      gdb_assert (sig != GDB_SIGNAL_0);
+	 that adjust_pc_after_break doesn't end up confused.
+
+         - In non-stop if we insert a breakpoint (e.g., a step-resume)
+	 in one thread after another thread that was stepping had been
+	 momentarily paused for a step-over.  When we re-resume the
+	 stepping thread, it may be resumed from that address with a
+	 breakpoint that hasn't trapped yet.  Seen with
+	 gdb.threads/non-stop-fair-events.exp, on targets that don't
+	 do displaced stepping.  */
+
+      if (debug_infrun)
+	fprintf_unfiltered (gdb_stdlog,
+			    "infrun: resume: [%s] stepped breakpoint\n",
+			    target_pid_to_str (tp->ptid));
 
       tp->stepped_breakpoint = 1;
 
@@ -2436,6 +2530,37 @@ clear_proceed_status_thread (struct thread_info *tp)
 			"infrun: clear_proceed_status_thread (%s)\n",
 			target_pid_to_str (tp->ptid));
 
+  /* If we're starting a new sequence, then the previous finished
+     single-step is no longer relevant.  */
+  if (tp->suspend.waitstatus_pending_p)
+    {
+      if (tp->suspend.stop_reason == TARGET_STOPPED_BY_SINGLE_STEP)
+	{
+	  if (debug_infrun)
+	    fprintf_unfiltered (gdb_stdlog,
+				"infrun: clear_proceed_status: pending "
+				"event of %s was a finished step. "
+				"Discarding.\n",
+				target_pid_to_str (tp->ptid));
+
+	  tp->suspend.waitstatus_pending_p = 0;
+	  tp->suspend.stop_reason = TARGET_STOPPED_BY_NO_REASON;
+	}
+      else if (debug_infrun)
+	{
+	  char *statstr;
+
+	  statstr = target_waitstatus_to_string (&tp->suspend.waitstatus);
+	  fprintf_unfiltered (gdb_stdlog,
+			      "infrun: clear_proceed_status_thread: thread %s "
+			      "has pending wait status %s "
+			      "(currently_stepping=%d).\n",
+			      target_pid_to_str (tp->ptid), statstr,
+			      currently_stepping (tp));
+	  xfree (statstr);
+	}
+    }
+
   /* If this signal should not be seen by program, give it zero.
      Used for debugging signals.  */
   if (!signal_pass_state (tp->suspend.stop_signal))
@@ -2499,8 +2624,6 @@ clear_proceed_status (int step)
 
   stop_after_trap = 0;
 
-  clear_step_over_info ();
-
   observer_notify_about_to_proceed ();
 
   if (stop_registers)
@@ -2740,16 +2863,39 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal)
     thread_step_over_chain_enqueue (tp);
 
   started = start_step_over (tp->ptid);
-  if (started)
+
+  if (!non_stop)
     {
-      /* A new displaced stepping sequence was started.  */
+      if (started)
+	{
+	  /* A new step-over sequence was started.  Even if we could
+	     resume other threads in case of displaced stepping, we
+	     wouldn't be able to do that with the remote target, as we
+	     can't send another packet until the target stops again.
+	     To simplify, we handle displaced stepping the same as
+	     in-line stepping, and resume all other threads only when
+	     the displaced stepping finishes.  */
+	}
+      else
+	{
+	  reset_ecs (ecs, tp);
+	  keep_going_pass (ecs);
+	  if (!ecs->wait_some_more)
+	    error ("Command aborted.");
+	}
     }
-  else if (tp->step_over_next == NULL)
+  else if (step_over_info_valid_p ())
+    ;
+  else
     {
-      reset_ecs (ecs, tp);
-      keep_going_pass (ecs);
-      if (!ecs->wait_some_more)
-	error ("Command aborted.");
+      /* If the thread wasn't started, and isn't queued, run it.  */
+      if (!tp->executing && tp->step_over_next == NULL)
+	{
+	  reset_ecs (ecs, tp);
+	  keep_going_pass (ecs);
+	  if (!ecs->wait_some_more)
+	    error ("Command aborted.");
+	}
     }
 
   discard_cleanups (old_chain);
@@ -3055,6 +3201,174 @@ print_target_wait_results (ptid_t waiton_ptid, ptid_t result_ptid,
   ui_file_delete (tmp_stream);
 }
 
+/* Select a thread at random, out of those which are resumed and have
+   had events.  */
+
+static struct thread_info *
+random_pending_event_thread (ptid_t waiton_ptid)
+{
+  struct thread_info *event_tp;
+  int num_events = 0;
+  int random_selector;
+
+  /* First see how many events we have.  Count only resumed threads
+     that have an event pending.  */
+  ALL_NON_EXITED_THREADS (event_tp)
+    if (ptid_match (event_tp->ptid, waiton_ptid)
+	&& event_tp->resumed
+	&& event_tp->suspend.waitstatus_pending_p)
+      num_events++;
+
+  if (num_events == 0)
+    return NULL;
+
+  /* Now randomly pick a thread out of those that have had events.  */
+  random_selector = (int)
+    ((num_events * (double) rand ()) / (RAND_MAX + 1.0));
+
+  if (debug_infrun && num_events > 1)
+    fprintf_unfiltered (gdb_stdlog,
+			"infrun: Found %d events, selecting #%d\n",
+			num_events, random_selector);
+
+  /* Select the Nth thread that has had an event.  */
+  ALL_NON_EXITED_THREADS (event_tp)
+    if (ptid_match (event_tp->ptid, waiton_ptid)
+	&& event_tp->resumed
+	&& event_tp->suspend.waitstatus_pending_p)
+      if (random_selector-- == 0)
+	break;
+
+  return event_tp;
+}
+
+/* Wrapper for target_wait that first checks whether threads have
+   pending status to report before actually asking the target for more
+   events.  */
+
+static ptid_t
+do_target_wait (ptid_t ptid, struct target_waitstatus *status, int options)
+{
+  ptid_t event_ptid;
+  struct thread_info *tp;
+
+  /* First check if there is a resumed thread with a wait status
+     pending.  */
+  if (ptid_equal (ptid, minus_one_ptid) || ptid_is_pid (ptid))
+    {
+      tp = random_pending_event_thread (ptid);
+    }
+  else
+    {
+      if (debug_infrun)
+	fprintf_unfiltered (gdb_stdlog,
+			    "infrun: Waiting for specific thread %s.\n",
+			    target_pid_to_str (ptid));
+
+      /* We have a specific thread to check.  */
+      tp = find_thread_ptid (ptid);
+      gdb_assert (tp != NULL);
+      if (!tp->suspend.waitstatus_pending_p)
+	tp = NULL;
+    }
+
+  if (tp != NULL
+      && (tp->suspend.stop_reason == TARGET_STOPPED_BY_SW_BREAKPOINT
+	  || tp->suspend.stop_reason == TARGET_STOPPED_BY_HW_BREAKPOINT))
+    {
+      struct regcache *regcache = get_thread_regcache (tp->ptid);
+      struct gdbarch *gdbarch = get_regcache_arch (regcache);
+      CORE_ADDR pc;
+      int discard = 0;
+
+      pc = regcache_read_pc (regcache);
+
+      if (pc != tp->suspend.stop_pc)
+	{
+	  if (debug_infrun)
+	    fprintf_unfiltered (gdb_stdlog,
+				"infrun: PC of %s changed.  was=%s, now=%s\n",
+				target_pid_to_str (tp->ptid),
+				paddress (target_gdbarch (), tp->prev_pc),
+				paddress (target_gdbarch (), pc));
+	  discard = 1;
+	}
+      else if (!breakpoint_inserted_here_p (get_regcache_aspace (regcache), pc))
+	{
+	  if (debug_infrun)
+	    fprintf_unfiltered (gdb_stdlog,
+				"infrun: previous breakpoint of %s, at %s gone\n",
+				target_pid_to_str (tp->ptid),
+				paddress (target_gdbarch (), pc));
+
+	  discard = 1;
+	}
+
+      if (discard)
+	{
+	  if (debug_infrun)
+	    fprintf_unfiltered (gdb_stdlog,
+				"infrun: pending event of %s cancelled.\n",
+				target_pid_to_str (tp->ptid));
+
+	  tp->suspend.waitstatus.kind = TARGET_WAITKIND_SPURIOUS;
+	  tp->suspend.stop_reason = TARGET_STOPPED_BY_NO_REASON;
+	}
+    }
+
+  if (tp != NULL)
+    {
+      if (debug_infrun)
+	{
+	  char *statstr;
+
+	  statstr = target_waitstatus_to_string (&tp->suspend.waitstatus);
+	  fprintf_unfiltered (gdb_stdlog,
+			      "infrun: Using pending wait status %s for %s.\n",
+			      statstr,
+			      target_pid_to_str (tp->ptid));
+	  xfree (statstr);
+	}
+
+      /* Now that we've selected our final event LWP, un-adjust its PC
+	 if it was a software breakpoint (and the target doesn't
+	 always adjust the PC itself).  */
+      if (tp->suspend.stop_reason == TARGET_STOPPED_BY_SW_BREAKPOINT
+	  && !target_supports_stopped_by_sw_breakpoint ())
+	{
+	  struct regcache *regcache;
+	  struct gdbarch *gdbarch;
+	  int decr_pc;
+
+	  regcache = get_thread_regcache (tp->ptid);
+	  gdbarch = get_regcache_arch (regcache);
+
+	  decr_pc = gdbarch_decr_pc_after_break (gdbarch);
+	  if (decr_pc != 0)
+	    {
+	      CORE_ADDR pc;
+
+	      pc = regcache_read_pc (regcache);
+	      regcache_write_pc (regcache, pc + decr_pc);
+	    }
+	}
+
+      tp->suspend.stop_reason = TARGET_STOPPED_BY_NO_REASON;
+      *status = tp->suspend.waitstatus;
+      tp->suspend.waitstatus_pending_p = 0;
+      return tp->ptid;
+    }
+
+  /* But if we don't find one, we'll have to wait.  */
+
+  if (deprecated_target_wait_hook)
+    event_ptid = deprecated_target_wait_hook (ptid, status, options);
+  else
+    event_ptid = target_wait (ptid, status, options);
+
+  return event_ptid;
+}
+
 /* Prepare and stabilize the inferior for detaching it.  E.g.,
    detaching while a thread is displaced stepping is a recipe for
    crashing it, as nothing would readjust the PC out of the scratch
@@ -3098,10 +3412,7 @@ prepare_for_detach (void)
 	 don't get any event.  */
       target_dcache_invalidate ();
 
-      if (deprecated_target_wait_hook)
-	ecs->ptid = deprecated_target_wait_hook (pid_ptid, &ecs->ws, 0);
-      else
-	ecs->ptid = target_wait (pid_ptid, &ecs->ws, 0);
+      ecs->ptid = do_target_wait (pid_ptid, &ecs->ws, 0);
 
       if (debug_infrun)
 	print_target_wait_results (pid_ptid, ecs->ptid, &ecs->ws);
@@ -3173,10 +3484,7 @@ wait_for_inferior (void)
 	 don't get any event.  */
       target_dcache_invalidate ();
 
-      if (deprecated_target_wait_hook)
-	ecs->ptid = deprecated_target_wait_hook (waiton_ptid, &ecs->ws, 0);
-      else
-	ecs->ptid = target_wait (waiton_ptid, &ecs->ws, 0);
+      ecs->ptid = do_target_wait (waiton_ptid, &ecs->ws, 0);
 
       if (debug_infrun)
 	print_target_wait_results (waiton_ptid, ecs->ptid, &ecs->ws);
@@ -3273,11 +3581,7 @@ fetch_inferior_event (void *client_data)
   make_cleanup_restore_integer (&execution_direction);
   execution_direction = target_execution_direction ();
 
-  if (deprecated_target_wait_hook)
-    ecs->ptid =
-      deprecated_target_wait_hook (waiton_ptid, &ecs->ws, TARGET_WNOHANG);
-  else
-    ecs->ptid = target_wait (waiton_ptid, &ecs->ws, TARGET_WNOHANG);
+  ecs->ptid = do_target_wait (waiton_ptid, &ecs->ws, TARGET_WNOHANG);
 
   if (debug_infrun)
     print_target_wait_results (waiton_ptid, ecs->ptid, &ecs->ws);
@@ -3650,6 +3954,319 @@ get_inferior_stop_soon (ptid_t ptid)
   return inf->control.stop_soon;
 }
 
+/* Wait for one event.  Store the resulting waitstatus in WS, and
+   return the event ptid.  */
+
+static ptid_t
+wait_one (struct target_waitstatus *ws)
+{
+  ptid_t event_ptid;
+  ptid_t wait_ptid = minus_one_ptid;
+
+  overlay_cache_invalid = 1;
+
+  /* Flush target cache before starting to handle each event.
+     Target was running and cache could be stale.  This is just a
+     heuristic.  Running threads may modify target memory, but we
+     don't get any event.  */
+  target_dcache_invalidate ();
+
+  if (deprecated_target_wait_hook)
+    event_ptid = deprecated_target_wait_hook (wait_ptid, ws, 0);
+  else
+    event_ptid = target_wait (wait_ptid, ws, 0);
+
+  if (debug_infrun)
+    print_target_wait_results (wait_ptid, event_ptid, ws);
+
+  if (ws->kind == TARGET_WAITKIND_SYSCALL_ENTRY
+      || ws->kind == TARGET_WAITKIND_SYSCALL_RETURN)
+    ws->value.syscall_number = UNKNOWN_SYSCALL;
+
+  return event_ptid;
+}
+
+/* Generate a wrapper for target_stopped_by_REASON that works on PTID
+   instead of the current thread.  */
+#define THREAD_STOPPED_BY(REASON)		\
+static int					\
+thread_stopped_by_ ## REASON (ptid_t ptid)	\
+{						\
+  struct cleanup *old_chain;			\
+  int res;					\
+						\
+  old_chain = save_inferior_ptid ();		\
+  inferior_ptid = ptid;				\
+						\
+  res = target_stopped_by_ ## REASON ();	\
+						\
+  do_cleanups (old_chain);			\
+						\
+  return res;					\
+}
+
+/* Generate thread_stopped_by_watchpoint.  */
+THREAD_STOPPED_BY (watchpoint)
+/* Generate thread_stopped_by_sw_breakpoint.  */
+THREAD_STOPPED_BY (sw_breakpoint)
+/* Generate thread_stopped_by_hw_breakpoint.  */
+THREAD_STOPPED_BY (hw_breakpoint)
+
+/* Cleanups that switches to the PTID pointed at by PTID_P.  */
+
+static void
+switch_to_thread_cleanup (void *ptid_p)
+{
+  ptid_t ptid = *(ptid_t *) ptid_p;
+
+  switch_to_thread (ptid);
+}
+
+/* Stop all threads.  */
+
+static void
+stop_all_threads (void)
+{
+  /* We may need multiple passes to discover all threads.  */
+  int pass;
+  int iterations = 0;
+  ptid_t entry_ptid;
+  struct cleanup *old_chain;
+
+  gdb_assert (non_stop);
+
+  if (debug_infrun)
+    fprintf_unfiltered (gdb_stdlog, "infrun.c: stop_all_threads\n");
+
+  entry_ptid = inferior_ptid;
+  old_chain = make_cleanup (switch_to_thread_cleanup, &entry_ptid);
+
+  /* Stop threads in two passes since threads could be spawning as we
+     go through the first pass.  In the second pass, we will stop such
+     spawned threads.  */
+  for (pass = 0; pass < 2; pass++, iterations++)
+    {
+      if (debug_infrun)
+	fprintf_unfiltered (gdb_stdlog,
+			    "infrun.c: stop_all_threads, pass=%d, "
+			    "iterations=%d\n", pass, iterations);
+      while (1)
+	{
+	  ptid_t event_ptid;
+	  struct target_waitstatus ws;
+	  int need_wait = 0;
+	  struct thread_info *t;
+
+	  update_thread_list ();
+
+	  /* Go through all threads looking for threads that we need
+	     to tell the target to stop.  */
+	  ALL_NON_EXITED_THREADS (t)
+	    {
+	      if (t->executing)
+		{
+		  /* If already stopping, don't request a stop again.
+		     We just haven't seen the notification yet.  */
+		  if (!t->stop_requested)
+		    {
+		      if (debug_infrun)
+			fprintf_unfiltered (gdb_stdlog,
+					    "infrun.c:   %s executing, "
+					    "need stop\n",
+					    target_pid_to_str (t->ptid));
+		      target_stop (t->ptid);
+		      t->stop_requested = 1;
+		    }
+		  else
+		    {
+		      if (debug_infrun)
+			fprintf_unfiltered (gdb_stdlog,
+					    "infrun.c:   %s executing, "
+					    "already stopping\n",
+					    target_pid_to_str (t->ptid));
+		    }
+
+		  if (t->stop_requested)
+		    need_wait = 1;
+		}
+	      else
+		{
+		  if (debug_infrun)
+		    fprintf_unfiltered (gdb_stdlog,
+					"infrun.c:   %s not executing\n",
+					target_pid_to_str (t->ptid));
+
+		  /* The thread may be not executing, but still be
+		     resumed with a pending status to process.  */
+		  t->resumed = 0;
+		}
+	    }
+
+	  if (!need_wait)
+	    break;
+
+	  /* If we find new threads on the second iteration, restart
+	     over.  We want to see two iterations in a row with all
+	     threads stopped.  */
+	  if (pass > 0)
+	    pass = -1;
+
+	  event_ptid = wait_one (&ws);
+	  if (ws.kind == TARGET_WAITKIND_NO_RESUMED)
+	    /* All resumed threads exited.  */
+	    ;
+	  else if (ws.kind == TARGET_WAITKIND_EXITED
+		   || ws.kind == TARGET_WAITKIND_SIGNALLED)
+	    {
+	      if (debug_infrun)
+		{
+		  ptid_t ptid = pid_to_ptid (ws.value.integer);
+
+		  fprintf_unfiltered (gdb_stdlog,
+				      "infrun: %s exited while "
+				      "stopping threads\n",
+				      target_pid_to_str (ptid));
+		}
+	    }
+	  else
+	    {
+	      if (!in_thread_list (event_ptid))
+		t = add_thread (event_ptid);
+	      else
+		t = find_thread_ptid (event_ptid);
+
+	      t->stop_requested = 0;
+	      t->executing = 0;
+	      t->resumed = 0;
+	      t->control.may_range_step = 0;
+
+	      if (ws.kind == TARGET_WAITKIND_STOPPED
+		  && ws.value.sig == GDB_SIGNAL_0)
+		{
+		  /* We caught the event that we intended to catch, so
+		     there's no event pending.  */
+		  t->suspend.waitstatus.kind = TARGET_WAITKIND_IGNORE;
+		  t->suspend.waitstatus_pending_p = 0;
+
+		  if (displaced_step_fixup (t->ptid, GDB_SIGNAL_0) < 0)
+		    {
+		      /* Add it back to the step-over queue.  */
+		      if (debug_infrun)
+			{
+			  fprintf_unfiltered (gdb_stdlog,
+					      "infrun: displaced-step of %s "
+					      "canceled: adding back to the "
+					      "step-over queue\n",
+					      target_pid_to_str (t->ptid));
+			}
+		      thread_step_over_chain_enqueue (t);
+		    }
+		}
+	      else
+		{
+		  enum gdb_signal sig;
+		  struct regcache *regcache;
+		  struct address_space *aspace;
+
+		  if (debug_infrun)
+		    {
+		      char *statstr;
+
+		      statstr = target_waitstatus_to_string (&ws);
+		      fprintf_unfiltered (gdb_stdlog,
+					  "infrun: target_wait %s, saving "
+					  "status for %d.%ld.%ld\n",
+					  statstr,
+					  ptid_get_pid (t->ptid),
+					  ptid_get_lwp (t->ptid),
+					  ptid_get_tid (t->ptid));
+		      xfree (statstr);
+		    }
+
+		  /* Record for later.  */
+		  t->suspend.waitstatus = ws;
+		  t->suspend.waitstatus_pending_p = 1;
+
+		  sig = (ws.kind == TARGET_WAITKIND_STOPPED
+			 ? ws.value.sig : GDB_SIGNAL_0);
+
+		  regcache = get_thread_regcache (t->ptid);
+		  aspace = get_regcache_aspace (regcache);
+
+		  if (ws.kind == TARGET_WAITKIND_STOPPED
+		      && ws.value.sig == GDB_SIGNAL_TRAP)
+		    {
+		      CORE_ADDR pc = regcache_read_pc (regcache);
+
+		      adjust_pc_after_break (t, &t->suspend.waitstatus);
+
+		      if (thread_stopped_by_watchpoint (t->ptid))
+			{
+			  t->suspend.stop_reason = TARGET_STOPPED_BY_WATCHPOINT;
+			}
+		      else if (target_supports_stopped_by_sw_breakpoint ()
+			       && thread_stopped_by_sw_breakpoint (t->ptid))
+			{
+			  t->suspend.stop_reason
+			    = TARGET_STOPPED_BY_SW_BREAKPOINT;
+			}
+		      else if (target_supports_stopped_by_hw_breakpoint ()
+			       && thread_stopped_by_hw_breakpoint (t->ptid))
+			{
+			  t->suspend.stop_reason
+			    = TARGET_STOPPED_BY_HW_BREAKPOINT;
+			}
+		      else if (!target_supports_stopped_by_hw_breakpoint ()
+			       && hardware_breakpoint_inserted_here_p (aspace,
+								       pc))
+			{
+			  t->suspend.stop_reason
+			    = TARGET_STOPPED_BY_HW_BREAKPOINT;
+			}
+		      else if (!target_supports_stopped_by_sw_breakpoint ()
+			       && software_breakpoint_inserted_here_p (aspace,
+								       pc))
+			{
+			  t->suspend.stop_reason
+			    = TARGET_STOPPED_BY_SW_BREAKPOINT;
+			}
+		      else if (!thread_has_single_step_breakpoints_set (t)
+			       && currently_stepping (t))
+			{
+			  t->suspend.stop_reason
+			    = TARGET_STOPPED_BY_SINGLE_STEP;
+			}
+		    }
+
+		  if (displaced_step_fixup (t->ptid, sig) < 0)
+		    {
+		      /* Add it back to the step-over queue.  */
+		      thread_step_over_chain_enqueue (t);
+		    }
+
+		  t->suspend.stop_pc = regcache_read_pc (regcache);
+
+		  if (debug_infrun)
+		    {
+		      fprintf_unfiltered (gdb_stdlog,
+					  "infrun: saved stop_pc=%s for %s "
+					  "(currently_stepping=%d)\n",
+					  paddress (target_gdbarch (),
+						    t->suspend.stop_pc),
+					  target_pid_to_str (t->ptid),
+					  currently_stepping (t));
+		    }
+		}
+	    }
+	}
+    }
+
+  do_cleanups (old_chain);
+
+  if (debug_infrun)
+    fprintf_unfiltered (gdb_stdlog, "infrun.c: stop_all_threads done\n");
+}
+
 /* Given an execution control state that has been freshly filled in by
    an event from the inferior, figure out what it means and take
    appropriate action.
@@ -3772,21 +4389,28 @@ handle_inferior_event (struct execution_control_state *ecs)
      we're handling a process exit in non-stop mode, there's nothing
      to do, as threads of the dead process are gone, and threads of
      any other process were left running.  */
-  if (!non_stop)
-    set_executing (minus_one_ptid, 0);
-  else if (ecs->ws.kind == TARGET_WAITKIND_SIGNALLED
-	   && ecs->ws.kind == TARGET_WAITKIND_EXITED)
-    {
-      ptid_t pid_ptid;
 
-      /* Some targets still have execution when a process exits.
-	 E.g., for "checkpoint", when when a fork exits and is
-	 mourned, linux-fork.c switches to another fork.  */
-      pid_ptid = pid_to_ptid (ptid_get_pid (ecs->ptid));
-      set_executing (pid_ptid, 0);
-    }
-  else
-    set_executing (ecs->ptid, 0);
+  {
+    ptid_t mark_ptid;
+
+    if (!non_stop)
+      mark_ptid = minus_one_ptid;
+    else if (ecs->ws.kind == TARGET_WAITKIND_SIGNALLED
+	     && ecs->ws.kind == TARGET_WAITKIND_EXITED)
+      {
+	/* Some targets still have execution when a process exits.
+	   E.g., for "checkpoint", when when a fork exits and is
+	   mourned, linux-fork.c switches to another fork.  */
+	mark_ptid = pid_to_ptid (ptid_get_pid (ecs->ptid));
+      }
+    else
+      mark_ptid = ecs->ptid;
+
+    set_executing (mark_ptid, 0);
+
+    /* Likewise the resumed flag.  */
+    set_resumed (mark_ptid, 0);
+  }
 
   switch (ecs->ws.kind)
     {
@@ -4199,16 +4823,105 @@ Cannot fill $_exitsignal with the correct signal number.\n"));
     }
 }
 
+/* Restart threads back to what they were trying to do back when we
+   paused them for an in-line step-over.  The EVENT_THREAD thread is
+   ignored.  */
+
+static void
+restart_threads (struct thread_info *event_thread)
+{
+  struct thread_info *tp;
+  struct thread_info *step_over = NULL;
+
+  /* In case the instruction just stepped spawned a new thread.  */
+  update_thread_list ();
+
+  ALL_NON_EXITED_THREADS (tp)
+    {
+      struct execution_control_state ecss;
+      struct execution_control_state *ecs = &ecss;
+
+      if (tp == event_thread)
+	{
+	  if (debug_infrun)
+	    fprintf_unfiltered (gdb_stdlog,
+				"infrun: restart threads: "
+				"[%s] is event thread\n",
+				target_pid_to_str (tp->ptid));
+	  continue;
+	}
+
+      if (!(tp->state == THREAD_RUNNING || tp->control.in_infcall))
+	{
+	  if (debug_infrun)
+	    fprintf_unfiltered (gdb_stdlog,
+				"infrun: restart threads: "
+				"[%s] not meant to be running\n",
+				target_pid_to_str (tp->ptid));
+	  continue;
+	}
+
+      if (tp->executing)
+	{
+	  if (debug_infrun)
+	    fprintf_unfiltered (gdb_stdlog,
+				"infrun: restart threads: [%s] executing\n",
+				target_pid_to_str (tp->ptid));
+	  gdb_assert (tp->resumed);
+	  continue;
+	}
+
+      if (tp->step_over_next != NULL)
+	{
+	  if (debug_infrun)
+	    fprintf_unfiltered (gdb_stdlog,
+				"infrun: restart threads: "
+				"[%s] needs step-over\n",
+				target_pid_to_str (tp->ptid));
+	  gdb_assert (!tp->resumed);
+	  continue;
+	}
+
+      /* If some thread needs to start a step-over at this point, it
+	 should still be in the step-over queue, and thus skipped
+	 above.  */
+      gdb_assert (!thread_still_needs_step_over (tp));
+
+      if (currently_stepping (tp))
+	{
+	  if (debug_infrun)
+	    fprintf_unfiltered (gdb_stdlog,
+				"infrun: restart threads: [%s] was stepping\n",
+				target_pid_to_str (tp->ptid));
+	  keep_going_stepped_thread (tp);
+	}
+      else
+	{
+	  if (debug_infrun)
+	    fprintf_unfiltered (gdb_stdlog,
+				"infrun: restart threads: [%s] continuing\n",
+				target_pid_to_str (tp->ptid));
+	  reset_ecs (ecs, tp);
+	  switch_to_thread (tp->ptid);
+	  keep_going_pass (ecs);
+	}
+    }
+}
+
 /* Called when we get an event that may finish an in-line or
    out-of-line (displaced stepping) step-over started previously.  */
 
 static void
 finish_step_over (struct execution_control_state *ecs)
 {
+  int had_step_over_info;
+
   displaced_step_fixup (ecs->ptid,
 			ecs->event_thread->suspend.stop_signal);
 
-  if (step_over_info_valid_p ())
+  had_step_over_info = step_over_info_valid_p ();
+
+  if (had_step_over_info)
     {
       /* If we're stepping over a breakpoint with all threads locked,
 	 then only the thread that was stepped should be reporting
@@ -4225,6 +4938,18 @@ finish_step_over (struct execution_control_state *ecs)
   /* Start a new step-over in another thread if there's one that
      needs it.  */
   start_step_over (ecs->ptid);
+
+  /* If we were stepping over a breakpoint before, and haven't started
+     a new in-line step-over sequence, then restart all other threads
+     (except the event thread).  We can't do this in all-stop, as then
+     e.g., we wouldn't be able to issue any other remote packet until
+     these other threads stop.  */
+  if (had_step_over_info && !step_over_info_valid_p ())
+    {
+      restart_threads (ecs->event_thread);
+
+      gdb_assert (!ecs->event_thread->resumed);
+    }
 }
 
 /* Come here when the program has stopped with a signal.  */
@@ -4701,6 +5426,7 @@ handle_signal_stop (struct execution_control_state *ecs)
                                 "infrun: signal arrived while stepping over "
                                 "breakpoint\n");
 
+	  clear_step_over_info ();
 	  insert_hp_step_resume_breakpoint_at_frame (frame);
 	  ecs->event_thread->step_after_step_resume_breakpoint = 1;
 	  /* Reset trap_expected to ensure breakpoints are re-inserted.  */
@@ -4734,6 +5460,7 @@ handle_signal_stop (struct execution_control_state *ecs)
                                 "infrun: signal may take us out of "
                                 "single-step range\n");
 
+	  clear_step_over_info ();
 	  insert_hp_step_resume_breakpoint_at_frame (frame);
 	  ecs->event_thread->step_after_step_resume_breakpoint = 1;
 	  /* Reset trap_expected to ensure breakpoints are re-inserted.  */
@@ -5739,6 +6466,27 @@ keep_going_stepped_thread (struct thread_info *tp)
   struct execution_control_state ecss;
   struct execution_control_state *ecs = &ecss;
 
+  if (tp->suspend.waitstatus_pending_p)
+    {
+      if (debug_infrun)
+	{
+	  char *statstr;
+
+	  statstr = target_waitstatus_to_string (&tp->suspend.waitstatus);
+	  fprintf_unfiltered (gdb_stdlog,
+			      "infrun: previously stepped thread %s "
+			      "has pending wait status %s.\n",
+			      target_pid_to_str (tp->ptid),  statstr);
+	  xfree (statstr);
+	}
+
+      tp->resumed = 1;
+
+      if (target_can_async_p ())
+	target_async (1);
+      return 1;
+    }
+
   /* If the stepping thread exited, then don't try to switch back and
      resume it, which could fail in several different ways depending
      on the target.  Instead, just keep going.
@@ -5817,6 +6565,7 @@ keep_going_stepped_thread (struct thread_info *tp)
 				     get_frame_address_space (frame),
 				     stop_pc);
 
+      tp->resumed = 1;
       resume_ptid = user_visible_resume_ptid (tp->control.stepping_command);
       do_target_resume (resume_ptid, 0, GDB_SIGNAL_0);
     }
@@ -6248,6 +6997,7 @@ keep_going_pass (struct execution_control_state *ecs)
   struct cleanup *old_cleanups = make_cleanup (resume_cleanups, 0);
 
   gdb_assert (ptid_equal (ecs->event_thread->ptid, inferior_ptid));
+  gdb_assert (!ecs->event_thread->resumed);
 
   /* Save the pc before execution, to compare with pc after stop.  */
   ecs->event_thread->prev_pc
@@ -6270,6 +7020,34 @@ keep_going_pass (struct execution_control_state *ecs)
       discard_cleanups (old_cleanups);
       resume (ecs->event_thread->suspend.stop_signal);
     }
+  else if (step_over_info_valid_p ())
+    {
+      /* Another thread is stepping over a breakpoint in-line.  If
+	 this thread needs a step-over too, queue the request.  In
+	 either case, this resume must be deferred for later.  */
+      struct thread_info *tp = ecs->event_thread;
+
+      if (ecs->hit_singlestep_breakpoint
+	  || thread_still_needs_step_over (tp))
+	{
+	  if (debug_infrun)
+	    fprintf_unfiltered (gdb_stdlog,
+				"infrun: step-over already in progress: "
+				"step-over for %s deferred\n",
+				target_pid_to_str (tp->ptid));
+	  thread_step_over_chain_enqueue (tp);
+	}
+      else
+	{
+	  if (debug_infrun)
+	    fprintf_unfiltered (gdb_stdlog,
+				"infrun: step-over in progress: "
+				"resume of %s deferred\n",
+				target_pid_to_str (tp->ptid));
+	}
+
+      discard_cleanups (old_cleanups);
+    }
   else
     {
       struct regcache *regcache = get_current_regcache ();
@@ -6317,6 +7095,14 @@ keep_going_pass (struct execution_control_state *ecs)
 
       ecs->event_thread->control.trap_expected = (remove_bp || remove_wps);
 
+      /* If we're now doing an in-line step-over, we need to stop all
+	 other threads.  Note this must be done before
+	 insert_breakpoints below, because that removes the breakpoint
+	 we're about to step over, otherwise other threads could miss
+	 it.  */
+      if (step_over_info_valid_p () && non_stop)
+	stop_all_threads ();
+
       /* Stop stepping if inserting breakpoints fails.  */
       TRY
 	{
@@ -7747,6 +8533,23 @@ static const struct internalvar_funcs siginfo_funcs =
   NULL
 };
 
+/* Callback for infrun's target events source.  This is marked when a
+   thread has a pending status to process.  */
+
+static void
+infrun_async_inferior_event_handler (gdb_client_data data)
+{
+  /* If the target is closed while this event source is marked, we
+     will reach here without execution, or a target to call
+     target_wait on, which is an error.  Instead of tracking whether
+     the target has been popped already, or whether we do have threads
+     with pending statutes, simply ignore the event.  */
+  if (!target_is_async_p ())
+    return;
+
+  inferior_event_handler (INF_REG_EVENT, NULL);
+}
+
 void
 _initialize_infrun (void)
 {
@@ -7754,6 +8557,10 @@ _initialize_infrun (void)
   int numsigs;
   struct cmd_list_element *c;
 
+  /* Register extra event sources in the event loop.  */
+  infrun_async_inferior_event_token
+    = create_async_event_handler (infrun_async_inferior_event_handler, NULL);
+
   add_info ("signals", signals_info, _("\
 What debugger does when program gets various signals.\n\
 Specify a signal as argument to print info on that signal only."));
diff --git a/gdb/infrun.h b/gdb/infrun.h
index 1f09e41..0a5d20d 100644
--- a/gdb/infrun.h
+++ b/gdb/infrun.h
@@ -196,4 +196,7 @@ extern void signal_catch_update (const unsigned int *);
    systems.  Use of symbolic signal names is strongly encouraged.  */
 enum gdb_signal gdb_signal_from_command (int num);
 
+/* Enables/disables infrun's async event source in the event loop.  */
+extern void infrun_async (int enable);
+
 #endif /* INFRUN_H */
diff --git a/gdb/target.c b/gdb/target.c
index 6f6029b..6d814c6 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -3693,6 +3693,15 @@ maintenance_print_target_stack (char *cmd, int from_tty)
     }
 }
 
+/* See target.h.  */
+
+void
+target_async (int enable)
+{
+  infrun_async (enable);
+  current_target.to_async (&current_target, enable);
+}
+
 /* Controls if targets can report that they can/are async.  This is
    just for maintainers to use when debugging gdb.  */
 int target_async_permitted = 1;
diff --git a/gdb/target.h b/gdb/target.h
index 19254d6..f400558 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -1687,8 +1687,7 @@ extern int target_async_permitted;
 #define target_is_async_p() (current_target.to_is_async_p (&current_target))
 
 /* Enables/disabled async target events.  */
-#define target_async(ENABLE) \
-     (current_target.to_async (&current_target, (ENABLE)))
+extern void target_async (int enable);
 
 #define target_execution_direction() \
   (current_target.to_execution_direction (&current_target))
diff --git a/gdb/target/waitstatus.h b/gdb/target/waitstatus.h
index d4ef3b8..ffaddc1 100644
--- a/gdb/target/waitstatus.h
+++ b/gdb/target/waitstatus.h
@@ -131,7 +131,10 @@ enum target_stop_reason
   TARGET_STOPPED_BY_HW_BREAKPOINT,
 
   /* Stopped by a watchpoint.  */
-  TARGET_STOPPED_BY_WATCHPOINT
+  TARGET_STOPPED_BY_WATCHPOINT,
+
+  /* Stopped by a single step finishing.  */
+  TARGET_STOPPED_BY_SINGLE_STEP
 };
 
 /* Prototypes */
diff --git a/gdb/thread.c b/gdb/thread.c
index a01f5d8..bb942c2 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -232,6 +232,7 @@ new_thread (ptid_t ptid)
   /* Nothing to follow yet.  */
   tp->pending_follow.kind = TARGET_WAITKIND_SPURIOUS;
   tp->state = THREAD_STOPPED;
+  tp->suspend.waitstatus.kind = TARGET_WAITKIND_IGNORE;
 
   return tp;
 }
@@ -848,6 +849,28 @@ thread_change_ptid (ptid_t old_ptid, ptid_t new_ptid)
   observer_notify_thread_ptid_changed (old_ptid, new_ptid);
 }
 
+/* See gdbthread.h.  */
+
+void
+set_resumed (ptid_t ptid, int resumed)
+{
+  struct thread_info *tp;
+  int all = ptid_equal (ptid, minus_one_ptid);
+
+  if (all || ptid_is_pid (ptid))
+    {
+      for (tp = thread_list; tp; tp = tp->next)
+	if (all || ptid_get_pid (tp->ptid) == ptid_get_pid (ptid))
+	  tp->resumed = resumed;
+    }
+  else
+    {
+      tp = find_thread_ptid (ptid);
+      gdb_assert (tp != NULL);
+      tp->resumed = resumed;
+    }
+}
+
 /* Helper for set_running, that marks one thread either running or
    stopped.  */
 
-- 
1.9.3

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 12/17] Implement all-stop on top of a target running non-stop mode
  2015-04-01 22:41 ` [PATCH 12/17] Implement all-stop on top of a target running non-stop mode Pedro Alves
@ 2015-04-02 14:58   ` Eli Zaretskii
  2015-04-07  9:51     ` Pedro Alves
  0 siblings, 1 reply; 26+ messages in thread
From: Eli Zaretskii @ 2015-04-02 14:58 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

> From: Pedro Alves <palves@redhat.com>
> Date: Wed,  1 Apr 2015 23:14:09 +0100
> 
> +@kindex maint set target-non-stop @var{mode} [on|off|auto]
> +@kindex maint show target-non-stop
> +@item maint set target-non-stop
> +@itemx maint show target-non-stop
> +This controls whether @value{GDBN} targets always operate in non-mode
                                                                ^^^^^^^^
"non-stop mode", I guess.

> +even if @code{set non-stop} is @code{off} (@pxref{Non-Stop Mode}).
> +This default is @code{auto}, meaning this is enabled if available; but
   ^^^^
"The"

> +this can be changed to more easily debug problems.

I find the last part (after the semi-colon) confusing and not really
adding any important information.  Perhaps consider dropping it.

What about NEWS?

Thanks.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 12/17] Implement all-stop on top of a target running non-stop mode
  2015-04-02 14:58   ` Eli Zaretskii
@ 2015-04-07  9:51     ` Pedro Alves
  2015-04-07 10:01       ` Eli Zaretskii
  0 siblings, 1 reply; 26+ messages in thread
From: Pedro Alves @ 2015-04-07  9:51 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

On 04/02/2015 03:58 PM, Eli Zaretskii wrote:
>> From: Pedro Alves <palves@redhat.com>
>> Date: Wed,  1 Apr 2015 23:14:09 +0100
>>
>> +@kindex maint set target-non-stop @var{mode} [on|off|auto]
>> +@kindex maint show target-non-stop
>> +@item maint set target-non-stop
>> +@itemx maint show target-non-stop
>> +This controls whether @value{GDBN} targets always operate in non-mode
>                                                                 ^^^^^^^^
> "non-stop mode", I guess.

Whoops.  Yes, fixed.

> 
>> +even if @code{set non-stop} is @code{off} (@pxref{Non-Stop Mode}).
>> +This default is @code{auto}, meaning this is enabled if available; but
>    ^^^^
> "The"

Fixed.

> 
>> +this can be changed to more easily debug problems.
> 
> I find the last part (after the semi-colon) confusing and not really
> adding any important information.  Perhaps consider dropping it.

Dropped.

> 
> What about NEWS?

How about this?

gdb/ChangeLog:
2015-04-07  Pedro Alves  <palves@redhat.com>

	* NEWS: Mention "maint set/show target-non-stop".
	(...)

gdb/doc/ChangeLog:
2015-04-07  Pedro Alves  <palves@redhat.com>

	* gdb.texinfo (Maintenance Commands): Document "maint set/show
	target-non-stop".

diff --git c/gdb/NEWS w/gdb/NEWS
index cd7c2b3..1526528 100644
--- c/gdb/NEWS
+++ w/gdb/NEWS
@@ -41,6 +41,12 @@ maint print symbol-cache-statistics
 maint flush-symbol-cache
   Flush the contents of the symbol cache.
 
+maint set target-non-stop (on|off|auto)
+maint show target-non-stop
+  Control whether GDB targets always operate in non-stop mode even if
+  "set non-stop" is "off".  The default is "auto", meaning this is
+  enabled if supported by the target.
+
 record btrace bts
 record bts
   Start branch trace recording using Branch Trace Store (BTS) format.
diff --git c/gdb/doc/gdb.texinfo w/gdb/doc/gdb.texinfo
index c6e9b9b..6b7ba98 100644
--- c/gdb/doc/gdb.texinfo
+++ w/gdb/doc/gdb.texinfo
@@ -34103,6 +34103,29 @@ asynchronous mode (@pxref{Background Execution}).  Normally the
 default is asynchronous, if it is available; but this can be changed
 to more easily debug problems occurring only in synchronous mode.
 
+@kindex maint set target-non-stop @var{mode} [on|off|auto]
+@kindex maint show target-non-stop
+@item maint set target-non-stop
+@itemx maint show target-non-stop
+This controls whether @value{GDBN} targets always operate in non-stop
+mode even if @code{set non-stop} is @code{off} (@pxref{Non-Stop
+Mode}).  The default is @code{auto}, meaning this is enabled if
+supported by the target.
+
+@table @code
+@item maint set target-non-stop auto
+This is the default mode.  @value{GDBN} controls the target in
+non-stop mode if the target supports it.
+
+@item maint set target-non-stop on
+@value{GDBN} controls the target in non-stop mode even if the target
+does not indicate support.
+
+@item maint set target-non-stop off
+@value{GDBN} does not control the target in non-stop mode even if the
+target supports it.
+@end table
+
 @kindex maint set per-command
 @kindex maint show per-command
 @item maint set per-command

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 12/17] Implement all-stop on top of a target running non-stop mode
  2015-04-07  9:51     ` Pedro Alves
@ 2015-04-07 10:01       ` Eli Zaretskii
  2015-04-07 10:11         ` Pedro Alves
  0 siblings, 1 reply; 26+ messages in thread
From: Eli Zaretskii @ 2015-04-07 10:01 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

> Date: Tue, 07 Apr 2015 10:51:50 +0100
> From: Pedro Alves <palves@redhat.com>
> CC: gdb-patches@sourceware.org
> 
> > What about NEWS?
> 
> How about this?
> 
> gdb/ChangeLog:
> 2015-04-07  Pedro Alves  <palves@redhat.com>
> 
> 	* NEWS: Mention "maint set/show target-non-stop".
> 	(...)
> 
> gdb/doc/ChangeLog:
> 2015-04-07  Pedro Alves  <palves@redhat.com>
> 
> 	* gdb.texinfo (Maintenance Commands): Document "maint set/show
> 	target-non-stop".
> 
> diff --git c/gdb/NEWS w/gdb/NEWS
> index cd7c2b3..1526528 100644
> --- c/gdb/NEWS
> +++ w/gdb/NEWS
> @@ -41,6 +41,12 @@ maint print symbol-cache-statistics
>  maint flush-symbol-cache
>    Flush the contents of the symbol cache.
>  
> +maint set target-non-stop (on|off|auto)
> +maint show target-non-stop
> +  Control whether GDB targets always operate in non-stop mode even if
> +  "set non-stop" is "off".  The default is "auto", meaning this is
> +  enabled if supported by the target.
> +

LGTM, except that maybe replace "this" by "non-stop mode", to be more
clear.

Thanks.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 12/17] Implement all-stop on top of a target running non-stop mode
  2015-04-07 10:01       ` Eli Zaretskii
@ 2015-04-07 10:11         ` Pedro Alves
  0 siblings, 0 replies; 26+ messages in thread
From: Pedro Alves @ 2015-04-07 10:11 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

On 04/07/2015 11:01 AM, Eli Zaretskii wrote:
>> Date: Tue, 07 Apr 2015 10:51:50 +0100
>> From: Pedro Alves <palves@redhat.com>
>> CC: gdb-patches@sourceware.org
>>
>>> What about NEWS?
>>
>> How about this?
>>
>> gdb/ChangeLog:
>> 2015-04-07  Pedro Alves  <palves@redhat.com>
>>
>> 	* NEWS: Mention "maint set/show target-non-stop".
>> 	(...)
>>
>> gdb/doc/ChangeLog:
>> 2015-04-07  Pedro Alves  <palves@redhat.com>
>>
>> 	* gdb.texinfo (Maintenance Commands): Document "maint set/show
>> 	target-non-stop".
>>
>> diff --git c/gdb/NEWS w/gdb/NEWS
>> index cd7c2b3..1526528 100644
>> --- c/gdb/NEWS
>> +++ w/gdb/NEWS
>> @@ -41,6 +41,12 @@ maint print symbol-cache-statistics
>>  maint flush-symbol-cache
>>    Flush the contents of the symbol cache.
>>  
>> +maint set target-non-stop (on|off|auto)
>> +maint show target-non-stop
>> +  Control whether GDB targets always operate in non-stop mode even if
>> +  "set non-stop" is "off".  The default is "auto", meaning this is
>> +  enabled if supported by the target.
>> +
> 
> LGTM, except that maybe replace "this" by "non-stop mode", to be more
> clear.
> 

Done, thanks.

I'll be sending a v2 series soon, and will include that change (and
equivalent to gdb.texinfo).

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 03/17] Displaced stepping debug: fetch the right regcache
  2015-04-01 22:14 ` [PATCH 03/17] Displaced stepping debug: fetch the right regcache Pedro Alves
@ 2015-04-07 10:47   ` Pedro Alves
  2015-04-07 13:55   ` Yao Qi
  1 sibling, 0 replies; 26+ messages in thread
From: Pedro Alves @ 2015-04-07 10:47 UTC (permalink / raw)
  To: gdb-patches

On 04/01/2015 11:14 PM, Pedro Alves wrote:
> Although not currently possible in practice when we get here,
> 'resume_ptid' can also be a wildcard throughout this function.  It's
> clearer to fetch the regcache using the thread's ptid.
> 
> gdb/ChangeLog:
> 2015-04-01  Pedro Alves  <pedro@codesourcery.com>
> 
> 	* infrun.c (resume) <displaced stepping debug output>: Get the
> 	leader thread's regcache, not resume_ptid's.

Not much point in carrying this one around.  I pushed it in.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [cancel] Re: [PATCH 00/17] All-stop on top of non-stop
  2015-04-01 22:14 [PATCH 00/17] All-stop on top of non-stop Pedro Alves
                   ` (16 preceding siblings ...)
  2015-04-01 23:08 ` [PATCH 11/17] Teach non-stop to do in-line step-overs (stop all, step, restart) Pedro Alves
@ 2015-04-07 12:52 ` Pedro Alves
  17 siblings, 0 replies; 26+ messages in thread
From: Pedro Alves @ 2015-04-07 12:52 UTC (permalink / raw)
  To: gdb-patches

I sent a v2 of this series here:

 https://sourceware.org/ml/gdb-patches/2015-04/msg00198.html

Most is unchanged, but wider testing revealed a few bugs
and the need for more fixing on non-x86_64 ports.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 03/17] Displaced stepping debug: fetch the right regcache
  2015-04-01 22:14 ` [PATCH 03/17] Displaced stepping debug: fetch the right regcache Pedro Alves
  2015-04-07 10:47   ` Pedro Alves
@ 2015-04-07 13:55   ` Yao Qi
  2015-04-07 14:12     ` Pedro Alves
  1 sibling, 1 reply; 26+ messages in thread
From: Yao Qi @ 2015-04-07 13:55 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

Pedro Alves <palves@redhat.com> writes:

> gdb/ChangeLog:
> 2015-04-01  Pedro Alves  <pedro@codesourcery.com>
>
> 	* infrun.c (resume) <displaced stepping debug output>: Get the
> 	leader thread's regcache, not resume_ptid's.

Hi Pedro,
From your change, I don't see why TP is the leader thread.

> ---
>  gdb/infrun.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index cf0ef32..f23e76e 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -2374,7 +2374,7 @@ resume (enum gdb_signal sig)
>        && tp->control.trap_expected
>        && use_displaced_stepping_now_p (gdbarch, sig))
>      {
> -      struct regcache *resume_regcache = get_thread_regcache (resume_ptid);
> +      struct regcache *resume_regcache = get_thread_regcache (tp->ptid);
>        struct gdbarch *resume_gdbarch = get_regcache_arch (resume_regcache);
>        CORE_ADDR actual_pc = regcache_read_pc (resume_regcache);

If we get recache from TP, can we remove local variables
resume_regcache, resume_gdbarch, actual_pc, and use regcache, gdbarch
and pc we've got at the beginning of function resume instead?

-- 
Yao (齐尧)

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 03/17] Displaced stepping debug: fetch the right regcache
  2015-04-07 13:55   ` Yao Qi
@ 2015-04-07 14:12     ` Pedro Alves
  0 siblings, 0 replies; 26+ messages in thread
From: Pedro Alves @ 2015-04-07 14:12 UTC (permalink / raw)
  To: Yao Qi; +Cc: gdb-patches

On 04/07/2015 02:55 PM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
> 
>> gdb/ChangeLog:
>> 2015-04-01  Pedro Alves  <pedro@codesourcery.com>
>>
>> 	* infrun.c (resume) <displaced stepping debug output>: Get the
>> 	leader thread's regcache, not resume_ptid's.
> 
> Hi Pedro,
> From your change, I don't see why TP is the leader thread.

'resume' and 'target_resume' assume inferior_ptid is the "leader"
thread we want resumed -- TP is just the current thread
initialized at the top:

  struct thread_info *tp = inferior_thread ();

But e.g., if we're resuming with "set scheduler-locking off",
this:

  resume_ptid = user_visible_resume_ptid (user_step);

makes resume_ptid be a whole-process ptid, or
minus_one_ptid (all-threads).  In that case, the target_resume
backend implementation knows the thread that is the "leader" is
inferior_ptid, not the one passed as argument.

(though when we start a displaced step, resume_ptid is always
pointing at the current thread, never a wildcard ptid.)

>> @@ -2374,7 +2374,7 @@ resume (enum gdb_signal sig)
>>        && tp->control.trap_expected
>>        && use_displaced_stepping_now_p (gdbarch, sig))
>>      {
>> -      struct regcache *resume_regcache = get_thread_regcache (resume_ptid);
>> +      struct regcache *resume_regcache = get_thread_regcache (tp->ptid);
>>        struct gdbarch *resume_gdbarch = get_regcache_arch (resume_regcache);
>>        CORE_ADDR actual_pc = regcache_read_pc (resume_regcache);
> 
> If we get recache from TP, can we remove local variables
> resume_regcache, resume_gdbarch, actual_pc, and use regcache, gdbarch
> and pc we've got at the beginning of function resume instead?

Good point.  Indeed we should be able to.  The PC we got at the top would
be incorrect, given the thread's PC now points at the scratch pad,
but we already do:

	  /* Update pc to reflect the new address from which we will
	     execute instructions due to displaced stepping.  */
	  pc = regcache_read_pc (get_thread_regcache (inferior_ptid));

so the current PC's contents should work.  And that line could be:

  pc = regcache_read_pc (regcache);

too.

Thanks,
Pedro Alves

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2015-04-07 14:12 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-01 22:14 [PATCH 00/17] All-stop on top of non-stop Pedro Alves
2015-04-01 22:14 ` [PATCH 10/17] Factor out code to re-resume stepped thread Pedro Alves
2015-04-01 22:14 ` [PATCH 13/17] Fix signal-while-stepping-over-bp-other-thread.exp on targets always in non-stop Pedro Alves
2015-04-01 22:14 ` [PATCH 03/17] Displaced stepping debug: fetch the right regcache Pedro Alves
2015-04-07 10:47   ` Pedro Alves
2015-04-07 13:55   ` Yao Qi
2015-04-07 14:12     ` Pedro Alves
2015-04-01 22:14 ` [PATCH 17/17] native Linux: enable always non-stop by default Pedro Alves
2015-04-01 22:14 ` [PATCH 01/17] Fix and test "checkpoint" in non-stop mode Pedro Alves
2015-04-01 22:14 ` [PATCH 08/17] Use keep_going in proceed and start_step_over too Pedro Alves
2015-04-01 22:14 ` [PATCH 02/17] PR13858 - Can't do displaced stepping with no symbols Pedro Alves
2015-04-01 22:14 ` [PATCH 04/17] Change adjust_pc_after_break's prototype Pedro Alves
2015-04-01 22:14 ` [PATCH 09/17] Misc switch_back_to_stepped_thread cleanups Pedro Alves
2015-04-01 22:24 ` [PATCH 07/17] Embed the pending step-over chain in thread_info objects Pedro Alves
2015-04-01 22:40 ` [PATCH 06/17] Make thread_still_needs_step_over consider stepping_over_watchpoint too Pedro Alves
2015-04-01 22:40 ` [PATCH 15/17] Fix step-over-trips-on-watchpoint.exp/step-over-lands-on-breakpoint.exp race Pedro Alves
2015-04-01 22:41 ` [PATCH 12/17] Implement all-stop on top of a target running non-stop mode Pedro Alves
2015-04-02 14:58   ` Eli Zaretskii
2015-04-07  9:51     ` Pedro Alves
2015-04-07 10:01       ` Eli Zaretskii
2015-04-07 10:11         ` Pedro Alves
2015-04-01 23:06 ` [PATCH 16/17] Disable displaced stepping if trying it fails Pedro Alves
2015-04-01 23:06 ` [PATCH 14/17] Fix interrupt-noterm.exp on targets always in non-stop Pedro Alves
2015-04-01 23:07 ` [PATCH 05/17] remote.c/all-stop: Implement TARGET_WAITKIND_NO_RESUMED and TARGET_WNOHANG Pedro Alves
2015-04-01 23:08 ` [PATCH 11/17] Teach non-stop to do in-line step-overs (stop all, step, restart) Pedro Alves
2015-04-07 12:52 ` [cancel] Re: [PATCH 00/17] All-stop on top of non-stop Pedro Alves

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).