Re: [PATCH] Fix PR cli/15603

[Tom Tromey <tromey at redhat dot com>]

Tom> I'm regression-testing it now.

It tested fine.  Let me know what you think.

Tom


This fixes PR cli/15603.

The bug here is that when a software watchpoint is being used, gdb
will stop responding to C-c.  This is a regression caused by the
"catch signal" patch.

The problem is that software watchpoints always end up on the bpstat
list.  However, this makes bpstat_explains_signal return
BPSTAT_SIGNAL_HIDE, causing infrun to think that the signal is not a
"random signal".

The fix is to change bpstat_explains_signal to handle this better.  I
chose to do it in a "clean API" way, by passing the signal value to
bpstat_explains_signal and then adding an explains_signal method for
watchpoints, which handles the specifics.

Built and regtested on x86-64 Fedora 18.
New test case included.

	* break-catch-sig.c (signal_catchpoint_explains_signal): Add 'sig'
	argument.
	* breakpoint.c (bpstat_explains_signal): Add 'sig' argument.
	Special case signals other than GDB_SIGNAL_TRAP.
	(explains_signal_watchpoint): New function.
	(base_breakpoint_explains_signal): Add 'sig' argument.
	(initialize_breakpoint_ops): Set 'explains_signal' method for
	watchpoints.
	* breakpoint.h (struct breakpoint_ops) <explains_signal>: Add
	signal argument.
	(bpstat_explains_signal): Likewise.
	* infrun.c (handle_syscall_event, handle_inferior_event): Update.

	* gdb.base/random-signal.c: New file.
	* gdb.base/random-signal.exp: New file.

fixup
---
 gdb/break-catch-sig.c                    |  2 +-
 gdb/breakpoint.c                         | 31 ++++++++++++++++++++---
 gdb/breakpoint.h                         |  6 +++--
 gdb/infrun.c                             | 18 +++++++++-----
 gdb/testsuite/gdb.base/random-signal.c   | 28 +++++++++++++++++++++
 gdb/testsuite/gdb.base/random-signal.exp | 42 ++++++++++++++++++++++++++++++++
 6 files changed, 114 insertions(+), 13 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/random-signal.c
 create mode 100644 gdb/testsuite/gdb.base/random-signal.exp

diff --git a/gdb/break-catch-sig.c b/gdb/break-catch-sig.c
index c162cc2..02d8b4a 100644
--- a/gdb/break-catch-sig.c
+++ b/gdb/break-catch-sig.c
@@ -351,7 +351,7 @@ signal_catchpoint_print_recreate (struct breakpoint *b, struct ui_file *fp)
    catchpoints.  */
 
 static enum bpstat_signal_value
-signal_catchpoint_explains_signal (struct breakpoint *b)
+signal_catchpoint_explains_signal (struct breakpoint *b, enum gdb_signal sig)
 {
   return BPSTAT_SIGNAL_PASS;
 }
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index d4ccc81..c1a9981 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -4153,7 +4153,7 @@ bpstat_find_breakpoint (bpstat bsp, struct breakpoint *breakpoint)
 /* See breakpoint.h.  */
 
 enum bpstat_signal_value
-bpstat_explains_signal (bpstat bsp)
+bpstat_explains_signal (bpstat bsp, enum gdb_signal sig)
 {
   enum bpstat_signal_value result = BPSTAT_SIGNAL_NO;
 
@@ -4163,8 +4163,16 @@ bpstat_explains_signal (bpstat bsp)
 	 return BPSTAT_SIGNAL_HIDE.  */
       enum bpstat_signal_value newval = BPSTAT_SIGNAL_HIDE;
 
-      if (bsp->breakpoint_at != NULL)
-	newval = bsp->breakpoint_at->ops->explains_signal (bsp->breakpoint_at);
+      if (bsp->breakpoint_at == NULL)
+	{
+	  /* A moribund location can never explain a signal other than
+	     GDB_SIGNAL_TRAP.  */
+	  if (sig != GDB_SIGNAL_TRAP)
+	    newval = BPSTAT_SIGNAL_NO;
+	}
+      else
+	newval = bsp->breakpoint_at->ops->explains_signal (bsp->breakpoint_at,
+							   sig);
 
       if (newval > result)
 	result = newval;
@@ -10630,6 +10638,20 @@ print_recreate_watchpoint (struct breakpoint *b, struct ui_file *fp)
   print_recreate_thread (b, fp);
 }
 
