This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Make watchpoints behave in always-inserted mode too.
- From: Pedro Alves <pedro at codesourcery dot com>
- To: gdb-patches at sourceware dot org
- Date: Fri, 20 Nov 2009 02:42:09 +0000
- Subject: Make watchpoints behave in always-inserted mode too.
This patch makes watchpoints work in always-inserted mode too,
so that watchpoints in non-stop mode aren't missed.
The main ideas here are:
- Teaching update_global_location_list to detect duplicate
watchpoint locations. A watchpoint location is duplicate
of another only if its type (watchpoint, access, read),
address and length fields match.
- Get rid of the explicit remove/insert breakpoints
sequence done by bpstat_stop_status. With the duplicate
watchpoint locations infrastructure in place, we can just
use update_watchpoints and update_global_location_list
directly.
- Have watch_command_1 not create an initial location
for the watchpoint. This (useless) location was always
created with address 0. update_watchpoint is responsible for
recreating the real watchpoint locations, but, in
always inserted mode, it could happen that we'd manage
to try to insert a watchpoint at 0 in the target, before
any update_watchpoint call was reached...
- Addressing the get_frame_program_space issue Daniel
pointed out last week, by avoiding creating locations
if the target hasn't execution yet. They're not needed
otherwise. This isn't an ideal fix, but has no
downsides, and gets the ball rolling. I'll need to take
a stab at making watchpoints actualy work properly with
multi inferiors on linux before coming up with a
better fix. This needed addressing, due to the new
call to update_watchpoints at the end of watch_command_1,
so that we can continue creating watchpoints before the
target has execution.
Comments?
This applies on top of the always-insert regression fix at
<http://sourceware.org/ml/gdb-patches/2009-11/msg00428.html>
No regressions on x86_64-linux native/gdbserver.
--
Pedro Alves
2009-11-20 Pedro Alves <pedro@codesourcery.com>
* breakpoint.c (update_watchpoint): Skip creating locations and
reading the selected frame if there's no execution.
(bpstat_stop_status): Use is_hardware_watchpoint. If not
stopping, update watchpoints and the global location list, instead
of removing and inserting all breakpoints.
(breakpoint_address_is_meaningful): Hardware watchpoints also have
a meaningful target address.
(watchpoint_locations_match): New.
(breakpoint_locations_match): New.
(watch_command_1): Create the watchpoint breakpoint without any
location initially. Use update_watchpoint to create the
watchpoint locations.
(update_global_location_list): Use breakpoint_locations_match, so
watchpoint locations are handled too. Also detect duplicate
watchpoint locations.
---
gdb/breakpoint.c | 133 +++++++++++++++++++++++++++++++++++--------------------
1 file changed, 86 insertions(+), 47 deletions(-)
Index: src/gdb/breakpoint.c
===================================================================
--- src.orig/gdb/breakpoint.c 2009-11-20 01:02:18.000000000 +0000
+++ src/gdb/breakpoint.c 2009-11-20 01:45:30.000000000 +0000
@@ -1019,7 +1019,6 @@ update_watchpoint (struct breakpoint *b,
struct bp_location *loc;
int frame_saved;
bpstat bs;
- struct program_space *frame_pspace;
/* If this is a local watchpoint, we only want to check if the
watchpoint frame is in scope if the current thread is the thread
@@ -1058,8 +1057,6 @@ update_watchpoint (struct breakpoint *b,
select_frame (fi);
}
- frame_pspace = get_frame_program_space (get_selected_frame (NULL));
-
if (within_current_scope && reparse)
{
char *s;
@@ -1084,9 +1081,16 @@ update_watchpoint (struct breakpoint *b,
don't try to insert watchpoint. We don't automatically delete
such watchpoint, though, since failure to parse expression
is different from out-of-scope watchpoint. */
- if (within_current_scope && b->exp)
+ if ( !target_has_execution)
+ {
+ /* Without execution, memory can't change. No use to try and
+ set watchpoint locations. The watchpoint will be reset when
+ the target gains execution, through breakpoint_re_set. */
+ }
+ else if (within_current_scope && b->exp)
{
struct value *val_chain, *v, *result, *next;
+ struct program_space *frame_pspace;
fetch_watchpoint_value (b->exp, &v, &result, &val_chain);
@@ -1125,6 +1129,8 @@ update_watchpoint (struct breakpoint *b,
}
}
+ frame_pspace = get_frame_program_space (get_selected_frame (NULL));
+
/* Look at each value on the value chain. */
for (v = val_chain; v; v = next)
{
@@ -3575,22 +3581,17 @@ bpstat_stop_status (struct address_space
for (bs = root_bs->next; bs != NULL; bs = bs->next)
if (!bs->stop
&& bs->breakpoint_at->owner
- && (bs->breakpoint_at->owner->type == bp_hardware_watchpoint
- || bs->breakpoint_at->owner->type == bp_read_watchpoint
- || bs->breakpoint_at->owner->type == bp_access_watchpoint))
- {
- /* remove/insert can invalidate bs->breakpoint_at, if this
- location is no longer used by the watchpoint. Prevent
- further code from trying to use it. */
+ && is_hardware_watchpoint (bs->breakpoint_at->owner))
+ {
+ update_watchpoint (bs->breakpoint_at->owner, 0 /* don't reparse. */);
+ /* Updating watchpoints invalidates bs->breakpoint_at.
+ Prevent further code from trying to use it. */
bs->breakpoint_at = NULL;
need_remove_insert = 1;
}
if (need_remove_insert)
- {
- remove_breakpoints ();
- insert_breakpoints ();
- }
+ update_global_location_list (1);
return root_bs->next;
}
@@ -4570,21 +4571,28 @@ set_default_breakpoint (int valid, struc
these types to be a duplicate of an actual breakpoint at address zero:
bp_watchpoint
- bp_hardware_watchpoint
- bp_read_watchpoint
- bp_access_watchpoint
- bp_catchpoint */
+ bp_catchpoint
+
+*/
static int
breakpoint_address_is_meaningful (struct breakpoint *bpt)
{
enum bptype type = bpt->type;
- return (type != bp_watchpoint
- && type != bp_hardware_watchpoint
- && type != bp_read_watchpoint
- && type != bp_access_watchpoint
- && type != bp_catchpoint);
+ return (type != bp_watchpoint && type != bp_catchpoint);
+}
+
+/* Assuming LOC1 and LOC2's owners are hardware watchpoints, returns
+ true if LOC1 and LOC2 represent the same watchpoint location. */
+
+static int
+watchpoint_locations_match (struct bp_location *loc1, struct bp_location *loc2)
+{
+ return (loc1->owner->type == loc2->owner->type
+ && loc1->pspace->aspace == loc2->pspace->aspace
+ && loc1->address == loc2->address
+ && loc1->length == loc2->length);
}
/* Returns true if {ASPACE1,ADDR1} and {ASPACE2,ADDR2} represent the
@@ -4601,6 +4609,22 @@ breakpoint_address_match (struct address
&& addr1 == addr2);
}
+
+static int
+breakpoint_locations_match (struct bp_location *loc1, struct bp_location *loc2)
+{
+ int hw_point1 = is_hardware_watchpoint (loc1->owner);
+ int hw_point2 = is_hardware_watchpoint (loc2->owner);
+
+ if (hw_point1 != hw_point2)
+ return 0;
+ else if (hw_point1)
+ return watchpoint_locations_match (loc1, loc2);
+ else
+ return breakpoint_address_match (loc1->pspace->aspace, loc1->address,
+ loc2->pspace->aspace, loc2->address);
+}
+
static void
breakpoint_adjustment_warning (CORE_ADDR from_addr, CORE_ADDR to_addr,
int bnum, int have_bnum)
@@ -6982,7 +7006,6 @@ watch_command_1 (char *arg, int accessfl
{
struct gdbarch *gdbarch = get_current_arch ();
struct breakpoint *b, *scope_breakpoint = NULL;
- struct symtab_and_line sal;
struct expression *exp;
struct block *exp_valid_block;
struct value *val, *mark;
@@ -6993,14 +7016,11 @@ watch_command_1 (char *arg, int accessfl
int toklen;
char *cond_start = NULL;
char *cond_end = NULL;
- struct expression *cond = NULL;
int i, other_type_used, target_resources_ok = 0;
enum bptype bp_type;
int mem_cnt = 0;
int thread = -1;
- init_sal (&sal); /* initialize to zeroes */
-
/* Make sure that we actually have parameters to parse. */
if (arg != NULL && arg[0] != '\0')
{
@@ -7062,8 +7082,6 @@ watch_command_1 (char *arg, int accessfl
}
}
- sal.pspace = current_program_space;
-
/* Parse the rest of the arguments. */
innermost_block = NULL;
exp_start = arg;
@@ -7092,8 +7110,11 @@ watch_command_1 (char *arg, int accessfl
toklen = end_tok - tok;
if (toklen >= 1 && strncmp (tok, "if", toklen) == 0)
{
+ struct expression *cond;
+
tok = cond_start = end_tok + 1;
cond = parse_exp_1 (&tok, 0, 0);
+ xfree (cond);
cond_end = tok;
}
if (*tok)
@@ -7163,7 +7184,7 @@ watch_command_1 (char *arg, int accessfl
}
/* Now set up the breakpoint. */
- b = set_raw_breakpoint (gdbarch, sal, bp_type);
+ b = set_raw_breakpoint_without_location (NULL, bp_type);
set_breakpoint_count (breakpoint_count + 1);
b->number = breakpoint_count;
b->thread = thread;
@@ -7173,7 +7194,6 @@ watch_command_1 (char *arg, int accessfl
b->exp_string = savestring (exp_start, exp_end - exp_start);
b->val = val;
b->val_valid = 1;
- b->loc->cond = cond;
if (cond_start)
b->cond_string = savestring (cond_start, cond_end - cond_start);
else
@@ -7199,6 +7219,11 @@ watch_command_1 (char *arg, int accessfl
}
value_free_to_mark (mark);
+
+ /* Finally update the new watchpoint. This creates the locations
+ that should be inserted. */
+ update_watchpoint (b, 1);
+
mention (b);
update_global_location_list (1);
}
@@ -8208,7 +8233,10 @@ update_global_location_list (int should_
/* The first bp_location being the only one non-DUPLICATE for the current run
of the same ADDRESS. */
- struct bp_location *loc_first;
+ struct bp_location *bp_loc_first;
+ struct bp_location *wp_loc_first;
+ struct bp_location *awp_loc_first;
+ struct bp_location *rwp_loc_first;
/* Saved former bp_location array which we compare against the newly built
bp_location from the current state of ALL_BREAKPOINTS. */
@@ -8307,10 +8335,7 @@ update_global_location_list (int should_
{
struct bp_location *loc2 = *loc2p;
- if (breakpoint_address_match (loc2->pspace->aspace,
- loc2->address,
- old_loc->pspace->aspace,
- old_loc->address))
+ if (breakpoint_locations_match (loc2, old_loc))
{
/* For the sake of should_be_inserted.
Duplicates check below will fix up this later. */
@@ -8377,12 +8402,19 @@ update_global_location_list (int should_
This is so that the bpt instruction is only inserted once.
If we have a permanent breakpoint at the same place as BPT, make
that one the official one, and the rest as duplicates. Permanent
- breakpoints are sorted first for the same address. */
+ breakpoints are sorted first for the same address.
+
+ Do the same for hardware watchpoints, but also considering the
+ watchpoint type (regular/access/read) and length. */
- loc_first = NULL;
+ bp_loc_first = NULL;
+ wp_loc_first = NULL;
+ awp_loc_first = NULL;
+ rwp_loc_first = NULL;
ALL_BP_LOCATIONS (loc, locp)
{
struct breakpoint *b = loc->owner;
+ struct bp_location **loc_first;
if (b->enable_state == bp_disabled
|| b->enable_state == bp_call_disabled
@@ -8398,21 +8430,28 @@ update_global_location_list (int should_
_("allegedly permanent breakpoint is not "
"actually inserted"));
- if (loc_first == NULL
- || (overlay_debugging && loc->section != loc_first->section)
- || !breakpoint_address_match (loc->pspace->aspace, loc->address,
- loc_first->pspace->aspace,
- loc_first->address))
+ if (b->type == bp_hardware_watchpoint)
+ loc_first = &wp_loc_first;
+ else if (b->type == bp_read_watchpoint)
+ loc_first = &rwp_loc_first;
+ else if (b->type == bp_access_watchpoint)
+ loc_first = &awp_loc_first;
+ else
+ loc_first = &bp_loc_first;
+
+ if (*loc_first == NULL
+ || (overlay_debugging && loc->section != (*loc_first)->section)
+ || !breakpoint_locations_match (loc, *loc_first))
{
- loc_first = loc;
+ *loc_first = loc;
loc->duplicate = 0;
continue;
}
loc->duplicate = 1;
- if (loc_first->owner->enable_state == bp_permanent && loc->inserted
- && b->enable_state != bp_permanent)
+ if ((*loc_first)->owner->enable_state == bp_permanent && loc->inserted
+ && b->enable_state != bp_permanent)
internal_error (__FILE__, __LINE__,
_("another breakpoint was inserted on top of "
"a permanent breakpoint"));