gdb/

*	breakpoint.c (breakpoint_init_inferior): Clean up the moribund
	locations list.
	(moribund_breakpoint_here_p): Record the moribund
	location in the moribund_locations vector.
	* breakpoint.h (moribund_breakpoint_here_p): Declare.
	(displaced_step_fixup): Check if the breakpoint the thread was
	trying to step over has been removed since having been placed in
	the displaced stepping queue.
	(adjust_pc_after_break): In non-stop mode, check for a moribund
	breakpoint at the stop pc.
	(handle_inferior_event): Don't retire moribund breakpoints on
	TARGET_WAITKIND_IGNORE.

gdb/testsuite/
	* gdb.mi/mi-nsmoribund.exp, gdb.mi/nsmoribund.c: New test.

7 files changed, 362 insertions, 32 deletions

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 6bf9358..d81cca8 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,4 +1,19 @@
-2008-10-14  Pedro Alves  <pedro@codesourcery.com>
+2008-10-15  Pedro Alves  <pedro@codesourcery.com>
+
+	* breakpoint.c (breakpoint_init_inferior): Clean up the moribund
+	locations list.
+	(moribund_breakpoint_here_p): Record the moribund
+	location in the moribund_locations vector.
+	* breakpoint.h (moribund_breakpoint_here_p): Declare.
+	(displaced_step_fixup): Check if the breakpoint the thread was
+	trying to step over has been removed since having been placed in
+	the displaced stepping queue.
+	(adjust_pc_after_break): In non-stop mode, check for a moribund
+	breakpoint at the stop pc.
+	(handle_inferior_event): Don't retire moribund breakpoints on
+	TARGET_WAITKIND_IGNORE.
+
+2008-10-15  Pedro Alves  <pedro@codesourcery.com>
 
 	* infrun.c (displaced_step_prepare): Switch thread temporarily
 	while we're here.
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index b289ead..7f535df 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -1744,6 +1744,7 @@ breakpoint_init_inferior (enum inf_context context)
 {
   struct breakpoint *b, *temp;
   struct bp_location *bpt;
+  int ix;
 
   ALL_BP_LOCATIONS (bpt)
     if (bpt->owner->enable_state != bp_permanent)
@@ -1786,6 +1787,11 @@ breakpoint_init_inferior (enum inf_context context)
 	break;
       }
   }
+
+  /* Get rid of the moribund locations.  */
+  for (ix = 0; VEC_iterate (bp_location_p, moribund_locations, ix, bpt); ++ix)
+    free_bp_location (bpt);
+  VEC_free (bp_location_p, moribund_locations);
 }
 
 /* breakpoint_here_p (PC) returns non-zero if an enabled breakpoint
@@ -1828,6 +1834,20 @@ breakpoint_here_p (CORE_ADDR pc)
   return any_breakpoint_here ? ordinary_breakpoint_here : 0;
 }
 
+/* Return true if there's a moribund breakpoint at PC.  */
+
+int
+moribund_breakpoint_here_p (CORE_ADDR pc)
+{
+  struct bp_location *loc;
+  int ix;
+
+  for (ix = 0; VEC_iterate (bp_location_p, moribund_locations, ix, loc); ++ix)
+    if (loc->address == pc)
+      return 1;
+
+  return 0;
+}
 
 /* Returns non-zero if there's a breakpoint inserted at PC, which is
    inserted using regular breakpoint_chain/bp_location_chain mechanism.
@@ -7107,8 +7127,8 @@ update_global_location_list (int should_insert)
 	}
 
       if (!found_object)
-	{	      
-	  if (removed)
+	{
+	  if (removed && non_stop)
 	    {
 	      /* This location was removed from the targets.  In non-stop mode,
 		 a race condition is possible where we've removed a breakpoint,
@@ -7116,20 +7136,22 @@ update_global_location_list (int should_insert)
 		 arrive later.  To suppress spurious SIGTRAPs reported to user,
 		 we keep this breakpoint location for a bit, and will retire it
 		 after we see 3 * thread_count events.
-		 The theory here is that reporting of events should, 
+		 The theory here is that reporting of events should,
 		 "on the average", be fair, so after that many event we'll see
 		 events from all threads that have anything of interest, and no
-		 longer need to keep this breakpoint.  This is just a 
+		 longer need to keep this breakpoint.  This is just a
 		 heuristic, but if it's wrong, we'll report unexpected SIGTRAP,
-		 which is usability issue, but not a correctness problem.  */	  
+		 which is usability issue, but not a correctness problem.  */
 	      loc->events_till_retirement = 3 * (thread_count () + 1);
 	      loc->owner = NULL;
-	    }
 
