diff options
| -rw-r--r-- | gdb/ChangeLog | 20 | ||||
| -rw-r--r-- | gdb/amd64-linux-nat.c | 30 | ||||
| -rw-r--r-- | gdb/breakpoint.c | 41 | ||||
| -rw-r--r-- | gdb/i386-linux-nat.c | 31 | ||||
| -rw-r--r-- | gdb/i386-nat.c | 9 | ||||
| -rw-r--r-- | gdb/i386-nat.h | 12 | ||||
| -rw-r--r-- | gdb/testsuite/ChangeLog | 9 | ||||
| -rw-r--r-- | gdb/testsuite/gdb.base/watchpoint-hw-hit-once.c | 34 | ||||
| -rw-r--r-- | gdb/testsuite/gdb.base/watchpoint-hw-hit-once.exp | 43 |
9 files changed, 221 insertions, 8 deletions
diff --git a/gdb/ChangeLog b/gdb/ChangeLog index b71505e4c8a..1a4fcd77152 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,4 +1,24 @@ 2009-11-20 Jan Kratochvil <jan.kratochvil@redhat.com> + + Fix repeated rwatch output. + * amd64-linux-nat.c (amd64_linux_dr_set, amd64_linux_dr_set_control) + (amd64_linux_dr_set_addr, amd64_linux_dr_reset_addr) + (amd64_linux_dr_get_status): New comments. + (amd64_linux_dr_unset_status): New function. + (_initialize_amd64_linux_nat): Install it. + * i386-linux-nat.c (i386_linux_dr_get, i386_linux_dr_set) + (i386_linux_dr_set_control, i386_linux_dr_set_addr) + (i386_linux_dr_reset_addr, i386_linux_dr_get_status): New comments. + (i386_linux_dr_unset_status): New function. + (_initialize_i386_linux_nat): Install it. + * i386-nat.c (I386_DR_WATCH_MASK): New macro. + (I386_DR_WATCH_HIT): Use I386_DR_WATCH_MASK. + (i386_insert_aligned_watchpoint): Call i386_dr_low.unset_status. + * i386-nat.h (struct i386_dr_low_type): Extend comments for + set_control, set_addr, reset_addr and get_status. New unset_status. + * breakpoint.c (update_watchpoint): Extend the comment. + +2009-11-20 Jan Kratochvil <jan.kratochvil@redhat.com> Pedro Alves <pedro@codesourcery.com> Fix reordered watchpoints triggered in other threads during all-stop. diff --git a/gdb/amd64-linux-nat.c b/gdb/amd64-linux-nat.c index 7d8461c9116..9ccd3b2e9fe 100644 --- a/gdb/amd64-linux-nat.c +++ b/gdb/amd64-linux-nat.c @@ -270,6 +270,8 @@ amd64_linux_dr_get (ptid_t ptid, int regnum) return value; } +/* Set debug register REGNUM to VALUE in only the one LWP of PTID. */ + static void amd64_linux_dr_set (ptid_t ptid, int regnum, unsigned long value) { @@ -286,6 +288,8 @@ amd64_linux_dr_set (ptid_t ptid, int regnum, unsigned long value) perror_with_name (_("Couldn't write debug register")); } +/* Set DR_CONTROL to ADDR in all LWPs of LWP_LIST. */ + static void amd64_linux_dr_set_control (unsigned long control) { @@ -297,6 +301,8 @@ amd64_linux_dr_set_control (unsigned long control) amd64_linux_dr_set (ptid, DR_CONTROL, control); } +/* Set address REGNUM (zero based) to ADDR in all LWPs of LWP_LIST. */ + static void amd64_linux_dr_set_addr (int regnum, CORE_ADDR addr) { @@ -310,18 +316,41 @@ amd64_linux_dr_set_addr (int regnum, CORE_ADDR addr) amd64_linux_dr_set (ptid, DR_FIRSTADDR + regnum, addr); } +/* Set address REGNUM (zero based) to zero in all LWPs of LWP_LIST. */ + static void amd64_linux_dr_reset_addr (int regnum) { amd64_linux_dr_set_addr (regnum, 0); } +/* Get DR_STATUS from only the one LWP of INFERIOR_PTID. */ + static unsigned long amd64_linux_dr_get_status (void) { return amd64_linux_dr_get (inferior_ptid, DR_STATUS); } +/* Unset MASK bits in DR_STATUS in all LWPs of LWP_LIST. */ + +static void +amd64_linux_dr_unset_status (unsigned long mask) +{ + struct lwp_info *lp; + ptid_t ptid; + + ALL_LWPS (lp, ptid) + { + unsigned long value; + + value = amd64_linux_dr_get (ptid, DR_STATUS); + value &= ~mask; + amd64_linux_dr_set (ptid, DR_STATUS, value); + } +} + + static void amd64_linux_new_thread (ptid_t ptid) { @@ -672,6 +701,7 @@ _initialize_amd64_linux_nat (void) i386_dr_low.set_addr = amd64_linux_dr_set_addr; i386_dr_low.reset_addr = amd64_linux_dr_reset_addr; i386_dr_low.get_status = amd64_linux_dr_get_status; + i386_dr_low.unset_status = amd64_linux_dr_unset_status; i386_set_debug_register_length (8); /* Override the GNU/Linux inferior startup hook. */ diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 1c30f6dcf06..d879b575413 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -993,7 +993,46 @@ fetch_watchpoint_value (struct expression *exp, struct value **valp, - Update the list of values that must be watched in B->loc. If the watchpoint disposition is disp_del_at_next_stop, then do nothing. - If this is local watchpoint that is out of scope, delete it. */ + If this is local watchpoint that is out of scope, delete it. + + Even with `set breakpoint always-inserted on' the watchpoints are removed + + inserted on each stop here. Normal breakpoints must never be removed + because they might be missed by a running thread when debugging in non-stop + mode. On the other hand, hardware watchpoints (is_hardware_watchpoint; + processed here) are specific to each LWP since they are stored in each LWP's + hardware debug registers. Therefore, such LWP must be stopped first in + order to be able to modify its hardware watchpoints. + + Hardware watchpoints must be reset exactly once after being presented to the + user. It cannot be done sooner, because it would reset the data used to + present the watchpoint hit to the user. And it must not be done later + because it could display the same single watchpoint hit during multiple GDB + stops. Note that the latter is relevant only to the hardware watchpoint + types bp_read_watchpoint and bp_access_watchpoint. False hit by + bp_hardware_watchpoint is not user-visible - its hit is suppressed if the + memory content has not changed. + + The following constraints influence the location where we can reset hardware + watchpoints: + + * target_stopped_by_watchpoint and target_stopped_data_address are called + several times when GDB stops. + + [linux] + * Multiple hardware watchpoints can be hit at the same time, causing GDB to + stop. GDB only presents one hardware watchpoint hit at a time as the + reason for stopping, and all the other hits are presented later, one after + the other, each time the user requests the execution to be resumed. + Execution is not resumed for the threads still having pending hit event + stored in LWP_INFO->STATUS. While the watchpoint is already removed from + the inferior on the first stop the thread hit event is kept being reported + from its cached value by linux_nat_stopped_data_address until the real + thread resume happens after the watchpoint gets presented and thus its + LWP_INFO->STATUS gets reset. + + Therefore the hardware watchpoint hit can get safely reset on the watchpoint + removal from inferior. */ + static void update_watchpoint (struct breakpoint *b, int reparse) { diff --git a/gdb/i386-linux-nat.c b/gdb/i386-linux-nat.c index fe848ff6460..186a2fd0ba5 100644 --- a/gdb/i386-linux-nat.c +++ b/gdb/i386-linux-nat.c @@ -586,6 +586,8 @@ i386_linux_store_inferior_registers (struct target_ops *ops, static unsigned long i386_linux_dr[DR_CONTROL + 1]; +/* Get debug register REGNUM value from only the one LWP of PTID. */ + static unsigned long i386_linux_dr_get (ptid_t ptid, int regnum) { @@ -614,6 +616,8 @@ i386_linux_dr_get (ptid_t ptid, int regnum) return value; } +/* Set debug register REGNUM to VALUE in only the one LWP of PTID. */ + static void i386_linux_dr_set (ptid_t ptid, int regnum, unsigned long value) { @@ -630,6 +634,8 @@ i386_linux_dr_set (ptid_t ptid, int regnum, unsigned long value) perror_with_name (_("Couldn't write debug register")); } +/* Set DR_CONTROL to ADDR in all LWPs of LWP_LIST. */ + static void i386_linux_dr_set_control (unsigned long control) { @@ -641,6 +647,8 @@ i386_linux_dr_set_control (unsigned long control) i386_linux_dr_set (ptid, DR_CONTROL, control); } +/* Set address REGNUM (zero based) to ADDR in all LWPs of LWP_LIST. */ + static void i386_linux_dr_set_addr (int regnum, CORE_ADDR addr) { @@ -654,18 +662,40 @@ i386_linux_dr_set_addr (int regnum, CORE_ADDR addr) i386_linux_dr_set (ptid, DR_FIRSTADDR + regnum, addr); } +/* Set address REGNUM (zero based) to zero in all LWPs of LWP_LIST. */ + static void i386_linux_dr_reset_addr (int regnum) { i386_linux_dr_set_addr (regnum, 0); } +/* Get DR_STATUS from only the one LWP of INFERIOR_PTID. */ + static unsigned long i386_linux_dr_get_status (void) { return i386_linux_dr_get (inferior_ptid, DR_STATUS); } +/* Unset MASK bits in DR_STATUS in all LWPs of LWP_LIST. */ + +static void +i386_linux_dr_unset_status (unsigned long mask) +{ + struct lwp_info *lp; + ptid_t ptid; + + ALL_LWPS (lp, ptid) + { + unsigned long value; + + value = i386_linux_dr_get (ptid, DR_STATUS); + value &= ~mask; + i386_linux_dr_set (ptid, DR_STATUS, value); + } +} + static void i386_linux_new_thread (ptid_t ptid) { @@ -837,6 +867,7 @@ _initialize_i386_linux_nat (void) i386_dr_low.set_addr = i386_linux_dr_set_addr; i386_dr_low.reset_addr = i386_linux_dr_reset_addr; i386_dr_low.get_status = i386_linux_dr_get_status; + i386_dr_low.unset_status = i386_linux_dr_unset_status; i386_set_debug_register_length (4); /* Override the default ptrace resume method. */ diff --git a/gdb/i386-nat.c b/gdb/i386-nat.c index eb16687ddb2..edb78bfadeb 100644 --- a/gdb/i386-nat.c +++ b/gdb/i386-nat.c @@ -137,8 +137,11 @@ struct i386_dr_low_type i386_dr_low; #define I386_DR_GET_RW_LEN(i) \ ((dr_control_mirror >> (DR_CONTROL_SHIFT + DR_CONTROL_SIZE * (i))) & 0x0f) +/* Mask that this I'th watchpoint has triggered. */ +#define I386_DR_WATCH_MASK(i) (1 << (i)) + /* Did the watchpoint whose address is in the I'th register break? */ -#define I386_DR_WATCH_HIT(i) (dr_status_mirror & (1 << (i))) +#define I386_DR_WATCH_HIT(i) (dr_status_mirror & I386_DR_WATCH_MASK (i)) /* A macro to loop over all debug registers. */ #define ALL_DEBUG_REGISTERS(i) for (i = 0; i < DR_NADDR; i++) @@ -358,6 +361,10 @@ i386_insert_aligned_watchpoint (CORE_ADDR addr, unsigned len_rw_bits) i386_dr_low.set_addr (i, addr); i386_dr_low.set_control (dr_control_mirror); + /* Only a sanity check for leftover bits (set possibly only by inferior). */ + if (i386_dr_low.unset_status) + i386_dr_low.unset_status (I386_DR_WATCH_MASK (i)); + return 0; } diff --git a/gdb/i386-nat.h b/gdb/i386-nat.h index f49b9f60bfd..892a8040197 100644 --- a/gdb/i386-nat.h +++ b/gdb/i386-nat.h @@ -49,16 +49,19 @@ extern void i386_use_watchpoints (struct target_ops *); functions are: set_control -- set the debug control (DR7) - register to a given value + register to a given value for all LWPs set_addr -- put an address into one debug - register + register for all LWPs reset_addr -- reset the address stored in - one debug register + one debug register for all LWPs get_status -- return the value of the debug - status (DR6) register. + status (DR6) register for current LWP + + unset_status -- unset the specified bits of the debug + status (DR6) register for all LWPs Additionally, the native file should set the debug_register_length field to 4 or 8 depending on the number of bytes used for @@ -70,6 +73,7 @@ struct i386_dr_low_type void (*set_addr) (int, CORE_ADDR); void (*reset_addr) (int); unsigned long (*get_status) (void); + void (*unset_status) (unsigned long); int debug_register_length; }; diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 83ae0522287..6bdf95aa409 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,7 +1,12 @@ 2009-11-20 Jan Kratochvil <jan.kratochvil@redhat.com> - * gdb.base/watchthreads-reorder.exp, - gdb.base/watchthreads-reorder.c: New. + * gdb.base/watchpoint-hw-hit-once.exp, + gdb.base/watchpoint-hw-hit-once.c: New. + +2009-11-20 Jan Kratochvil <jan.kratochvil@redhat.com> + + * gdb.threads/watchthreads-reorder.exp, + gdb.threads/watchthreads-reorder.c: New. 2009-11-17 Nathan Sidwell <nathan@codesourcery.com> diff --git a/gdb/testsuite/gdb.base/watchpoint-hw-hit-once.c b/gdb/testsuite/gdb.base/watchpoint-hw-hit-once.c new file mode 100644 index 00000000000..cccc9e01c57 --- /dev/null +++ b/gdb/testsuite/gdb.base/watchpoint-hw-hit-once.c @@ -0,0 +1,34 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2009 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 int watchee; + +int +main (void) +{ + volatile int dummy; + + /* Stub lines are present as no breakpoints/watchpoint gets hit if current PC + already stays on the line PC while entering "step"/"continue". */ + + dummy = 0; /* Stub to catch WATCHEE access after runto_main. */ + dummy = watchee; + dummy = 1; /* Stub to catch break-at-exit after WATCHEE has been hit. */ + dummy = 2; /* break-at-exit */ + + return 0; +} diff --git a/gdb/testsuite/gdb.base/watchpoint-hw-hit-once.exp b/gdb/testsuite/gdb.base/watchpoint-hw-hit-once.exp new file mode 100644 index 00000000000..bd3ad9f56c0 --- /dev/null +++ b/gdb/testsuite/gdb.base/watchpoint-hw-hit-once.exp @@ -0,0 +1,43 @@ +# Copyright 2009 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/>. + +# Arch not supporting hw watchpoints does not imply no_hardware_watchpoints set. +if {(![istarget "i?86-*-*"] && ![istarget "x86_64-*-*"] + && ![istarget "ia64-*-*"] && ![istarget "s390*-*-*"]) + || [target_info exists gdb,no_hardware_watchpoints]} then { + verbose "Skipping watchpoint-hw-hit-once test." + return +} + +set test watchpoint-hw-hit-once +set srcfile ${test}.c +if { [prepare_for_testing ${test}.exp ${test} ${srcfile}] } { + return -1 +} + +if ![runto_main] { + return -1 +} + +gdb_test "rwatch watchee" + +gdb_breakpoint [gdb_get_line_number "break-at-exit"] + +gdb_test "continue" "Continuing.\r\nHardware read watchpoint \[0-9\]+: watchee\r\n\r\nValue = 0\r\n.*" + +# Here should be no repeated notification of the read watchpoint. +gdb_test "continue" \ + "Continuing\\.\[ \r\n\]+Breakpoint \[0-9\]+, .*break-at-exit.*" \ + "continue to break-at-exit" |