From 1ced966e3458bf3db742913f4d0a55549824e298 Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Fri, 22 Jul 2011 16:58:32 +0000 Subject: [PATCH] 2011-07-22 Pedro Alves gdb/testsuite/ * gdb.arch/i386-dr3-watch.exp: Test that the i386 watchpoints backend doesn't leave used debug registers behind. gdb/gdbserver/ * i386-low.c (i386_insert_aligned_watchpoint): Don't pass the info to the inferior here. (i386_remove_aligned_watchpoint): Ditto. (i386_handle_nonaligned_watchpoint): Return immediate on fail to fit part of the watchpoint in the debug registers. (i386_update_inferior_debug_regs): New. (i386_low_insert_watchpoint): Work on a local mirror of the debug registers, and only update the inferior on success. (i386_low_remove_watchpoint): Ditto. gdb/ * i386-nat.c (I386_DR_VACANT, I386_DR_LOCAL_ENABLE) (I386_DR_GLOBAL_ENABLE, I386_DR_DISABLE, I386_DR_SET_RW_LEN) (I386_DR_GET_RW_LEN, I386_DR_WATCH_HIT): Add state parameter and adjust. (dr_mirror, dr_status_mirror, dr_control_mirror): Delete. (struct i386_debug_reg_state): New. (i386_init_dregs): New. (dr_mirror): New. (i386_cleanup_dregs): Use i386_init_dregs. (i386_show_dr): Add state parameter and adjust. (i386_insert_aligned_watchpoint): Ditto. Don't pass the info to the inferior here. (i386_remove_aligned_watchpoint): Likewise. (i386_handle_nonaligned_watchpoint): Add state parameter and adjust. (i386_update_inferior_debug_regs): New. (i386_insert_watchpoint): Work on a local mirror of the debug registers, and only update the inferior on success. (i386_remove_watchpoint): Ditto. (i386_region_ok_for_watchpoint): Adjust. (i386_stopped_data_address): Adjust. (i386_insert_hw_breakpoint): Adjust. (i386_remove_hw_breakpoint): Adjust. --- gdb/ChangeLog | 25 ++ gdb/gdbserver/ChangeLog | 12 + gdb/gdbserver/i386-low.c | 69 ++++-- gdb/i386-nat.c | 281 ++++++++++++++-------- gdb/testsuite/ChangeLog | 5 + gdb/testsuite/gdb.arch/i386-dr3-watch.exp | 44 ++++ 6 files changed, 313 insertions(+), 123 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 37214d35d30..e8b5d057c36 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,28 @@ +2011-07-22 Pedro Alves + + * i386-nat.c (I386_DR_VACANT, I386_DR_LOCAL_ENABLE) + (I386_DR_GLOBAL_ENABLE, I386_DR_DISABLE, I386_DR_SET_RW_LEN) + (I386_DR_GET_RW_LEN, I386_DR_WATCH_HIT): Add state parameter and + adjust. + (dr_mirror, dr_status_mirror, dr_control_mirror): Delete. + (struct i386_debug_reg_state): New. + (i386_init_dregs): New. + (dr_mirror): New. + (i386_cleanup_dregs): Use i386_init_dregs. + (i386_show_dr): Add state parameter and adjust. + (i386_insert_aligned_watchpoint): Ditto. Don't pass the info to + the inferior here. + (i386_remove_aligned_watchpoint): Likewise. + (i386_handle_nonaligned_watchpoint): Add state parameter and adjust. + (i386_update_inferior_debug_regs): New. + (i386_insert_watchpoint): Work on a local mirror of the debug + registers, and only update the inferior on success. + (i386_remove_watchpoint): Ditto. + (i386_region_ok_for_watchpoint): Adjust. + (i386_stopped_data_address): Adjust. + (i386_insert_hw_breakpoint): Adjust. + (i386_remove_hw_breakpoint): Adjust. + 2011-07-22 Tom Tromey * amd64-tdep.c (amd64_pseudo_register_read_value): Rename diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog index d0a914faaca..338f2c62bc2 100644 --- a/gdb/gdbserver/ChangeLog +++ b/gdb/gdbserver/ChangeLog @@ -1,3 +1,15 @@ +2011-07-22 Pedro Alves + + * i386-low.c (i386_insert_aligned_watchpoint): Don't pass the info + to the inferior here. + (i386_remove_aligned_watchpoint): Ditto. + (i386_handle_nonaligned_watchpoint): Return immediate on fail to + fit part of the watchpoint in the debug registers. + (i386_update_inferior_debug_regs): New. + (i386_low_insert_watchpoint): Work on a local mirror of the debug + registers, and only update the inferior on success. + (i386_low_remove_watchpoint): Ditto. + 2011-07-22 Kwok Cheung Yeung * linux-low.c (compare_ints, unique, list_threads, show_process, diff --git a/gdb/gdbserver/i386-low.c b/gdb/gdbserver/i386-low.c index 1baa23dd964..a9591794680 100644 --- a/gdb/gdbserver/i386-low.c +++ b/gdb/gdbserver/i386-low.c @@ -307,10 +307,6 @@ i386_insert_aligned_watchpoint (struct i386_debug_reg_state *state, state->dr_control_mirror |= DR_LOCAL_SLOWDOWN; state->dr_control_mirror &= I386_DR_CONTROL_MASK; - /* Finally, actually pass the info to the inferior. */ - i386_dr_low_set_addr (state, i); - i386_dr_low_set_control (state); - return 0; } @@ -337,9 +333,6 @@ i386_remove_aligned_watchpoint (struct i386_debug_reg_state *state, /* Reset our mirror. */ state->dr_mirror[i] = 0; I386_DR_DISABLE (state, i); - /* Reset it in the inferior. */ - i386_dr_low_set_control (state); - i386_dr_low_set_addr (state, i); } retval = 0; } @@ -360,7 +353,7 @@ i386_handle_nonaligned_watchpoint (struct i386_debug_reg_state *state, i386_wp_op_t what, CORE_ADDR addr, int len, enum target_hw_bp_type type) { - int retval = 0, status = 0; + int retval = 0; int max_wp_len = TARGET_HAS_DR_LEN_8 ? 8 : 4; static const int size_try_array[8][8] = @@ -398,25 +391,16 @@ i386_handle_nonaligned_watchpoint (struct i386_debug_reg_state *state, unsigned len_rw = i386_length_and_rw_bits (size, type); if (what == WP_INSERT) - status = i386_insert_aligned_watchpoint (state, addr, len_rw); + retval = i386_insert_aligned_watchpoint (state, addr, len_rw); else if (what == WP_REMOVE) - status = i386_remove_aligned_watchpoint (state, addr, len_rw); + retval = i386_remove_aligned_watchpoint (state, addr, len_rw); else fatal ("\ Invalid value %d of operation in i386_handle_nonaligned_watchpoint.\n", (int) what); - /* We keep the loop going even after a failure, because some - of the other aligned watchpoints might still succeed - (e.g. if they watch addresses that are already watched, - in which case we just increment the reference counts of - occupied debug registers). If we break out of the loop - too early, we could cause those addresses watched by - other watchpoints to be disabled when breakpoint.c reacts - to our failure to insert this watchpoint and tries to - remove it. */ - if (status) - retval = status; + if (retval) + break; } addr += size; @@ -448,6 +432,29 @@ Z_packet_to_hw_type (char type) } } +/* Update the inferior debug registers state, in INF_STATE, with the + new debug registers state, in NEW_STATE. */ + +static void +i386_update_inferior_debug_regs (struct i386_debug_reg_state *inf_state, + struct i386_debug_reg_state *new_state) +{ + int i; + + ALL_DEBUG_REGISTERS (i) + { + if (I386_DR_VACANT (new_state, i) != I386_DR_VACANT (inf_state, i)) + i386_dr_low_set_addr (new_state, i); + else + gdb_assert (new_state->dr_mirror[i] == inf_state->dr_mirror[i]); + } + + if (new_state->dr_control_mirror != inf_state->dr_control_mirror) + i386_dr_low_set_control (new_state); + + *inf_state = *new_state; +} + /* Insert a watchpoint to watch a memory region which starts at address ADDR and whose length is LEN bytes. Watch memory accesses of the type TYPE_FROM_PACKET. Return 0 on success, -1 on failure. */ @@ -458,6 +465,9 @@ i386_low_insert_watchpoint (struct i386_debug_reg_state *state, { int retval; enum target_hw_bp_type type = Z_packet_to_hw_type (type_from_packet); + /* Work on a local copy of the debug registers, and on success, + commit the change back to the inferior. */ + struct i386_debug_reg_state local_state = *state; if (type == hw_read) return 1; /* unsupported */ @@ -466,16 +476,19 @@ i386_low_insert_watchpoint (struct i386_debug_reg_state *state, && !(TARGET_HAS_DR_LEN_8 && len == 8)) || addr % len != 0) { - retval = i386_handle_nonaligned_watchpoint (state, WP_INSERT, + retval = i386_handle_nonaligned_watchpoint (&local_state, WP_INSERT, addr, len, type); } else { unsigned len_rw = i386_length_and_rw_bits (len, type); - retval = i386_insert_aligned_watchpoint (state, addr, len_rw); + retval = i386_insert_aligned_watchpoint (&local_state, addr, len_rw); } + if (retval == 0) + i386_update_inferior_debug_regs (state, &local_state); + if (debug_hw_points) i386_show_dr (state, "insert_watchpoint", addr, len, type); @@ -492,21 +505,27 @@ i386_low_remove_watchpoint (struct i386_debug_reg_state *state, { int retval; enum target_hw_bp_type type = Z_packet_to_hw_type (type_from_packet); + /* Work on a local copy of the debug registers, and on success, + commit the change back to the inferior. */ + struct i386_debug_reg_state local_state = *state; if (((len != 1 && len != 2 && len != 4) && !(TARGET_HAS_DR_LEN_8 && len == 8)) || addr % len != 0) { - retval = i386_handle_nonaligned_watchpoint (state, WP_REMOVE, + retval = i386_handle_nonaligned_watchpoint (&local_state, WP_REMOVE, addr, len, type); } else { unsigned len_rw = i386_length_and_rw_bits (len, type); - retval = i386_remove_aligned_watchpoint (state, addr, len_rw); + retval = i386_remove_aligned_watchpoint (&local_state, addr, len_rw); } + if (retval == 0) + i386_update_inferior_debug_regs (state, &local_state); + if (debug_hw_points) i386_show_dr (state, "remove_watchpoint", addr, len, type); diff --git a/gdb/i386-nat.c b/gdb/i386-nat.c index 7e6814ebad7..4c6c4e959f8 100644 --- a/gdb/i386-nat.c +++ b/gdb/i386-nat.c @@ -111,45 +111,88 @@ struct i386_dr_low_type i386_dr_low; /* The I'th debug register is vacant if its Local and Global Enable bits are reset in the Debug Control register. */ -#define I386_DR_VACANT(i) \ - ((dr_control_mirror & (3 << (DR_ENABLE_SIZE * (i)))) == 0) +#define I386_DR_VACANT(state, i) \ + (((state)->dr_control_mirror & (3 << (DR_ENABLE_SIZE * (i)))) == 0) /* Locally enable the break/watchpoint in the I'th debug register. */ -#define I386_DR_LOCAL_ENABLE(i) \ - dr_control_mirror |= (1 << (DR_LOCAL_ENABLE_SHIFT + DR_ENABLE_SIZE * (i))) +#define I386_DR_LOCAL_ENABLE(state, i) \ + do { \ + (state)->dr_control_mirror |= \ + (1 << (DR_LOCAL_ENABLE_SHIFT + DR_ENABLE_SIZE * (i))); \ + } while (0) /* Globally enable the break/watchpoint in the I'th debug register. */ -#define I386_DR_GLOBAL_ENABLE(i) \ - dr_control_mirror |= (1 << (DR_GLOBAL_ENABLE_SHIFT + DR_ENABLE_SIZE * (i))) +#define I386_DR_GLOBAL_ENABLE(state, i) \ + do { \ + (state)->dr_control_mirror |= \ + (1 << (DR_GLOBAL_ENABLE_SHIFT + DR_ENABLE_SIZE * (i))); \ + } while (0) /* Disable the break/watchpoint in the I'th debug register. */ -#define I386_DR_DISABLE(i) \ - dr_control_mirror &= ~(3 << (DR_ENABLE_SIZE * (i))) +#define I386_DR_DISABLE(state, i) \ + do { \ + (state)->dr_control_mirror &= \ + ~(3 << (DR_ENABLE_SIZE * (i))); \ + } while (0) /* Set in DR7 the RW and LEN fields for the I'th debug register. */ -#define I386_DR_SET_RW_LEN(i,rwlen) \ +#define I386_DR_SET_RW_LEN(state, i, rwlen) \ do { \ - dr_control_mirror &= ~(0x0f << (DR_CONTROL_SHIFT+DR_CONTROL_SIZE*(i))); \ - dr_control_mirror |= ((rwlen) << (DR_CONTROL_SHIFT+DR_CONTROL_SIZE*(i))); \ + (state)->dr_control_mirror &= \ + ~(0x0f << (DR_CONTROL_SHIFT + DR_CONTROL_SIZE * (i))); \ + (state)->dr_control_mirror |= \ + ((rwlen) << (DR_CONTROL_SHIFT + DR_CONTROL_SIZE * (i))); \ } while (0) /* Get from DR7 the RW and LEN fields for the I'th debug register. */ -#define I386_DR_GET_RW_LEN(i) \ - ((dr_control_mirror >> (DR_CONTROL_SHIFT + DR_CONTROL_SIZE * (i))) & 0x0f) +#define I386_DR_GET_RW_LEN(dr7, i) \ + (((dr7) \ + >> (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 & I386_DR_WATCH_MASK (i)) +#define I386_DR_WATCH_HIT(dr6, i) ((dr6) & (1 << (i))) /* A macro to loop over all debug registers. */ #define ALL_DEBUG_REGISTERS(i) for (i = 0; i < DR_NADDR; i++) -/* Mirror the inferior's DRi registers. We keep the status and - control registers separated because they don't hold addresses. */ -static CORE_ADDR dr_mirror[DR_NADDR]; -static unsigned long dr_status_mirror, dr_control_mirror; + +/* Global state needed to track h/w watchpoints. */ + +struct i386_debug_reg_state +{ + /* Mirror the inferior's DRi registers. We keep the status and + control registers separated because they don't hold addresses. + Note that since we can change these mirrors while threads are + running, we never trust them to explain a cause of a trap. + For that, we need to peek directly in the inferior registers. */ + CORE_ADDR dr_mirror[DR_NADDR]; + unsigned dr_status_mirror, dr_control_mirror; + + /* Reference counts for each debug register. */ + int dr_ref_count[DR_NADDR]; +}; + +/* Clear the reference counts and forget everything we knew about the + debug registers. */ + +static void +i386_init_dregs (struct i386_debug_reg_state *state) +{ + int i; + + ALL_DEBUG_REGISTERS (i) + { + state->dr_mirror[i] = 0; + state->dr_ref_count[i] = 0; + } + state->dr_control_mirror = 0; + state->dr_status_mirror = 0; +} + +static struct i386_debug_reg_state dr_mirror; /* Reference counts for each debug register. */ static int dr_ref_count[DR_NADDR]; @@ -172,7 +215,8 @@ static unsigned i386_length_and_rw_bits (int len, enum target_hw_bp_type type); value of the bit-field from DR7 which describes the length and access type of the region to be watched by this watchpoint. Return 0 on success, -1 on failure. */ -static int i386_insert_aligned_watchpoint (CORE_ADDR addr, +static int i386_insert_aligned_watchpoint (struct i386_debug_reg_state *state, + CORE_ADDR addr, unsigned len_rw_bits); /* Remove a watchpoint at address ADDR, which is assumed to be aligned @@ -180,7 +224,8 @@ static int i386_insert_aligned_watchpoint (CORE_ADDR addr, value of the bits from DR7 which describes the length and access type of the region watched by this watchpoint. Return 0 on success, -1 on failure. */ -static int i386_remove_aligned_watchpoint (CORE_ADDR addr, +static int i386_remove_aligned_watchpoint (struct i386_debug_reg_state *state, + CORE_ADDR addr, unsigned len_rw_bits); /* Insert or remove a (possibly non-aligned) watchpoint, or count the @@ -189,7 +234,8 @@ static int i386_remove_aligned_watchpoint (CORE_ADDR addr, successful insertion or removal, a positive number when queried about the number of registers, or -1 on failure. If WHAT is not a valid value, bombs through internal_error. */ -static int i386_handle_nonaligned_watchpoint (i386_wp_op_t what, +static int i386_handle_nonaligned_watchpoint (struct i386_debug_reg_state *state, + i386_wp_op_t what, CORE_ADDR addr, int len, enum target_hw_bp_type type); @@ -201,15 +247,7 @@ static int i386_handle_nonaligned_watchpoint (i386_wp_op_t what, void i386_cleanup_dregs (void) { - int i; - - ALL_DEBUG_REGISTERS(i) - { - dr_mirror[i] = 0; - dr_ref_count[i] = 0; - } - dr_control_mirror = 0; - dr_status_mirror = 0; + i386_init_dregs (&dr_mirror); } /* Print the values of the mirrored debug registers. This is called @@ -217,7 +255,8 @@ i386_cleanup_dregs (void) show-debug-regs" at GDB's prompt. */ static void -i386_show_dr (const char *func, CORE_ADDR addr, +i386_show_dr (struct i386_debug_reg_state *state, + const char *func, CORE_ADDR addr, int len, enum target_hw_bp_type type) { int addr_size = gdbarch_addr_bit (target_gdbarch) / 8; @@ -239,13 +278,16 @@ i386_show_dr (const char *func, CORE_ADDR addr, : "??unknown??")))); puts_unfiltered (":\n"); printf_unfiltered ("\tCONTROL (DR7): %s STATUS (DR6): %s\n", - phex (dr_control_mirror, 8), phex (dr_status_mirror, 8)); + phex (state->dr_control_mirror, 8), + phex (state->dr_status_mirror, 8)); ALL_DEBUG_REGISTERS(i) { printf_unfiltered ("\ \tDR%d: addr=0x%s, ref.count=%d DR%d: addr=0x%s, ref.count=%d\n", - i, phex (dr_mirror[i], addr_size), dr_ref_count[i], - i+1, phex (dr_mirror[i+1], addr_size), dr_ref_count[i+1]); + i, phex (state->dr_mirror[i], addr_size), + state->dr_ref_count[i], + i + 1, phex (state->dr_mirror[i + 1], addr_size), + state->dr_ref_count[i+1]); i++; } } @@ -311,7 +353,8 @@ Invalid hardware breakpoint length %d in i386_length_and_rw_bits.\n"), len); success, -1 on failure. */ static int -i386_insert_aligned_watchpoint (CORE_ADDR addr, unsigned len_rw_bits) +i386_insert_aligned_watchpoint (struct i386_debug_reg_state *state, + CORE_ADDR addr, unsigned len_rw_bits) { int i; @@ -323,11 +366,11 @@ i386_insert_aligned_watchpoint (CORE_ADDR addr, unsigned len_rw_bits) reuse it for this watchpoint as well (and save a register). */ ALL_DEBUG_REGISTERS(i) { - if (!I386_DR_VACANT (i) - && dr_mirror[i] == addr - && I386_DR_GET_RW_LEN (i) == len_rw_bits) + if (!I386_DR_VACANT (state, i) + && state->dr_mirror[i] == addr + && I386_DR_GET_RW_LEN (state->dr_control_mirror, i) == len_rw_bits) { - dr_ref_count[i]++; + state->dr_ref_count[i]++; return 0; } } @@ -335,7 +378,7 @@ i386_insert_aligned_watchpoint (CORE_ADDR addr, unsigned len_rw_bits) /* Next, look for a vacant debug register. */ ALL_DEBUG_REGISTERS(i) { - if (I386_DR_VACANT (i)) + if (I386_DR_VACANT (state, i)) break; } @@ -346,9 +389,9 @@ i386_insert_aligned_watchpoint (CORE_ADDR addr, unsigned len_rw_bits) /* Now set up the register I to watch our region. */ /* Record the info in our local mirrored array. */ - dr_mirror[i] = addr; - dr_ref_count[i] = 1; - I386_DR_SET_RW_LEN (i, len_rw_bits); + state->dr_mirror[i] = addr; + state->dr_ref_count[i] = 1; + I386_DR_SET_RW_LEN (state, i, len_rw_bits); /* Note: we only enable the watchpoint locally, i.e. in the current task. Currently, no i386 target allows or supports global watchpoints; however, if any target would want that in the @@ -356,17 +399,9 @@ i386_insert_aligned_watchpoint (CORE_ADDR addr, unsigned len_rw_bits) to enable watchpoints globally or locally, and the code below should use global or local enable and slow-down flags as appropriate. */ - I386_DR_LOCAL_ENABLE (i); - dr_control_mirror |= DR_LOCAL_SLOWDOWN; - dr_control_mirror &= I386_DR_CONTROL_MASK; - - /* Finally, actually pass the info to the inferior. */ - 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)); + I386_DR_LOCAL_ENABLE (state, i); + state->dr_control_mirror |= DR_LOCAL_SLOWDOWN; + state->dr_control_mirror &= I386_DR_CONTROL_MASK; return 0; } @@ -378,25 +413,22 @@ i386_insert_aligned_watchpoint (CORE_ADDR addr, unsigned len_rw_bits) success, -1 on failure. */ static int -i386_remove_aligned_watchpoint (CORE_ADDR addr, unsigned len_rw_bits) +i386_remove_aligned_watchpoint (struct i386_debug_reg_state *state, + CORE_ADDR addr, unsigned len_rw_bits) { int i, retval = -1; ALL_DEBUG_REGISTERS(i) { - if (!I386_DR_VACANT (i) - && dr_mirror[i] == addr - && I386_DR_GET_RW_LEN (i) == len_rw_bits) + if (!I386_DR_VACANT (state, i) + && state->dr_mirror[i] == addr + && I386_DR_GET_RW_LEN (state->dr_control_mirror, i) == len_rw_bits) { - if (--dr_ref_count[i] == 0) /* no longer in use? */ + if (--state->dr_ref_count[i] == 0) /* no longer in use? */ { /* Reset our mirror. */ - dr_mirror[i] = 0; - I386_DR_DISABLE (i); - /* Reset it in the inferior. */ - i386_dr_low.set_control (dr_control_mirror); - if (i386_dr_low.reset_addr) - i386_dr_low.reset_addr (i); + state->dr_mirror[i] = 0; + I386_DR_DISABLE (state, i); } retval = 0; } @@ -413,10 +445,11 @@ i386_remove_aligned_watchpoint (CORE_ADDR addr, unsigned len_rw_bits) valid value, bombs through internal_error. */ static int -i386_handle_nonaligned_watchpoint (i386_wp_op_t what, CORE_ADDR addr, int len, +i386_handle_nonaligned_watchpoint (struct i386_debug_reg_state *state, + i386_wp_op_t what, CORE_ADDR addr, int len, enum target_hw_bp_type type) { - int retval = 0, status = 0; + int retval = 0; int max_wp_len = TARGET_HAS_DR_LEN_8 ? 8 : 4; static int size_try_array[8][8] = @@ -454,24 +487,15 @@ i386_handle_nonaligned_watchpoint (i386_wp_op_t what, CORE_ADDR addr, int len, unsigned len_rw = i386_length_and_rw_bits (size, type); if (what == WP_INSERT) - status = i386_insert_aligned_watchpoint (addr, len_rw); + retval = i386_insert_aligned_watchpoint (state, addr, len_rw); else if (what == WP_REMOVE) - status = i386_remove_aligned_watchpoint (addr, len_rw); + retval = i386_remove_aligned_watchpoint (state, addr, len_rw); else internal_error (__FILE__, __LINE__, _("\ Invalid value %d of operation in i386_handle_nonaligned_watchpoint.\n"), (int)what); - /* We keep the loop going even after a failure, because some - of the other aligned watchpoints might still succeed - (e.g. if they watch addresses that are already watched, - in which case we just increment the reference counts of - occupied debug registers). If we break out of the loop - too early, we could cause those addresses watched by - other watchpoints to be disabled when breakpoint.c reacts - to our failure to insert this watchpoint and tries to - remove it. */ - if (status) - retval = status; + if (retval) + break; } addr += size; @@ -481,6 +505,43 @@ Invalid value %d of operation in i386_handle_nonaligned_watchpoint.\n"), return retval; } +/* Update the inferior's debug registers with the new debug registers + state, in NEW_STATE, and then update our local mirror to match. */ + +static void +i386_update_inferior_debug_regs (struct i386_debug_reg_state *new_state) +{ + int i; + + ALL_DEBUG_REGISTERS (i) + { + if (I386_DR_VACANT (new_state, i) != I386_DR_VACANT (&dr_mirror, i)) + { + if (!I386_DR_VACANT (new_state, i)) + { + i386_dr_low.set_addr (i, new_state->dr_mirror[i]); + + /* 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)); + } + else + { + if (i386_dr_low.reset_addr) + i386_dr_low.reset_addr (i); + } + } + else + gdb_assert (new_state->dr_mirror[i] == dr_mirror.dr_mirror[i]); + } + + if (new_state->dr_control_mirror != dr_mirror.dr_control_mirror) + i386_dr_low.set_control (new_state->dr_control_mirror); + + dr_mirror = *new_state; +} + /* Insert a watchpoint to watch a memory region which starts at address ADDR and whose length is LEN bytes. Watch memory accesses of the type TYPE. Return 0 on success, -1 on failure. */ @@ -490,22 +551,30 @@ i386_insert_watchpoint (CORE_ADDR addr, int len, int type, struct expression *cond) { int retval; + /* Work on a local copy of the debug registers, and on success, + commit the change back to the inferior. */ + struct i386_debug_reg_state local_state = dr_mirror; if (type == hw_read) return 1; /* unsupported */ if (((len != 1 && len !=2 && len !=4) && !(TARGET_HAS_DR_LEN_8 && len == 8)) || addr % len != 0) - retval = i386_handle_nonaligned_watchpoint (WP_INSERT, addr, len, type); + retval = i386_handle_nonaligned_watchpoint (&local_state, + WP_INSERT, addr, len, type); else { unsigned len_rw = i386_length_and_rw_bits (len, type); - retval = i386_insert_aligned_watchpoint (addr, len_rw); + retval = i386_insert_aligned_watchpoint (&local_state, + addr, len_rw); } + if (retval == 0) + i386_update_inferior_debug_regs (&local_state); + if (maint_show_dr) - i386_show_dr ("insert_watchpoint", addr, len, type); + i386_show_dr (&dr_mirror, "insert_watchpoint", addr, len, type); return retval; } @@ -518,19 +587,27 @@ i386_remove_watchpoint (CORE_ADDR addr, int len, int type, struct expression *cond) { int retval; + /* Work on a local copy of the debug registers, and on success, + commit the change back to the inferior. */ + struct i386_debug_reg_state local_state = dr_mirror; if (((len != 1 && len !=2 && len !=4) && !(TARGET_HAS_DR_LEN_8 && len == 8)) || addr % len != 0) - retval = i386_handle_nonaligned_watchpoint (WP_REMOVE, addr, len, type); + retval = i386_handle_nonaligned_watchpoint (&local_state, + WP_REMOVE, addr, len, type); else { unsigned len_rw = i386_length_and_rw_bits (len, type); - retval = i386_remove_aligned_watchpoint (addr, len_rw); + retval = i386_remove_aligned_watchpoint (&local_state, + addr, len_rw); } + if (retval == 0) + i386_update_inferior_debug_regs (&local_state); + if (maint_show_dr) - i386_show_dr ("remove_watchpoint", addr, len, type); + i386_show_dr (&dr_mirror, "remove_watchpoint", addr, len, type); return retval; } @@ -545,7 +622,8 @@ i386_region_ok_for_watchpoint (CORE_ADDR addr, int len) /* Compute how many aligned watchpoints we would need to cover this region. */ - nregs = i386_handle_nonaligned_watchpoint (WP_COUNT, addr, len, hw_write); + nregs = i386_handle_nonaligned_watchpoint (&dr_mirror, + WP_COUNT, addr, len, hw_write); return nregs <= DR_NADDR ? 1 : 0; } @@ -559,30 +637,35 @@ i386_stopped_data_address (struct target_ops *ops, CORE_ADDR *addr_p) CORE_ADDR addr = 0; int i; int rc = 0; + unsigned status; + unsigned control; + struct i386_debug_reg_state *state = &dr_mirror; - dr_status_mirror = i386_dr_low.get_status (); + dr_mirror.dr_status_mirror = i386_dr_low.get_status (); + status = dr_mirror.dr_status_mirror; + control = dr_mirror.dr_control_mirror; ALL_DEBUG_REGISTERS(i) { - if (I386_DR_WATCH_HIT (i) + if (I386_DR_WATCH_HIT (status, i) /* This second condition makes sure DRi is set up for a data watchpoint, not a hardware breakpoint. The reason is that GDB doesn't call the target_stopped_data_address method except for data watchpoints. In other words, I'm being paranoiac. */ - && I386_DR_GET_RW_LEN (i) != 0 + && I386_DR_GET_RW_LEN (control, i) != 0 /* This third condition makes sure DRi is not vacant, this avoids false positives in windows-nat.c. */ - && !I386_DR_VACANT (i)) + && !I386_DR_VACANT (state, i)) { - addr = dr_mirror[i]; + addr = state->dr_mirror[i]; rc = 1; if (maint_show_dr) - i386_show_dr ("watchpoint_hit", addr, -1, hw_write); + i386_show_dr (&dr_mirror, "watchpoint_hit", addr, -1, hw_write); } } if (maint_show_dr && addr == 0) - i386_show_dr ("stopped_data_addr", 0, 0, hw_write); + i386_show_dr (&dr_mirror, "stopped_data_addr", 0, 0, hw_write); if (rc) *addr_p = addr; @@ -604,10 +687,11 @@ i386_insert_hw_breakpoint (struct gdbarch *gdbarch, { unsigned len_rw = i386_length_and_rw_bits (1, hw_execute); CORE_ADDR addr = bp_tgt->placed_address; - int retval = i386_insert_aligned_watchpoint (addr, len_rw) ? EBUSY : 0; + int retval = i386_insert_aligned_watchpoint (&dr_mirror, + addr, len_rw) ? EBUSY : 0; if (maint_show_dr) - i386_show_dr ("insert_hwbp", addr, 1, hw_execute); + i386_show_dr (&dr_mirror, "insert_hwbp", addr, 1, hw_execute); return retval; } @@ -621,10 +705,11 @@ i386_remove_hw_breakpoint (struct gdbarch *gdbarch, { unsigned len_rw = i386_length_and_rw_bits (1, hw_execute); CORE_ADDR addr = bp_tgt->placed_address; - int retval = i386_remove_aligned_watchpoint (addr, len_rw); + int retval = i386_remove_aligned_watchpoint (&dr_mirror, + addr, len_rw); if (maint_show_dr) - i386_show_dr ("remove_hwbp", addr, 1, hw_execute); + i386_show_dr (&dr_mirror, "remove_hwbp", addr, 1, hw_execute); return retval; } diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index a907aa160c4..9a909522148 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2011-07-22 Pedro Alves + + * gdb.arch/i386-dr3-watch.exp: Test that the i386 watchpoints + backend doesn't leave used debug registers behind. + 2011-07-22 Tom Tromey * gdb.dwarf2/typeddwarf.c: XFAIL 'z' on x86-64. diff --git a/gdb/testsuite/gdb.arch/i386-dr3-watch.exp b/gdb/testsuite/gdb.arch/i386-dr3-watch.exp index f560b013fe6..35d758706d3 100644 --- a/gdb/testsuite/gdb.arch/i386-dr3-watch.exp +++ b/gdb/testsuite/gdb.arch/i386-dr3-watch.exp @@ -38,6 +38,8 @@ if ![runto_main] then { gdb_test_no_output "set breakpoint always-inserted on" +# Test that we handle watchpoints in all of DR0-DR3. + gdb_test "watch i1" "Hardware watchpoint .*: i1" gdb_test "watch i2" "Hardware watchpoint .*: i2" gdb_test "watch i3" "Hardware watchpoint .*: i3" @@ -47,3 +49,45 @@ gdb_test "c" "Hardware watchpoint.*: i1.*" "continue to i1 watchpoint" gdb_test "c" "Hardware watchpoint.*: i2.*" "continue to i2 watchpoint" gdb_test "c" "Hardware watchpoint.*: i3.*" "continue to i3 watchpoint" gdb_test "c" "Hardware watchpoint.*: i4.*" "continue to i4 watchpoint" + +delete_breakpoints + +# Regression test for a bug where the i386 watchpoints support backend +# would leave some debug registers occupied even if not enough debug +# registers were available to cover a single watchpoint location. + +gdb_test "watch i1" \ + "Hardware watchpoint .*: i1" \ + "set watchpoint occuping one debug register" + +# gap1 too long to fit the 3 left over debug registers (but would fit +# 4 if all were available). +set test "watchpoint on gap1 does not fit debug registers" +gdb_test_multiple "watch gap1" "$test" { + -re "Hardware watchpoint .*: gap1.*Warning:.*Could not insert hardware watchpoint.*You may have requested too many.*" { + pass $test + } + -re "Hardware watchpoint .*: gap1\r\n$gdb_prompt $" { + pass "$test (target emulates hardware watchpoints)" + return + } + -re "Watchpoint .*: gap1\r\n$gdb_prompt $" { + pass "$test (gdb figured out itself the watchpoint does not fit)" + return + } +} + +# Start over. +gdb_test "delete" \ + "" \ + "delete all watchpoints" \ + "Delete all breakpoints.*$" \ + "y" + +# If debug registers were left occupied by mistake, we'll fail to set +# some of these watchpoints. Each watchpoint should fit in one of the +# 4 debug registers available. +gdb_test "watch i1" "Hardware watchpoint .*: i1" "watch i1 still fits" +gdb_test "watch i2" "Hardware watchpoint .*: i2" "watch i2 still fits" +gdb_test "watch i3" "Hardware watchpoint .*: i3" "watch i3 still fits" +gdb_test "watch i4" "Hardware watchpoint .*: i4" "watch i4 still fits" -- 2.30.2