-	  free_bp_location (loc);
+	      VEC_safe_push (bp_location_p, moribund_locations, loc);
+	    }
+	  else
+	    free_bp_location (loc);
 	}
     }
-    
+
   ALL_BREAKPOINTS (b)
     {
       check_duplicates (b);
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index de21b9a..65f5ae1 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -681,6 +681,8 @@ enum breakpoint_here
 
 extern enum breakpoint_here breakpoint_here_p (CORE_ADDR);
 
+extern int moribund_breakpoint_here_p (CORE_ADDR);
+
 extern int breakpoint_inserted_here_p (CORE_ADDR);
 
 extern int regular_breakpoint_inserted_here_p (CORE_ADDR);
diff --git a/gdb/infrun.c b/gdb/infrun.c
index f60f938..150bf4c 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -789,40 +789,71 @@ displaced_step_fixup (ptid_t event_ptid, enum target_signal signal)
 
   do_cleanups (old_cleanups);
 
+  displaced_step_ptid = null_ptid;
+
   /* Are there any pending displaced stepping requests?  If so, run
      one now.  */
-  if (displaced_step_request_queue)
+  while (displaced_step_request_queue)
     {
       struct displaced_step_request *head;
       ptid_t ptid;
+      CORE_ADDR actual_pc;
 
       head = displaced_step_request_queue;
       ptid = head->ptid;
       displaced_step_request_queue = head->next;
       xfree (head);
 
-      if (debug_displaced)
-	fprintf_unfiltered (gdb_stdlog,
-			    "displaced: stepping queued %s now\n",
-			    target_pid_to_str (ptid));
-
-      displaced_step_ptid = null_ptid;
-      displaced_step_prepare (ptid);
       context_switch (ptid);
 
-      if (debug_displaced)
+      actual_pc = read_pc ();
+
+      if (breakpoint_here_p (actual_pc))
 	{
-	  struct regcache *resume_regcache = get_thread_regcache (ptid);
-	  CORE_ADDR actual_pc = regcache_read_pc (resume_regcache);
-	  gdb_byte buf[4];
-
-	  fprintf_unfiltered (gdb_stdlog, "displaced: run 0x%s: ",
-			      paddr_nz (actual_pc));
-	  read_memory (actual_pc, buf, sizeof (buf));
-	  displaced_step_dump_bytes (gdb_stdlog, buf, sizeof (buf));
+	  if (debug_displaced)
+	    fprintf_unfiltered (gdb_stdlog,
+				"displaced: stepping queued %s now\n",
+				target_pid_to_str (ptid));
+
+	  displaced_step_prepare (ptid);
+
+	  if (debug_displaced)
+	    {
+	      gdb_byte buf[4];
+
+	      fprintf_unfiltered (gdb_stdlog, "displaced: run 0x%s: ",
+				  paddr_nz (actual_pc));
+	      read_memory (actual_pc, buf, sizeof (buf));
+	      displaced_step_dump_bytes (gdb_stdlog, buf, sizeof (buf));
+	    }
+
+	  target_resume (ptid, 1, TARGET_SIGNAL_0);
+
+	  /* Done, we're stepping a thread.  */
+	  break;
 	}
+      else
+	{
+	  int step;
+	  struct thread_info *tp = inferior_thread ();
+
+	  /* The breakpoint we were sitting under has since been
+	     removed.  */
+	  tp->trap_expected = 0;
+
+	  /* Go back to what we were trying to do.  */
+	  step = currently_stepping (tp);
 
-      target_resume (ptid, 1, TARGET_SIGNAL_0);
+	  if (debug_displaced)
+	    fprintf_unfiltered (gdb_stdlog, "breakpoint is gone %s: step(%d)\n",
+				target_pid_to_str (tp->ptid), step);
+
+	  target_resume (ptid, step, TARGET_SIGNAL_0);
+	  tp->stop_signal = TARGET_SIGNAL_0;
+
+	  /* This request was discarded.  See if there's any other
+	     thread waiting for its turn.  */
+	}
     }
 }
 
@@ -1798,9 +1829,16 @@ adjust_pc_after_break (struct execution_control_state *ecs)
   breakpoint_pc = regcache_read_pc (regcache)
 		  - gdbarch_decr_pc_after_break (gdbarch);
 
