Breakpoint locations, enableness and MI's =breakpoint-modified event
Jan Vrany
jan.vrany@fit.cvut.cz
Wed May 23 10:14:00 GMT 2018
Hi there,
when using MI, an MI event =breakpoint-modified is emitted if
breakpoint
enableness is changed via CLI command (such as 'dis 1'). However, if
breakpoint location is changed (such as 'dis 1.1'), no event is emitted
so the frontend may get out of sync. Is there any reason for this?
Also, why the event is not emitted when breakpoint enablement is
changed via MI command -break-enable?
In case this is a bug, I tried to fix it and emit the event even for
breakpoint location changes (see the patch below, testcase included).
In short, I have extended gdb::observers::breakpoint_modified to take
also the location (or nullptr) and notify() it also when location
enableness changes, passing the specific location. The MI hook is
updated accordingly, the rest is left as it was. Other uses of
gdb::observers::breakpoint_modified may need to be updated too (python,
guile & tui comes to mind).
Best, Jan
commit e766ec361a76749f14e63abb6ac588ebed59ad12
Author: Jan Vrany <jan.vrany@fit.cvut.cz>
Date: Wed May 23 10:27:16 2018 +0100
MI: emit =breakpoint-modified when breakpoint location enableness
changes
When a CLI command enabled/disables a breakpoint, MI frontend is
notified
via a =breakpoint-modified event. However, if only a particular
location
is enabled/disabled, this event was not emitted which may cause
frontend
to be out of sync (displaying wrong information to the user and so
on).
This commit fixes this by emmiting =breakpoint-modified event with
only the
location that has changed.
diff --git a/gdb/annotate.c b/gdb/annotate.c
index 495aa2dba6..e3c1978cd5 100644
--- a/gdb/annotate.c
+++ b/gdb/annotate.c
@@ -33,6 +33,7 @@
static void print_value_flags (struct type *);
static void breakpoint_changed (struct breakpoint *b);
+static void breakpoint_modified (struct breakpoint *b, struct
bp_location *l);
void (*deprecated_annotate_signalled_hook) (void);
@@ -589,10 +590,16 @@ breakpoint_changed (struct breakpoint *b)
annotate_breakpoints_invalid ();
}
+static void
+breakpoint_modified (struct breakpoint *b, struct bp_location *l)
+{
+ breakpoint_changed (b);
+}
+
void
_initialize_annotate (void)
{
gdb::observers::breakpoint_created.attach (breakpoint_changed);
gdb::observers::breakpoint_deleted.attach (breakpoint_changed);
- gdb::observers::breakpoint_modified.attach (breakpoint_changed);
+ gdb::observers::breakpoint_modified.attach (breakpoint_modified);
}
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 721afd2c04..21d480f713 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -904,7 +904,7 @@ set_breakpoint_condition (struct breakpoint *b,
const char *exp,
}
mark_breakpoint_modified (b);
- gdb::observers::breakpoint_modified.notify (b);
+ gdb::observers::breakpoint_modified.notify (b, nullptr);
}
/* Completion for the "condition" command. */
@@ -1173,7 +1173,7 @@ breakpoint_set_commands (struct breakpoint *b,
validate_commands_for_breakpoint (b, commands.get ());
b->commands = std::move (commands);
- gdb::observers::breakpoint_modified.notify (b);
+ gdb::observers::breakpoint_modified.notify (b, nullptr);
}
/* Set the internal `silent' flag on the breakpoint. Note that this
@@ -1187,7 +1187,7 @@ breakpoint_set_silent (struct breakpoint *b, int
silent)
b->silent = silent;
if (old_silent != silent)
- gdb::observers::breakpoint_modified.notify (b);
+ gdb::observers::breakpoint_modified.notify (b, nullptr);
}
/* Set the thread for this breakpoint. If THREAD is -1, make the
@@ -1200,7 +1200,7 @@ breakpoint_set_thread (struct breakpoint *b, int
thread)
b->thread = thread;
if (old_thread != thread)
- gdb::observers::breakpoint_modified.notify (b);
+ gdb::observers::breakpoint_modified.notify (b, nullptr);
}
/* Set the task for this breakpoint. If TASK is 0, make the
@@ -1213,7 +1213,7 @@ breakpoint_set_task (struct breakpoint *b, int
task)
b->task = task;
if (old_task != task)
- gdb::observers::breakpoint_modified.notify (b);
+ gdb::observers::breakpoint_modified.notify (b, nullptr);
}
static void
@@ -1266,7 +1266,7 @@ commands_command_1 (const char *arg, int
from_tty,
{
validate_commands_for_breakpoint (b, cmd.get ());
b->commands = cmd;
- gdb::observers::breakpoint_modified.notify (b);
+ gdb::observers::breakpoint_modified.notify (b, nullptr);
}
});
}
@@ -2644,7 +2644,7 @@ insert_bp_location (struct bp_location *bl,
{
/* See also: disable_breakpoints_in_shlibs. */
bl->shlib_disabled = 1;
- gdb::observers::breakpoint_modified.notify (bl->owner);
+ gdb::observers::breakpoint_modified.notify (bl->owner,
nullptr);
if (!*disabled_breaks)
{
fprintf_unfiltered (tmp_error_stream,
@@ -5292,7 +5292,7 @@ bpstat_check_breakpoint_conditions (bpstat bs,
ptid_t ptid)
bs->stop = 0;
/* Increase the hit count even though we don't stop. */
++(b->hit_count);
- gdb::observers::breakpoint_modified.notify (b);
+ gdb::observers::breakpoint_modified.notify (b, nullptr);
}
}
@@ -5444,7 +5444,7 @@ bpstat_stop_status (const address_space *aspace,
if (bs->stop)
{
++(b->hit_count);
- gdb::observers::breakpoint_modified.notify (b);
+ gdb::observers::breakpoint_modified.notify (b, nullptr);
/* We will stop here. */
if (b->disposition == disp_disable)
@@ -6443,10 +6443,19 @@ breakpoint_address_bits (struct breakpoint *b)
/* See breakpoint.h. */
void
-print_breakpoint (breakpoint *b)
+print_breakpoint (breakpoint *b, bp_location *loc)
{
struct bp_location *dummy_loc = NULL;
- print_one_breakpoint (b, &dummy_loc, 0);
+ if (!loc)
+ print_one_breakpoint (b, &dummy_loc, 0);
+ else
+ {
+ int n = 1;
+ struct bp_location *l;
+ ui_out_emit_tuple tuple_emitter (current_uiout, "bkpt");
+ for (l = b->loc; l != loc; l = l->next, ++n);
+ print_one_breakpoint_location(b, loc, n, &dummy_loc, 0);
+ }
}
/* Return true if this breakpoint was set by the user, false if it is
@@ -7600,7 +7609,7 @@ disable_breakpoints_in_unloaded_shlib (struct
so_list *solib)
loc->inserted = 0;
/* This may cause duplicate notifications for the same
breakpoint. */
- gdb::observers::breakpoint_modified.notify (b);
+ gdb::observers::breakpoint_modified.notify (b, nullptr);
if (!disabled_shlib_breaks)
{
@@ -7683,7 +7692,7 @@ disable_breakpoints_in_freed_objfile (struct
objfile *objfile)
}
if (bp_modified)
- gdb::observers::breakpoint_modified.notify (b);
+ gdb::observers::breakpoint_modified.notify (b, nullptr);
}
}
@@ -11691,7 +11700,7 @@ download_tracepoint_locations (void)
t = (struct tracepoint *) b;
t->number_on_target = b->number;
if (bp_location_downloaded)
- gdb::observers::breakpoint_modified.notify (b);
+ gdb::observers::breakpoint_modified.notify (b, nullptr);
}
}
@@ -13671,7 +13680,7 @@ update_breakpoint_locations (struct breakpoint
*b,
}
if (!locations_are_equal (existing_locations, b->loc))
- gdb::observers::breakpoint_modified.notify (b);
+ gdb::observers::breakpoint_modified.notify (b, nullptr);
}
/* Find the SaL locations corresponding to the given LOCATION.
@@ -13968,7 +13977,7 @@ set_ignore_count (int bptnum, int count, int
from_tty)
"crossings of breakpoint %d."),
count, bptnum);
}
- gdb::observers::breakpoint_modified.notify (b);
+ gdb::observers::breakpoint_modified.notify (b, nullptr);
return;
}
@@ -14225,6 +14234,8 @@ enable_disable_bp_num_loc (int bp_num, int
loc_num, bool enable)
target_disable_tracepoint (loc);
}
update_global_location_list (UGLL_DONT_INSERT);
+
+ gdb::observers::breakpoint_modified.notify (loc->owner, loc);
}
/* Enable or disable a range of breakpoint locations. BP_NUM is the
@@ -14271,7 +14282,7 @@ disable_breakpoint (struct breakpoint *bpt)
update_global_location_list (UGLL_DONT_INSERT);
- gdb::observers::breakpoint_modified.notify (bpt);
+ gdb::observers::breakpoint_modified.notify (bpt, nullptr);
}
/* Enable or disable the breakpoint(s) or breakpoint location(s)
@@ -14397,7 +14408,7 @@ enable_breakpoint_disp (struct breakpoint *bpt,
enum bpdisp disposition,
bpt->enable_count = count;
update_global_location_list (UGLL_MAY_INSERT);
- gdb::observers::breakpoint_modified.notify (bpt);
+ gdb::observers::breakpoint_modified.notify (bpt, nullptr);
}
@@ -14878,7 +14889,7 @@ static void
trace_pass_set_count (struct tracepoint *tp, int count, int from_tty)
{
tp->pass_count = count;
- gdb::observers::breakpoint_modified.notify (tp);
+ gdb::observers::breakpoint_modified.notify (tp, nullptr);
if (from_tty)
printf_filtered (_("Setting tracepoint %d's passcount to %d\n"),
tp->number, count);
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 4223158fbc..2e01b018bd 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -1683,7 +1683,8 @@ extern const char *ep_parse_optional_if_clause
(const char **arg);
UIOUT iff debugging multiple threads. */
extern void maybe_print_thread_hit_breakpoint (struct ui_out *uiout);
-/* Print the specified breakpoint. */
-extern void print_breakpoint (breakpoint *bp);
+/* Print the specified breakpoint BP and all its locations.
+ If LOC is not null, print only the LOC. */
+extern void print_breakpoint (breakpoint *bp, bp_location *loc =
nullptr);
#endif /* !defined (BREAKPOINT_H) */
diff --git a/gdb/mi/mi-cmd-break.c b/gdb/mi/mi-cmd-break.c
index 8897117bb8..f8eb151032 100644
--- a/gdb/mi/mi-cmd-break.c
+++ b/gdb/mi/mi-cmd-break.c
@@ -395,7 +395,7 @@ mi_cmd_break_passcount (const char *command, char
**argv, int argc)
if (t)
{
t->pass_count = p;
- gdb::observers::breakpoint_modified.notify (t);
+ gdb::observers::breakpoint_modified.notify (t, nullptr);
}
else
{
diff --git a/gdb/mi/mi-interp.c b/gdb/mi/mi-interp.c
index cec8af1278..98804000df 100644
--- a/gdb/mi/mi-interp.c
+++ b/gdb/mi/mi-interp.c
@@ -81,7 +81,7 @@ static void mi_tsv_deleted (const struct
trace_state_variable *tsv);
static void mi_tsv_modified (const struct trace_state_variable *tsv);
static void mi_breakpoint_created (struct breakpoint *b);
static void mi_breakpoint_deleted (struct breakpoint *b);
-static void mi_breakpoint_modified (struct breakpoint *b);
+static void mi_breakpoint_modified (struct breakpoint *b, struct
bp_location *l);
static void mi_command_param_changed (const char *param, const char
*value);
static void mi_memory_changed (struct inferior *inf, CORE_ADDR
memaddr,
ssize_t len, const bfd_byte *myaddr);
@@ -829,10 +829,10 @@ mi_tsv_modified (const struct
trace_state_variable *tsv)
}
}
-/* Print breakpoint BP on MI's event channel. */
+/* Print breakpoint BP or LOC (if not nullptr) on MI's event
channel. */
static void
-mi_print_breakpoint_for_event (struct mi_interp *mi, breakpoint *bp)
+mi_print_breakpoint_for_event (struct mi_interp *mi, breakpoint *bp,
struct bp_location *loc)
{
ui_out *mi_uiout = interp_ui_out (mi);
@@ -850,7 +850,8 @@ mi_print_breakpoint_for_event (struct mi_interp
*mi, breakpoint *bp)
scoped_restore restore_uiout
= make_scoped_restore (¤t_uiout, mi_uiout);
- print_breakpoint (bp);
+ print_breakpoint(bp, loc);
+
}
CATCH (ex, RETURN_MASK_ALL)
{
@@ -884,7 +885,7 @@ mi_breakpoint_created (struct breakpoint *b)
fprintf_unfiltered (mi->event_channel,
"breakpoint-created");
- mi_print_breakpoint_for_event (mi, b);
+ mi_print_breakpoint_for_event (mi, b, nullptr);
gdb_flush (mi->event_channel);
}
@@ -921,7 +922,7 @@ mi_breakpoint_deleted (struct breakpoint *b)
/* Emit notification about modified breakpoint. */
static void
-mi_breakpoint_modified (struct breakpoint *b)
+mi_breakpoint_modified (struct breakpoint *b, struct bp_location *l)
{
if (mi_suppress_notification.breakpoint)
return;
@@ -940,7 +941,7 @@ mi_breakpoint_modified (struct breakpoint *b)
target_terminal::ours_for_output ();
fprintf_unfiltered (mi->event_channel,
"breakpoint-modified");
- mi_print_breakpoint_for_event (mi, b);
+ mi_print_breakpoint_for_event (mi, b, l);
gdb_flush (mi->event_channel);
}
diff --git a/gdb/observable.h b/gdb/observable.h
index 7a3cb39268..d36e73d569 100644
--- a/gdb/observable.h
+++ b/gdb/observable.h
@@ -149,7 +149,7 @@ extern observable<struct breakpoint *>
breakpoint_deleted;
/* A breakpoint has been modified in some way. The argument b
is the modified breakpoint. */
-extern observable<struct breakpoint *> breakpoint_modified;
+extern observable<struct breakpoint *, struct bp_location *>
breakpoint_modified;
/* The trace frame is changed to tfnum (e.g., by using the 'tfind'
command). If tfnum is negative, it means gdb resumes live
diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index a66e553cf8..47bd7f46df 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -1089,7 +1089,7 @@ gdbpy_breakpoint_deleted (struct breakpoint *b)
/* Callback that is used when a breakpoint is modified. */
static void
-gdbpy_breakpoint_modified (struct breakpoint *b)
+gdbpy_breakpoint_modified (struct breakpoint *b, struct bp_location
*l)
{
int num = b->number;
PyGILState_STATE state;
diff --git a/gdb/testsuite/gdb.mi/mi-breakpoint-changed-2.c
b/gdb/testsuite/gdb.mi/mi-breakpoint-changed-2.c
new file mode 100644
index 0000000000..c4a295aa21
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-breakpoint-changed-2.c
@@ -0,0 +1,42 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2018 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/
>. */
+
+#include <stdio.h>
+#include <stdlib.h>
+
+static inline int
+add(int a, int b) {
+ return a + b;
+}
+
+int
+my_rand_1(void) {
+ return add(rand(), rand());
+}
+
+int
+my_rand_2(void) {
+ int r = my_rand_1();
+ return add(r, r);
+}
+
+int
+main (void)
+{
+ int i = my_rand_1();
+ return 1; /* next-line */
+}
\ No newline at end of file
diff --git a/gdb/testsuite/gdb.mi/mi-breakpoint-changed-2.exp
b/gdb/testsuite/gdb.mi/mi-breakpoint-changed-2.exp
new file mode 100644
index 0000000000..c5b3e1d7b8
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-breakpoint-changed-2.exp
@@ -0,0 +1,57 @@
+# Copyright 2018 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/>
.
+
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+gdb_exit
+if {[mi_gdb_start]} {
+ continue
+}
+
+#
+# Start here
+#
+standard_testfile
+
+if {[gdb_compile "$srcdir/$subdir/$srcfile" $binfile executable {debug
additional_flags=-O2}] != "" } {
+ return -1
+}
+
+mi_run_to_main
+
+mi_gdb_test "break add" \
+ {(&.*)*.*~"Breakpoint 2 at.*\\n".*=breakpoint-
created,bkpt=\{number="2",type="breakpoint".*\},\{number="2.1",enabled=
"y".*\}.*\n\^done} \
+ "break add"
+
+# Modify enableness through MI commands shouldn't trigger MI
+# notification.
+mi_gdb_test "-break-disable 2.1" "\\^done" "-break-disable 3"
+mi_gdb_test "-break-enable 2.1" "\\^done" "-break-enable 3"
+
+# Modify enableness through CLI commands should trigger MI
+# notification.
+mi_gdb_test "dis 2.2" \
+ {.*=breakpoint-
modified,bkpt=\{number="2.2".*,enabled=\"n\".*\}.*\n\^done} \
+ "dis 2.2"
+mi_gdb_test "en 2.2" \
+ {.*=breakpoint-
modified,bkpt=\{number="2.2".*,enabled=\"y\".*\}.*\n\^done} \
+ "en 2.2"
+
+
+mi_gdb_exit
+
+
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index e170d704bc..e9125ef9f6 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -1636,7 +1636,7 @@ start_tracing (const char *notes)
loc->gdbarch);
if (bp_location_downloaded)
- gdb::observers::breakpoint_modified.notify (b);
+ gdb::observers::breakpoint_modified.notify (b, nullptr);
}
VEC_free (breakpoint_p, tp_vec);
@@ -3169,7 +3169,7 @@ merge_uploaded_tracepoints (struct uploaded_tp
**uploaded_tps)
/* Notify 'breakpoint-modified' observer that at least one of B's
locations was changed. */
for (ix = 0; VEC_iterate (breakpoint_p, modified_tp, ix, b); ix++)
- gdb::observers::breakpoint_modified.notify (b);
+ gdb::observers::breakpoint_modified.notify (b, nullptr);
VEC_free (breakpoint_p, modified_tp);
free_uploaded_tps (uploaded_tps);
diff --git a/gdb/tui/tui-hooks.c b/gdb/tui/tui-hooks.c
index efa02e2f08..f206a3b773 100644
--- a/gdb/tui/tui-hooks.c
+++ b/gdb/tui/tui-hooks.c
@@ -102,7 +102,7 @@ tui_event_delete_breakpoint (struct breakpoint *b)
}
static void
-tui_event_modify_breakpoint (struct breakpoint *b)
+tui_event_modify_breakpoint (struct breakpoint *b, struct bp_location
*l)
{
tui_update_all_breakpoint_info ();
}
More information about the Gdb
mailing list