[commit] [patch 2/2] Fix watchpoints for multi-inferior #2

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

On Tue, 24 Jan 2012 14:19:34 +0100, Pedro Alves wrote:
> Please try to keep an open spirit.

This patch series took 4+ years (Oct 2007) with 5 repost series total, that is
a long time.


> Ah.  Could you please split the gdbserver bits into a separate patch?

Yes.  To be reposted.


> The non-gdbserver bits look okay to me.

Checked in.


Thanks,
Jan


http://sourceware.org/ml/gdb-cvs/2012-01/msg00197.html

--- src/gdb/ChangeLog	2012/01/24 13:46:50	1.13767
+++ src/gdb/ChangeLog	2012/01/24 13:49:53	1.13768
@@ -1,5 +1,17 @@
 2012-01-24  Jan Kratochvil  <jan.kratochvil@redhat.com>
 
+	Fix watchpoints to be specific for each inferior.
+	* breakpoint.c (watchpoint_in_thread_scope): Verify also
+	current_program_space.
+	* i386-nat.c (i386_inferior_data_cleanup): New.
+	(i386_inferior_data_get): Replace variable inf_data_local by an
+	inferior_data call.
+	(i386_use_watchpoints): Initialize i386_inferior_data.
+	* linux-nat.c (linux_nat_iterate_watchpoint_lwps): Use INFERIOR_PTID
+	specific iterate_over_lwps.
+
+2012-01-24  Jan Kratochvil  <jan.kratochvil@redhat.com>
+
 	Fix watchpoints across inferior fork.
 	* amd64-linux-nat.c (update_debug_registers_callback): Update the
 	comment for linux_nat_iterate_watchpoint_lwps.
--- src/gdb/testsuite/ChangeLog	2012/01/24 13:46:54	1.3033
+++ src/gdb/testsuite/ChangeLog	2012/01/24 13:49:58	1.3034
@@ -1,5 +1,11 @@
 2012-01-24  Jan Kratochvil  <jan.kratochvil@redhat.com>
 
+	Fix watchpoints to be specific for each inferior.
+	* gdb.multi/watchpoint-multi.c: New file.
+	* gdb.multi/watchpoint-multi.exp: New file.
+
+2012-01-24  Jan Kratochvil  <jan.kratochvil@redhat.com>
+
 	Fix watchpoints across inferior fork.
 	* gdb.threads/watchpoint-fork-child.c: New file.
 	* gdb.threads/watchpoint-fork-mt.c: New file.