-  /* Check whether there actually is a software breakpoint inserted
-     at that location.  */
-  if (software_breakpoint_inserted_here_p (breakpoint_pc))
+  /* Check whether there actually is a software breakpoint inserted at
+     that location.
+
+     If in non-stop mode, a race condition is possible where we've
+     removed a breakpoint, but stop events for that breakpoint were
+     already queued and arrive later.  To suppress those spurious
+     SIGTRAPs, we keep a list of such breakpoint locations for a bit,
+     and retire them after a number of stop events are reported.  */
+  if (software_breakpoint_inserted_here_p (breakpoint_pc)
+      || (non_stop && moribund_breakpoint_here_p (breakpoint_pc)))
     {
       /* When using hardware single-step, a SIGTRAP is reported for both
 	 a completed single-step and a software breakpoint.  Need to
@@ -1873,8 +1911,6 @@ handle_inferior_event (struct execution_control_state *ecs)
   else
     stop_soon = NO_STOP_QUIETLY;
 
-  breakpoint_retire_moribund ();
-
   /* Cache the last pid/waitstatus. */
   target_last_wait_ptid = ecs->ptid;
   target_last_waitstatus = ecs->ws;
@@ -1902,6 +1938,8 @@ handle_inferior_event (struct execution_control_state *ecs)
 
   if (ecs->ws.kind != TARGET_WAITKIND_IGNORE)
     {
+      breakpoint_retire_moribund ();
+
       /* Mark the non-executing threads accordingly.  */
       if (!non_stop
  	  || ecs->ws.kind == TARGET_WAITKIND_EXITED
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 98c2188..1dd8d4b 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,7 @@
+2008-10-15  Pedro Alves  <pedro@codesourcery.com>
+
+	* gdb.mi/mi-nsmoribund.exp, gdb.mi/nsmoribund.c: New test.
+
 2008-10-15  Denis Pilat  <denis.pilat@st.com>
 
 	* gdb.cp/mb-ctor.exp: Fix a typo.
diff --git a/gdb/testsuite/gdb.mi/mi-nsmoribund.exp b/gdb/testsuite/gdb.mi/mi-nsmoribund.exp
new file mode 100644
index 0000000..85e5138
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-nsmoribund.exp
@@ -0,0 +1,177 @@
+# Copyright 2008 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/>.
+
+# This only works with native configurations
+if {![isnative]} {
+  return
+}
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+gdb_exit
+if {[mi_gdb_start]} {
+    continue
+}
+
+#
+# Start here
+#
+set testfile "nsmoribund"
+set srcfile "$testfile.c"
+set binfile "$objdir/$subdir/mi-$testfile"
+
+set options [list debug incdir=$objdir]
+if {[gdb_compile_pthreads "$srcdir/$subdir/$srcfile" $binfile executable $options] != "" } {
+    return -1
+}
+
+mi_gdb_reinitialize_dir $srcdir/$subdir
+mi_gdb_load $binfile
+
+set supported 0
+send_gdb "-gdb-show non-stop\n"
+gdb_expect {
+    -re ".*\\^done,value=\"off\",supported=\"(\[^\"\]+)\"\r\n$mi_gdb_prompt$" {
+	if { $expect_out(1,string) == "1" } {
+	    set supported 1
+	}
+    }
+    -re ".$mi_gdb_prompt$" {
+    }
+}
+
+mi_gdb_test "-gdb-set non-stop 1" ".*"
+mi_gdb_test "-gdb-set target-async 1" ".*"
+detect_async
+
+mi_gdb_test "200-break-insert -t main" ".*"
+
+set created "=thread-created,id=\"$decimal\"\r\n"
+set running "\\*running,thread-id=\"$decimal\"\r\n"
+
+set notifs "($created)*($running)*"
+
+# Note: presently, we skip this test on non-native targets,
+# so 'run' is OK.  As soon as we start to run this on remote
+# target, the logic from mi_run_cmd will have to be refactored.
+send_gdb "-exec-run\n"
+gdb_expect {
+    -re "\\^running\r\n$notifs$mi_gdb_prompt" {
+    }
+    -re "\\^error,msg=\"The target does not support running in non-stop mode.\"" {
+	verbose -log "Non-stop mode not supported, skipping all tests"
+	return
+    }
+    -re "\r\n$mi_gdb_prompt" {
+	perror "Cannot start target (unknown output after running)"
+	return -1
+    }
+    timeout {
+	perror "Cannot start target (timeout)"
+	return -1
+    }
+}
+mi_expect_stop "breakpoint-hit" main ".*" ".*" "\[0-9\]+" \
+    { "" "disp=\"del\"" } "run to main"
+
+# Keep this in sync with THREADS in the the $srcfile.
+set nthreads 10
+
+# Set a breakpoint and let all threads hit it (except the main
+# thread).
+
+set bkpt_line [gdb_get_line_number "set breakpoint here"]
+
+mi_create_breakpoint "$srcfile:$bkpt_line" 2 keep thread_function .* .* .* \
+    "breakpoint at thread_function"
+
+mi_send_resuming_command "exec-continue --all" "resume all"
+for {set i 0} {$i < $nthreads} {incr i} {
+    mi_expect_stop "breakpoint-hit" "thread_function" "\[^\n\]*" "$srcfile" \
+	"\[0-9\]*" {"" "disp=\"keep\""} "stop $i"
+}
+
+# All but the main thread should have hit it.
+
+mi_check_thread_states \
+    {"running" "stopped" "stopped" "stopped" "stopped" "stopped" "stopped" "stopped" "stopped" "stopped" "stopped"} \
+    "thread state: all stopped except the main thread"
+
+# Select a stopped thread to make sure we're able to delete
+# breakpoints
+mi_gdb_test "-thread-select 5" "\\^done.*" "select thread 5"
+
+# Now that we know about all the threads, we can get rid of
+# breakpoint.
+mi_delete_breakpoints
+
+# Recreate the same breakpoint, but this time, specific to thread 5.
+mi_create_breakpoint "-p 5 $srcfile:$bkpt_line" 3 keep thread_function .* .* .* \
+    "thread specific breakpoint at thread_function"
+
+# Resume all threads.  Only thread 5 should report a stop.
+
+set running_re ""
+for {set i $nthreads} {$i > 0} {incr i -1} {
+    set running_re "$running_re\\*running,thread-id=\"$decimal\"\r\n"
+}
+
+send_gdb "-exec-continue --all\n"
+gdb_expect {
+    -re ".*$running_re$mi_gdb_prompt" {
+	pass "resume all, thread specific breakpoint"
+    }
+    timeout {
+	fail "resume all, thread specific breakpoint (timeout)"
+    }
+}
+
+mi_expect_stop "breakpoint-hit" "thread_function" "\[^\n\]*" "$srcfile" \
+    "\[0-9\]*" {"" "disp=\"keep\""} "hit thread specific breakpoint"
+
+# All threads except both thread 5 (and the main thread) should now be
+# repeatedly hitting the thread specific breakpoint and stepping over
+# it transparently.  These are internal events, so the frontend should
+# see those threads as running.
+
+mi_check_thread_states \
+    {"running" "running" "running" "running" "stopped" "running" "running" "running" "running"} \
+    "thread state: all running except the breakpoint thread"
+
+# Get rid of the breakpoint while the other threads are stepping over
+# it, and tell all threads to exit.  The program should exit
+# gracefully shortly.  Send all commands in a row, since if something
+# goes wrong with moribund locations support or displaced stepping (or
+# a target bug if it can step over breakpoints itself), a spurious
+# SIGTRAP/SIGSEGV can come at any time after deleting the breakpoint.
+
+send_gdb "102-break-delete\n"
+send_gdb "print done = 1\n"
+send_gdb "103-exec-continue --all\n"
+
+gdb_expect {
+    -re "\\*stopped,reason=\"exited-normally\"" {
+	pass "resume all, program exited normally"
+    }
+    -re "\\*stopped" {
+	fail "unexpected stop"
+    }
+    timeout {
+	fail "resume all, waiting for program exit (timeout)"
+    }
+}
+
+mi_gdb_exit
diff --git a/gdb/testsuite/gdb.mi/nsmoribund.c b/gdb/testsuite/gdb.mi/nsmoribund.c
new file mode 100644
index 0000000..aa41dbd
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/nsmoribund.c
@@ -0,0 +1,72 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2002, 2003, 2004, 2007, 2008 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/>.
+
+   This file is based on schedlock.c.  */
+
+#include <stdio.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include <pthread.h>
+
+int THREADS = 10;
+unsigned int *args;
+volatile int done = 0;
+
+void *
+thread_function (void *arg)
+{
+  int my_number = (long) arg;
+  volatile int *myp = (volatile int *) &args[my_number];
+
+  /* Don't run forever.  Run just short of it :)  */
+  while (*myp > 0 && !done)
+    {
+      (*myp)++; /* set breakpoint here */
+    }
+
+  if (done)
+    usleep (100); /* Some time to make sure we don't mask any bad
+		     SIGTRAP handling.  */
+
+  pthread_exit (NULL);
+}
+
+int
+main (int argc, char **argv)
+{
+  int res;
+  pthread_t *threads;
+  void *thread_result;
+  long i = 0;
+
+  threads = malloc (THREADS * sizeof (pthread_t));
+  args = malloc (THREADS * sizeof (unsigned int));
+
+  for (i = 0; i < THREADS; i++)
+    {
+      args[i] = 1; /* Init value.  */
+      res = pthread_create (&threads[i],
+			    NULL,
+			    thread_function,
+			    (void *) i);
+    }
+
+  for (i = 0; i < THREADS; i++)
+    pthread_join (threads[i], &thread_result);
+
+  exit(EXIT_SUCCESS);
+}