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

Jan Kratochvil
2012-01-02T16:47:15Z

Hi,

this is a new post of:
[patch 4/4] hw watchpoints made multi-inferior
http://sourceware.org/ml/gdb-patches/2010-12/msg00045.html

Currently watchpoints somehow apply to all the inferiors but they do not work
that way anyway. I believe watchpoints should be specific for each inferior.

The testcase has some problems with gdbserver, it has been skipped now.

No regressions on {x86_64,x86_64-m32,i686}-fedorarawhide-linux-gnu.

For ppc* the testcase will be difficult, to test the issue properly one needs
`awatch' (that is `watch' can hide a problem) but PPC does not feature
`awatch'.

Thanks,
Jan

gdb/
2012-01-02 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.

gdb/testsuite/
2012-01-02 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.

--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -1225,9 +1225,10 @@ is_watchpoint (const struct breakpoint *bpt)
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
--- a/gdb/i386-nat.c
+++ b/gdb/i386-nat.c
@@ -181,16 +181,31 @@ struct i386_inferior_data
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))
{
@@ -856,6 +871,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
@@ -1288,7 +1288,11 @@ linux_nat_iterate_watchpoint_lwps

if (inf->pid == inferior_pid)
{
- iterate_over_lwps (minus_one_ptid, callback, callback_data);
+ /* Iterate all the threads of the current inferior. Without specifying
+ INFERIOR_PID it would iterate all threads of all inferiors, which is
+ inappropriate for watchpoints. */
+
+ iterate_over_lwps (pid_to_ptid (inferior_pid), callback, callback_data);
}
else
{
--- /dev/null
+++ b/gdb/testsuite/gdb.multi/watchpoint-multi.c
@@ -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;
+}
--- /dev/null
+++ b/gdb/testsuite/gdb.multi/watchpoint-multi.exp
@@ -0,0 +1,92 @@
+# 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/>.
+
+if [is_remote target] {
+ # It is KFAIL.
+ continue
+}
+
+set testfile "watchpoint-multi"
+
+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
+
+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"
+# displaced-stepping is also needed as other GDB sometimes still removes the
+# breakpoints, even with always-inserted on.
+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"
+
+# Do not use simple hardware watchpoint ("watch") as its false hit may be
+# unnoticed by GDB if it reads it still has the same value.
+gdb_test "awatch c" "Hardware access \\(read/write\\) watchpoint \[0-9\]+: c"
+
+gdb_breakpoint "marker_exit"
+
+gdb_test "inferior 1" "witching to inferior 1 .*"
+
+set have_awatch_b 0
+set test "awatch b"
+gdb_test_multiple $test $test {
+ -re "Hardware access \\(read/write\\) watchpoint \[0-9\]+: b\r\n$gdb_prompt $" {
+ pass $test
+ set have_awatch_b 1
+ }
+ -re "There are not enough available hardware resources for this watchpoint\\.\r\n$gdb_prompt $" {
+ untested $test
+ return
+ }
+}
+
+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"