+/* Implement the "explains_signal" breakpoint_ops method for
+   watchpoints.  */
+
+static enum bpstat_signal_value
+explains_signal_watchpoint (struct breakpoint *b, enum gdb_signal sig)
+{
+  /* A software watchpoint cannot cause a signal other than
+     GDB_SIGNAL_TRAP.  */
+  if (b->type == bp_watchpoint && sig != GDB_SIGNAL_TRAP)
+    return BPSTAT_SIGNAL_NO;
+
+  return BPSTAT_SIGNAL_HIDE;
+}
+
 /* The breakpoint_ops structure to be used in hardware watchpoints.  */
 
 static struct breakpoint_ops watchpoint_breakpoint_ops;
@@ -12733,7 +12755,7 @@ base_breakpoint_decode_linespec (struct breakpoint *b, char **s,
 /* The default 'explains_signal' method.  */
 
 static enum bpstat_signal_value
-base_breakpoint_explains_signal (struct breakpoint *b)
+base_breakpoint_explains_signal (struct breakpoint *b, enum gdb_signal sig)
 {
   return BPSTAT_SIGNAL_HIDE;
 }
@@ -15743,6 +15765,7 @@ initialize_breakpoint_ops (void)
   ops->print_it = print_it_watchpoint;
   ops->print_mention = print_mention_watchpoint;
   ops->print_recreate = print_recreate_watchpoint;
+  ops->explains_signal = explains_signal_watchpoint;
 
   /* Masked watchpoints.  */
   ops = &masked_watchpoint_breakpoint_ops;
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 94210f4..43ab906 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -613,7 +613,8 @@ struct breakpoint_ops
      should still be delivered to the inferior.  This is used to make
      'catch signal' interact properly with 'handle'; see
      bpstat_explains_signal.  */
-  enum bpstat_signal_value (*explains_signal) (struct breakpoint *);
+  enum bpstat_signal_value (*explains_signal) (struct breakpoint *,
+					       enum gdb_signal);
 };
 
 /* Helper for breakpoint_ops->print_recreate implementations.  Prints
@@ -1009,7 +1010,8 @@ bpstat bpstat_find_breakpoint (bpstat, struct breakpoint *);
 /* Nonzero if a signal that we got in wait() was due to circumstances
    explained by the bpstat; and the signal should therefore not be
    delivered.  */
-extern enum bpstat_signal_value bpstat_explains_signal (bpstat);
+extern enum bpstat_signal_value bpstat_explains_signal (bpstat,
+							enum gdb_signal);
 
 /* Nonzero is this bpstat causes a stop.  */
 extern int bpstat_causes_stop (bpstat);
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 51a032b..720c4ed 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -3124,7 +3124,8 @@ handle_syscall_event (struct execution_control_state *ecs)
 	= bpstat_stop_status (get_regcache_aspace (regcache),
 			      stop_pc, ecs->ptid, &ecs->ws);
 
-      sval = bpstat_explains_signal (ecs->event_thread->control.stop_bpstat);
+      sval = bpstat_explains_signal (ecs->event_thread->control.stop_bpstat,
+				     GDB_SIGNAL_TRAP);
       ecs->random_signal = sval == BPSTAT_SIGNAL_NO;
 
       if (!ecs->random_signal)
@@ -3374,7 +3375,8 @@ handle_inferior_event (struct execution_control_state *ecs)
 				  stop_pc, ecs->ptid, &ecs->ws);
 
 	  sval
-	    = bpstat_explains_signal (ecs->event_thread->control.stop_bpstat);
+	    = bpstat_explains_signal (ecs->event_thread->control.stop_bpstat,
+				      GDB_SIGNAL_TRAP);
 	  ecs->random_signal = sval == BPSTAT_SIGNAL_NO;
 
 	  if (!ecs->random_signal)
