new file mode 100644
@@ -0,0 +1,48 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright (C) 2019 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 program exits if its first argument is --stop, otherwise it
+ tries to exec the first argument, passing --stop. In both cases it
+ writes to a global variable, which GDB will watch in this test. It
+ is meant to be called with its own pathname, so that it writes to
+ the variable, execs itself once, writes to the variable again, and
+ exits. */
+
+#include <unistd.h>
+#include <string.h>
+
+int watched_var = 0;
+
+int main (int argc, char *argv[])
+{
+ char *new_argv[3] = {"", "--stop", NULL};
+
+ if (argc != 2)
+ return 1;
+
+ new_argv[0] = argv[0];
+
+ watched_var = 1;
+
+ if (strcmp (argv[1], "--stop") == 0)
+ return 0;
+
+ if (execvp (argv[1], &new_argv[0]) < 0)
+ return 1;
+
+ return 0;
+}
new file mode 100644
@@ -0,0 +1,53 @@
+# Copyright (C) 2019 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 part of the gdb testsuite.
+
+# Test if a hardware watchpoint triggers before and after a program
+# self-execs. This should happen since the watched global symbol must
+# be valid after the exec.
+
+if {[skip_hw_watchpoint_tests]} {
+ return
+}
+
+# The testsuite doesn't currently allow sending args to the stub.
+if [use_gdb_stub] {
+ unsupported "using gdb stub"
+ return
+}
+
+standard_testfile
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
+ return
+}
+
+gdb_test_no_output "set args $binfile" ""
+
+runto_main
+
+# We don't want to hit the breakpoint from runto_main after the exec.
+delete_breakpoints
+
+gdb_test "watch watched_var" "Hardware watchpoint.*: watched_var.*"
+
+set watchpoint_hit_re \
+ "Hardware watchpoint.*: watched_var.*Old value = 0.*New value = 1"
+
+gdb_test "continue" "$watchpoint_hit_re.*" "continue before exec"
+
+gdb_test "continue" "is executing new program.*$watchpoint_hit_re.*" \
+ "continue after exec"
@@ -65,6 +65,9 @@ struct x86_linux_nat_target : public x86_nat_target<linux_nat_target>
void low_forget_process (pid_t pid) override
{ x86_forget_process (pid); }
+ void low_post_exec (struct lwp_info *lwp) override
+ { x86_cleanup_dregs (lwp->ptid.pid ()); }
+
void low_prepare_to_resume (struct lwp_info *lwp) override
{ x86_linux_prepare_to_resume (lwp); }
@@ -133,13 +133,22 @@ x86_forget_process (pid_t pid)
}
/* Clear the reference counts and forget everything we knew about the
- debug registers. */
+ debug registers of process PID. */
void
-x86_cleanup_dregs (void)
+x86_cleanup_dregs (pid_t pid)
{
/* Starting from scratch has the same effect. */
- x86_forget_process (inferior_ptid.pid ());
+ x86_forget_process (pid);
+}
+
+/* Convenience overloaded function that calls x86_cleanup_dregs with
+ the pid of inferior_ptid. */
+
+void
+x86_cleanup_dregs (void)
+{
+ x86_cleanup_dregs (inferior_ptid.pid ());
}
/* Insert a watchpoint to watch a memory region which starts at
@@ -36,7 +36,13 @@
extern void x86_set_debug_register_length (int len);
-/* Use this function to reset the x86-nat.c debug register state. */
+/* Use this function to reset the x86-nat.c debug register state for
+ PID. */
+
+extern void x86_cleanup_dregs (pid_t pid);
+
+/* Use this function to reset the x86-nat.c debug register state for
+ the pid of inferior_ptid. */
extern void x86_cleanup_dregs (void);
This patch changes the linux native x86 target so that it updates its debug register state after an exec call is detected, by overriding the architecture-specific low target method low_post_exec from linux_nat_target. Previously, an exec event could cause the debug register state to become inconsistent. The infrun logic tries to re-insert symbolic watchpoints and breakpoints after an exec event, in case that they are still meaningful in the new program. However, it assumes watchpoints and breakpoints are cleared across the exec event, including those that use hardware debug registers, so it doesn't try to remove them before re-inserting them. From the target's perspective, this is seen as two successive calls to insert_watchpoint, with no remove_watchpoint in between, as it is sometimes done when the inferior stops and is resumed again. The first insertion comes before the inferior is resumed and eventually calls exec, the second happens after the exec call when infrun updates the symbolic breakpoints and watchpoints that are still meaningful. This double insertion can cause the state of the debug registers tracked by the low target to become inconsistent. In some cases this can cause a watchpoint not to be installed. The x86 linux native target keeps track of requests for equivalent hardware watchpoints with a reference count so that it doesn't need to duplicate debug register usage. This type of request doesn't cause the debug registers in the threads to be updated, since the target assumes the thread already has the debug registers properly configured. If a program self-execs, a global symbol can have the same address and length in its new instance. This will cause the double insertion across an exec call to not have any effect, so the watchpoint won't trigger if the second instance of the program writes to the watched region. This issue is reflected by the testcase included in this patch. This specific issue probably doesn't happen in ARM and s390, and possibly other targets, since they don't seem to handle duplicated hardware watchpoints specially, but any debug register state tracked by these targets is likely to be incorrect, and could cause other issues, so they should also redefine low_post_exec to reset the debug register state that they track. The new testcase will likely pass for these. The Power linux native target currently has other issues that need to be addressed first. This patch makes no changes to the x86 linux-low gdbserver stub, because when the generic linux stub detects an exec event, and if exec events are configured to be reported to the client ("exec-events+" in the query packet), it deletes the process state and recreates one, which causes the debug register data of the linux x86 stub to be cleared. Linux stubs for other arches that support hardware watchpoints can use this same mechanism. When the generic linux stub detects an exec event, but these are not configured to be reported back to the client, the process state is not re-initialized. However, if infrun isn't notified of the exec event in the first place, it won't try to insert the watchpoint twice without a removal in-between, so the debug register state in the stub will match what infrun knows about the watchpoints, even though it doesn't match what is actually installed in the threads, if the watchpoint was cleared by the exec call. gdb/ChangeLog: 2019-05-16 Pedro Franco de Carvalho <pedromfc@linux.ibm.com> * x86-nat.h (x86_cleanup_dregs): Declare overloaded function with a pid argument. Adjust the comment for the original function. * x86-nat.c (x86_cleanup_dregs): Define overloaded function. Change the original version to use it. * x86-linux-nat.h (x86_linux_nat_target) <low_post_exec>: Override and define. gdb/testsuite/ChangeLog: 2019-05-16 Pedro Franco de Carvalho <pedromfc@linux.ibm.com> * gdb.base/watchpoint-exec.exp: New testcase. * gdb.base/watchpoint-exec.c: New file. --- gdb/testsuite/gdb.base/watchpoint-exec.c | 48 ++++++++++++++++++++ gdb/testsuite/gdb.base/watchpoint-exec.exp | 53 ++++++++++++++++++++++ gdb/x86-linux-nat.h | 3 ++ gdb/x86-nat.c | 15 ++++-- gdb/x86-nat.h | 8 +++- 5 files changed, 123 insertions(+), 4 deletions(-) create mode 100644 gdb/testsuite/gdb.base/watchpoint-exec.c create mode 100644 gdb/testsuite/gdb.base/watchpoint-exec.exp