This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: don't run watchpoint commands when the watchpoint goes out of scope
On Tuesday 24 May 2011 19:56:22, Tom Tromey wrote:
> >>>>> "Pedro" == Pedro Alves <pedro@codesourcery.com> writes:
>
> Pedro> Walking the related breakpoints in map_breakpoint_numbers
> Pedro> is bogus. It should be the map_breakpoint_numbers' callback
> Pedro> responsibility to iterate over the related breakpoints if it
> Pedro> so needs/wants.
>
> I totally agree; but I wonder what relies on this.
> Annotation indicates that this code has been there forever.
Hmm, I thought I had made sure nothing relies on this, but
I may have misread the code. Sorry about that.
On "delete", we'd previously delete watchpoint's "related" breakpoints,
but now we're leaving in place with disp == del on next stop (done
directly from within delete_breakpoint. That shouldn't hurt, other than
causing a stop on the scope breakpoint once for nothing. On
"enable"/"disable", it shouldn't make a difference for watchpoint
"related" breakpoints either, since we never disable those
breakpoints.
The new ifunc "related" breakpoints is the large piece that I apparently
missed (most of it is outside breakpoint.c). It looks like that it may
happen that we may end up with "related" breakpoints pending off a
bp_gnu_ifunc_resolver breakpoint, and we should delete them all
with "delete". Thing is, "delete FOO" used to delete all the
related breakpoints (because it goes through map_breakpoint_numbers),
while "delete" does not, because that just calls delete_breakpoint
directly on each user breakpoint, and delete_breakpoint doesn't go
through ifunc related breakpoints deleting them. Hmm. Either
make delete_breakpoint itself always handle related breakpoints, or make
the "delete" command (and others) walk related breakpoints in the
paths that don't use map_breakpoint_numbers? This looks like will
need a bit more fixing than I'd like to propose myself to at the
moment (my fix-bugs-as-I-trip-on-them stack is already nested
too deep). Better keep the old bugs than add new different
bugs, so this patch restores the old behavior of iterating
over the "related" breakpoints, except in the case of the
"commands" command. I'll apply it tomorrow, barring comments.
Tested on x86_64-linux, no regressions.
Thanks,
--
Pedro Alves
2011-05-25 Pedro Alves <pedro@codesourcery.com>
gdb/
* breakpoint.c (iterate_related_breakpoints): New.
(do_map_delete_breakpoint): New.
(delete_command): Pass do_map_delete_breakpoint to
map_breakpoint_numbers.
(do_disable_breakpoint): New.
(do_map_disable_breakpoint): Iterate over the breakpoint's related
breakpoints.
(do_enable_breakpoint): Rename to ...
(enable_breakpoint_disp): ... this.
(enable_breakpoint): Adjust.
(do_enable_breakpoint): New.
(enable_once_breakpoint): Delete.
(do_map_enable_breakpoint): New.
(do_map_enable_once_breakpoint): New.
(enable_once_command, enable_delete_command)
(delete_trace_command): Iterate over the breakpoint's related
breakpoints.
---
gdb/breakpoint.c | 99 ++++++++++++++++++++++++++++++++++++++++++++++---------
1 file changed, 84 insertions(+), 15 deletions(-)
Index: src/gdb/breakpoint.c
===================================================================
--- src.orig/gdb/breakpoint.c 2011-05-25 18:17:31.000000000 +0100
+++ src/gdb/breakpoint.c 2011-05-25 20:56:06.637364386 +0100
@@ -176,7 +176,7 @@ static void hbreak_command (char *, int)
static void thbreak_command (char *, int);
-static void do_enable_breakpoint (struct breakpoint *, enum bpdisp);
+static void enable_breakpoint_disp (struct breakpoint *, enum bpdisp);
static void stop_command (char *arg, int from_tty);
@@ -10812,8 +10812,42 @@ make_cleanup_delete_breakpoint (struct b
return make_cleanup (do_delete_breakpoint_cleanup, b);
}
-/* A callback for map_breakpoint_numbers that calls
- delete_breakpoint. */
+/* Iterator function to call a user-provided callback function once
+ for each of B and its related breakpoints. */
+
+static void
+iterate_over_related_breakpoints (struct breakpoint *b,
+ void (*function) (struct breakpoint *,
+ void *),
+ void *data)
+{
+ struct breakpoint *related;
+
+ related = b;
+ do
+ {
+ struct breakpoint *next;
+
+ /* FUNCTION may delete RELATED. */
+ next = related->related_breakpoint;
+
+ if (next == related)
+ {
+ /* RELATED is the last ring entry. */
+ function (related, data);
+
+ /* FUNCTION may have deleted it, so we'd never reach back to
+ B. There's nothing left to do anyway, so just break
+ out. */
+ break;
+ }
+ else
+ function (related, data);
+
+ related = next;
+ }
+ while (related != b);
+}
static void
do_delete_breakpoint (struct breakpoint *b, void *ignore)
@@ -10821,6 +10855,15 @@ do_delete_breakpoint (struct breakpoint
delete_breakpoint (b);
}
+/* A callback for map_breakpoint_numbers that calls
+ delete_breakpoint. */
+
+static void
+do_map_delete_breakpoint (struct breakpoint *b, void *ignore)
+{
+ iterate_over_related_breakpoints (b, do_delete_breakpoint, NULL);
+}
+
void
delete_command (char *arg, int from_tty)
{
@@ -10852,7 +10895,7 @@ delete_command (char *arg, int from_tty)
}
}
else
- map_breakpoint_numbers (arg, do_delete_breakpoint, NULL);
+ map_breakpoint_numbers (arg, do_map_delete_breakpoint, NULL);
}
static int
@@ -11672,13 +11715,21 @@ disable_breakpoint (struct breakpoint *b
observer_notify_breakpoint_modified (bpt);
}
+/* A callback for iterate_over_related_breakpoints. */
+
+static void
+do_disable_breakpoint (struct breakpoint *b, void *ignore)
+{
+ disable_breakpoint (b);
+}
+
/* A callback for map_breakpoint_numbers that calls
disable_breakpoint. */
static void
do_map_disable_breakpoint (struct breakpoint *b, void *ignore)
{
- disable_breakpoint (b);
+ iterate_over_related_breakpoints (b, do_disable_breakpoint, NULL);
}
static void
@@ -11710,7 +11761,7 @@ disable_command (char *args, int from_tt
}
static void
-do_enable_breakpoint (struct breakpoint *bpt, enum bpdisp disposition)
+enable_breakpoint_disp (struct breakpoint *bpt, enum bpdisp disposition)
{
int target_resources_ok;
@@ -11771,7 +11822,13 @@ do_enable_breakpoint (struct breakpoint
void
enable_breakpoint (struct breakpoint *bpt)
{
- do_enable_breakpoint (bpt, bpt->disposition);
+ enable_breakpoint_disp (bpt, bpt->disposition);
+}
+
+static void
+do_enable_breakpoint (struct breakpoint *bpt, void *arg)
+{
+ enable_breakpoint (bpt);
}
/* A callback for map_breakpoint_numbers that calls
@@ -11780,7 +11837,7 @@ enable_breakpoint (struct breakpoint *bp
static void
do_map_enable_breakpoint (struct breakpoint *b, void *ignore)
{
- enable_breakpoint (b);
+ iterate_over_related_breakpoints (b, do_enable_breakpoint, NULL);
}
/* The enable command enables the specified breakpoints (or all defined
@@ -11816,27 +11873,39 @@ enable_command (char *args, int from_tty
}
static void
-enable_once_breakpoint (struct breakpoint *bpt, void *ignore)
+do_enable_breakpoint_disp (struct breakpoint *bpt, void *arg)
+{
+ enum bpdisp disp = *(enum bpdisp *) arg;
+
+ enable_breakpoint_disp (bpt, disp);
+}
+
+static void
+do_map_enable_once_breakpoint (struct breakpoint *bpt, void *ignore)
{
- do_enable_breakpoint (bpt, disp_disable);
+ enum bpdisp disp = disp_disable;
+
+ iterate_over_related_breakpoints (bpt, do_enable_breakpoint_disp, &disp);
}
static void
enable_once_command (char *args, int from_tty)
{
- map_breakpoint_numbers (args, enable_once_breakpoint, NULL);
+ map_breakpoint_numbers (args, do_map_enable_once_breakpoint, NULL);
}
static void
-enable_delete_breakpoint (struct breakpoint *bpt, void *ignore)
+do_map_enable_delete_breakpoint (struct breakpoint *bpt, void *ignore)
{
- do_enable_breakpoint (bpt, disp_del);
+ enum bpdisp disp = disp_del;
+
+ iterate_over_related_breakpoints (bpt, do_enable_breakpoint_disp, &disp);
}
static void
enable_delete_command (char *args, int from_tty)
{
- map_breakpoint_numbers (args, enable_delete_breakpoint, NULL);
+ map_breakpoint_numbers (args, do_map_enable_delete_breakpoint, NULL);
}
static void
@@ -12362,7 +12431,7 @@ delete_trace_command (char *arg, int fro
}
}
else
- map_breakpoint_numbers (arg, do_delete_breakpoint, NULL);
+ map_breakpoint_numbers (arg, do_map_delete_breakpoint, NULL);
}
/* Helper function for trace_pass_command. */