[patch 4/4] hw watchpoints made multi-inferior

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

Hi,

the multi-inferior watchpoints (see [patch 0/4]) were implemented rather to
prove the chosen style of the fork()-compatibility makes sense and is really
usable even for multi-inferior.

Unfortunately currently the breakpoints IMO do not work for multi-inferior,
they are automatically multi-location and GDB even internal errors in some
cases.

This means the workarounds I have chosen require support of "hbreak".
ppc: Not tested as I do not have a BookE/hbreak-capable box.  I guess the
     patch may work in multi-inferior, though.
s390: Not tested as s390 does not support "hbreak".  There are really changes
      needed to be made to make s390 hw-watchpoints multi-inferior capable.

Also I tried to get rid of ALL_LWPS in this patch as it is in fact duplicate
to iterate_over_lwps which is (a) multi-inferior capable (by its FILTER) and
(b) such complicated macros should be discouraged anyway from GDB.  Still some
targets use ALL_LWPS so it still has to stay defined.

After the regular (non-hardware) breakpoints start working the multi-inferior
watchpoints can get tested across more architectures.

I only tried to verify the multi-inferior code, I do not say the
multi-inferior for hw watchpoints support is anyhow complete with this patch.



Thanks,
Jan


gdb/
2010-12-06  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Fix watchpoints for multi-inferior.
	* breakpoint.c
	(update_watchpoint) <b->pspace != current_program_space>: New.
	(insert_breakpoint_locations): New variable
	saved_current_program_space.  Exclude breakpoints not matching it.
	(watch_command_1): Initialize b->pspace.
	(update_global_location_list): Exclude old_loc not matching
	current_program_space.
	* i386-nat.c (i386_inferior_data_cleanup): New.
	(i386_inferior_data_get): Remove variable inf_data_local.  Initialize
	inf_data from an inferior_data call.
	(i386_use_watchpoints): Initialize I386_INFERIOR_DATA.
	* linux-nat.c (linux_nat_iterate_watchpoint_lwps): Fix
	iterate_over_lwps FILTER.

gdb/testsuite/
2010-12-06  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Fix watchpoints for multi-inferior.
	* gdb.multi/watchpoint-multi.c: New file.
	* gdb.multi/watchpoint-multi.exp: New file.

--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -1302,6 +1302,9 @@ update_watchpoint (struct breakpoint *b, int reparse)
   if (!watchpoint_in_thread_scope (b))
     return;
 
+  if (b->pspace != current_program_space)
+    return;
+
   /* We don't free locations.  They are stored in bp_location array and
      update_global_locations will eventually delete them and remove
      breakpoints if needed.  */
@@ -1879,6 +1882,7 @@ insert_breakpoint_locations (void)
   int val = 0;
   int disabled_breaks = 0;
   int hw_breakpoint_error = 0;
+  struct program_space *saved_current_program_space = current_program_space;
 
   struct ui_file *tmp_error_stream = mem_fileopen ();
   struct cleanup *cleanups = make_cleanup_ui_file_delete (tmp_error_stream);