--- src/gdb/breakpoint.c	2012/01/16 20:40:50	1.642
+++ src/gdb/breakpoint.c	2012/01/24 13:49:55	1.643
@@ -1239,9 +1239,10 @@
 static int
 watchpoint_in_thread_scope (struct watchpoint *b)
 {
-  return (ptid_equal (b->watchpoint_thread, null_ptid)
-	  || (ptid_equal (inferior_ptid, b->watchpoint_thread)
-	      && !is_executing (inferior_ptid)));
+  return (b->base.pspace == current_program_space
+	  && (ptid_equal (b->watchpoint_thread, null_ptid)
+	      || (ptid_equal (inferior_ptid, b->watchpoint_thread)
+		  && !is_executing (inferior_ptid))));
 }
 
 /* Set watchpoint B to disp_del_at_next_stop, even including its possible
--- src/gdb/i386-nat.c	2012/01/24 13:46:54	1.39
+++ src/gdb/i386-nat.c	2012/01/24 13:49:56	1.40
@@ -181,16 +181,31 @@
   struct i386_debug_reg_state state;
 };
 
+/* Per-inferior hook for register_inferior_data_with_cleanup.  */
+
+static void
+i386_inferior_data_cleanup (struct inferior *inf, void *arg)
+{
+  struct i386_inferior_data *inf_data = arg;
+
+  xfree (inf_data);
+}
+
 /* Get data specific for INFERIOR_PTID LWP.  Return special data area
    for processes being detached.  */
 
 static struct i386_inferior_data *
 i386_inferior_data_get (void)
 {
-  /* Intermediate patch stub.  */
-  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;
+
+  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))
     {
@@ -855,6 +870,10 @@
   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
--- src/gdb/testsuite/gdb.multi/watchpoint-multi.c
+++ src/gdb/testsuite/gdb.multi/watchpoint-multi.c	2012-01-24 13:52:56.759120000 +0000
@@ -0,0 +1,51 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2012 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/>.  */
+
+#include <pthread.h>
+#include <assert.h>
+
+static volatile int a, b, c;
+
+static void
+marker_exit (void)
+{
+  a = 1;
+}
+
+static void *
+start (void *arg)
+{
+  b = 2;
+  c = 3;
+
+  return NULL;
+}
+
+int
+main (void)
+{
+  pthread_t thread;
+  int i;
+
+  i = pthread_create (&thread, NULL, start, NULL);
+  assert (i == 0);
+  i = pthread_join (thread, NULL);
+  assert (i == 0);
+
+  marker_exit ();
+  return 0;
+}
--- src/gdb/testsuite/gdb.multi/watchpoint-multi.exp
+++ src/gdb/testsuite/gdb.multi/watchpoint-multi.exp	2012-01-24 13:52:57.147228000 +0000
@@ -0,0 +1,91 @@
+# Copyright 2012 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/>.
+
+set testfile "watchpoint-multi"
+
+# Multiple inferiors are needed, therefore both native and extended gdbserver
+# modes are supported.  Only non-extended gdbserver is not supported.
+if [target_info exists use_gdb_stub] {
+    untested ${testfile}.exp
+    return
+}
+
+# Do not use simple hardware watchpoints ("watch") as its false hit may be
+# unnoticed by GDB if it reads it still has the same value.
+if [skip_hw_watchpoint_access_tests] {
+    untested "${testfile} (no hardware access watchpoints)"
+    return
+}
+
+set executable ${testfile}
+set srcfile ${testfile}.c
+set binfile ${objdir}/${subdir}/${executable}
+
+if { [gdb_compile_pthreads "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+    untested ${testfile}.exp
+    return -1
+}
+
+clean_restart $executable
+
+# Simulate non-stop+target-async which also uses breakpoint always-inserted.
+gdb_test_no_output "set breakpoint always-inserted on"
+# displaced-stepping is also needed as other GDB sometimes still removes the
+# breakpoints, even with always-inserted on.
+# Without the support this test just is not as thorough as it could be.
+if [support_displaced_stepping] {
+    gdb_test_no_output "set displaced-stepping on"
+}
+
+# Debugging of this testcase:
+#gdb_test_no_output "maintenance set show-debug-regs on"
+#gdb_test_no_output "set debug infrun 1"
+
+gdb_breakpoint main {temporary}
+gdb_test "run" "Temporary breakpoint.* main .*" "start to main inferior 1"
+
+gdb_test "add-inferior" "Added inferior 2"
+gdb_test "inferior 2" "witching to inferior 2 .*"
+gdb_load $binfile
+
+gdb_breakpoint main {temporary}
+gdb_test "run" "Temporary breakpoint.* main .*" "start to main inferior 2"
+
+gdb_test "awatch c" "Hardware access \\(read/write\\) watchpoint \[0-9\]+: c"
+
+gdb_breakpoint "marker_exit"
+
+gdb_test "inferior 1" "witching to inferior 1 .*"
+
+if [skip_hw_watchpoint_multi_tests] {
+    # On single hardware watchpoint at least test the watchpoint in inferior
+    # 2 is not hit.
+} else {
+    gdb_test "awatch b" "Hardware access \\(read/write\\) watchpoint \[0-9\]+: b"
+
+    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 access \\(read/write\\) 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_exit .*" "catch marker_exit in inferior 2"
+
+    gdb_test "inferior 1" "witching to inferior 1 .*"
+
+    gdb_test "continue" "Hardware access \\(read/write\\) 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_exit .*" "catch marker_exit in inferior 1"