This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch] Fix crash on conditional watchpoints (PR 11371)
- From: Pedro Alves <pedro at codesourcery dot com>
- To: gdb-patches at sourceware dot org
- Cc: Jan Kratochvil <jan dot kratochvil at redhat dot com>, Chris Moller <cmoller at redhat dot com>
- Date: Sat, 14 Aug 2010 03:38:17 +0100
- Subject: Re: [patch] Fix crash on conditional watchpoints (PR 11371)
- References: <20100811202053.GB16057@host1.dyn.jankratochvil.net>
On Wednesday 11 August 2010 21:20:53, Jan Kratochvil wrote:
> on a watchpoint with condition using inferior function call GDB crashes.
>
> == Invalid read of size 8
> == at 0x63949B: bpstat_stop_status (breakpoint.c:4138)
> == by 0x69C145: handle_inferior_event (infrun.c:3873)
...
> == Address 0xcc727b0 is 16 bytes inside a block of size 184 free'd
> == at 0x4C25D72: free (vg_replace_malloc.c:325)
> == by 0x48E503: xfree (utils.c:1505)
> == by 0x63B9CA: free_bp_location (breakpoint.c:5402)
> == by 0x643B54: update_global_location_list (breakpoint.c:9428)
...
> It is due to bpstat_stop_status calling bpstat_check_breakpoint_conditions
> which calls update_global_location_list which rebuilds bp_location's which
> bpstat_stop_status does not expect.
Thanks.
> With the fix it just unfortunately does not stop when it should. This is due
> to all-stop mode always removing and re-creating watchpoint locations and
> bpstat_what() never stops on `bs->breakpoint_at == NULL'.
Yeah.
> Anyway non-stop
> mode should solve it as it no longer re-creates watchpoints each time.
> Unfortunately I could not verify it as non-stop fails for me in this case in
> some general way.
There's always "set breakpoint always-inserted on".
> I would turn the PR from a crashing one into a non-working
> one, there is a KFAIL in the testcase for it below. I believe the problem it
> does not stop is unrelated to this one.
I think we should try to address that problem too. Otherwise, we're still
papering over some deeper problems.
I think the main points are:
- we need to be careful about hidden inferior function calls while building
the bpstat list (or more generally, when walking breakpoint/location lists),
as what we're walking over may change below our feet.
- we may see a bp_location that is pointed by a bpstat be removed from the
global location list, and detached from its owner breakpoint, but, that does
not mean that it no longer explains the stop, and, does not mean we
should not report the stop. In fact, if the original owner is still listed,
we should report the stop. This is the case of the KFAIL in your test.
Please give this alternative patch a try, and let me know what you think.
No regressions on x86_64-linux, your test passes cleanly for me, and
valgrind doesn't complain when I ran the test manually.
--
Pedro Alves
2010-08-14 Pedro Alves <pedro@codesourcery.com>
PR breakpoints/11371
* breakpoint.c (breakpoint_init_inferior): Decrement the
location's reference count instead of deleting right away.
(bpstat_free): Decrement the location's reference count. Make
static.
(bpstat_copy): Increment the location's reference count.
(bpstat_find_breakpoint): Adjust.
(bpstat_num): Adjust.
(print_it_typical): Adjust. Use the breakpoint pointer in the
bpstat instead of the location's owner.
(bpstat_alloc): Remove const qualifier from the 'bl' parameter.
Adjust to record the location's owner in the bpstat.
(watchpoint_check): Use the breakpoint pointer in the bpstat
instead of the location's owner.
(bpstat_check_breakpoint_conditions): Don't handle
bp_watchpoint_scope here. Use the breakpoint pointer in the
bpstat instead of the location's owner.
(bpstat_stop_status): Defer inferior function calls to after
building the bpstat list. Handle bp_watchpoint_scope here. Use
the breakpoint pointer in the bpstat instead of the location's
owner.
(bpstat_what): Use the breakpoint pointer in the bpstat instead of
the location's owner.
(free_bp_location): Don't walk bpstats clearing locations.
(incref_bp_location): New.
(decref_bp_location): New.
(breakpoint_auto_delete): Use the breakpoint pointer in the bpstat
instead of the location's owner.
(update_global_location_list): Clear the location's owner, and
decrement the location's reference count instead of deleting it
right away.
(breakpoint_retire_moribund): Decrement the location's reference
count instead of deleting it right away.
(bpstat_remove_bp_location): Delete.
(bpstat_remove_breakpoint): New.
(bpstat_remove_bp_location_callback): Delete.
(bpstat_remove_breakpoint_callback): New.
(delete_breakpoint): Iterate over all threads' stop_bpstat's
clearing references to the breakpoint that is being deleted.
* breakpoint.h (struct bp_location) <refc>: New field.
<owner>: Update comments.
(bpstat_free): Delete declaration.
(struct bpstats): Change the type of the breakpoint_at field to
struct breakpoint point, from struct bp_location pointer. Add new
field bp_location_at.
---
gdb/breakpoint.c | 240 ++++++++++++++++++++++++++++++++-----------------------
gdb/breakpoint.h | 48 ++++++++---
2 files changed, 180 insertions(+), 108 deletions(-)
Index: src/gdb/breakpoint.c
===================================================================
--- src.orig/gdb/breakpoint.c 2010-08-14 01:20:58.000000000 +0100
+++ src/gdb/breakpoint.c 2010-08-14 02:26:32.000000000 +0100
@@ -135,7 +135,7 @@ static void watchpoints_info (char *, in
static int breakpoint_1 (int, int, int (*) (const struct breakpoint *));
-static bpstat bpstat_alloc (const struct bp_location *, bpstat);
+static bpstat bpstat_alloc (struct bp_location *, bpstat);
static int breakpoint_cond_eval (void *);
@@ -202,6 +202,8 @@ static int single_step_breakpoint_insert
CORE_ADDR pc);
static void free_bp_location (struct bp_location *loc);
+static void incref_bp_location (struct bp_location *loc);
+static void decref_bp_location (struct bp_location **loc);
static struct bp_location *allocate_bp_location (struct breakpoint *bpt);
@@ -209,9 +211,6 @@ static void update_global_location_list
static void update_global_location_list_nothrow (int);
-static int bpstat_remove_bp_location_callback (struct thread_info *th,
- void *data);
-
static int is_hardware_watchpoint (const struct breakpoint *bpt);
static int is_watchpoint (const struct breakpoint *bpt);
@@ -2606,7 +2605,7 @@ breakpoint_init_inferior (enum inf_conte
/* Get rid of the moribund locations. */
for (ix = 0; VEC_iterate (bp_location_p, moribund_locations, ix, bpt); ++ix)
- free_bp_location (bpt);
+ decref_bp_location (&bpt);
VEC_free (bp_location_p, moribund_locations);
}
@@ -2854,12 +2853,16 @@ ep_is_catchpoint (struct breakpoint *ep)
return (ep->type == bp_catchpoint);
}
-void
+/* Frees any storage that is part of a bpstat. Does not walk the
+ 'next' chain. */
+
+static void
bpstat_free (bpstat bs)
{
if (bs->old_val != NULL)
value_free (bs->old_val);
decref_counted_command_line (&bs->commands);
+ decref_bp_location (&bs->bp_location_at);
xfree (bs);
}
@@ -2902,6 +2905,7 @@ bpstat_copy (bpstat bs)
tmp = (bpstat) xmalloc (sizeof (*tmp));
memcpy (tmp, bs, sizeof (*tmp));
incref_counted_command_line (tmp->commands);
+ incref_bp_location (tmp->bp_location_at);
if (bs->old_val != NULL)
{
tmp->old_val = value_copy (bs->old_val);
@@ -2929,7 +2933,7 @@ bpstat_find_breakpoint (bpstat bsp, stru
for (; bsp != NULL; bsp = bsp->next)
{
- if (bsp->breakpoint_at && bsp->breakpoint_at->owner == breakpoint)
+ if (bsp->breakpoint_at == breakpoint)
return bsp;
}
return NULL;
@@ -2956,7 +2960,7 @@ bpstat_num (bpstat *bsp, int *num)
correspond to a single breakpoint -- otherwise,
this function might return the same number more
than once and this will look ugly. */
- b = (*bsp)->breakpoint_at ? (*bsp)->breakpoint_at->owner : NULL;
+ b = (*bsp)->breakpoint_at;
*bsp = (*bsp)->next;
if (b == NULL)
return -1; /* breakpoint that's been deleted since */
@@ -3168,13 +3172,11 @@ print_it_typical (bpstat bs)
which has since been deleted. */
if (bs->breakpoint_at == NULL)
return PRINT_UNKNOWN;
- bl = bs->breakpoint_at;
- /* bl->owner can be NULL if it was a momentary breakpoint
- which has since been placed into moribund_locations. */
- if (bl->owner == NULL)
- return PRINT_UNKNOWN;
- b = bl->owner;
+ gdb_assert (bs->bp_location_at != NULL);
+
+ bl = bs->bp_location_at;
+ b = bs->breakpoint_at;
stb = ui_out_stream_new (uiout);
old_chain = make_cleanup_ui_out_stream_delete (stb);
@@ -3183,7 +3185,7 @@ print_it_typical (bpstat bs)
{
case bp_breakpoint:
case bp_hardware_breakpoint:
- bp_temp = bs->breakpoint_at->owner->disposition == disp_del;
+ bp_temp = b->disposition == disp_del;
if (bl->address != bl->requested_address)
breakpoint_adjustment_warning (bl->requested_address,
bl->address,
@@ -3364,9 +3366,8 @@ print_bp_stop_message (bpstat bs)
case print_it_normal:
{
- const struct bp_location *bl = bs->breakpoint_at;
- struct breakpoint *b = bl ? bl->owner : NULL;
-
+ struct breakpoint *b = bs->breakpoint_at;
+
/* Normal case. Call the breakpoint's print_it method, or
print_it_typical. */
/* FIXME: how breakpoint can ever be NULL here? */
@@ -3445,13 +3446,15 @@ breakpoint_cond_eval (void *exp)
/* Allocate a new bpstat and chain it to the current one. */
static bpstat
-bpstat_alloc (const struct bp_location *bl, bpstat cbs /* Current "bs" value */ )
+bpstat_alloc (struct bp_location *bl, bpstat cbs /* Current "bs" value */ )
{
bpstat bs;
bs = (bpstat) xmalloc (sizeof (*bs));
cbs->next = bs;
- bs->breakpoint_at = bl;
+ bs->breakpoint_at = bl->owner;
+ bs->bp_location_at = bl;
+ incref_bp_location (bl);
/* If the condition is false, etc., don't do the commands. */
bs->commands = NULL;
bs->commands_left = NULL;
@@ -3544,10 +3547,9 @@ watchpoint_check (void *p)
struct frame_info *fr;
int within_current_scope;
- /* BS is built for existing struct breakpoint. */
+ /* BS is built from an existing struct breakpoint. */
gdb_assert (bs->breakpoint_at != NULL);
- gdb_assert (bs->breakpoint_at->owner != NULL);
- b = bs->breakpoint_at->owner;
+ b = bs->breakpoint_at;
/* 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
@@ -3738,9 +3740,9 @@ bpstat_check_watchpoint (bpstat bs)
struct breakpoint *b;
/* BS is built for existing struct breakpoint. */
- bl = bs->breakpoint_at;
+ bl = bs->bp_location_at;
gdb_assert (bl != NULL);
- b = bl->owner;
+ b = bs->breakpoint_at;
gdb_assert (b != NULL);
if (is_watchpoint (b))
@@ -3889,6 +3891,7 @@ bpstat_check_watchpoint (bpstat bs)
/* Check conditions (condition proper, frame, thread and ignore count)
of breakpoint referred to by BS. If we should not stop for this
breakpoint, set BS->stop to 0. */
+
static void
bpstat_check_breakpoint_conditions (bpstat bs, ptid_t ptid)
{
@@ -3897,9 +3900,9 @@ bpstat_check_breakpoint_conditions (bpst
struct breakpoint *b;
/* BS is built for existing struct breakpoint. */
- bl = bs->breakpoint_at;
+ bl = bs->bp_location_at;
gdb_assert (bl != NULL);
- b = bl->owner;
+ b = bs->breakpoint_at;
gdb_assert (b != NULL);
if (frame_id_p (b->frame_id)
@@ -3910,19 +3913,12 @@ bpstat_check_breakpoint_conditions (bpst
int value_is_zero = 0;
struct expression *cond;
- /* If this is a scope breakpoint, mark the associated
- watchpoint as triggered so that we will handle the
- out-of-scope event. We'll get to the watchpoint next
- iteration. */
- if (b->type == bp_watchpoint_scope)
- b->related_breakpoint->watchpoint_triggered = watch_triggered_yes;
-
if (is_watchpoint (b))
cond = b->cond_exp;
else
cond = bl->cond;
- if (cond && bl->owner->disposition != disp_del_at_next_stop)
+ if (cond && b->disposition != disp_del_at_next_stop)
{
int within_current_scope = 1;
@@ -4033,10 +4029,14 @@ bpstat_stop_status (struct address_space
bpstat bs = root_bs;
int ix;
int need_remove_insert;
+ int removed_any;
- /* ALL_BP_LOCATIONS iteration would break across
- update_global_location_list possibly executed by
- bpstat_check_breakpoint_conditions's inferior call. */
+ /* First, build the bpstat chain with locations that explain a
+ target stop, while being careful to not set the target running,
+ as that may invalidate locations (in particular watchpoint
+ locations are recreated). Resuming will happen here with
+ breakpoint conditions or watchpoint expressions that include
+ inferior function calls. */
ALL_BREAKPOINTS (b)
{
@@ -4063,15 +4063,53 @@ bpstat_stop_status (struct address_space
bs = bpstat_alloc (bl, bs); /* Alloc a bpstat to explain stop */
- /* Assume we stop. Should we find watchpoint that is not actually
- triggered, or if condition of breakpoint is false, we'll reset
- 'stop' to 0. */
+ /* Assume we stop. Should we find a watchpoint that is not
+ actually triggered, or if the condition of the breakpoint
+ evaluates as false, we'll reset 'stop' to 0. */
bs->stop = 1;
bs->print = 1;
- bpstat_check_watchpoint (bs);
- if (!bs->stop)
- continue;
+ /* If this is a scope breakpoint, mark the associated
+ watchpoint as triggered so that we will handle the
+ out-of-scope event. We'll get to the watchpoint next
+ iteration. */
+ if (b->type == bp_watchpoint_scope)
+ b->related_breakpoint->watchpoint_triggered = watch_triggered_yes;
+ }
+ }
+
+ for (ix = 0; VEC_iterate (bp_location_p, moribund_locations, ix, loc); ++ix)
+ {
+ if (breakpoint_address_match (loc->pspace->aspace, loc->address,
+ aspace, bp_addr))
+ {
+ bs = bpstat_alloc (loc, bs);
+ /* For hits of moribund locations, we should just proceed. */
+ bs->stop = 0;
+ bs->print = 0;
+ bs->print_it = print_it_noop;
+ }
+ }
+
+ /* Terminate the chain. */
+ bs->next = NULL;
+
+ /* Now go through the locations that caused the target to stop, and
+ check whether we're interested in reporting this stop to higher
+ layers, or whether we should resume the target transparently. */
+
+ removed_any = 0;
+
+ for (bs = root_bs->next; bs != NULL; bs = bs->next)
+ {
+ if (!bs->stop)
+ continue;
+
+ bpstat_check_watchpoint (bs);
+ if (!bs->stop)
+ continue;
+
+ b = bs->breakpoint_at;
if (b->type == bp_thread_event || b->type == bp_overlay_event
|| b->type == bp_longjmp_master
@@ -4080,7 +4118,7 @@ bpstat_stop_status (struct address_space
bs->stop = 0;
else
bpstat_check_breakpoint_conditions (bs, ptid);
-
+
if (bs->stop)
{
++(b->hit_count);
@@ -4090,7 +4128,7 @@ bpstat_stop_status (struct address_space
{
if (b->enable_state != bp_permanent)
b->enable_state = bp_disabled;
- update_global_location_list (0);
+ removed_any = 1;
}
if (b->silent)
bs->print = 0;
@@ -4111,24 +4149,8 @@ bpstat_stop_status (struct address_space
/* Print nothing for this entry if we dont stop or dont print. */
if (bs->stop == 0 || bs->print == 0)
bs->print_it = print_it_noop;
- }
}
- for (ix = 0; VEC_iterate (bp_location_p, moribund_locations, ix, loc); ++ix)
- {
- if (breakpoint_address_match (loc->pspace->aspace, loc->address,
- aspace, bp_addr))
- {
- bs = bpstat_alloc (loc, bs);
- /* For hits of moribund locations, we should just proceed. */
- bs->stop = 0;
- bs->print = 0;
- bs->print_it = print_it_noop;
- }
- }
-
- bs->next = NULL; /* Terminate the chain */
-
/* If we aren't stopping, the value of some hardware watchpoint may
not have changed, but the intermediate memory locations we are
watching may have. Don't bother if we're stopping; this will get
@@ -4137,18 +4159,17 @@ bpstat_stop_status (struct address_space
if (! bpstat_causes_stop (root_bs->next))
for (bs = root_bs->next; bs != NULL; bs = bs->next)
if (!bs->stop
- && bs->breakpoint_at->owner
- && is_hardware_watchpoint (bs->breakpoint_at->owner))
+ && bs->breakpoint_at
+ && is_hardware_watchpoint (bs->breakpoint_at))
{
- 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;
+ update_watchpoint (bs->breakpoint_at, 0 /* don't reparse. */);
need_remove_insert = 1;
}
if (need_remove_insert)
update_global_location_list (1);
+ else if (removed_any)
+ update_global_location_list (0);
return root_bs->next;
}
@@ -4201,10 +4222,10 @@ bpstat_what (bpstat bs)
breakpoint which has since been deleted. */
bptype = bp_none;
}
- else if (bs->breakpoint_at->owner == NULL)
+ else if (bs->breakpoint_at == NULL)
bptype = bp_none;
else
- bptype = bs->breakpoint_at->owner->type;
+ bptype = bs->breakpoint_at->type;
switch (bptype)
{
@@ -5379,32 +5400,41 @@ allocate_bp_location (struct breakpoint
internal_error (__FILE__, __LINE__, _("unknown breakpoint type"));
}
+ loc->refc = 1;
return loc;
}
-static void free_bp_location (struct bp_location *loc)
+static void
+free_bp_location (struct bp_location *loc)
{
- /* Be sure no bpstat's are pointing at it after it's been freed. */
- /* FIXME, how can we find all bpstat's?
- We just check stop_bpstat for now. Note that we cannot just
- remove bpstats pointing at bpt from the stop_bpstat list
- entirely, as breakpoint commands are associated with the bpstat;
- if we remove it here, then the later call to
- bpstat_do_actions (&stop_bpstat);
- in event-top.c won't do anything, and temporary breakpoints
- with commands won't work. */
-
- iterate_over_threads (bpstat_remove_bp_location_callback, loc);
-
if (loc->cond)
xfree (loc->cond);
if (loc->function_name)
xfree (loc->function_name);
-
+
xfree (loc);
}
+/* Increment reference count. */
+
+static void
+incref_bp_location (struct bp_location *bl)
+{
+ ++bl->refc;
+}
+
+/* Decrement reference count. If the reference count reaches 0,
+ destroy the bp_location. Sets *BLP to NULL. */
+
+static void
+decref_bp_location (struct bp_location **blp)
+{
+ if (--(*blp)->refc == 0)
+ free_bp_location (*blp);
+ *blp = NULL;
+}
+
/* Helper to set_raw_breakpoint below. Creates a breakpoint
that has type BPTYPE and has no locations as yet. */
/* This function is used in gdbtk sources and thus can not be made static. */
@@ -9112,11 +9142,10 @@ breakpoint_auto_delete (bpstat bs)
struct breakpoint *b, *temp;
for (; bs; bs = bs->next)
- if (bs->breakpoint_at
- && bs->breakpoint_at->owner
- && bs->breakpoint_at->owner->disposition == disp_del
+ if (bs->breakpoint_at
+ && bs->breakpoint_at->disposition == disp_del
&& bs->stop)
- delete_breakpoint (bs->breakpoint_at->owner);
+ delete_breakpoint (bs->breakpoint_at);
ALL_BREAKPOINTS_SAFE (b, temp)
{
@@ -9427,7 +9456,10 @@ update_global_location_list (int should_
VEC_safe_push (bp_location_p, moribund_locations, old_loc);
}
else
- free_bp_location (old_loc);
+ {
+ old_loc->owner = NULL;
+ decref_bp_location (&old_loc);
+ }
}
}
@@ -9510,7 +9542,7 @@ breakpoint_retire_moribund (void)
for (ix = 0; VEC_iterate (bp_location_p, moribund_locations, ix, loc); ++ix)
if (--(loc->events_till_retirement) == 0)
{
- free_bp_location (loc);
+ decref_bp_location (&loc);
VEC_unordered_remove (bp_location_p, moribund_locations, ix);
--ix;
}
@@ -9525,14 +9557,15 @@ update_global_location_list_nothrow (int
update_global_location_list (inserting);
}
-/* Clear LOC from a BPS. */
+/* Clear BKP from a BPS. */
+
static void
-bpstat_remove_bp_location (bpstat bps, struct bp_location *loc)
+bpstat_remove_bp_location (bpstat bps, struct breakpoint *bpt)
{
bpstat bs;
for (bs = bps; bs; bs = bs->next)
- if (bs->breakpoint_at == loc)
+ if (bs->breakpoint_at == bpt)
{
bs->breakpoint_at = NULL;
bs->old_val = NULL;
@@ -9542,16 +9575,16 @@ bpstat_remove_bp_location (bpstat bps, s
/* Callback for iterate_over_threads. */
static int
-bpstat_remove_bp_location_callback (struct thread_info *th, void *data)
+bpstat_remove_breakpoint_callback (struct thread_info *th, void *data)
{
- struct bp_location *loc = data;
+ struct breakpoint *bpt = data;
- bpstat_remove_bp_location (th->stop_bpstat, loc);
+ bpstat_remove_bp_location (th->stop_bpstat, bpt);
return 0;
}
/* Delete a breakpoint and clean up all traces of it in the data
- structures. */
+ structures. */
void
delete_breakpoint (struct breakpoint *bpt)
@@ -9609,6 +9642,19 @@ delete_breakpoint (struct breakpoint *bp
xfree (bpt->exec_pathname);
clean_up_filters (&bpt->syscalls_to_be_caught);
+
+ /* Be sure no bpstat's are pointing at the breakpoint after it's
+ been freed. */
+ /* FIXME, how can we find all bpstat's? We just check stop_bpstat
+ in all threeds for now. Note that we cannot just remove bpstats
+ pointing at bpt from the stop_bpstat list entirely, as breakpoint
+ commands are associated with the bpstat; if we remove it here,
+ then the later call to bpstat_do_actions (&stop_bpstat); in
+ event-top.c won't do anything, and temporary breakpoints with
+ commands won't work. */
+
+ iterate_over_threads (bpstat_remove_breakpoint_callback, bpt);
+
/* Now that breakpoint is removed from breakpoint
list, update the global location list. This
will remove locations that used to belong to
Index: src/gdb/breakpoint.h
===================================================================
--- src.orig/gdb/breakpoint.h 2010-08-14 01:20:58.000000000 +0100
+++ src/gdb/breakpoint.h 2010-08-14 01:42:59.000000000 +0100
@@ -240,13 +240,18 @@ struct bp_location
the same parent breakpoint. */
struct bp_location *next;
+ /* The reference count. */
+ int refc;
+
/* Type of this breakpoint location. */
enum bp_loc_type loc_type;
/* Each breakpoint location must belong to exactly one higher-level
- breakpoint. This and the DUPLICATE flag are more straightforward
- than reference counting. This pointer is NULL iff this bp_location is in
- (and therefore only in) moribund_locations. */
+ breakpoint. This pointer is NULL iff this bp_location is no
+ longer attached to a breakpoint. For example, when a breakpoint
+ is deleted, its locations may still be found in the
+ moribund_locations list, or if we had stopped for it, in
+ bpstats. */
struct breakpoint *owner;
/* Conditional. Break only if this expression's value is nonzero.
@@ -560,10 +565,6 @@ DEF_VEC_P(breakpoint_p);
typedef struct bpstats *bpstat;
-/* Frees any storage that is part of a bpstat.
- Does not walk the 'next' chain. */
-extern void bpstat_free (bpstat);
-
/* Clears a chain of bpstat, freeing storage
of each. */
extern void bpstat_clear (bpstat *);
@@ -728,16 +729,41 @@ enum bp_print_how
struct bpstats
{
- /* Linked list because there can be two breakpoints at the same
- place, and a bpstat reflects the fact that both have been hit. */
+ /* Linked list because there can be more than one breakpoint at
+ the same place, and a bpstat reflects the fact that all have
+ been hit. */
bpstat next;
- /* Breakpoint that we are at. */
- const struct bp_location *breakpoint_at;
+
+ /* Location that caused the stop. Locations are refcounted, so
+ this will never be NULL. Note that this location may end up
+ detached from a breakpoint, but that does not necessary mean
+ that the struct breakpoint is gone. E.g., consider a
+ watchpoint with a condition that involves an inferior function
+ call. Watchpoint locations are recreated often (on resumes,
+ hence on infcalls too). Between creating the bpstat and after
+ evaluating the watchpoint condition, this location may hence
+ end up detached from its original owner watchpoint, even though
+ the watchpoint is still listed. If it's condition evaluates as
+ true, we still want this location to cause a stop, and we will
+ still need to know which watchpoint it was originally attached.
+ What this means is that we should not (in most cases) follow
+ the `bpstat->bp_location->owner' link, but instead use the
+ `breakpoint_at' field below. */
+ struct bp_location *bp_location_at;
+
+ /* Breakpoint that caused the stop. This is nullified if the
+ breakpoint ends up being deleted. See comments on
+ `bp_location_at' above for why do we need this field instead of
+ following the location's owner. */
+ struct breakpoint *breakpoint_at;
+
/* The associated command list. */
struct counted_command_line *commands;
+
/* Commands left to be done. This points somewhere in
base_command. */
struct command_line *commands_left;
+
/* Old value associated with a watchpoint. */
struct value *old_val;