@@ -1908,7 +1912,8 @@ insert_breakpoint_locations (void)
 	 if we aren't attached to any process yet, we should still
 	 insert breakpoints.  */
       if (!gdbarch_has_global_breakpoints (target_gdbarch)
-	  && ptid_equal (inferior_ptid, null_ptid))
+	  && (ptid_equal (inferior_ptid, null_ptid)
+	      || b->pspace != saved_current_program_space))
 	continue;
 
       val = insert_bp_location (b, tmp_error_stream,
@@ -8265,6 +8270,7 @@ watch_command_1 (char *arg, int accessflag, int from_tty,
   b = set_raw_breakpoint_without_location (NULL, bp_type);
   set_breakpoint_number (internal, b);
   b->thread = thread;
+  b->pspace = current_program_space;
   b->disposition = disp_donttouch;
   b->exp = exp;
   b->exp_valid_block = exp_valid_block;
@@ -9404,6 +9410,9 @@ update_global_location_list (int should_insert)
       int keep_in_target = 0;
       int removed = 0;
 
+      if (old_loc->pspace != current_program_space)
+	continue;
+
       /* Skip LOCP entries which will definitely never be needed.  Stop either
 	 at or being the one matching OLD_LOC.  */
       while (locp < bp_location + bp_location_count
--- a/gdb/i386-nat.c
+++ b/gdb/i386-nat.c
@@ -229,15 +229,29 @@ struct i386_inferior_data
     struct dr_mirror dr_mirror;
   };
 
+static void
+i386_inferior_data_cleanup (struct inferior *inf, void *arg)
+{
+  struct i386_inferior_data *inf_data = arg;
+
+  xfree (inf_data);
+}
+
 static struct i386_inferior_data *
 i386_inferior_data_get (void)
 {
-  static struct i386_inferior_data inf_data_local;
   struct inferior *inf = current_inferior ();
-  struct i386_inferior_data *inf_data = &inf_data_local;
+  struct i386_inferior_data *inf_data;
   static struct i386_inferior_data *detached_inf_data;
   static int detached_inf_pid = -1;
 
+  inf_data = inferior_data (inf, i386_inferior_data);
+  if (inf_data == NULL)
+    {
+      inf_data = xzalloc (sizeof (*inf_data));
+      set_inferior_data (current_inferior (), i386_inferior_data, inf_data);
+    }
+
   if (inf->pid != ptid_get_pid (inferior_ptid))
     {
       if (detached_inf_pid != ptid_get_pid (inferior_ptid))
@@ -779,6 +793,10 @@ i386_use_watchpoints (struct target_ops *t)
   t->to_remove_watchpoint = i386_remove_watchpoint;
   t->to_insert_hw_breakpoint = i386_insert_hw_breakpoint;
   t->to_remove_hw_breakpoint = i386_remove_hw_breakpoint;
+
+  if (i386_inferior_data == NULL)
+    i386_inferior_data
+      = register_inferior_data_with_cleanup (i386_inferior_data_cleanup);
 }
 
 void
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -1287,7 +1287,7 @@ linux_nat_iterate_watchpoint_lwps
   if (inf->pid == inferior_pid)
     {
       /* Standard mode.  */
-      iterate_over_lwps (minus_one_ptid,
+      iterate_over_lwps (pid_to_ptid (inferior_pid),
 			 iterate_watchpoint_lwps_callback, &data);
     }
   else
--- /dev/null
+++ b/gdb/testsuite/gdb.multi/watchpoint-multi.c
@@ -0,0 +1,46 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2010 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 volatile int a, b, c;
+
+static void
+marker_exit1 (void)
+{
+  a = 1;
+}
+
+/* Workaround PR breakpoints/12272 by two different breakpoint locations.  */
+static void
+marker_exit2 (void)
+{
+  a = 1;
+}
+
+int
+main (void)
+{
+  a = 1;
+  a = 1;
+  b = 2;
+  b = 2;
+  c = 3;
+  c = 3;
+
+  marker_exit1 ();
+  marker_exit2 ();
+  return 0;
+}
--- /dev/null
+++ b/gdb/testsuite/gdb.multi/watchpoint-multi.exp
@@ -0,0 +1,84 @@
+# Copyright 2010 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 { [is_remote target] || ![isnative] } then {
+    continue
+}
+
+set testfile "watchpoint-multi"
+
+set executable ${testfile}
+set srcfile ${testfile}.c
+set binfile ${objdir}/${subdir}/${executable}
+
+if { [prepare_for_testing ${testfile}.exp ${executable} ${srcfile}] } {
+    return -1
+}
+
+if ![runto_main] {
+    return
+}
+# Never keep/use any non-hw breakpoints to workaround a multi-inferior bug.
+delete_breakpoints
+
+gdb_test "add-inferior" "Added inferior 2"
+gdb_test "inferior 2" "witching to inferior 2 .*"
+gdb_load $binfile
+
+if ![runto_main] {
+    return
+}
+delete_breakpoints
+
+# Simulate non-stop+target-async which also uses breakpoint always-inserted.
+gdb_test_no_output "set breakpoint always-inserted on"
+
+# Debugging of this testcase:
+#gdb_test_no_output "maintenance set show-debug-regs on"
+#gdb_test_no_output "set debug infrun 1"
+
+gdb_test "watch c" "Hardware watchpoint \[0-9\]+: c"
+# Never keep/use any non-hw breakpoints to workaround a multi-inferior bug.
+# Use `*' to workaround a multi-inferior bug.
+set test "hbreak *marker_exit2"
+gdb_test_multiple $test $test {
+    -re "Hardware assisted breakpoint \[0-9\]+ at .*\r\n$gdb_prompt $" {
+	pass $test
+    }
+    -re "(No hardware breakpoint support in the target\\.|Hardware breakpoints used exceeds limit\\.)\r\n$gdb_prompt $" {
+	pass $test
+	untested ${testfile}.exp
+	return
+    }
+}
+
+gdb_test "inferior 1" "witching to inferior 1 .*"
+
+gdb_test "watch b" "Hardware watchpoint \[0-9\]+: b"
+gdb_test "hbreak *marker_exit1" {Hardware assisted breakpoint [0-9]+ at .*}
+
+gdb_test "inferior 2" "witching to inferior 2 .*"
+
+# FAIL would be a hit on watchpoint for `b' - that one is for the other
+# inferior.
+gdb_test "continue" "Hardware watchpoint \[0-9\]+: c\r\n\r\nOld value = 0\r\nNew value = 3\r\n.*" "catch c"
+
+gdb_test "continue" {Breakpoint [0-9]+, marker_exit2 .*} "catch marker_exit2"
+
+gdb_test "inferior 1" "witching to inferior 1 .*"
+
+gdb_test "continue" "Hardware watchpoint \[0-9\]+: b\r\n\r\nOld value = 0\r\nNew value = 2\r\n.*" "catch b"
+
+gdb_test "continue" {Breakpoint [0-9]+, marker_exit1 .*} "catch marker_exit2"