@@ -3673,7 +3675,8 @@ handle_inferior_event (struct execution_control_state *ecs)
 	= bpstat_stop_status (get_regcache_aspace (get_current_regcache ()),
 			      stop_pc, ecs->ptid, &ecs->ws);
       ecs->random_signal
-	= (bpstat_explains_signal (ecs->event_thread->control.stop_bpstat)
+	= (bpstat_explains_signal (ecs->event_thread->control.stop_bpstat,
+				   GDB_SIGNAL_TRAP)
 	   == BPSTAT_SIGNAL_NO);
 
       /* Note that this may be referenced from inside
@@ -4248,7 +4251,8 @@ handle_inferior_event (struct execution_control_state *ecs)
 
   if (debug_infrun
       && ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP
-      && (bpstat_explains_signal (ecs->event_thread->control.stop_bpstat)
+      && (bpstat_explains_signal (ecs->event_thread->control.stop_bpstat,
+				  GDB_SIGNAL_TRAP)
 	  == BPSTAT_SIGNAL_NO)
       && stopped_by_watchpoint)
     fprintf_unfiltered (gdb_stdlog,
@@ -4277,7 +4281,8 @@ handle_inferior_event (struct execution_control_state *ecs)
 
   if (ecs->event_thread->suspend.stop_signal == GDB_SIGNAL_TRAP)
     ecs->random_signal
-      = !((bpstat_explains_signal (ecs->event_thread->control.stop_bpstat)
+      = !((bpstat_explains_signal (ecs->event_thread->control.stop_bpstat,
+				   GDB_SIGNAL_TRAP)
 	   != BPSTAT_SIGNAL_NO)
 	  || stopped_by_watchpoint
 	  || ecs->event_thread->control.trap_expected
@@ -4288,7 +4293,8 @@ handle_inferior_event (struct execution_control_state *ecs)
     {
       enum bpstat_signal_value sval;
 
-      sval = bpstat_explains_signal (ecs->event_thread->control.stop_bpstat);
+      sval = bpstat_explains_signal (ecs->event_thread->control.stop_bpstat,
+				     ecs->event_thread->suspend.stop_signal);
       ecs->random_signal = (sval == BPSTAT_SIGNAL_NO);
 
       if (sval == BPSTAT_SIGNAL_HIDE)
diff --git a/gdb/testsuite/gdb.base/random-signal.c b/gdb/testsuite/gdb.base/random-signal.c
new file mode 100644
index 0000000..bd00d66
--- /dev/null
+++ b/gdb/testsuite/gdb.base/random-signal.c
@@ -0,0 +1,28 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2013 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/>.  */
+
+int v;
+
+void do_something () { }
+
+int main()
+{
+  do_something();
+
+  for (;;)
+    ;
+}
diff --git a/gdb/testsuite/gdb.base/random-signal.exp b/gdb/testsuite/gdb.base/random-signal.exp
new file mode 100644
index 0000000..bd23513
--- /dev/null
+++ b/gdb/testsuite/gdb.base/random-signal.exp
@@ -0,0 +1,42 @@
+# Copyright 2013 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/>.
+
+if [target_info exists gdb,nosignals] {
+    verbose "Skipping catch-signal.exp because of nosignals."
+    continue
+}
+
+standard_testfile
+
+if {[prepare_for_testing $testfile.exp $testfile $srcfile debug]} {
+    return -1
+}
+
+if {![runto_main]} {
+    return -1
+}
+
+gdb_test_no_output "set can-use-hw-watchpoints 0"
+gdb_test "watch v" "Watchpoint .*"
+gdb_test_multiple "continue" "continue" {
+    -re "Continuing" {
+	pass "continue"
+    }
+}
+
+# For this to work we must be sure to consume the "Continuing."
+# message first, or GDB's signal handler may not be in place.
+after 500 {send_gdb "\003"}
+gdb_test "" "Program received signal SIGINT.*" "stop with control-c"
-- 
1.8.1.4