* [PATCH 0/9] Convert breakpoint iteration macros to ranges
@ 2021-05-27 15:35 Simon Marchi
2021-05-27 15:35 ` [PATCH 1/9] gdb: add all_breakpoints function Simon Marchi
` (9 more replies)
0 siblings, 10 replies; 21+ messages in thread
From: Simon Marchi @ 2021-05-27 15:35 UTC (permalink / raw)
To: gdb-patches
This all started with me finding that the update_global_location_list
function was hard to read, notably because the breakpoint iteration
macros are a bit hard to read (require multiple local variables and
pointers to pointers). So this removes all breakpoint-related iteration
macros in favor of functions that return iterable ranges.
Patch 5 also converts the bp_locations array into a vector, which
simplifies things a bit.
In the end, the update_global_location_list is still hard to understand
(big surprise), but I think that the code in general is a bit tidier
like this.
I regtested this on Linux, with the unix and native-extended-gdbserver
boards.
Simon Marchi (9):
gdb: add all_breakpoints function
gdb: add all_breakpoints_safe function
gdb: add all_tracepoints function
gdb: add breakpoint::locations method
gdb: make bp_locations an std::vector
gdb: add all_bp_locations function
gdb: add all_bp_locations_at_addr function
gdb: remove iterate_over_breakpoints function
gdb: remove iterate_over_bp_locations function
gdb/ada-lang.c | 4 +-
gdb/breakpoint.c | 866 +++++++++++--------------------
gdb/breakpoint.h | 69 ++-
gdb/dummy-frame.c | 7 +-
gdb/guile/scm-breakpoint.c | 10 +-
gdb/jit.c | 2 +-
gdb/python/py-breakpoint.c | 27 +-
gdb/python/py-finishbreakpoint.c | 17 +-
gdb/record-full.c | 26 +-
gdb/remote.c | 3 +-
gdb/solib-svr4.c | 7 +-
gdb/tracepoint.c | 31 +-
gdb/tui/tui-winsource.c | 9 +-
13 files changed, 420 insertions(+), 658 deletions(-)
--
2.31.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 1/9] gdb: add all_breakpoints function
2021-05-27 15:35 [PATCH 0/9] Convert breakpoint iteration macros to ranges Simon Marchi
@ 2021-05-27 15:35 ` Simon Marchi
2021-05-27 15:35 ` [PATCH 2/9] gdb: add all_breakpoints_safe function Simon Marchi
` (8 subsequent siblings)
9 siblings, 0 replies; 21+ messages in thread
From: Simon Marchi @ 2021-05-27 15:35 UTC (permalink / raw)
To: gdb-patches
Introduce the all_breakpoints function, which returns a range that can
be used to iterate on breakpoints. Replace all uses of the
ALL_BREAKPOINTS macro with this.
In one instance, I could replace the breakpoint iteration with a call to
get_breakpoint.
gdb/ChangeLog:
* breakpoint.c (ALL_BREAKPOINTS): Remove, replace all uses with
all_breakpoints.
(breakpoint_iterator): New.
(breakpoint_range): New.
(all_breakpoints): New.
Change-Id: I229595bddad7c9100b179a9dd56b04b8c206e86c
---
gdb/breakpoint.c | 189 ++++++++++++++++++-----------------------------
1 file changed, 70 insertions(+), 119 deletions(-)
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index d479f0089489..b6c32db2ec02 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -486,8 +486,6 @@ bool target_exact_watchpoints = false;
ALL_BREAKPOINTS_SAFE does so even if the statement deletes the
current breakpoint. */
-#define ALL_BREAKPOINTS(B) for (B = breakpoint_chain; B; B = B->next)
-
#define ALL_BREAKPOINTS_SAFE(B,TMP) \
for (B = breakpoint_chain; \
B ? (TMP=B->next, 1): 0; \
@@ -526,6 +524,22 @@ bool target_exact_watchpoints = false;
static struct breakpoint *breakpoint_chain;
+/* Breakpoint linked list iterator. */
+
+using breakpoint_iterator = next_iterator<breakpoint>;
+
+/* Breakpoint linked list range. */
+
+using breakpoint_range = next_adapter<breakpoint, breakpoint_iterator>;
+
+/* Return a range to iterate over all breakpoints. */
+
+static breakpoint_range
+all_breakpoints ()
+{
+ return breakpoint_range (breakpoint_chain);
+}
+
/* Array is sorted by bp_location_is_less_than - primarily by the ADDRESS. */
static struct bp_location **bp_locations;
@@ -579,15 +593,11 @@ struct breakpoint *
breakpoint_find_if (int (*func) (struct breakpoint *b, void *d),
void *user_data)
{
- struct breakpoint *b = NULL;
-
- ALL_BREAKPOINTS (b)
- {
- if (func (b, user_data) != 0)
- break;
- }
+ for (breakpoint *b : all_breakpoints ())
+ if (func (b, user_data) != 0)
+ return b;
- return b;
+ return nullptr;
}
/* Return whether a breakpoint is an active enabled breakpoint. */
@@ -632,9 +642,7 @@ scoped_rbreak_breakpoints::~scoped_rbreak_breakpoints ()
void
clear_breakpoint_hit_counts (void)
{
- struct breakpoint *b;
-
- ALL_BREAKPOINTS (b)
+ for (breakpoint *b : all_breakpoints ())
b->hit_count = 0;
}
@@ -645,13 +653,11 @@ clear_breakpoint_hit_counts (void)
struct breakpoint *
get_breakpoint (int num)
{
- struct breakpoint *b;
-
- ALL_BREAKPOINTS (b)
+ for (breakpoint *b : all_breakpoints ())
if (b->number == num)
return b;
- return NULL;
+ return nullptr;
}
\f
@@ -979,8 +985,7 @@ void
set_breakpoint_condition (int bpnum, const char *exp, int from_tty,
bool force)
{
- struct breakpoint *b;
- ALL_BREAKPOINTS (b)
+ for (breakpoint *b : all_breakpoints ())
if (b->number == bpnum)
{
/* Check if this breakpoint has a "stop" method implemented in an
@@ -1052,7 +1057,6 @@ condition_completer (struct cmd_list_element *cmd,
if (*space == '\0')
{
int len;
- struct breakpoint *b;
if (text[0] == '$')
{
@@ -1073,7 +1077,7 @@ condition_completer (struct cmd_list_element *cmd,
/* We're completing the breakpoint number. */
len = strlen (text);
- ALL_BREAKPOINTS (b)
+ for (breakpoint *b : all_breakpoints ())
{
char number[50];
@@ -1270,11 +1274,10 @@ validate_commands_for_breakpoint (struct breakpoint *b,
std::vector<breakpoint *>
static_tracepoints_here (CORE_ADDR addr)
{
- struct breakpoint *b;
std::vector<breakpoint *> found;
struct bp_location *loc;
- ALL_BREAKPOINTS (b)
+ for (breakpoint *b : all_breakpoints ())
if (b->type == bp_static_tracepoint)
{
for (loc = b->loc; loc; loc = loc->next)
@@ -2929,9 +2932,7 @@ breakpoint_program_space_exit (struct program_space *pspace)
void
insert_breakpoints (void)
{
- struct breakpoint *bpt;
-
- ALL_BREAKPOINTS (bpt)
+ for (breakpoint *bpt : all_breakpoints ())
if (is_hardware_watchpoint (bpt))
{
struct watchpoint *w = (struct watchpoint *) bpt;
@@ -3025,7 +3026,6 @@ update_inserted_breakpoint_locations (void)
static void
insert_breakpoint_locations (void)
{
- struct breakpoint *bpt;
struct bp_location *bl, **blp_tmp;
int error_flag = 0;
int val = 0;
@@ -3071,7 +3071,7 @@ insert_breakpoint_locations (void)
/* If we failed to insert all locations of a watchpoint, remove
them, as half-inserted watchpoint is of limited use. */
- ALL_BREAKPOINTS (bpt)
+ for (breakpoint *bpt : all_breakpoints ())
{
int some_failed = 0;
struct bp_location *loc;
@@ -4280,9 +4280,7 @@ int
hardware_watchpoint_inserted_in_range (const address_space *aspace,
CORE_ADDR addr, ULONGEST len)
{
- struct breakpoint *bpt;
-
- ALL_BREAKPOINTS (bpt)
+ for (breakpoint *bpt : all_breakpoints ())
{
struct bp_location *loc;
@@ -4861,13 +4859,12 @@ watchpoints_triggered (struct target_waitstatus *ws)
{
bool stopped_by_watchpoint = target_stopped_by_watchpoint ();
CORE_ADDR addr;
- struct breakpoint *b;
if (!stopped_by_watchpoint)
{
/* We were not stopped by a watchpoint. Mark all watchpoints
as not triggered. */
- ALL_BREAKPOINTS (b)
+ for (breakpoint *b : all_breakpoints ())
if (is_hardware_watchpoint (b))
{
struct watchpoint *w = (struct watchpoint *) b;
@@ -4882,7 +4879,7 @@ watchpoints_triggered (struct target_waitstatus *ws)
{
/* We were stopped by a watchpoint, but we don't know where.
Mark all watchpoints as unknown. */
- ALL_BREAKPOINTS (b)
+ for (breakpoint *b : all_breakpoints ())
if (is_hardware_watchpoint (b))
{
struct watchpoint *w = (struct watchpoint *) b;
@@ -4897,7 +4894,7 @@ watchpoints_triggered (struct target_waitstatus *ws)
affected by this data address as triggered, and all others as not
triggered. */
- ALL_BREAKPOINTS (b)
+ for (breakpoint *b : all_breakpoints ())
if (is_hardware_watchpoint (b))
{
struct watchpoint *w = (struct watchpoint *) b;
@@ -5223,9 +5220,7 @@ bpstat_check_watchpoint (bpstat bs)
if (bl->watchpoint_type == hw_read)
{
- struct breakpoint *other_b;
-
- ALL_BREAKPOINTS (other_b)
+ for (breakpoint *other_b : all_breakpoints ())
if (other_b->type == bp_hardware_watchpoint
|| other_b->type == bp_access_watchpoint)
{
@@ -5441,10 +5436,9 @@ bpstat
build_bpstat_chain (const address_space *aspace, CORE_ADDR bp_addr,
const struct target_waitstatus *ws)
{
- struct breakpoint *b;
bpstat bs_head = NULL, *bs_link = &bs_head;
- ALL_BREAKPOINTS (b)
+ for (breakpoint *b : all_breakpoints ())
{
if (!breakpoint_enabled (b))
continue;
@@ -5858,11 +5852,10 @@ bpstat_run_callbacks (bpstat bs_head)
bool
bpstat_should_step ()
{
- struct breakpoint *b;
-
- ALL_BREAKPOINTS (b)
+ for (breakpoint *b : all_breakpoints ())
if (breakpoint_enabled (b) && b->type == bp_watchpoint && b->loc != NULL)
return true;
+
return false;
}
@@ -6594,7 +6587,6 @@ static int
breakpoint_1 (const char *bp_num_list, bool show_internal,
bool (*filter) (const struct breakpoint *))
{
- struct breakpoint *b;
struct bp_location *last_loc = NULL;
int nr_printable_breakpoints;
struct value_print_options opts;
@@ -6608,7 +6600,7 @@ breakpoint_1 (const char *bp_num_list, bool show_internal,
/* Compute the number of rows in the table, as well as the size
required for address fields. */
nr_printable_breakpoints = 0;
- ALL_BREAKPOINTS (b)
+ for (breakpoint *b : all_breakpoints ())
{
/* If we have a filter, only list the breakpoints it accepts. */
if (filter && !filter (b))
@@ -6676,7 +6668,7 @@ breakpoint_1 (const char *bp_num_list, bool show_internal,
if (nr_printable_breakpoints > 0)
annotate_breakpoints_table ();
- ALL_BREAKPOINTS (b)
+ for (breakpoint *b : all_breakpoints ())
{
QUIT;
/* If we have a filter, only list the breakpoints it accepts. */
@@ -6820,18 +6812,18 @@ describe_other_breakpoints (struct gdbarch *gdbarch,
struct obj_section *section, int thread)
{
int others = 0;
- struct breakpoint *b;
- ALL_BREAKPOINTS (b)
+ for (breakpoint *b : all_breakpoints ())
others += (user_breakpoint_p (b)
&& breakpoint_has_pc (b, pspace, pc, section));
+
if (others > 0)
{
if (others == 1)
printf_filtered (_("Note: breakpoint "));
else /* if (others == ???) */
printf_filtered (_("Note: breakpoints "));
- ALL_BREAKPOINTS (b)
+ for (breakpoint *b : all_breakpoints ())
if (user_breakpoint_p (b) && breakpoint_has_pc (b, pspace, pc, section))
{
others--;
@@ -7422,9 +7414,9 @@ delete_longjmp_breakpoint_at_next_stop (int thread)
struct breakpoint *
set_longjmp_breakpoint_for_call_dummy (void)
{
- struct breakpoint *b, *retval = NULL;
+ breakpoint *retval = nullptr;
- ALL_BREAKPOINTS (b)
+ for (breakpoint *b : all_breakpoints ())
if (b->pspace == current_program_space && b->type == bp_longjmp_master)
{
struct breakpoint *new_b;
@@ -7487,9 +7479,7 @@ check_longjmp_breakpoint_for_call_dummy (struct thread_info *tp)
void
enable_overlay_breakpoints (void)
{
- struct breakpoint *b;
-
- ALL_BREAKPOINTS (b)
+ for (breakpoint *b : all_breakpoints ())
if (b->type == bp_overlay_event)
{
b->enable_state = bp_enabled;
@@ -7501,9 +7491,7 @@ enable_overlay_breakpoints (void)
void
disable_overlay_breakpoints (void)
{
- struct breakpoint *b;
-
- ALL_BREAKPOINTS (b)
+ for (breakpoint *b : all_breakpoints ())
if (b->type == bp_overlay_event)
{
b->enable_state = bp_disabled;
@@ -7733,8 +7721,6 @@ disable_breakpoints_in_unloaded_shlib (struct so_list *solib)
static void
disable_breakpoints_in_freed_objfile (struct objfile *objfile)
{
- struct breakpoint *b;
-
if (objfile == NULL)
return;
@@ -7753,7 +7739,7 @@ disable_breakpoints_in_freed_objfile (struct objfile *objfile)
|| (objfile->flags & OBJF_USERLOADED) == 0)
return;
- ALL_BREAKPOINTS (b)
+ for (breakpoint *b : all_breakpoints ())
{
struct bp_location *loc;
int bp_modified = 0;
@@ -8085,12 +8071,11 @@ breakpoint_hit_catch_solib (const struct bp_location *bl,
const struct target_waitstatus *ws)
{
struct solib_catchpoint *self = (struct solib_catchpoint *) bl->owner;
- struct breakpoint *other;
if (ws->kind == TARGET_WAITKIND_LOADED)
return 1;
- ALL_BREAKPOINTS (other)
+ for (breakpoint *other : all_breakpoints ())
{
struct bp_location *other_bl;
@@ -8451,11 +8436,9 @@ static int
hw_breakpoint_used_count (void)
{
int i = 0;
- struct breakpoint *b;
struct bp_location *bl;
- ALL_BREAKPOINTS (b)
- {
+ for (breakpoint *b : all_breakpoints ())
if (b->type == bp_hardware_breakpoint && breakpoint_enabled (b))
for (bl = b->loc; bl; bl = bl->next)
{
@@ -8463,7 +8446,6 @@ hw_breakpoint_used_count (void)
one register. */
i += b->ops->resources_needed (bl);
}
- }
return i;
}
@@ -8500,10 +8482,9 @@ hw_watchpoint_used_count_others (struct breakpoint *except,
enum bptype type, int *other_type_used)
{
int i = 0;
- struct breakpoint *b;
*other_type_used = 0;
- ALL_BREAKPOINTS (b)
+ for (breakpoint *b : all_breakpoints ())
{
if (b == except)
continue;
@@ -8522,31 +8503,23 @@ hw_watchpoint_used_count_others (struct breakpoint *except,
void
disable_watchpoints_before_interactive_call_start (void)
{
- struct breakpoint *b;
-
- ALL_BREAKPOINTS (b)
- {
+ for (breakpoint *b : all_breakpoints ())
if (is_watchpoint (b) && breakpoint_enabled (b))
{
b->enable_state = bp_call_disabled;
update_global_location_list (UGLL_DONT_INSERT);
}
- }
}
void
enable_watchpoints_after_interactive_call_stop (void)
{
- struct breakpoint *b;
-
- ALL_BREAKPOINTS (b)
- {
+ for (breakpoint *b : all_breakpoints ())
if (is_watchpoint (b) && b->enable_state == bp_call_disabled)
{
b->enable_state = bp_enabled;
update_global_location_list (UGLL_MAY_INSERT);
}
- }
}
void
@@ -8896,13 +8869,9 @@ static void
update_dprintf_commands (const char *args, int from_tty,
struct cmd_list_element *c)
{
- struct breakpoint *b;
-
- ALL_BREAKPOINTS (b)
- {
- if (b->type == bp_dprintf)
+ for (breakpoint *b : all_breakpoints ())
+ if (b->type == bp_dprintf)
update_dprintf_command_list (b);
- }
}
/* Create a breakpoint with SAL as location. Use LOCATION
@@ -11496,7 +11465,6 @@ compare_breakpoints (const breakpoint *a, const breakpoint *b)
static void
clear_command (const char *arg, int from_tty)
{
- struct breakpoint *b;
int default_match;
std::vector<symtab_and_line> decoded_sals;
@@ -11567,7 +11535,7 @@ clear_command (const char *arg, int from_tty)
? NULL : symtab_to_fullname (sal.symtab));
/* Find all matching breakpoints and add them to 'found'. */
- ALL_BREAKPOINTS (b)
+ for (breakpoint *b : all_breakpoints ())
{
int match = 0;
/* Are we going to delete b? */
@@ -11893,7 +11861,6 @@ force_breakpoint_reinsertion (struct bp_location *bl)
static void
update_global_location_list (enum ugll_insert_mode insert_mode)
{
- struct breakpoint *b;
struct bp_location **locp, *loc;
/* Last breakpoint location address that was marked for update. */
CORE_ADDR last_addr = 0;
@@ -11921,13 +11888,13 @@ update_global_location_list (enum ugll_insert_mode insert_mode)
bp_locations = NULL;
bp_locations_count = 0;
- ALL_BREAKPOINTS (b)
+ for (breakpoint *b : all_breakpoints ())
for (loc = b->loc; loc; loc = loc->next)
bp_locations_count++;
bp_locations = XNEWVEC (struct bp_location *, bp_locations_count);
locp = bp_locations;
- ALL_BREAKPOINTS (b)
+ for (breakpoint *b : all_breakpoints ())
for (loc = b->loc; loc; loc = loc->next)
*locp++ = loc;
@@ -12188,7 +12155,7 @@ update_global_location_list (enum ugll_insert_mode insert_mode)
/* ALL_BP_LOCATIONS bp_location has LOC->OWNER always
non-NULL. */
struct bp_location **loc_first_p;
- b = loc->owner;
+ breakpoint *b = loc->owner;
if (!unduplicated_should_be_inserted (loc)
|| !bl_address_is_meaningful (loc)
@@ -13283,8 +13250,6 @@ strace_marker_p (struct breakpoint *b)
void
delete_breakpoint (struct breakpoint *bpt)
{
- struct breakpoint *b;
-
gdb_assert (bpt != NULL);
/* Has this bp already been deleted? This can happen because
@@ -13339,7 +13304,7 @@ delete_breakpoint (struct breakpoint *bpt)
if (breakpoint_chain == bpt)
breakpoint_chain = bpt->next;
- ALL_BREAKPOINTS (b)
+ for (breakpoint *b : all_breakpoints ())
if (b->next == bpt)
{
b->next = bpt->next;
@@ -13410,7 +13375,7 @@ iterate_over_related_breakpoints (struct breakpoint *b,
static void
delete_command (const char *arg, int from_tty)
{
- struct breakpoint *b, *b_tmp;
+ breakpoint *b_tmp;
dont_repeat ();
@@ -13421,7 +13386,7 @@ delete_command (const char *arg, int from_tty)
/* Delete all breakpoints if no argument. Do not delete
internal breakpoints, these have to be deleted with an
explicit breakpoint number argument. */
- ALL_BREAKPOINTS (b)
+ for (breakpoint *b : all_breakpoints ())
if (user_breakpoint_p (b))
{
breaks_to_delete = 1;
@@ -13432,6 +13397,8 @@ delete_command (const char *arg, int from_tty)
if (!from_tty
|| (breaks_to_delete && query (_("Delete all breakpoints? "))))
{
+ breakpoint *b;
+
ALL_BREAKPOINTS_SAFE (b, b_tmp)
if (user_breakpoint_p (b))
delete_breakpoint (b);
@@ -14084,12 +14051,10 @@ breakpoint_re_set_thread (struct breakpoint *b)
void
set_ignore_count (int bptnum, int count, int from_tty)
{
- struct breakpoint *b;
-
if (count < 0)
count = 0;
- ALL_BREAKPOINTS (b)
+ for (breakpoint *b : all_breakpoints ())
if (b->number == bptnum)
{
if (is_tracepoint (b))
@@ -14205,13 +14170,7 @@ map_breakpoint_numbers (const char *args,
static struct bp_location *
find_location_by_number (int bp_num, int loc_num)
{
- struct breakpoint *b;
-
- ALL_BREAKPOINTS (b)
- if (b->number == bp_num)
- {
- break;
- }
+ breakpoint *b = get_breakpoint (bp_num);
if (!b || b->number != bp_num)
error (_("Bad breakpoint number '%d'"), bp_num);
@@ -14437,9 +14396,7 @@ enable_disable_command (const char *args, int from_tty, bool enable)
{
if (args == 0)
{
- struct breakpoint *bpt;
-
- ALL_BREAKPOINTS (bpt)
+ for (breakpoint *bpt : all_breakpoints ())
if (user_breakpoint_p (bpt))
{
if (enable)
@@ -14628,9 +14585,7 @@ invalidate_bp_value_on_memory_change (struct inferior *inferior,
CORE_ADDR addr, ssize_t len,
const bfd_byte *data)
{
- struct breakpoint *bp;
-
- ALL_BREAKPOINTS (bp)
+ for (breakpoint *bp : all_breakpoints ())
if (bp->enable_state == bp_enabled
&& bp->type == bp_hardware_watchpoint)
{
@@ -14726,9 +14681,7 @@ int
single_step_breakpoint_inserted_here_p (const address_space *aspace,
CORE_ADDR pc)
{
- struct breakpoint *bpt;
-
- ALL_BREAKPOINTS (bpt)
+ for (breakpoint *bpt : all_breakpoints ())
{
if (bpt->type == bp_single_step
&& breakpoint_has_location_inserted_here (bpt, aspace, pc))
@@ -15169,7 +15122,6 @@ static void
save_breakpoints (const char *filename, int from_tty,
bool (*filter) (const struct breakpoint *))
{
- struct breakpoint *tp;
int any = 0;
int extra_trace_bits = 0;
@@ -15177,7 +15129,7 @@ save_breakpoints (const char *filename, int from_tty,
error (_("Argument required (file name in which to save)"));
/* See if we have anything to save. */
- ALL_BREAKPOINTS (tp)
+ for (breakpoint *tp : all_breakpoints ())
{
/* Skip internal and momentary breakpoints. */
if (!user_breakpoint_p (tp))
@@ -15215,7 +15167,7 @@ save_breakpoints (const char *filename, int from_tty,
if (extra_trace_bits)
save_trace_state_variables (&fp);
- ALL_BREAKPOINTS (tp)
+ for (breakpoint *tp : all_breakpoints ())
{
/* Skip internal and momentary breakpoints. */
if (!user_breakpoint_p (tp))
@@ -15432,10 +15384,9 @@ int
pc_at_non_inline_function (const address_space *aspace, CORE_ADDR pc,
const struct target_waitstatus *ws)
{
- struct breakpoint *b;
struct bp_location *bl;
- ALL_BREAKPOINTS (b)
+ for (breakpoint *b : all_breakpoints ())
{
if (!is_non_inline_function (b))
continue;
--
2.31.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 2/9] gdb: add all_breakpoints_safe function
2021-05-27 15:35 [PATCH 0/9] Convert breakpoint iteration macros to ranges Simon Marchi
2021-05-27 15:35 ` [PATCH 1/9] gdb: add all_breakpoints function Simon Marchi
@ 2021-05-27 15:35 ` Simon Marchi
2021-05-27 17:35 ` Tom Tromey
2021-05-27 15:35 ` [PATCH 3/9] gdb: add all_tracepoints function Simon Marchi
` (7 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: Simon Marchi @ 2021-05-27 15:35 UTC (permalink / raw)
To: gdb-patches
Same as the previous patch, but intended to replace the
ALL_BREAKPOINTS_SAFE macro, which allows deleting the current breakpoint
while iterating. The new range type simply wraps the range added by the
previous patch with basic_safe_range.
I didn't remove the ALL_BREAKPOINTS_SAFE macro, because there is one
spot where it's more tricky to remove, in the
check_longjmp_breakpoint_for_call_dummy function. More thought it
needed for this one.
gdb/ChangeLog:
* breakpoint.c (breakpoint_safe_range): New.
(all_breakpoints_safe): New. Use instead of
ALL_BREAKPOINTS_SAFE where possible.
Change-Id: Ifccab29f135e1f85700e3697ed60f0b643c7682f
---
gdb/breakpoint.c | 109 ++++++++++++++++++-----------------------------
1 file changed, 41 insertions(+), 68 deletions(-)
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index b6c32db2ec02..06012c7374ab 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -540,6 +540,20 @@ all_breakpoints ()
return breakpoint_range (breakpoint_chain);
}
+/* Breakpoint linked list range, safe against deletion of the current
+ breakpoint while iterating. */
+
+using breakpoint_safe_range = basic_safe_range<breakpoint_range>;
+
+/* Return a range to iterate over all breakpoints. This range is safe against
+ deletion of the current breakpoint while iterating. */
+
+static breakpoint_safe_range
+all_breakpoints_safe ()
+{
+ return breakpoint_safe_range (all_breakpoints ());
+}
+
/* Array is sorted by bp_location_is_less_than - primarily by the ADDRESS. */
static struct bp_location **bp_locations;
@@ -2889,15 +2903,12 @@ of catchpoint."), bl->owner->number);
void
breakpoint_program_space_exit (struct program_space *pspace)
{
- struct breakpoint *b, *b_temp;
struct bp_location *loc, **loc_temp;
/* Remove any breakpoint that was set through this program space. */
- ALL_BREAKPOINTS_SAFE (b, b_temp)
- {
- if (b->pspace == pspace)
- delete_breakpoint (b);
- }
+ for (breakpoint *b : all_breakpoints_safe ())
+ if (b->pspace == pspace)
+ delete_breakpoint (b);
/* Breakpoints set through other program spaces could have locations
bound to PSPACE as well. Remove those. */
@@ -3143,9 +3154,7 @@ remove_breakpoints (void)
static void
remove_threaded_breakpoints (struct thread_info *tp, int silent)
{
- struct breakpoint *b, *b_tmp;
-
- ALL_BREAKPOINTS_SAFE (b, b_tmp)
+ for (breakpoint *b : all_breakpoints ())
{
if (b->thread == tp->global_num && user_breakpoint_p (b))
{
@@ -3653,7 +3662,6 @@ breakpoint_event_location_empty_p (const struct breakpoint *b)
void
update_breakpoints_after_exec (void)
{
- struct breakpoint *b, *b_tmp;
struct bp_location *bploc, **bplocp_tmp;
/* We're about to delete breakpoints from GDB's lists. If the
@@ -3668,7 +3676,7 @@ update_breakpoints_after_exec (void)
if (bploc->pspace == current_program_space)
gdb_assert (!bploc->inserted);
- ALL_BREAKPOINTS_SAFE (b, b_tmp)
+ for (breakpoint *b : all_breakpoints_safe ())
{
if (b->pspace != current_program_space)
continue;
@@ -4002,7 +4010,6 @@ mark_breakpoints_out (void)
void
breakpoint_init_inferior (enum inf_context context)
{
- struct breakpoint *b, *b_tmp;
struct program_space *pspace = current_program_space;
/* If breakpoint locations are shared across processes, then there's
@@ -4012,7 +4019,7 @@ breakpoint_init_inferior (enum inf_context context)
mark_breakpoints_out ();
- ALL_BREAKPOINTS_SAFE (b, b_tmp)
+ for (breakpoint *b : all_breakpoints_safe ())
{
if (b->loc && b->loc->pspace != pspace)
continue;
@@ -7354,14 +7361,13 @@ set_raw_breakpoint (struct gdbarch *gdbarch,
void
set_longjmp_breakpoint (struct thread_info *tp, struct frame_id frame)
{
- struct breakpoint *b, *b_tmp;
int thread = tp->global_num;
/* To avoid having to rescan all objfile symbols at every step,
we maintain a list of continually-inserted but always disabled
longjmp "master" breakpoints. Here, we simply create momentary
clones of those and enable them for the requested thread. */
- ALL_BREAKPOINTS_SAFE (b, b_tmp)
+ for (breakpoint *b : all_breakpoints_safe ())
if (b->pspace == current_program_space
&& (b->type == bp_longjmp_master
|| b->type == bp_exception_master))
@@ -7383,9 +7389,7 @@ set_longjmp_breakpoint (struct thread_info *tp, struct frame_id frame)
void
delete_longjmp_breakpoint (int thread)
{
- struct breakpoint *b, *b_tmp;
-
- ALL_BREAKPOINTS_SAFE (b, b_tmp)
+ for (breakpoint *b : all_breakpoints_safe ())
if (b->type == bp_longjmp || b->type == bp_exception)
{
if (b->thread == thread)
@@ -7396,9 +7400,7 @@ delete_longjmp_breakpoint (int thread)
void
delete_longjmp_breakpoint_at_next_stop (int thread)
{
- struct breakpoint *b, *b_tmp;
-
- ALL_BREAKPOINTS_SAFE (b, b_tmp)
+ for (breakpoint *b : all_breakpoints_safe ())
if (b->type == bp_longjmp || b->type == bp_exception)
{
if (b->thread == thread)
@@ -7505,9 +7507,7 @@ disable_overlay_breakpoints (void)
void
set_std_terminate_breakpoint (void)
{
- struct breakpoint *b, *b_tmp;
-
- ALL_BREAKPOINTS_SAFE (b, b_tmp)
+ for (breakpoint *b : all_breakpoints_safe ())
if (b->pspace == current_program_space
&& b->type == bp_std_terminate_master)
{
@@ -7520,9 +7520,7 @@ set_std_terminate_breakpoint (void)
void
delete_std_terminate_breakpoint (void)
{
- struct breakpoint *b, *b_tmp;
-
- ALL_BREAKPOINTS_SAFE (b, b_tmp)
+ for (breakpoint *b : all_breakpoints_safe ())
if (b->type == bp_std_terminate)
delete_breakpoint (b);
}
@@ -7564,9 +7562,7 @@ create_jit_event_breakpoint (struct gdbarch *gdbarch, CORE_ADDR address)
void
remove_jit_event_breakpoints (void)
{
- struct breakpoint *b, *b_tmp;
-
- ALL_BREAKPOINTS_SAFE (b, b_tmp)
+ for (breakpoint *b : all_breakpoints_safe ())
if (b->type == bp_jit_event
&& b->loc->pspace == current_program_space)
delete_breakpoint (b);
@@ -7575,9 +7571,7 @@ remove_jit_event_breakpoints (void)
void
remove_solib_event_breakpoints (void)
{
- struct breakpoint *b, *b_tmp;
-
- ALL_BREAKPOINTS_SAFE (b, b_tmp)
+ for (breakpoint *b : all_breakpoints_safe ())
if (b->type == bp_shlib_event
&& b->loc->pspace == current_program_space)
delete_breakpoint (b);
@@ -7588,9 +7582,7 @@ remove_solib_event_breakpoints (void)
void
remove_solib_event_breakpoints_at_next_stop (void)
{
- struct breakpoint *b, *b_tmp;
-
- ALL_BREAKPOINTS_SAFE (b, b_tmp)
+ for (breakpoint *b : all_breakpoints_safe ())
if (b->type == bp_shlib_event
&& b->loc->pspace == current_program_space)
b->disposition = disp_del_at_next_stop;
@@ -11625,19 +11617,15 @@ clear_command (const char *arg, int from_tty)
void
breakpoint_auto_delete (bpstat bs)
{
- struct breakpoint *b, *b_tmp;
-
for (; bs; bs = bs->next)
if (bs->breakpoint_at
&& bs->breakpoint_at->disposition == disp_del
&& bs->stop)
delete_breakpoint (bs->breakpoint_at);
- ALL_BREAKPOINTS_SAFE (b, b_tmp)
- {
+ for (breakpoint *b : all_breakpoints_safe ())
if (b->disposition == disp_del_at_next_stop)
delete_breakpoint (b);
- }
}
/* A comparison function for bp_location AP and BP being interfaced to
@@ -13375,8 +13363,6 @@ iterate_over_related_breakpoints (struct breakpoint *b,
static void
delete_command (const char *arg, int from_tty)
{
- breakpoint *b_tmp;
-
dont_repeat ();
if (arg == 0)
@@ -13396,13 +13382,9 @@ delete_command (const char *arg, int from_tty)
/* Ask user only if there are some breakpoints to delete. */
if (!from_tty
|| (breaks_to_delete && query (_("Delete all breakpoints? "))))
- {
- breakpoint *b;
-
- ALL_BREAKPOINTS_SAFE (b, b_tmp)
- if (user_breakpoint_p (b))
- delete_breakpoint (b);
- }
+ for (breakpoint *b : all_breakpoints_safe ())
+ if (user_breakpoint_p (b))
+ delete_breakpoint (b);
}
else
map_breakpoint_numbers
@@ -13974,8 +13956,6 @@ breakpoint_re_set_one (breakpoint *b)
void
breakpoint_re_set (void)
{
- struct breakpoint *b, *b_tmp;
-
{
scoped_restore_current_language save_language;
scoped_restore save_input_radix = make_scoped_restore (&input_radix);
@@ -13999,7 +13979,7 @@ breakpoint_re_set (void)
breakpoint 1, we'd insert the locations of breakpoint 2, which
hadn't been re-set yet, and thus may have stale locations. */
- ALL_BREAKPOINTS_SAFE (b, b_tmp)
+ for (breakpoint *b : all_breakpoints_safe ())
{
try
{
@@ -14126,13 +14106,11 @@ map_breakpoint_number_range (std::pair<int, int> bp_num_range,
}
else
{
- struct breakpoint *b, *tmp;
-
for (int i = bp_num_range.first; i <= bp_num_range.second; i++)
{
bool match = false;
- ALL_BREAKPOINTS_SAFE (b, tmp)
+ for (breakpoint *b : all_breakpoints_safe ())
if (b->number == i)
{
match = true;
@@ -14927,20 +14905,19 @@ disable_trace_command (const char *args, int from_tty)
static void
delete_trace_command (const char *arg, int from_tty)
{
- struct breakpoint *b, *b_tmp;
-
dont_repeat ();
if (arg == 0)
{
int breaks_to_delete = 0;
+ breakpoint *tp;
/* Delete all breakpoints if no argument.
Do not delete internal or call-dummy breakpoints, these
have to be deleted with an explicit breakpoint number
argument. */
- ALL_TRACEPOINTS (b)
- if (is_tracepoint (b) && user_breakpoint_p (b))
+ ALL_TRACEPOINTS (tp)
+ if (is_tracepoint (tp) && user_breakpoint_p (tp))
{
breaks_to_delete = 1;
break;
@@ -14950,7 +14927,7 @@ delete_trace_command (const char *arg, int from_tty)
if (!from_tty
|| (breaks_to_delete && query (_("Delete all tracepoints? "))))
{
- ALL_BREAKPOINTS_SAFE (b, b_tmp)
+ for (breakpoint *b : all_breakpoints_safe ())
if (is_tracepoint (b) && user_breakpoint_p (b))
delete_breakpoint (b);
}
@@ -15352,13 +15329,9 @@ add_catch_command (const char *name, const char *docstring,
struct breakpoint *
iterate_over_breakpoints (gdb::function_view<bool (breakpoint *)> callback)
{
- struct breakpoint *b, *b_tmp;
-
- ALL_BREAKPOINTS_SAFE (b, b_tmp)
- {
- if (callback (b))
- return b;
- }
+ for (breakpoint *b : all_breakpoints_safe ())
+ if (callback (b))
+ return b;
return NULL;
}
--
2.31.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 3/9] gdb: add all_tracepoints function
2021-05-27 15:35 [PATCH 0/9] Convert breakpoint iteration macros to ranges Simon Marchi
2021-05-27 15:35 ` [PATCH 1/9] gdb: add all_breakpoints function Simon Marchi
2021-05-27 15:35 ` [PATCH 2/9] gdb: add all_breakpoints_safe function Simon Marchi
@ 2021-05-27 15:35 ` Simon Marchi
2021-05-27 15:35 ` [PATCH 4/9] gdb: add breakpoint::locations method Simon Marchi
` (6 subsequent siblings)
9 siblings, 0 replies; 21+ messages in thread
From: Simon Marchi @ 2021-05-27 15:35 UTC (permalink / raw)
To: gdb-patches
Same idea as the previous patches, but to replace the ALL_TRACEPOINTS
macro. Define a new filtered_iterator that only keeps the breakpoints
for which is_tracepoint returns true (just like the macro did).
I would have like to make it so tracepoint_range yields some
`tracepoint *` instead of some `breakpoint *`, that would help simplify
the callers, who wouldn't have to do the cast themselves. But I didn't
find an obvious way to do it. It can always be added later.
It turns out there is already an all_tracepoints function, which returns
a vector containing all the breakpoints that are tracepoint. Remove it,
most users will just work seamlessly with the new function. The
exception is start_tracing, which iterated multiple times on the vector.
Adapt this one so it iterates multiple times on the returned range.
Since the existing users of all_tracepoints are outside of breakpoint.c,
this requires defining all_tracepoints and a few supporting types in
breakpoint.h. So, move breakpoint_iterator from breakpoint.c to
breakpoint.h.
gdb/ChangeLog:
* breakpoint.h (all_tracepoints): Remove.
(breakpoint_iterator): Move here.
(struct tracepoint_filter): New.
(tracepoint_iterator): New.
(tracepoint_range): New.
(all_tracepoints): New.
* breakpoint.c (ALL_TRACEPOINTS): Remove, replace all users with
all_tracepoints.
(breakpoint_iterator): Move to header.
(all_tracepoints): New.
* tracepoint.c (start_tracing): Adjust.
Change-Id: I76b1bba4215dbec7a03846c568368aeef7f1e05a
---
gdb/breakpoint.c | 55 ++++++++++++------------------------------------
gdb/breakpoint.h | 29 ++++++++++++++++++++++---
gdb/tracepoint.c | 8 +++----
3 files changed, 44 insertions(+), 48 deletions(-)
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 06012c7374ab..eb7fc553f9c4 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -514,20 +514,10 @@ bool target_exact_watchpoints = false;
&& (*BP_LOCP_TMP)->address == ADDRESS); \
BP_LOCP_TMP++)
-/* Iterator for tracepoints only. */
-
-#define ALL_TRACEPOINTS(B) \
- for (B = breakpoint_chain; B; B = B->next) \
- if (is_tracepoint (B))
-
/* Chains of all breakpoints defined. */
static struct breakpoint *breakpoint_chain;
-/* Breakpoint linked list iterator. */
-
-using breakpoint_iterator = next_iterator<breakpoint>;
-
/* Breakpoint linked list range. */
using breakpoint_range = next_adapter<breakpoint, breakpoint_iterator>;
@@ -554,6 +544,14 @@ all_breakpoints_safe ()
return breakpoint_safe_range (all_breakpoints ());
}
+/* See breakpoint.h. */
+
+tracepoint_range
+all_tracepoints ()
+{
+ return tracepoint_range (breakpoint_chain);
+}
+
/* Array is sorted by bp_location_is_less_than - primarily by the ADDRESS. */
static struct bp_location **bp_locations;
@@ -11714,12 +11712,11 @@ bp_locations_target_extensions_update (void)
static void
download_tracepoint_locations (void)
{
- struct breakpoint *b;
enum tribool can_download_tracepoint = TRIBOOL_UNKNOWN;
scoped_restore_current_pspace_and_thread restore_pspace_thread;
- ALL_TRACEPOINTS (b)
+ for (breakpoint *b : all_tracepoints ())
{
struct bp_location *bl;
struct tracepoint *t;
@@ -14910,13 +14907,12 @@ delete_trace_command (const char *arg, int from_tty)
if (arg == 0)
{
int breaks_to_delete = 0;
- breakpoint *tp;
/* Delete all breakpoints if no argument.
Do not delete internal or call-dummy breakpoints, these
have to be deleted with an explicit breakpoint number
argument. */
- ALL_TRACEPOINTS (tp)
+ for (breakpoint *tp : all_tracepoints ())
if (is_tracepoint (tp) && user_breakpoint_p (tp))
{
breaks_to_delete = 1;
@@ -14973,13 +14969,11 @@ trace_pass_command (const char *args, int from_tty)
args = skip_spaces (args);
if (*args && strncasecmp (args, "all", 3) == 0)
{
- struct breakpoint *b;
-
args += 3; /* Skip special argument "all". */
if (*args)
error (_("Junk at end of arguments."));
- ALL_TRACEPOINTS (b)
+ for (breakpoint *b : all_tracepoints ())
{
t1 = (struct tracepoint *) b;
trace_pass_set_count (t1, count, from_tty);
@@ -15006,9 +15000,7 @@ trace_pass_command (const char *args, int from_tty)
struct tracepoint *
get_tracepoint (int num)
{
- struct breakpoint *t;
-
- ALL_TRACEPOINTS (t)
+ for (breakpoint *t : all_tracepoints ())
if (t->number == num)
return (struct tracepoint *) t;
@@ -15022,9 +15014,7 @@ get_tracepoint (int num)
struct tracepoint *
get_tracepoint_by_number_on_target (int num)
{
- struct breakpoint *b;
-
- ALL_TRACEPOINTS (b)
+ for (breakpoint *b : all_tracepoints ())
{
struct tracepoint *t = (struct tracepoint *) b;
@@ -15044,7 +15034,6 @@ struct tracepoint *
get_tracepoint_by_number (const char **arg,
number_or_range_parser *parser)
{
- struct breakpoint *t;
int tpnum;
const char *instring = arg == NULL ? NULL : *arg;
@@ -15068,7 +15057,7 @@ get_tracepoint_by_number (const char **arg,
return NULL;
}
- ALL_TRACEPOINTS (t)
+ for (breakpoint *t : all_tracepoints ())
if (t->number == tpnum)
{
return (struct tracepoint *) t;
@@ -15225,22 +15214,6 @@ save_tracepoints_command (const char *args, int from_tty)
save_breakpoints (args, from_tty, is_tracepoint);
}
-/* Create a vector of all tracepoints. */
-
-std::vector<breakpoint *>
-all_tracepoints (void)
-{
- std::vector<breakpoint *> tp_vec;
- struct breakpoint *tp;
-
- ALL_TRACEPOINTS (tp)
- {
- tp_vec.push_back (tp);
- }
-
- return tp_vec;
-}
-
\f
/* This help string is used to consolidate all the help string for specifying
locations used by several commands. */
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 54c5e423e10a..5a10839603d7 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -28,6 +28,7 @@
#include "location.h"
#include <vector>
#include "gdbsupport/array-view.h"
+#include "gdbsupport/filtered-iterator.h"
#include "gdbsupport/function-view.h"
#include "gdbsupport/refcounted-object.h"
#include "cli/cli-script.h"
@@ -1683,9 +1684,6 @@ extern struct tracepoint *
get_tracepoint_by_number (const char **arg,
number_or_range_parser *parser);
-/* Return a vector of all tracepoints currently defined. */
-extern std::vector<breakpoint *> all_tracepoints (void);
-
/* Return true if B is of tracepoint kind. */
extern bool is_tracepoint (const struct breakpoint *b);
@@ -1717,6 +1715,31 @@ class scoped_rbreak_breakpoints
extern struct breakpoint *iterate_over_breakpoints
(gdb::function_view<bool (breakpoint *)>);
+/* Breakpoint linked list iterator. */
+
+using breakpoint_iterator = next_iterator<breakpoint>;
+
+/* Breakpoint filter to only keep tracepoints. */
+
+struct tracepoint_filter
+{
+ bool operator() (breakpoint *b)
+ { return is_tracepoint (b); }
+};
+
+/* Breakpoint linked list iterator, filtering to only keep tracepoints. */
+
+using tracepoint_iterator
+ = filtered_iterator<breakpoint_iterator, tracepoint_filter>;
+
+/* Breakpoint linked list range, filtering to only keep tracepoints. */
+
+using tracepoint_range = next_adapter<breakpoint, tracepoint_iterator>;
+
+/* Return a range to iterate over all tracepoints. */
+
+tracepoint_range all_tracepoints ();
+
/* Nonzero if the specified PC cannot be a location where functions
have been inlined. */
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index da0e976c8694..863fb6d34d8d 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -1603,13 +1603,13 @@ start_tracing (const char *notes)
int any_enabled = 0, num_to_download = 0;
int ret;
- std::vector<breakpoint *> tp_vec = all_tracepoints ();
+ auto tracepoint_range = all_tracepoints ();
/* No point in tracing without any tracepoints... */
- if (tp_vec.empty ())
+ if (tracepoint_range.begin () == tracepoint_range.end ())
error (_("No tracepoints defined, not starting trace"));
- for (breakpoint *b : tp_vec)
+ for (breakpoint *b : tracepoint_range)
{
if (b->enable_state == bp_enabled)
any_enabled = 1;
@@ -1640,7 +1640,7 @@ start_tracing (const char *notes)
target_trace_init ();
- for (breakpoint *b : tp_vec)
+ for (breakpoint *b : tracepoint_range)
{
struct tracepoint *t = (struct tracepoint *) b;
struct bp_location *loc;
--
2.31.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 4/9] gdb: add breakpoint::locations method
2021-05-27 15:35 [PATCH 0/9] Convert breakpoint iteration macros to ranges Simon Marchi
` (2 preceding siblings ...)
2021-05-27 15:35 ` [PATCH 3/9] gdb: add all_tracepoints function Simon Marchi
@ 2021-05-27 15:35 ` Simon Marchi
2021-05-27 15:35 ` [PATCH 5/9] gdb: make bp_locations an std::vector Simon Marchi
` (5 subsequent siblings)
9 siblings, 0 replies; 21+ messages in thread
From: Simon Marchi @ 2021-05-27 15:35 UTC (permalink / raw)
To: gdb-patches
Add the breakpoint::locations method, which returns a range that can be
used to iterate over a breakpoint's locations. This shortens
for (bp_location *loc = b->loc; loc != nullptr; loc = loc->next)
into
for (bp_location *loc : b->locations ())
Change all the places that I found that could use it.
gdb/ChangeLog:
* breakpoint.h (bp_locations_range): New.
(struct breakpoint) <locations>: New. Use where possible.
Change-Id: I1ba2f7d93d57e544e1f8609124587dcf2e1da037
---
gdb/ada-lang.c | 4 +-
gdb/breakpoint.c | 132 ++++++++++++++++++----------------------
gdb/breakpoint.h | 7 +++
gdb/jit.c | 2 +-
gdb/remote.c | 3 +-
gdb/solib-svr4.c | 4 +-
gdb/tracepoint.c | 23 +++----
gdb/tui/tui-winsource.c | 4 +-
8 files changed, 78 insertions(+), 101 deletions(-)
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 1b5067662593..f0c1aa2c8a67 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -11552,8 +11552,6 @@ static void
create_excep_cond_exprs (struct ada_catchpoint *c,
enum ada_exception_catchpoint_kind ex)
{
- struct bp_location *bl;
-
/* Nothing to do if there's no specific exception to catch. */
if (c->excep_string.empty ())
return;
@@ -11569,7 +11567,7 @@ create_excep_cond_exprs (struct ada_catchpoint *c,
/* Iterate over all the catchpoint's locations, and parse an
expression for each. */
- for (bl = c->loc; bl != NULL; bl = bl->next)
+ for (bp_location *bl : c->locations ())
{
struct ada_catchpoint_location *ada_loc
= (struct ada_catchpoint_location *) bl;
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index eb7fc553f9c4..1d33262520a0 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -680,8 +680,6 @@ get_breakpoint (int num)
static void
mark_breakpoint_modified (struct breakpoint *b)
{
- struct bp_location *loc;
-
/* This is only meaningful if the target is
evaluating conditions and if the user has
opted for condition evaluation on the target's
@@ -693,7 +691,7 @@ mark_breakpoint_modified (struct breakpoint *b)
if (!is_breakpoint (b))
return;
- for (loc = b->loc; loc; loc = loc->next)
+ for (bp_location *loc : b->locations ())
loc->condition_changed = condition_modified;
}
@@ -910,7 +908,7 @@ set_breakpoint_condition (struct breakpoint *b, const char *exp,
else
{
int loc_num = 1;
- for (bp_location *loc = b->loc; loc != nullptr; loc = loc->next)
+ for (bp_location *loc : b->locations ())
{
loc->cond.reset ();
if (loc->disabled_by_cond && loc->enabled)
@@ -952,7 +950,7 @@ set_breakpoint_condition (struct breakpoint *b, const char *exp,
the error and the condition string will be rejected.
This two-pass approach is taken to avoid setting the
state of locations in case of a reject. */
- for (bp_location *loc = b->loc; loc != nullptr; loc = loc->next)
+ for (bp_location *loc : b->locations ())
{
try
{
@@ -975,9 +973,11 @@ set_breakpoint_condition (struct breakpoint *b, const char *exp,
/* If we reach here, the condition is valid at some locations. */
int loc_num = 1;
- for (bp_location *loc = b->loc; loc != nullptr;
- loc = loc->next, loc_num++)
- set_breakpoint_location_condition (exp, loc, b->number, loc_num);
+ for (bp_location *loc : b->locations ())
+ {
+ set_breakpoint_location_condition (exp, loc, b->number, loc_num);
+ loc_num++;
+ }
}
/* We know that the new condition parsed successfully. The
@@ -1287,12 +1287,11 @@ std::vector<breakpoint *>
static_tracepoints_here (CORE_ADDR addr)
{
std::vector<breakpoint *> found;
- struct bp_location *loc;
for (breakpoint *b : all_breakpoints ())
if (b->type == bp_static_tracepoint)
{
- for (loc = b->loc; loc; loc = loc->next)
+ for (bp_location *loc : b->locations ())
if (loc->address == addr)
found.push_back (b);
}
@@ -2032,7 +2031,6 @@ update_watchpoint (struct watchpoint *b, int reparse)
{
int reg_cnt;
enum bp_loc_type loc_type;
- struct bp_location *bl;
reg_cnt = can_use_hardware_watchpoint (val_chain);
@@ -2108,7 +2106,7 @@ update_watchpoint (struct watchpoint *b, int reparse)
loc_type = (b->type == bp_watchpoint? bp_loc_other
: bp_loc_hardware_watchpoint);
- for (bl = b->loc; bl; bl = bl->next)
+ for (bp_location *bl : b->locations ())
bl->loc_type = loc_type;
}
@@ -3083,7 +3081,6 @@ insert_breakpoint_locations (void)
for (breakpoint *bpt : all_breakpoints ())
{
int some_failed = 0;
- struct bp_location *loc;
if (!is_hardware_watchpoint (bpt))
continue;
@@ -3094,15 +3091,16 @@ insert_breakpoint_locations (void)
if (bpt->disposition == disp_del_at_next_stop)
continue;
- for (loc = bpt->loc; loc; loc = loc->next)
+ for (bp_location *loc : bpt->locations ())
if (!loc->inserted && should_be_inserted (loc))
{
some_failed = 1;
break;
}
+
if (some_failed)
{
- for (loc = bpt->loc; loc; loc = loc->next)
+ for (bp_location *loc : bpt->locations ())
if (loc->inserted)
remove_breakpoint (loc);
@@ -4287,8 +4285,6 @@ hardware_watchpoint_inserted_in_range (const address_space *aspace,
{
for (breakpoint *bpt : all_breakpoints ())
{
- struct bp_location *loc;
-
if (bpt->type != bp_hardware_watchpoint
&& bpt->type != bp_access_watchpoint)
continue;
@@ -4296,7 +4292,7 @@ hardware_watchpoint_inserted_in_range (const address_space *aspace,
if (!breakpoint_enabled (bpt))
continue;
- for (loc = bpt->loc; loc; loc = loc->next)
+ for (bp_location *loc : bpt->locations ())
if (loc->pspace->aspace == aspace && loc->inserted)
{
CORE_ADDR l, h;
@@ -4903,10 +4899,9 @@ watchpoints_triggered (struct target_waitstatus *ws)
if (is_hardware_watchpoint (b))
{
struct watchpoint *w = (struct watchpoint *) b;
- struct bp_location *loc;
w->watchpoint_triggered = watch_triggered_no;
- for (loc = b->loc; loc; loc = loc->next)
+ for (bp_location *loc : b->locations ())
{
if (is_masked_watchpoint (b))
{
@@ -5448,7 +5443,7 @@ build_bpstat_chain (const address_space *aspace, CORE_ADDR bp_addr,
if (!breakpoint_enabled (b))
continue;
- for (bp_location *bl = b->loc; bl != NULL; bl = bl->next)
+ for (bp_location *bl : b->locations ())
{
/* For hardware watchpoints, we look only at the first
location. The watchpoint_check function will work on the
@@ -5918,7 +5913,6 @@ wrap_indent_at_field (struct ui_out *uiout, const char *col_name)
static const char *
bp_condition_evaluator (struct breakpoint *b)
{
- struct bp_location *bl;
char host_evals = 0;
char target_evals = 0;
@@ -5932,7 +5926,7 @@ bp_condition_evaluator (struct breakpoint *b)
|| !target_supports_evaluation_of_breakpoint_conditions ())
return condition_evaluation_host;
- for (bl = b->loc; bl; bl = bl->next)
+ for (bp_location *bl : b->locations ())
{
if (bl->cond_bytecode)
target_evals++;
@@ -6516,11 +6510,12 @@ print_one_breakpoint (struct breakpoint *b,
locations_list.emplace (uiout, "locations");
int n = 1;
- for (bp_location *loc = b->loc; loc != NULL; loc = loc->next, ++n)
+ for (bp_location *loc : b->locations ())
{
ui_out_emit_tuple loc_tuple_emitter (uiout, NULL);
print_one_breakpoint_location (b, loc, n, last_loc,
allflag, allflag);
+ n++;
}
}
}
@@ -6530,14 +6525,13 @@ static int
breakpoint_address_bits (struct breakpoint *b)
{
int print_address_bits = 0;
- struct bp_location *loc;
/* Software watchpoints that aren't watching memory don't have an
address to print. */
if (is_no_memory_software_watchpoint (b))
return 0;
- for (loc = b->loc; loc; loc = loc->next)
+ for (bp_location *loc : b->locations ())
{
int addr_bit;
@@ -6701,7 +6695,7 @@ breakpoint_1 (const char *bp_num_list, bool show_internal,
if (show_internal || user_breakpoint_p (b))
{
print_one_breakpoint (b, &last_loc, show_internal);
- for (bp_location *loc = b->loc; loc != NULL; loc = loc->next)
+ for (bp_location *loc : b->locations ())
if (loc->disabled_by_cond)
has_disabled_by_cond_location = true;
}
@@ -6795,9 +6789,7 @@ breakpoint_has_pc (struct breakpoint *b,
struct program_space *pspace,
CORE_ADDR pc, struct obj_section *section)
{
- struct bp_location *bl = b->loc;
-
- for (; bl; bl = bl->next)
+ for (bp_location *bl : b->locations ())
{
if (bl->pspace == pspace
&& bl->address == pc
@@ -7731,13 +7723,12 @@ disable_breakpoints_in_freed_objfile (struct objfile *objfile)
for (breakpoint *b : all_breakpoints ())
{
- struct bp_location *loc;
int bp_modified = 0;
if (!is_breakpoint (b) && !is_tracepoint (b))
continue;
- for (loc = b->loc; loc != NULL; loc = loc->next)
+ for (bp_location *loc : b->locations ())
{
CORE_ADDR loc_addr = loc->address;
@@ -8067,8 +8058,6 @@ breakpoint_hit_catch_solib (const struct bp_location *bl,
for (breakpoint *other : all_breakpoints ())
{
- struct bp_location *other_bl;
-
if (other == bl->owner)
continue;
@@ -8078,7 +8067,7 @@ breakpoint_hit_catch_solib (const struct bp_location *bl,
if (self->pspace != NULL && other->pspace != self->pspace)
continue;
- for (other_bl = other->loc; other_bl != NULL; other_bl = other_bl->next)
+ for (bp_location *other_bl : other->locations ())
{
if (other->ops->breakpoint_hit (other_bl, aspace, bp_addr, ws))
return 1;
@@ -8426,11 +8415,10 @@ static int
hw_breakpoint_used_count (void)
{
int i = 0;
- struct bp_location *bl;
for (breakpoint *b : all_breakpoints ())
if (b->type == bp_hardware_breakpoint && breakpoint_enabled (b))
- for (bl = b->loc; bl; bl = bl->next)
+ for (bp_location *bl : b->locations ())
{
/* Special types of hardware breakpoints may use more than
one register. */
@@ -8447,12 +8435,11 @@ static int
hw_watchpoint_use_count (struct breakpoint *b)
{
int i = 0;
- struct bp_location *bl;
if (!breakpoint_enabled (b))
return 0;
- for (bl = b->loc; bl; bl = bl->next)
+ for (bp_location *bl : b->locations ())
{
/* Special types of hardware watchpoints may use more than
one register. */
@@ -8995,7 +8982,7 @@ init_breakpoint_sal (struct breakpoint *b, struct gdbarch *gdbarch,
/* The order of the locations is now stable. Set the location
condition using the location's number. */
int loc_num = 1;
- for (bp_location *loc = b->loc; loc != nullptr; loc = loc->next)
+ for (bp_location *loc : b->locations ())
{
if (b->cond_string != nullptr)
set_breakpoint_location_condition (b->cond_string, loc, b->number,
@@ -11531,8 +11518,7 @@ clear_command (const char *arg, int from_tty)
/* Are we going to delete b? */
if (b->type != bp_none && !is_watchpoint (b))
{
- struct bp_location *loc = b->loc;
- for (; loc; loc = loc->next)
+ for (bp_location *loc : b->locations ())
{
/* If the user specified file:line, don't allow a PC
match. This matches historical gdb behavior. */
@@ -11718,7 +11704,6 @@ download_tracepoint_locations (void)
for (breakpoint *b : all_tracepoints ())
{
- struct bp_location *bl;
struct tracepoint *t;
int bp_location_downloaded = 0;
@@ -11738,7 +11723,7 @@ download_tracepoint_locations (void)
if (can_download_tracepoint == TRIBOOL_FALSE)
break;
- for (bl = b->loc; bl; bl = bl->next)
+ for (bp_location *bl : b->locations ())
{
/* In tracepoint, locations are _never_ duplicated, so
should_be_inserted is equivalent to
@@ -11846,7 +11831,7 @@ force_breakpoint_reinsertion (struct bp_location *bl)
static void
update_global_location_list (enum ugll_insert_mode insert_mode)
{
- struct bp_location **locp, *loc;
+ struct bp_location **locp;
/* Last breakpoint location address that was marked for update. */
CORE_ADDR last_addr = 0;
/* Last breakpoint location program space that was marked for update. */
@@ -11874,13 +11859,13 @@ update_global_location_list (enum ugll_insert_mode insert_mode)
bp_locations_count = 0;
for (breakpoint *b : all_breakpoints ())
- for (loc = b->loc; loc; loc = loc->next)
+ for (bp_location *loc ATTRIBUTE_UNUSED : b->locations ())
bp_locations_count++;
bp_locations = XNEWVEC (struct bp_location *, bp_locations_count);
locp = bp_locations;
for (breakpoint *b : all_breakpoints ())
- for (loc = b->loc; loc; loc = loc->next)
+ for (bp_location *loc : b->locations ())
*locp++ = loc;
/* See if we need to "upgrade" a software breakpoint to a hardware
@@ -11891,7 +11876,7 @@ update_global_location_list (enum ugll_insert_mode insert_mode)
locp < bp_locations + bp_locations_count;
locp++)
{
- loc = *locp;
+ bp_location *loc = *locp;
if (!loc->inserted && should_be_inserted (loc))
handle_automatic_hardware_breakpoints (loc);
}
@@ -12135,6 +12120,8 @@ update_global_location_list (enum ugll_insert_mode insert_mode)
wp_loc_first = NULL;
awp_loc_first = NULL;
rwp_loc_first = NULL;
+
+ bp_location *loc;
ALL_BP_LOCATIONS (loc, locp)
{
/* ALL_BP_LOCATIONS bp_location has LOC->OWNER always
@@ -12352,6 +12339,13 @@ breakpoint::~breakpoint ()
xfree (this->extra_string);
}
+/* See breakpoint.h. */
+
+bp_locations_range breakpoint::locations ()
+{
+ return bp_locations_range (this->loc);
+}
+
static struct bp_location *
base_breakpoint_allocate_location (struct breakpoint *self)
{
@@ -13398,9 +13392,7 @@ delete_command (const char *arg, int from_tty)
static int
all_locations_are_pending (struct breakpoint *b, struct program_space *pspace)
{
- struct bp_location *loc;
-
- for (loc = b->loc; loc != NULL; loc = loc->next)
+ for (bp_location *loc : b->locations ())
if ((pspace == NULL
|| loc->pspace == pspace)
&& !loc->shlib_disabled
@@ -13719,10 +13711,9 @@ update_breakpoint_locations (struct breakpoint *b,
{
if ((!e->enabled || e->disabled_by_cond) && e->function_name)
{
- struct bp_location *l = b->loc;
if (have_ambiguous_names)
{
- for (; l; l = l->next)
+ for (bp_location *l : b->locations ())
{
/* Ignore software vs hardware location type at
this point, because with "set breakpoint
@@ -13741,7 +13732,7 @@ update_breakpoint_locations (struct breakpoint *b,
}
else
{
- for (; l; l = l->next)
+ for (bp_location *l : b->locations ())
if (l->function_name
&& strcmp (e->function_name, l->function_name) == 0)
{
@@ -14154,7 +14145,7 @@ find_location_by_number (int bp_num, int loc_num)
error (_("Bad breakpoint location number '%d'"), loc_num);
int n = 0;
- for (bp_location *loc = b->loc; loc != NULL; loc = loc->next)
+ for (bp_location *loc : b->locations ())
if (++n == loc_num)
return loc;
@@ -14350,9 +14341,7 @@ disable_breakpoint (struct breakpoint *bpt)
if (target_supports_enable_disable_tracepoint ()
&& current_trace_status ()->running && is_tracepoint (bpt))
{
- struct bp_location *location;
-
- for (location = bpt->loc; location; location = location->next)
+ for (bp_location *location : bpt->locations ())
target_disable_tracepoint (location);
}
@@ -14471,9 +14460,7 @@ enable_breakpoint_disp (struct breakpoint *bpt, enum bpdisp disposition,
if (target_supports_enable_disable_tracepoint ()
&& current_trace_status ()->running && is_tracepoint (bpt))
{
- struct bp_location *location;
-
- for (location = bpt->loc; location; location = location->next)
+ for (bp_location *location : bpt->locations ())
target_enable_tracepoint (location);
}
@@ -14568,9 +14555,7 @@ invalidate_bp_value_on_memory_change (struct inferior *inferior,
if (wp->val_valid && wp->val != nullptr)
{
- struct bp_location *loc;
-
- for (loc = bp->loc; loc != NULL; loc = loc->next)
+ for (bp_location *loc : bp->locations ())
if (loc->loc_type == bp_loc_hardware_watchpoint
&& loc->address + loc->length > addr
&& addr + len > loc->address)
@@ -14639,9 +14624,7 @@ breakpoint_has_location_inserted_here (struct breakpoint *bp,
const address_space *aspace,
CORE_ADDR pc)
{
- struct bp_location *loc;
-
- for (loc = bp->loc; loc != NULL; loc = loc->next)
+ for (bp_location *loc : bp->locations ())
if (loc->inserted
&& breakpoint_location_address_match (loc, aspace, pc))
return 1;
@@ -15182,12 +15165,15 @@ save_breakpoints (const char *filename, int from_tty,
special, and not user visible. */
if (!is_watchpoint (tp) && tp->loc && tp->loc->next)
{
- struct bp_location *loc;
int n = 1;
- for (loc = tp->loc; loc != NULL; loc = loc->next, n++)
- if (!loc->enabled)
- fp.printf ("disable $bpnum.%d\n", n);
+ for (bp_location *loc : tp->locations ())
+ {
+ if (!loc->enabled)
+ fp.printf ("disable $bpnum.%d\n", n);
+
+ n++;
+ }
}
}
@@ -15330,14 +15316,12 @@ int
pc_at_non_inline_function (const address_space *aspace, CORE_ADDR pc,
const struct target_waitstatus *ws)
{
- struct bp_location *bl;
-
for (breakpoint *b : all_breakpoints ())
{
if (!is_non_inline_function (b))
continue;
- for (bl = b->loc; bl != NULL; bl = bl->next)
+ for (bp_location *bl : b->locations ())
{
if (!bl->shlib_disabled
&& bpstat_check_location (bl, aspace, pc, ws))
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 5a10839603d7..f31498a54eb8 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -703,6 +703,10 @@ enum watchpoint_triggered
extern bool target_exact_watchpoints;
+/* bp_location linked list range. */
+
+using bp_locations_range = next_adapter<bp_location>;
+
/* Note that the ->silent field is not currently used by any commands
(though the code is in there if it was to be, and set_raw_breakpoint
does set it to 0). I implemented it because I thought it would be
@@ -715,6 +719,9 @@ struct breakpoint
{
virtual ~breakpoint ();
+ /* Return a range of this breakpoint's locations. */
+ bp_locations_range locations ();
+
/* Methods associated with this breakpoint. */
const breakpoint_ops *ops = NULL;
diff --git a/gdb/jit.c b/gdb/jit.c
index be10f197fd69..1de785b8de08 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -783,7 +783,7 @@ jit_breakpoint_deleted (struct breakpoint *b)
if (b->type != bp_jit_event)
return;
- for (bp_location *iter = b->loc; iter != nullptr; iter = iter->next)
+ for (bp_location *iter : b->locations ())
{
for (objfile *objf : iter->pspace->objfiles ())
{
diff --git a/gdb/remote.c b/gdb/remote.c
index aa98f5f951e8..9d83003f1e51 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -13518,7 +13518,6 @@ remote_target::get_tracepoint_status (struct breakpoint *bp,
{
struct remote_state *rs = get_remote_state ();
char *reply;
- struct bp_location *loc;
struct tracepoint *tp = (struct tracepoint *) bp;
size_t size = get_remote_packet_size ();
@@ -13526,7 +13525,7 @@ remote_target::get_tracepoint_status (struct breakpoint *bp,
{
tp->hit_count = 0;
tp->traceframe_usage = 0;
- for (loc = tp->loc; loc; loc = loc->next)
+ for (bp_location *loc : tp->locations ())
{
/* If the tracepoint was never downloaded, don't go asking for
any status. */
diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index 60eeac5f4bef..2642e1ad2fdb 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -2019,15 +2019,13 @@ svr4_handle_solib_event (void)
static bool
svr4_update_solib_event_breakpoint (struct breakpoint *b)
{
- struct bp_location *loc;
-
if (b->type != bp_shlib_event)
{
/* Continue iterating. */
return false;
}
- for (loc = b->loc; loc != NULL; loc = loc->next)
+ for (bp_location *loc : b->locations ())
{
struct svr4_info *info;
struct probe_and_action *pa;
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index 863fb6d34d8d..2b601290a164 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -636,7 +636,6 @@ validate_actionline (const char *line, struct breakpoint *b)
struct cmd_list_element *c;
const char *tmp_p;
const char *p;
- struct bp_location *loc;
struct tracepoint *t = (struct tracepoint *) b;
/* If EOF is typed, *line is NULL. */
@@ -682,7 +681,7 @@ validate_actionline (const char *line, struct breakpoint *b)
/* else fall thru, treat p as an expression and parse it! */
}
tmp_p = p;
- for (loc = t->loc; loc; loc = loc->next)
+ for (bp_location *loc : t->locations ())
{
p = tmp_p;
expression_up exp = parse_exp_1 (&p, loc->address,
@@ -732,7 +731,7 @@ validate_actionline (const char *line, struct breakpoint *b)
p = skip_spaces (p);
tmp_p = p;
- for (loc = t->loc; loc; loc = loc->next)
+ for (bp_location *loc : t->locations ())
{
p = tmp_p;
@@ -1565,9 +1564,7 @@ process_tracepoint_on_disconnect (void)
}
else
{
- struct bp_location *loc1;
-
- for (loc1 = b->loc; loc1; loc1 = loc1->next)
+ for (bp_location *loc1 : b->locations ())
{
if (loc1->shlib_disabled)
{
@@ -1643,11 +1640,10 @@ start_tracing (const char *notes)
for (breakpoint *b : tracepoint_range)
{
struct tracepoint *t = (struct tracepoint *) b;
- struct bp_location *loc;
int bp_location_downloaded = 0;
/* Clear `inserted' flag. */
- for (loc = b->loc; loc; loc = loc->next)
+ for (bp_location *loc : b->locations ())
loc->inserted = 0;
if ((b->type == bp_fast_tracepoint
@@ -1657,7 +1653,7 @@ start_tracing (const char *notes)
t->number_on_target = 0;
- for (loc = b->loc; loc; loc = loc->next)
+ for (bp_location *loc : b->locations ())
{
/* Since tracepoint locations are never duplicated, `inserted'
flag should be zero. */
@@ -1671,7 +1667,7 @@ start_tracing (const char *notes)
t->number_on_target = b->number;
- for (loc = b->loc; loc; loc = loc->next)
+ for (bp_location *loc : b->locations ())
if (loc->probe.prob != NULL)
loc->probe.prob->set_semaphore (loc->probe.objfile,
loc->gdbarch);
@@ -1750,14 +1746,12 @@ stop_tracing (const char *note)
for (breakpoint *t : all_tracepoints ())
{
- struct bp_location *loc;
-
if ((t->type == bp_fast_tracepoint
? !may_insert_fast_tracepoints
: !may_insert_tracepoints))
continue;
- for (loc = t->loc; loc; loc = loc->next)
+ for (bp_location *loc : t->locations ())
{
/* GDB can be totally absent in some disconnected trace scenarios,
but we don't really care if this semaphore goes out of sync.
@@ -2763,7 +2757,6 @@ struct bp_location *
get_traceframe_location (int *stepping_frame_p)
{
struct tracepoint *t;
- struct bp_location *tloc;
struct regcache *regcache;
if (tracepoint_number == -1)
@@ -2784,7 +2777,7 @@ get_traceframe_location (int *stepping_frame_p)
locations, assume it is a direct hit rather than a while-stepping
frame. (FIXME this is not reliable, should record each frame's
type.) */
- for (tloc = t->loc; tloc; tloc = tloc->next)
+ for (bp_location *tloc : t->locations ())
if (tloc->address == regcache_read_pc (regcache))
{
*stepping_frame_p = 0;
diff --git a/gdb/tui/tui-winsource.c b/gdb/tui/tui-winsource.c
index 908d9015bf14..738f69156485 100644
--- a/gdb/tui/tui-winsource.c
+++ b/gdb/tui/tui-winsource.c
@@ -459,12 +459,10 @@ tui_source_window_base::update_breakpoint_info
tui_bp_flags mode = 0;
iterate_over_breakpoints ([&] (breakpoint *bp) -> bool
{
- struct bp_location *loc;
-
if (bp == being_deleted)
return false;
- for (loc = bp->loc; loc != NULL; loc = loc->next)
+ for (bp_location *loc : bp->locations ())
{
if (location_matches_p (loc, i))
{
--
2.31.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 5/9] gdb: make bp_locations an std::vector
2021-05-27 15:35 [PATCH 0/9] Convert breakpoint iteration macros to ranges Simon Marchi
` (3 preceding siblings ...)
2021-05-27 15:35 ` [PATCH 4/9] gdb: add breakpoint::locations method Simon Marchi
@ 2021-05-27 15:35 ` Simon Marchi
2021-05-27 15:35 ` [PATCH 6/9] gdb: add all_bp_locations function Simon Marchi
` (4 subsequent siblings)
9 siblings, 0 replies; 21+ messages in thread
From: Simon Marchi @ 2021-05-27 15:35 UTC (permalink / raw)
To: gdb-patches
Change the type of the global location list, bp_locations, to be an
std::vector.
Adjust the users to deal with that, mostly in an obvious way by using
.data() and .size(). The user where it's slightly less obvious is
update_global_location_list. There, we std::move the old location list
out of the global vector into a local variable. The code to fill the
new location list gets simpler, as it's now simply using .push_back(),
no need to count the locations beforehand.
In the rest of update_global_location_list, the code is adjusted to work
with indices instead of `bp_location **`, to iterate on the location
list. I believe it's a bit easier to understand this way. But more
importantly, when we build with _GLIBCXX_DEBUG, the operator[] of the
vector does bound checking, so we will know if we ever access past a
vector size (which we won't if we access by raw pointer). I think that
work can further be done to make that function easier to understand,
notably find better names than "loc" and "loc2" for variables, but
that's work for later.
gdb/ChangeLog:
* breakpoint.c (bp_locations): Change to std::vector, update all
users.
(bp_locations_count): Remove.
(update_global_location_list): Change to work with indices
rather than bp_location**.
Change-Id: I193ce40f84d5dc930fbab8867cf946e78ff0df0b
---
gdb/breakpoint.c | 90 +++++++++++++++++-------------------------------
1 file changed, 32 insertions(+), 58 deletions(-)
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 1d33262520a0..42e4c65f8418 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -496,8 +496,8 @@ bool target_exact_watchpoints = false;
while executing the block of ALL_BP_LOCATIONS. */
#define ALL_BP_LOCATIONS(B,BP_TMP) \
- for (BP_TMP = bp_locations; \
- BP_TMP < bp_locations + bp_locations_count && (B = *BP_TMP);\
+ for (BP_TMP = bp_locations.data (); \
+ BP_TMP < bp_locations.data () + bp_locations.size () && (B = *BP_TMP);\
BP_TMP++)
/* Iterates through locations with address ADDRESS for the currently selected
@@ -510,7 +510,7 @@ bool target_exact_watchpoints = false;
for (BP_LOCP_START = BP_LOCP_START == NULL ? get_first_locp_gte_addr (ADDRESS) : BP_LOCP_START, \
BP_LOCP_TMP = BP_LOCP_START; \
BP_LOCP_START \
- && (BP_LOCP_TMP < bp_locations + bp_locations_count \
+ && (BP_LOCP_TMP < bp_locations.data () + bp_locations.size () \
&& (*BP_LOCP_TMP)->address == ADDRESS); \
BP_LOCP_TMP++)
@@ -554,11 +554,7 @@ all_tracepoints ()
/* Array is sorted by bp_location_is_less_than - primarily by the ADDRESS. */
-static struct bp_location **bp_locations;
-
-/* Number of elements of BP_LOCATIONS. */
-
-static unsigned bp_locations_count;
+static std::vector<bp_location *> bp_locations;
/* Maximum alignment offset between bp_target_info.PLACED_ADDRESS and
ADDRESS for the current elements of BP_LOCATIONS which get a valid
@@ -827,7 +823,7 @@ get_first_locp_gte_addr (CORE_ADDR address)
/* Find a close match to the first location at ADDRESS. */
locp_found = ((struct bp_location **)
- bsearch (&dummy_locp, bp_locations, bp_locations_count,
+ bsearch (&dummy_locp, bp_locations.data (), bp_locations.size (),
sizeof (struct bp_location **),
bp_locations_compare_addrs));
@@ -837,7 +833,7 @@ get_first_locp_gte_addr (CORE_ADDR address)
/* We may have found a location that is at ADDRESS but is not the first in the
location's list. Go backwards (if possible) and locate the first one. */
- while ((locp_found - 1) >= bp_locations
+ while ((locp_found - 1) >= bp_locations.data ()
&& (*(locp_found - 1))->address == address)
locp_found--;
@@ -1582,7 +1578,7 @@ breakpoint_xfer_memory (gdb_byte *readbuf, gdb_byte *writebuf,
report higher one. */
bc_l = 0;
- bc_r = bp_locations_count;
+ bc_r = bp_locations.size ();
while (bc_l + 1 < bc_r)
{
struct bp_location *bl;
@@ -1627,7 +1623,7 @@ breakpoint_xfer_memory (gdb_byte *readbuf, gdb_byte *writebuf,
/* Now do full processing of the found relevant range of elements. */
- for (bc = bc_l; bc < bp_locations_count; bc++)
+ for (bc = bc_l; bc < bp_locations.size (); bc++)
{
struct bp_location *bl = bp_locations[bc];
@@ -11831,7 +11827,6 @@ force_breakpoint_reinsertion (struct bp_location *bl)
static void
update_global_location_list (enum ugll_insert_mode insert_mode)
{
- struct bp_location **locp;
/* Last breakpoint location address that was marked for update. */
CORE_ADDR last_addr = 0;
/* Last breakpoint location program space that was marked for update. */
@@ -11850,38 +11845,22 @@ update_global_location_list (enum ugll_insert_mode insert_mode)
/* Saved former bp_locations array which we compare against the newly
built bp_locations from the current state of ALL_BREAKPOINTS. */
- struct bp_location **old_locp;
- unsigned old_locations_count;
- gdb::unique_xmalloc_ptr<struct bp_location *> old_locations (bp_locations);
-
- old_locations_count = bp_locations_count;
- bp_locations = NULL;
- bp_locations_count = 0;
-
- for (breakpoint *b : all_breakpoints ())
- for (bp_location *loc ATTRIBUTE_UNUSED : b->locations ())
- bp_locations_count++;
+ std::vector<bp_location *> old_locations = std::move (bp_locations);
+ bp_locations.clear ();
- bp_locations = XNEWVEC (struct bp_location *, bp_locations_count);
- locp = bp_locations;
for (breakpoint *b : all_breakpoints ())
for (bp_location *loc : b->locations ())
- *locp++ = loc;
+ bp_locations.push_back (loc);
/* See if we need to "upgrade" a software breakpoint to a hardware
breakpoint. Do this before deciding whether locations are
duplicates. Also do this before sorting because sorting order
depends on location type. */
- for (locp = bp_locations;
- locp < bp_locations + bp_locations_count;
- locp++)
- {
- bp_location *loc = *locp;
- if (!loc->inserted && should_be_inserted (loc))
+ for (bp_location *loc : bp_locations)
+ if (!loc->inserted && should_be_inserted (loc))
handle_automatic_hardware_breakpoints (loc);
- }
- std::sort (bp_locations, bp_locations + bp_locations_count,
+ std::sort (bp_locations.begin (), bp_locations.end (),
bp_location_is_less_than);
bp_locations_target_extensions_update ();
@@ -11896,14 +11875,9 @@ update_global_location_list (enum ugll_insert_mode insert_mode)
LOCP is kept in sync with OLD_LOCP, each pointing to the current
and former bp_location array state respectively. */
- locp = bp_locations;
- for (old_locp = old_locations.get ();
- old_locp < old_locations.get () + old_locations_count;
- old_locp++)
+ size_t loc_i = 0;
+ for (bp_location *old_loc : old_locations)
{
- struct bp_location *old_loc = *old_locp;
- struct bp_location **loc2p;
-
/* Tells if 'old_loc' is found among the new locations. If
not, we have to free it. */
int found_object = 0;
@@ -11913,28 +11887,28 @@ update_global_location_list (enum ugll_insert_mode insert_mode)
/* Skip LOCP entries which will definitely never be needed.
Stop either at or being the one matching OLD_LOC. */
- while (locp < bp_locations + bp_locations_count
- && (*locp)->address < old_loc->address)
- locp++;
+ while (loc_i < bp_locations.size ()
+ && bp_locations[loc_i]->address < old_loc->address)
+ loc_i++;
- for (loc2p = locp;
- (loc2p < bp_locations + bp_locations_count
- && (*loc2p)->address == old_loc->address);
- loc2p++)
+ for (size_t loc2_i = loc_i;
+ (loc2_i < bp_locations.size ()
+ && bp_locations[loc2_i]->address == old_loc->address);
+ loc2_i++)
{
/* Check if this is a new/duplicated location or a duplicated
location that had its condition modified. If so, we want to send
its condition to the target if evaluation of conditions is taking
place there. */
- if ((*loc2p)->condition_changed == condition_modified
+ if (bp_locations[loc2_i]->condition_changed == condition_modified
&& (last_addr != old_loc->address
|| last_pspace_num != old_loc->pspace->num))
{
- force_breakpoint_reinsertion (*loc2p);
+ force_breakpoint_reinsertion (bp_locations[loc2_i]);
last_pspace_num = old_loc->pspace->num;
}
- if (*loc2p == old_loc)
+ if (bp_locations[loc2_i] == old_loc)
found_object = 1;
}
@@ -11977,12 +11951,12 @@ update_global_location_list (enum ugll_insert_mode insert_mode)
/* OLD_LOC comes from existing struct breakpoint. */
if (bl_address_is_meaningful (old_loc))
{
- for (loc2p = locp;
- (loc2p < bp_locations + bp_locations_count
- && (*loc2p)->address == old_loc->address);
- loc2p++)
+ for (size_t loc2_i = loc_i;
+ (loc2_i < bp_locations.size ()
+ && bp_locations[loc2_i]->address == old_loc->address);
+ loc2_i++)
{
- struct bp_location *loc2 = *loc2p;
+ bp_location *loc2 = bp_locations[loc2_i];
if (loc2 == old_loc)
continue;
@@ -12121,7 +12095,7 @@ update_global_location_list (enum ugll_insert_mode insert_mode)
awp_loc_first = NULL;
rwp_loc_first = NULL;
- bp_location *loc;
+ bp_location *loc, **locp;
ALL_BP_LOCATIONS (loc, locp)
{
/* ALL_BP_LOCATIONS bp_location has LOC->OWNER always
--
2.31.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 6/9] gdb: add all_bp_locations function
2021-05-27 15:35 [PATCH 0/9] Convert breakpoint iteration macros to ranges Simon Marchi
` (4 preceding siblings ...)
2021-05-27 15:35 ` [PATCH 5/9] gdb: make bp_locations an std::vector Simon Marchi
@ 2021-05-27 15:35 ` Simon Marchi
2021-05-27 15:35 ` [PATCH 7/9] gdb: add all_bp_locations_at_addr function Simon Marchi
` (3 subsequent siblings)
9 siblings, 0 replies; 21+ messages in thread
From: Simon Marchi @ 2021-05-27 15:35 UTC (permalink / raw)
To: gdb-patches
Add the all_bp_locations function to replace the ALL_BP_LOCATIONS macro.
For simplicity, all_bp_locations simply returns a const reference to the
bp_locations vector. But the callers just treat it as a range to
iterate on, so if we ever change the breakpoint location storage, we can
change the all_bp_locations function to return some other range type,
and the callers won't need to be changed.
gdb/ChangeLog:
* breakpoint.c (ALL_BP_LOCATIONS): Remove, update users to use
all_bp_locations.
(all_bp_locations): New.
Change-Id: Iae71a1ba135c1a5bcdb4658bf3cf9793f0e9f81c
---
gdb/breakpoint.c | 84 +++++++++++++++---------------------------------
1 file changed, 26 insertions(+), 58 deletions(-)
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 42e4c65f8418..18cabee39846 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -491,15 +491,6 @@ bool target_exact_watchpoints = false;
B ? (TMP=B->next, 1): 0; \
B = TMP)
-/* Similar iterator for the low-level breakpoints. SAFE variant is
- not provided so update_global_location_list must not be called
- while executing the block of ALL_BP_LOCATIONS. */
-
-#define ALL_BP_LOCATIONS(B,BP_TMP) \
- for (BP_TMP = bp_locations.data (); \
- BP_TMP < bp_locations.data () + bp_locations.size () && (B = *BP_TMP);\
- BP_TMP++)
-
/* Iterates through locations with address ADDRESS for the currently selected
program space. BP_LOCP_TMP points to each object. BP_LOCP_START points
to where the loop should start from.
@@ -556,6 +547,12 @@ all_tracepoints ()
static std::vector<bp_location *> bp_locations;
+static const std::vector<bp_location *> &
+all_bp_locations ()
+{
+ return bp_locations;
+}
+
/* Maximum alignment offset between bp_target_info.PLACED_ADDRESS and
ADDRESS for the current elements of BP_LOCATIONS which get a valid
result from bp_location_has_shadow. You can use it for roughly
@@ -740,7 +737,6 @@ set_condition_evaluation_mode (const char *args, int from_tty,
/* Only update the mode if the user picked a different one. */
if (new_mode != old_mode)
{
- struct bp_location *loc, **loc_tmp;
/* If the user switched to a different evaluation mode, we
need to synch the changes with the target as follows:
@@ -752,7 +748,7 @@ set_condition_evaluation_mode (const char *args, int from_tty,
{
/* Mark everything modified and synch conditions with the
target. */
- ALL_BP_LOCATIONS (loc, loc_tmp)
+ for (bp_location *loc : all_bp_locations ())
mark_breakpoint_location_modified (loc);
}
else
@@ -760,7 +756,7 @@ set_condition_evaluation_mode (const char *args, int from_tty,
/* Manually mark non-duplicate locations to synch conditions
with the target. We do this to remove all the conditions the
target knows about. */
- ALL_BP_LOCATIONS (loc, loc_tmp)
+ for (bp_location *loc : all_bp_locations ())
if (is_breakpoint (loc->owner) && loc->inserted)
loc->needs_update = 1;
}
@@ -2825,12 +2821,10 @@ insert_bp_location (struct bp_location *bl,
supported, try emulating one with an access watchpoint. */
if (val == 1 && bl->watchpoint_type == hw_read)
{
- struct bp_location *loc, **loc_temp;
-
/* But don't try to insert it, if there's already another
hw_access location that would be considered a duplicate
of this one. */
- ALL_BP_LOCATIONS (loc, loc_temp)
+ for (bp_location *loc : all_bp_locations ())
if (loc != bl
&& loc->watchpoint_type == hw_access
&& watchpoint_locations_match (bl, loc))
@@ -2895,8 +2889,6 @@ of catchpoint."), bl->owner->number);
void
breakpoint_program_space_exit (struct program_space *pspace)
{
- struct bp_location *loc, **loc_temp;
-
/* Remove any breakpoint that was set through this program space. */
for (breakpoint *b : all_breakpoints_safe ())
if (b->pspace == pspace)
@@ -2904,7 +2896,7 @@ breakpoint_program_space_exit (struct program_space *pspace)
/* Breakpoints set through other program spaces could have locations
bound to PSPACE as well. Remove those. */
- ALL_BP_LOCATIONS (loc, loc_temp)
+ for (bp_location *loc : all_bp_locations ())
{
struct bp_location *tmp;
@@ -2958,12 +2950,8 @@ insert_breakpoints (void)
void
iterate_over_bp_locations (gdb::function_view<void (bp_location *)> callback)
{
- struct bp_location *loc, **loc_tmp;
-
- ALL_BP_LOCATIONS (loc, loc_tmp)
- {
- callback (loc);
- }
+ for (bp_location *loc : all_bp_locations ())
+ callback (loc);
}
/* This is used when we need to synch breakpoint conditions between GDB and the
@@ -2973,7 +2961,6 @@ iterate_over_bp_locations (gdb::function_view<void (bp_location *)> callback)
static void
update_inserted_breakpoint_locations (void)
{
- struct bp_location *bl, **blp_tmp;
int error_flag = 0;
int val = 0;
int disabled_breaks = 0;
@@ -2988,7 +2975,7 @@ update_inserted_breakpoint_locations (void)
scoped_restore_current_pspace_and_thread restore_pspace_thread;
- ALL_BP_LOCATIONS (bl, blp_tmp)
+ for (bp_location *bl : all_bp_locations ())
{
/* We only want to update software breakpoints and hardware
breakpoints. */
@@ -3029,7 +3016,6 @@ update_inserted_breakpoint_locations (void)
static void
insert_breakpoint_locations (void)
{
- struct bp_location *bl, **blp_tmp;
int error_flag = 0;
int val = 0;
int disabled_breaks = 0;
@@ -3044,7 +3030,7 @@ insert_breakpoint_locations (void)
scoped_restore_current_pspace_and_thread restore_pspace_thread;
- ALL_BP_LOCATIONS (bl, blp_tmp)
+ for (bp_location *bl : all_bp_locations ())
{
if (!should_be_inserted (bl) || (bl->inserted && !bl->needs_update))
continue;
@@ -3129,10 +3115,9 @@ You may have requested too many hardware breakpoints/watchpoints.\n");
int
remove_breakpoints (void)
{
- struct bp_location *bl, **blp_tmp;
int val = 0;
- ALL_BP_LOCATIONS (bl, blp_tmp)
+ for (bp_location *bl : all_bp_locations ())
{
if (bl->inserted && !is_tracepoint (bl->owner))
val |= remove_breakpoint (bl);
@@ -3167,10 +3152,9 @@ Thread-specific breakpoint %d deleted - thread %s no longer in the thread list.\
void
remove_breakpoints_inf (inferior *inf)
{
- struct bp_location *bl, **blp_tmp;
int val;
- ALL_BP_LOCATIONS (bl, blp_tmp)
+ for (bp_location *bl : all_bp_locations ())
{
if (bl->pspace != inf->pspace)
continue;
@@ -3654,8 +3638,6 @@ breakpoint_event_location_empty_p (const struct breakpoint *b)
void
update_breakpoints_after_exec (void)
{
- struct bp_location *bploc, **bplocp_tmp;
-
/* We're about to delete breakpoints from GDB's lists. If the
INSERTED flag is true, GDB will try to lift the breakpoints by
writing the breakpoints' "shadow contents" back into memory. The
@@ -3664,7 +3646,7 @@ update_breakpoints_after_exec (void)
breakpoints out as soon as it detects an exec. We don't do that
here instead, because there may be other attempts to delete
breakpoints after detecting an exec and before reaching here. */
- ALL_BP_LOCATIONS (bploc, bplocp_tmp)
+ for (bp_location *bploc : all_bp_locations ())
if (bploc->pspace == current_program_space)
gdb_assert (!bploc->inserted);
@@ -3775,7 +3757,6 @@ update_breakpoints_after_exec (void)
int
detach_breakpoints (ptid_t ptid)
{
- struct bp_location *bl, **blp_tmp;
int val = 0;
scoped_restore save_inferior_ptid = make_scoped_restore (&inferior_ptid);
struct inferior *inf = current_inferior ();
@@ -3785,7 +3766,7 @@ detach_breakpoints (ptid_t ptid)
/* Set inferior_ptid; remove_breakpoint_1 uses this global. */
inferior_ptid = ptid;
- ALL_BP_LOCATIONS (bl, blp_tmp)
+ for (bp_location *bl : all_bp_locations ())
{
if (bl->pspace != inf->pspace)
continue;
@@ -3980,9 +3961,7 @@ remove_breakpoint (struct bp_location *bl)
void
mark_breakpoints_out (void)
{
- struct bp_location *bl, **blp_tmp;
-
- ALL_BP_LOCATIONS (bl, blp_tmp)
+ for (bp_location *bl : all_bp_locations ())
if (bl->pspace == current_program_space)
bl->inserted = 0;
}
@@ -4114,10 +4093,9 @@ breakpoint_init_inferior (enum inf_context context)
enum breakpoint_here
breakpoint_here_p (const address_space *aspace, CORE_ADDR pc)
{
- struct bp_location *bl, **blp_tmp;
int any_breakpoint_here = 0;
- ALL_BP_LOCATIONS (bl, blp_tmp)
+ for (bp_location *bl : all_bp_locations ())
{
if (bl->loc_type != bp_loc_software_breakpoint
&& bl->loc_type != bp_loc_hardware_breakpoint)
@@ -4148,9 +4126,7 @@ int
breakpoint_in_range_p (const address_space *aspace,
CORE_ADDR addr, ULONGEST len)
{
- struct bp_location *bl, **blp_tmp;
-
- ALL_BP_LOCATIONS (bl, blp_tmp)
+ for (bp_location *bl : all_bp_locations ())
{
if (bl->loc_type != bp_loc_software_breakpoint
&& bl->loc_type != bp_loc_hardware_breakpoint)
@@ -7620,9 +7596,7 @@ create_and_insert_solib_event_breakpoint (struct gdbarch *gdbarch, CORE_ADDR add
void
disable_breakpoints_in_shlibs (void)
{
- struct bp_location *loc, **locp_tmp;
-
- ALL_BP_LOCATIONS (loc, locp_tmp)
+ for (bp_location *loc : all_bp_locations ())
{
/* ALL_BP_LOCATIONS bp_location has LOC->OWNER always non-NULL. */
struct breakpoint *b = loc->owner;
@@ -7653,10 +7627,9 @@ disable_breakpoints_in_shlibs (void)
static void
disable_breakpoints_in_unloaded_shlib (struct so_list *solib)
{
- struct bp_location *loc, **locp_tmp;
int disabled_shlib_breaks = 0;
- ALL_BP_LOCATIONS (loc, locp_tmp)
+ for (bp_location *loc : all_bp_locations ())
{
/* ALL_BP_LOCATIONS bp_location has LOC->OWNER always non-NULL. */
struct breakpoint *b = loc->owner;
@@ -11660,12 +11633,10 @@ bp_location_is_less_than (const bp_location *a, const bp_location *b)
static void
bp_locations_target_extensions_update (void)
{
- struct bp_location *bl, **blp_tmp;
-
bp_locations_placed_address_before_address_max = 0;
bp_locations_shadow_len_after_address_max = 0;
- ALL_BP_LOCATIONS (bl, blp_tmp)
+ for (bp_location *bl : all_bp_locations ())
{
CORE_ADDR start, end, addr;
@@ -12095,8 +12066,7 @@ update_global_location_list (enum ugll_insert_mode insert_mode)
awp_loc_first = NULL;
rwp_loc_first = NULL;
- bp_location *loc, **locp;
- ALL_BP_LOCATIONS (loc, locp)
+ for (bp_location *loc : all_bp_locations ())
{
/* ALL_BP_LOCATIONS bp_location has LOC->OWNER always
non-NULL. */
@@ -15311,9 +15281,7 @@ pc_at_non_inline_function (const address_space *aspace, CORE_ADDR pc,
void
breakpoint_free_objfile (struct objfile *objfile)
{
- struct bp_location **locp, *loc;
-
- ALL_BP_LOCATIONS (loc, locp)
+ for (bp_location *loc : all_bp_locations ())
if (loc->symtab != NULL && SYMTAB_OBJFILE (loc->symtab) == objfile)
loc->symtab = NULL;
}
--
2.31.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 7/9] gdb: add all_bp_locations_at_addr function
2021-05-27 15:35 [PATCH 0/9] Convert breakpoint iteration macros to ranges Simon Marchi
` (5 preceding siblings ...)
2021-05-27 15:35 ` [PATCH 6/9] gdb: add all_bp_locations function Simon Marchi
@ 2021-05-27 15:35 ` Simon Marchi
2021-05-27 18:04 ` Tom Tromey
2021-05-27 15:35 ` [PATCH 8/9] gdb: remove iterate_over_breakpoints function Simon Marchi
` (2 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: Simon Marchi @ 2021-05-27 15:35 UTC (permalink / raw)
To: gdb-patches
Add the all_bp_locations_at_addr function, which returns a range of all
breakpoint locations at exactly the given address. This lets us
replace:
bp_location *loc, **loc2p, *locp;
ALL_BP_LOCATIONS_AT_ADDR (loc2p, locp, address)
{
loc = *loc2p;
// use loc
}
with
for (bp_location *loc : all_bp_locations_at_addr (address))
{
// use loc
}
The all_bp_locations_at_addr returns a bp_locations_at_addr_range
object, which is really just a wrapper around two std::vector iterators
representing the beginning and end of the interesting range. These
iterators are found when constructing the bp_locations_at_addr_range
object using std::equal_range, which seems a perfect fit for this use
case.
One thing I noticed about the current ALL_BP_LOCATIONS_AT_ADDR is that
if you call it with a NULL start variable, that variable gets filled in
and can be re-used for subsequent iterations. This avoids the cost of
finding the start of the interesting range again for the subsequent
iterations. This happens in build_target_command_list, for example.
The same effect can be achieved by storing the range in a local
variable, it can be iterated on multiple times.
Note that the original comment over ALL_BP_LOCATIONS_AT_ADDR says:
Iterates through locations with address ADDRESS for the currently
selected program space.
I don't see anything restricting the iteration to a given program space,
as we iterate over all bp_locations, which as far as I know contains all
breakpoint locations, regardless of the program space. So I just
dropped that part of the comment.
gdb/ChangeLog:
* breakpoint.c (get_first_locp_gte_addr): Remove.
(ALL_BP_LOCATIONS_AT_ADDR): Remove. Replace all uses with
all_bp_locations_at_addr.
(struct bp_locations_at_addr_range): New.
(all_bp_locations_at_addr): New.
(bp_locations_compare_addrs): New.
Change-Id: Icc8c92302045c47a48f507b7f1872bdd31d4ba59
---
gdb/breakpoint.c | 258 ++++++++++++++++++++---------------------------
1 file changed, 110 insertions(+), 148 deletions(-)
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 18cabee39846..79f82f29dff3 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -223,8 +223,6 @@ static void set_tracepoint_count (int num);
static bool is_masked_watchpoint (const struct breakpoint *b);
-static struct bp_location **get_first_locp_gte_addr (CORE_ADDR address);
-
/* Return 1 if B refers to a static tracepoint set by marker ("-m"), zero
otherwise. */
@@ -491,20 +489,6 @@ bool target_exact_watchpoints = false;
B ? (TMP=B->next, 1): 0; \
B = TMP)
-/* Iterates through locations with address ADDRESS for the currently selected
- program space. BP_LOCP_TMP points to each object. BP_LOCP_START points
- to where the loop should start from.
- If BP_LOCP_START is a NULL pointer, the macro automatically seeks the
- appropriate location to start with. */
-
-#define ALL_BP_LOCATIONS_AT_ADDR(BP_LOCP_TMP, BP_LOCP_START, ADDRESS) \
- for (BP_LOCP_START = BP_LOCP_START == NULL ? get_first_locp_gte_addr (ADDRESS) : BP_LOCP_START, \
- BP_LOCP_TMP = BP_LOCP_START; \
- BP_LOCP_START \
- && (BP_LOCP_TMP < bp_locations.data () + bp_locations.size () \
- && (*BP_LOCP_TMP)->address == ADDRESS); \
- BP_LOCP_TMP++)
-
/* Chains of all breakpoints defined. */
static struct breakpoint *breakpoint_chain;
@@ -553,6 +537,65 @@ all_bp_locations ()
return bp_locations;
}
+/* Range to iterate over breakpoint locations at a given address. */
+
+struct bp_locations_at_addr_range
+{
+ using iterator = std::vector<bp_location *>::iterator;
+
+ bp_locations_at_addr_range (CORE_ADDR addr)
+ {
+ struct compare
+ {
+ bool operator() (const bp_location *loc, CORE_ADDR addr_) const
+ { return loc->address < addr_; }
+
+ bool operator() (CORE_ADDR addr_, const bp_location *loc) const
+ { return addr_ < loc->address; }
+ };
+
+ auto it_pair = std::equal_range (bp_locations.begin (), bp_locations.end (),
+ addr, compare ());
+
+ m_begin = it_pair.first;
+ m_end = it_pair.second;
+ }
+
+ iterator begin () const
+ { return m_begin; }
+
+ iterator end () const
+ { return m_end; }
+
+private:
+ std::vector<bp_location *>::iterator m_begin;
+ std::vector<bp_location *>::iterator m_end;
+};
+
+/* Return a range to iterate over all breakpoint locations exactly at address
+ ADDR.
+
+ If it's needed to iterate multiple times on the same range, it's possible
+ to save the range in a local variable and use it multiple times:
+
+ auto range = all_bp_locations_at_addr (addr);
+
+ for (bp_location *loc : range)
+ // use loc
+
+ for (bp_location *loc : range)
+ // use loc
+
+ This saves a bit of time, as it avoids re-doing the binary searches to find
+ the range's boundaries. Just remember not to change the bp_locations vector
+ in the mean time, as it could make the range's iterators stale. */
+
+static bp_locations_at_addr_range
+all_bp_locations_at_addr (CORE_ADDR addr)
+{
+ return bp_locations_at_addr_range (addr);
+}
+
/* Maximum alignment offset between bp_target_info.PLACED_ADDRESS and
ADDRESS for the current elements of BP_LOCATIONS which get a valid
result from bp_location_has_shadow. You can use it for roughly
@@ -786,56 +829,6 @@ show_condition_evaluation_mode (struct ui_file *file, int from_tty,
value);
}
-/* A comparison function for bp_location AP and BP that is used by
- bsearch. This comparison function only cares about addresses, unlike
- the more general bp_location_is_less_than function. */
-
-static int
-bp_locations_compare_addrs (const void *ap, const void *bp)
-{
- const struct bp_location *a = *(const struct bp_location **) ap;
- const struct bp_location *b = *(const struct bp_location **) bp;
-
- if (a->address == b->address)
- return 0;
- else
- return ((a->address > b->address) - (a->address < b->address));
-}
-
-/* Helper function to skip all bp_locations with addresses
- less than ADDRESS. It returns the first bp_location that
- is greater than or equal to ADDRESS. If none is found, just
- return NULL. */
-
-static struct bp_location **
-get_first_locp_gte_addr (CORE_ADDR address)
-{
- struct bp_location dummy_loc;
- struct bp_location *dummy_locp = &dummy_loc;
- struct bp_location **locp_found = NULL;
-
- /* Initialize the dummy location's address field. */
- dummy_loc.address = address;
-
- /* Find a close match to the first location at ADDRESS. */
- locp_found = ((struct bp_location **)
- bsearch (&dummy_locp, bp_locations.data (), bp_locations.size (),
- sizeof (struct bp_location **),
- bp_locations_compare_addrs));
-
- /* Nothing was found, nothing left to do. */
- if (locp_found == NULL)
- return NULL;
-
- /* We may have found a location that is at ADDRESS but is not the first in the
- location's list. Go backwards (if possible) and locate the first one. */
- while ((locp_found - 1) >= bp_locations.data ()
- && (*(locp_found - 1))->address == address)
- locp_found--;
-
- return locp_found;
-}
-
/* Parse COND_STRING in the context of LOC and set as the condition
expression of LOC. BP_NUM is the number of LOC's owner, LOC_NUM is
the number of LOC within its owner. In case of parsing error, mark
@@ -2248,10 +2241,8 @@ parse_cond_to_aexpr (CORE_ADDR scope, struct expression *cond)
static void
build_target_condition_list (struct bp_location *bl)
{
- struct bp_location **locp = NULL, **loc2p;
int null_condition_or_parse_error = 0;
int modified = bl->needs_update;
- struct bp_location *loc;
/* Release conditions left over from a previous insert. */
bl->target_info.conditions.clear ();
@@ -2264,6 +2255,8 @@ build_target_condition_list (struct bp_location *bl)
|| !target_supports_evaluation_of_breakpoint_conditions ())
return;
+ auto loc_range = all_bp_locations_at_addr (bl->address);
+
/* Do a first pass to check for locations with no assigned
conditions or conditions that fail to parse to a valid agent
expression bytecode. If any of these happen, then it's no use to
@@ -2273,9 +2266,8 @@ build_target_condition_list (struct bp_location *bl)
even if the locations aren't considered duplicates (e.g.,
software breakpoint and hardware breakpoint at the same
address). */
- ALL_BP_LOCATIONS_AT_ADDR (loc2p, locp, bl->address)
+ for (bp_location *loc : loc_range)
{
- loc = (*loc2p);
if (is_breakpoint (loc->owner) && loc->pspace->num == bl->pspace->num)
{
if (modified)
@@ -2306,9 +2298,8 @@ build_target_condition_list (struct bp_location *bl)
being evaluated by GDB or the remote stub. */
if (null_condition_or_parse_error)
{
- ALL_BP_LOCATIONS_AT_ADDR (loc2p, locp, bl->address)
+ for (bp_location *loc : loc_range)
{
- loc = (*loc2p);
if (is_breakpoint (loc->owner) && loc->pspace->num == bl->pspace->num)
{
/* Only go as far as the first NULL bytecode is
@@ -2327,21 +2318,18 @@ build_target_condition_list (struct bp_location *bl)
considered duplicates, but we still marge all the conditions
anyway, as it's simpler, and doesn't really make a practical
difference. */
- ALL_BP_LOCATIONS_AT_ADDR (loc2p, locp, bl->address)
- {
- loc = (*loc2p);
- if (loc->cond
- && is_breakpoint (loc->owner)
- && loc->pspace->num == bl->pspace->num
- && loc->owner->enable_state == bp_enabled
- && loc->enabled
- && !loc->disabled_by_cond)
- {
- /* Add the condition to the vector. This will be used later
- to send the conditions to the target. */
- bl->target_info.conditions.push_back (loc->cond_bytecode.get ());
- }
- }
+ for (bp_location *loc : loc_range)
+ if (loc->cond
+ && is_breakpoint (loc->owner)
+ && loc->pspace->num == bl->pspace->num
+ && loc->owner->enable_state == bp_enabled
+ && loc->enabled
+ && !loc->disabled_by_cond)
+ {
+ /* Add the condition to the vector. This will be used later
+ to send the conditions to the target. */
+ bl->target_info.conditions.push_back (loc->cond_bytecode.get ());
+ }
return;
}
@@ -2430,10 +2418,8 @@ parse_cmd_to_aexpr (CORE_ADDR scope, char *cmd)
static void
build_target_command_list (struct bp_location *bl)
{
- struct bp_location **locp = NULL, **loc2p;
int null_command_or_parse_error = 0;
int modified = bl->needs_update;
- struct bp_location *loc;
/* Clear commands left over from a previous insert. */
bl->target_info.tcommands.clear ();
@@ -2445,27 +2431,25 @@ build_target_command_list (struct bp_location *bl)
if (dprintf_style != dprintf_style_agent)
return;
+ auto loc_range = all_bp_locations_at_addr (bl->address);
+
/* For now, if we have any location at the same address that isn't a
dprintf, don't install the target-side commands, as that would
make the breakpoint not be reported to the core, and we'd lose
control. */
- ALL_BP_LOCATIONS_AT_ADDR (loc2p, locp, bl->address)
- {
- loc = (*loc2p);
- if (is_breakpoint (loc->owner)
- && loc->pspace->num == bl->pspace->num
- && loc->owner->type != bp_dprintf)
- return;
- }
+ for (bp_location *loc : loc_range)
+ if (is_breakpoint (loc->owner)
+ && loc->pspace->num == bl->pspace->num
+ && loc->owner->type != bp_dprintf)
+ return;
/* Do a first pass to check for locations with no assigned
conditions or conditions that fail to parse to a valid agent expression
bytecode. If any of these happen, then it's no use to send conditions
to the target since this location will always trigger and generate a
response back to GDB. */
- ALL_BP_LOCATIONS_AT_ADDR (loc2p, locp, bl->address)
+ for (bp_location *loc : loc_range)
{
- loc = (*loc2p);
if (is_breakpoint (loc->owner) && loc->pspace->num == bl->pspace->num)
{
if (modified)
@@ -2493,20 +2477,17 @@ build_target_command_list (struct bp_location *bl)
and so clean up. */
if (null_command_or_parse_error)
{
- ALL_BP_LOCATIONS_AT_ADDR (loc2p, locp, bl->address)
- {
- loc = (*loc2p);
- if (is_breakpoint (loc->owner)
- && loc->pspace->num == bl->pspace->num)
- {
- /* Only go as far as the first NULL bytecode is
- located. */
- if (loc->cmd_bytecode == NULL)
- return;
+ for (bp_location *loc : loc_range)
+ if (is_breakpoint (loc->owner)
+ && loc->pspace->num == bl->pspace->num)
+ {
+ /* Only go as far as the first NULL bytecode is
+ located. */
+ if (loc->cmd_bytecode == NULL)
+ return;
- loc->cmd_bytecode.reset ();
- }
- }
+ loc->cmd_bytecode.reset ();
+ }
}
/* No NULL commands or failed bytecode generation. Build a command
@@ -2517,22 +2498,19 @@ build_target_command_list (struct bp_location *bl)
could end up running the commands twice. For the moment, we only
support targets-side commands with dprintf, but it doesn't hurt
to be pedantically correct in case that changes. */
- ALL_BP_LOCATIONS_AT_ADDR (loc2p, locp, bl->address)
- {
- loc = (*loc2p);
- if (breakpoint_locations_match (bl, loc)
- && loc->owner->extra_string
- && is_breakpoint (loc->owner)
- && loc->pspace->num == bl->pspace->num
- && loc->owner->enable_state == bp_enabled
- && loc->enabled
- && !loc->disabled_by_cond)
- {
- /* Add the command to the vector. This will be used later
- to send the commands to the target. */
- bl->target_info.tcommands.push_back (loc->cmd_bytecode.get ());
- }
- }
+ for (bp_location *loc : loc_range)
+ if (breakpoint_locations_match (bl, loc)
+ && loc->owner->extra_string
+ && is_breakpoint (loc->owner)
+ && loc->pspace->num == bl->pspace->num
+ && loc->owner->enable_state == bp_enabled
+ && loc->enabled
+ && !loc->disabled_by_cond)
+ {
+ /* Add the command to the vector. This will be used later
+ to send the commands to the target. */
+ bl->target_info.tcommands.push_back (loc->cmd_bytecode.get ());
+ }
bl->target_info.persist = 0;
/* Maybe flag this location as persistent. */
@@ -4190,12 +4168,8 @@ bp_location_inserted_here_p (struct bp_location *bl,
int
breakpoint_inserted_here_p (const address_space *aspace, CORE_ADDR pc)
{
- struct bp_location **blp, **blp_tmp = NULL;
-
- ALL_BP_LOCATIONS_AT_ADDR (blp, blp_tmp, pc)
+ for (bp_location *bl : all_bp_locations_at_addr (pc))
{
- struct bp_location *bl = *blp;
-
if (bl->loc_type != bp_loc_software_breakpoint
&& bl->loc_type != bp_loc_hardware_breakpoint)
continue;
@@ -4213,12 +4187,8 @@ int
software_breakpoint_inserted_here_p (const address_space *aspace,
CORE_ADDR pc)
{
- struct bp_location **blp, **blp_tmp = NULL;
-
- ALL_BP_LOCATIONS_AT_ADDR (blp, blp_tmp, pc)
+ for (bp_location *bl : all_bp_locations_at_addr (pc))
{
- struct bp_location *bl = *blp;
-
if (bl->loc_type != bp_loc_software_breakpoint)
continue;
@@ -4235,12 +4205,8 @@ int
hardware_breakpoint_inserted_here_p (const address_space *aspace,
CORE_ADDR pc)
{
- struct bp_location **blp, **blp_tmp = NULL;
-
- ALL_BP_LOCATIONS_AT_ADDR (blp, blp_tmp, pc)
+ for (bp_location *bl : all_bp_locations_at_addr (pc))
{
- struct bp_location *bl = *blp;
-
if (bl->loc_type != bp_loc_hardware_breakpoint)
continue;
@@ -11746,8 +11712,6 @@ swap_insertion (struct bp_location *left, struct bp_location *right)
static void
force_breakpoint_reinsertion (struct bp_location *bl)
{
- struct bp_location **locp = NULL, **loc2p;
- struct bp_location *loc;
CORE_ADDR address = 0;
int pspace_num;
@@ -11766,10 +11730,8 @@ force_breakpoint_reinsertion (struct bp_location *bl)
the same program space as the location
as "its condition has changed". We need to
update the conditions on the target's side. */
- ALL_BP_LOCATIONS_AT_ADDR (loc2p, locp, address)
+ for (bp_location *loc : all_bp_locations_at_addr (address))
{
- loc = *loc2p;
-
if (!is_breakpoint (loc->owner)
|| pspace_num != loc->pspace->num)
continue;
--
2.31.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 8/9] gdb: remove iterate_over_breakpoints function
2021-05-27 15:35 [PATCH 0/9] Convert breakpoint iteration macros to ranges Simon Marchi
` (6 preceding siblings ...)
2021-05-27 15:35 ` [PATCH 7/9] gdb: add all_bp_locations_at_addr function Simon Marchi
@ 2021-05-27 15:35 ` Simon Marchi
2021-10-21 10:20 ` Tom de Vries
2021-05-27 15:35 ` [PATCH 9/9] gdb: remove iterate_over_bp_locations function Simon Marchi
2021-05-27 18:14 ` [PATCH 0/9] Convert breakpoint iteration macros to ranges Tom Tromey
9 siblings, 1 reply; 21+ messages in thread
From: Simon Marchi @ 2021-05-27 15:35 UTC (permalink / raw)
To: gdb-patches
Now that we have range functions that let us use ranged for loops, we
can remove iterate_over_breakpoints in favor of those, which are easier
to read and write. This requires exposing the declaration of
all_breakpoints and all_breakpoints_safe in breakpoint.h, as well as the
supporting types.
Change some users of iterate_over_breakpoints to use all_breakpoints,
when they don't need to delete the breakpoint, and all_breakpoints_safe
otherwise.
gdb/ChangeLog:
* breakpoint.h (iterate_over_breakpoints): Remove. Update
callers to use all_breakpoints or all_breakpoints_safe.
(breakpoint_range, all_breakpoints, breakpoint_safe_range,
all_breakpoints_safe): Move here.
* breakpoint.c (all_breakpoints, all_breakpoints_safe): Make
non-static.
(iterate_over_breakpoints): Remove.
* python/py-finishbreakpoint.c (bpfinishpy_detect_out_scope_cb):
Return void.
* python/py-breakpoint.c (build_bp_list): Add comment, reverse
return value logic.
* guile/scm-breakpoint.c (bpscm_build_bp_list): Return void.
Change-Id: Idde764a1f577de0423e4f2444a7d5cdb01ba5e48
---
gdb/breakpoint.c | 28 ++++------------------------
gdb/breakpoint.h | 30 +++++++++++++++++++-----------
gdb/dummy-frame.c | 7 +++----
gdb/guile/scm-breakpoint.c | 10 +++-------
gdb/python/py-breakpoint.c | 27 +++++++++++----------------
gdb/python/py-finishbreakpoint.c | 17 +++++------------
gdb/solib-svr4.c | 3 ++-
gdb/tui/tui-winsource.c | 5 +++--
8 files changed, 50 insertions(+), 77 deletions(-)
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 79f82f29dff3..751ed51cc7d3 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -493,27 +493,17 @@ bool target_exact_watchpoints = false;
static struct breakpoint *breakpoint_chain;
-/* Breakpoint linked list range. */
-
-using breakpoint_range = next_adapter<breakpoint, breakpoint_iterator>;
-
-/* Return a range to iterate over all breakpoints. */
+/* See breakpoint.h. */
-static breakpoint_range
+breakpoint_range
all_breakpoints ()
{
return breakpoint_range (breakpoint_chain);
}
-/* Breakpoint linked list range, safe against deletion of the current
- breakpoint while iterating. */
-
-using breakpoint_safe_range = basic_safe_range<breakpoint_range>;
-
-/* Return a range to iterate over all breakpoints. This range is safe against
- deletion of the current breakpoint while iterating. */
+/* See breakpoint.h. */
-static breakpoint_safe_range
+breakpoint_safe_range
all_breakpoints_safe ()
{
return breakpoint_safe_range (all_breakpoints ());
@@ -15191,16 +15181,6 @@ add_catch_command (const char *name, const char *docstring,
set_cmd_completer (command, completer);
}
-struct breakpoint *
-iterate_over_breakpoints (gdb::function_view<bool (breakpoint *)> callback)
-{
- for (breakpoint *b : all_breakpoints_safe ())
- if (callback (b))
- return b;
-
- return NULL;
-}
-
/* Zero if any of the breakpoint's locations could be a location where
functions have been inlined, nonzero otherwise. */
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index f31498a54eb8..ffe042459eef 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -31,6 +31,7 @@
#include "gdbsupport/filtered-iterator.h"
#include "gdbsupport/function-view.h"
#include "gdbsupport/refcounted-object.h"
+#include "gdbsupport/safe-iterator.h"
#include "cli/cli-script.h"
struct block;
@@ -1711,21 +1712,28 @@ class scoped_rbreak_breakpoints
DISABLE_COPY_AND_ASSIGN (scoped_rbreak_breakpoints);
};
-/* Breakpoint iterator function.
-
- Calls a callback function once for each breakpoint, so long as the
- callback function returns false. If the callback function returns
- true, the iteration will end and the current breakpoint will be
- returned. This can be useful for implementing a search for a
- breakpoint with arbitrary attributes, or for applying an operation
- to every breakpoint. */
-extern struct breakpoint *iterate_over_breakpoints
- (gdb::function_view<bool (breakpoint *)>);
-
/* Breakpoint linked list iterator. */
using breakpoint_iterator = next_iterator<breakpoint>;
+/* Breakpoint linked list range. */
+
+using breakpoint_range = next_adapter<breakpoint, breakpoint_iterator>;
+
+/* Return a range to iterate over all breakpoints. */
+
+breakpoint_range all_breakpoints ();
+
+/* Breakpoint linked list range, safe against deletion of the current
+ breakpoint while iterating. */
+
+using breakpoint_safe_range = basic_safe_range<breakpoint_range>;
+
+/* Return a range to iterate over all breakpoints. This range is safe against
+ deletion of the current breakpoint while iterating. */
+
+breakpoint_safe_range all_breakpoints_safe ();
+
/* Breakpoint filter to only keep tracepoints. */
struct tracepoint_filter
diff --git a/gdb/dummy-frame.c b/gdb/dummy-frame.c
index 155dec377f34..68a693797492 100644
--- a/gdb/dummy-frame.c
+++ b/gdb/dummy-frame.c
@@ -166,10 +166,9 @@ pop_dummy_frame (struct dummy_frame **dummy_ptr)
restore_infcall_suspend_state (dummy->caller_state);
- iterate_over_breakpoints ([dummy] (breakpoint* bp)
- {
- return pop_dummy_frame_bpt (bp, dummy);
- });
+ for (breakpoint *bp : all_breakpoints_safe ())
+ if (pop_dummy_frame_bpt (bp, dummy))
+ break;
/* restore_infcall_control_state frees inf_state,
all that remains is to pop *dummy_ptr. */
diff --git a/gdb/guile/scm-breakpoint.c b/gdb/guile/scm-breakpoint.c
index 826dfa9b0a38..4ff197e48a45 100644
--- a/gdb/guile/scm-breakpoint.c
+++ b/gdb/guile/scm-breakpoint.c
@@ -508,7 +508,7 @@ gdbscm_delete_breakpoint_x (SCM self)
/* iterate_over_breakpoints function for gdbscm_breakpoints. */
-static bool
+static void
bpscm_build_bp_list (struct breakpoint *bp, SCM *list)
{
breakpoint_smob *bp_smob = bp->scm_bp_object;
@@ -535,8 +535,6 @@ bpscm_build_bp_list (struct breakpoint *bp, SCM *list)
if (bp_smob != NULL)
*list = scm_cons (bp_smob->containing_scm, *list);
-
- return false;
}
/* (breakpoints) -> list
@@ -547,10 +545,8 @@ gdbscm_breakpoints (void)
{
SCM list = SCM_EOL;
- iterate_over_breakpoints ([&] (breakpoint *bp)
- {
- return bpscm_build_bp_list(bp, &list);
- });
+ for (breakpoint *bp : all_breakpoints ())
+ bpscm_build_bp_list (bp, &list);
return scm_reverse_x (list, SCM_EOL);
}
diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index 4710f3e9e6a6..a2ce4cdd06fd 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -898,25 +898,24 @@ bppy_init (PyObject *self, PyObject *args, PyObject *kwargs)
return 0;
}
-\f
+/* Append to LIST the breakpoint Python object associated to B.
+
+ Return true on success. Return false on failure, with the Python error
+ indicator set. */
static bool
build_bp_list (struct breakpoint *b, PyObject *list)
{
PyObject *bp = (PyObject *) b->py_bp_object;
- int iserr = 0;
/* Not all breakpoints will have a companion Python object.
Only breakpoints that were created via bppy_new, or
breakpoints that were created externally and are tracked by
the Python Scripting API. */
- if (bp)
- iserr = PyList_Append (list, bp);
-
- if (iserr == -1)
+ if (bp == nullptr)
return true;
- return false;
+ return PyList_Append (list, bp) == 0;
}
/* Static function to return a tuple holding all breakpoints. */
@@ -931,15 +930,11 @@ gdbpy_breakpoints (PyObject *self, PyObject *args)
if (list == NULL)
return NULL;
- /* If iterate_over_breakpoints returns non NULL it signals an error
- condition. In that case abandon building the list and return
- NULL. */
- auto callback = [&] (breakpoint *bp)
- {
- return build_bp_list(bp, list.get ());
- };
- if (iterate_over_breakpoints (callback) != NULL)
- return NULL;
+ /* If build_bp_list returns false, it signals an error condition. In that
+ case abandon building the list and return nullptr. */
+ for (breakpoint *bp : all_breakpoints ())
+ if (!build_bp_list (bp, list.get ()))
+ return nullptr;
return PyList_AsTuple (list.get ());
}
diff --git a/gdb/python/py-finishbreakpoint.c b/gdb/python/py-finishbreakpoint.c
index d2a1ec4fa98e..1d8373d807ed 100644
--- a/gdb/python/py-finishbreakpoint.c
+++ b/gdb/python/py-finishbreakpoint.c
@@ -342,7 +342,7 @@ bpfinishpy_out_of_scope (struct finish_breakpoint_object *bpfinish_obj)
/* Callback for `bpfinishpy_detect_out_scope'. Triggers Python's
`B->out_of_scope' function if B is a FinishBreakpoint out of its scope. */
-static bool
+static void
bpfinishpy_detect_out_scope_cb (struct breakpoint *b,
struct breakpoint *bp_stopped)
{
@@ -372,8 +372,6 @@ bpfinishpy_detect_out_scope_cb (struct breakpoint *b,
}
}
}
-
- return 0;
}
/* Attached to `stop' notifications, check if the execution has run
@@ -384,11 +382,8 @@ bpfinishpy_handle_stop (struct bpstats *bs, int print_frame)
{
gdbpy_enter enter_py (get_current_arch (), current_language);
- iterate_over_breakpoints ([&] (breakpoint *bp)
- {
- return bpfinishpy_detect_out_scope_cb
- (bp, bs == NULL ? NULL : bs->breakpoint_at);
- });
+ for (breakpoint *bp : all_breakpoints_safe ())
+ bpfinishpy_detect_out_scope_cb (bp, bs == NULL ? NULL : bs->breakpoint_at);
}
/* Attached to `exit' notifications, triggers all the necessary out of
@@ -399,10 +394,8 @@ bpfinishpy_handle_exit (struct inferior *inf)
{
gdbpy_enter enter_py (target_gdbarch (), current_language);
- iterate_over_breakpoints ([&] (breakpoint *bp)
- {
- return bpfinishpy_detect_out_scope_cb (bp, nullptr);
- });
+ for (breakpoint *bp : all_breakpoints_safe ())
+ bpfinishpy_detect_out_scope_cb (bp, nullptr);
}
/* Initialize the Python finish breakpoint code. */
diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index 2642e1ad2fdb..a8a7d1171dc6 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -2059,7 +2059,8 @@ svr4_update_solib_event_breakpoint (struct breakpoint *b)
static void
svr4_update_solib_event_breakpoints (void)
{
- iterate_over_breakpoints (svr4_update_solib_event_breakpoint);
+ for (breakpoint *bp : all_breakpoints_safe ())
+ svr4_update_solib_event_breakpoint (bp);
}
/* Create and register solib event breakpoints. PROBES is an array
diff --git a/gdb/tui/tui-winsource.c b/gdb/tui/tui-winsource.c
index 738f69156485..afd51e95980c 100644
--- a/gdb/tui/tui-winsource.c
+++ b/gdb/tui/tui-winsource.c
@@ -457,7 +457,7 @@ tui_source_window_base::update_breakpoint_info
do with it. Identify enable/disabled breakpoints as well as
those that we already hit. */
tui_bp_flags mode = 0;
- iterate_over_breakpoints ([&] (breakpoint *bp) -> bool
+ for (breakpoint *bp : all_breakpoints ())
{
if (bp == being_deleted)
return false;
@@ -479,7 +479,8 @@ tui_source_window_base::update_breakpoint_info
}
}
return false;
- });
+ }
+
if (line->break_mode != mode)
{
line->break_mode = mode;
--
2.31.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 9/9] gdb: remove iterate_over_bp_locations function
2021-05-27 15:35 [PATCH 0/9] Convert breakpoint iteration macros to ranges Simon Marchi
` (7 preceding siblings ...)
2021-05-27 15:35 ` [PATCH 8/9] gdb: remove iterate_over_breakpoints function Simon Marchi
@ 2021-05-27 15:35 ` Simon Marchi
2021-05-27 18:14 ` [PATCH 0/9] Convert breakpoint iteration macros to ranges Tom Tromey
9 siblings, 0 replies; 21+ messages in thread
From: Simon Marchi @ 2021-05-27 15:35 UTC (permalink / raw)
To: gdb-patches
Remove it, change users (well, a single one) to use all_bp_locations.
This requires moving all_bp_locations to breakpoint.h to expose it.
gdb/ChangeLog:
* breakpoint.h (iterate_over_bp_locations): Remove. Update
users to use all_bp_locations.
(all_bp_locations): New.
* breakpoint.c (all_bp_locations): Make non-static.
(iterate_over_bp_locations): Remove.
Change-Id: Iaf1f716d6c2c5b2975579b3dc113a86f5d0975be
---
gdb/breakpoint.c | 13 +++----------
gdb/breakpoint.h | 7 ++++---
gdb/record-full.c | 26 ++++++++++----------------
3 files changed, 17 insertions(+), 29 deletions(-)
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 751ed51cc7d3..225294f2bb26 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -521,7 +521,9 @@ all_tracepoints ()
static std::vector<bp_location *> bp_locations;
-static const std::vector<bp_location *> &
+/* See breakpoint.h. */
+
+const std::vector<bp_location *> &
all_bp_locations ()
{
return bp_locations;
@@ -2913,15 +2915,6 @@ insert_breakpoints (void)
update_global_location_list (UGLL_INSERT);
}
-/* Invoke CALLBACK for each of bp_location. */
-
-void
-iterate_over_bp_locations (gdb::function_view<void (bp_location *)> callback)
-{
- for (bp_location *loc : all_bp_locations ())
- callback (loc);
-}
-
/* This is used when we need to synch breakpoint conditions between GDB and the
target. It is the case with deleting and disabling of breakpoints when using
always-inserted mode. */
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index ffe042459eef..e40504f14ed3 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -1313,9 +1313,6 @@ extern void breakpoint_init_inferior (enum inf_context);
extern void breakpoint_auto_delete (bpstat);
-extern void iterate_over_bp_locations
- (gdb::function_view<void (bp_location *)> callback);
-
/* Return the chain of command lines to execute when this breakpoint
is hit. */
extern struct command_line *breakpoint_commands (struct breakpoint *b);
@@ -1755,6 +1752,10 @@ using tracepoint_range = next_adapter<breakpoint, tracepoint_iterator>;
tracepoint_range all_tracepoints ();
+/* Return a range to iterate over all breakpoint locations. */
+
+const std::vector<bp_location *> &all_bp_locations ();
+
/* Nonzero if the specified PC cannot be a location where functions
have been inlined. */
diff --git a/gdb/record-full.c b/gdb/record-full.c
index 91da1076194f..98d48e677443 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -1719,21 +1719,6 @@ struct record_full_breakpoint
active. */
static std::vector<record_full_breakpoint> record_full_breakpoints;
-static void
-record_full_sync_record_breakpoints (struct bp_location *loc)
-{
- if (loc->loc_type != bp_loc_software_breakpoint)
- return;
-
- if (loc->inserted)
- {
- record_full_breakpoints.emplace_back
- (loc->target_info.placed_address_space,
- loc->target_info.placed_address,
- 1);
- }
-}
-
/* Sync existing breakpoints to record_full_breakpoints. */
static void
@@ -1741,7 +1726,16 @@ record_full_init_record_breakpoints (void)
{
record_full_breakpoints.clear ();
- iterate_over_bp_locations (record_full_sync_record_breakpoints);
+ for (bp_location *loc : all_bp_locations ())
+ {
+ if (loc->loc_type != bp_loc_software_breakpoint)
+ continue;
+
+ if (loc->inserted)
+ record_full_breakpoints.emplace_back
+ (loc->target_info.placed_address_space,
+ loc->target_info.placed_address, 1);
+ }
}
/* Behavior is conditional on RECORD_FULL_IS_REPLAY. We will not actually
--
2.31.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/9] gdb: add all_breakpoints_safe function
2021-05-27 15:35 ` [PATCH 2/9] gdb: add all_breakpoints_safe function Simon Marchi
@ 2021-05-27 17:35 ` Tom Tromey
2021-05-27 17:58 ` Simon Marchi
0 siblings, 1 reply; 21+ messages in thread
From: Tom Tromey @ 2021-05-27 17:35 UTC (permalink / raw)
To: Simon Marchi via Gdb-patches
>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
Simon> @@ -3143,9 +3154,7 @@ remove_breakpoints (void)
Simon> static void
Simon> remove_threaded_breakpoints (struct thread_info *tp, int silent)
Simon> {
Simon> - struct breakpoint *b, *b_tmp;
Simon> -
Simon> - ALL_BREAKPOINTS_SAFE (b, b_tmp)
Simon> + for (breakpoint *b : all_breakpoints ())
Simon> {
Simon> if (b->thread == tp->global_num && user_breakpoint_p (b))
Simon> {
I had to go look, but this one is ok to convert to the non-safe
iterator, as it doesn't modify the collection in the loop.
Tom
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/9] gdb: add all_breakpoints_safe function
2021-05-27 17:35 ` Tom Tromey
@ 2021-05-27 17:58 ` Simon Marchi
2021-05-27 18:15 ` Tom Tromey
0 siblings, 1 reply; 21+ messages in thread
From: Simon Marchi @ 2021-05-27 17:58 UTC (permalink / raw)
To: Tom Tromey, Simon Marchi via Gdb-patches
On 2021-05-27 1:35 p.m., Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
>
> Simon> @@ -3143,9 +3154,7 @@ remove_breakpoints (void)
> Simon> static void
> Simon> remove_threaded_breakpoints (struct thread_info *tp, int silent)
> Simon> {
> Simon> - struct breakpoint *b, *b_tmp;
> Simon> -
> Simon> - ALL_BREAKPOINTS_SAFE (b, b_tmp)
> Simon> + for (breakpoint *b : all_breakpoints ())
> Simon> {
> Simon> if (b->thread == tp->global_num && user_breakpoint_p (b))
> Simon> {
>
> I had to go look, but this one is ok to convert to the non-safe
> iterator, as it doesn't modify the collection in the loop.
I don't think it was intentional to use all_breakpoints and not
all_breakpoints_safe here. I'm leaning towards changing it back to
all_breakpoints_safe, because it's not the goal of this patch to change
the behavior.
Simon
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 7/9] gdb: add all_bp_locations_at_addr function
2021-05-27 15:35 ` [PATCH 7/9] gdb: add all_bp_locations_at_addr function Simon Marchi
@ 2021-05-27 18:04 ` Tom Tromey
2021-05-27 18:13 ` Simon Marchi
0 siblings, 1 reply; 21+ messages in thread
From: Tom Tromey @ 2021-05-27 18:04 UTC (permalink / raw)
To: Simon Marchi via Gdb-patches
>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
Simon> + using iterator = std::vector<bp_location *>::iterator;
Simon> +private:
Simon> + std::vector<bp_location *>::iterator m_begin;
Simon> + std::vector<bp_location *>::iterator m_end;
It's slightly more future-proof to just write 'iterator' for these types.
Tom
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 7/9] gdb: add all_bp_locations_at_addr function
2021-05-27 18:04 ` Tom Tromey
@ 2021-05-27 18:13 ` Simon Marchi
0 siblings, 0 replies; 21+ messages in thread
From: Simon Marchi @ 2021-05-27 18:13 UTC (permalink / raw)
To: Tom Tromey, Simon Marchi via Gdb-patches
On 2021-05-27 2:04 p.m., Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
>
> Simon> + using iterator = std::vector<bp_location *>::iterator;
>
>
> Simon> +private:
> Simon> + std::vector<bp_location *>::iterator m_begin;
> Simon> + std::vector<bp_location *>::iterator m_end;
>
> It's slightly more future-proof to just write 'iterator' for these types.
>
> Tom
>
Good idea, done.
Simon
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/9] Convert breakpoint iteration macros to ranges
2021-05-27 15:35 [PATCH 0/9] Convert breakpoint iteration macros to ranges Simon Marchi
` (8 preceding siblings ...)
2021-05-27 15:35 ` [PATCH 9/9] gdb: remove iterate_over_bp_locations function Simon Marchi
@ 2021-05-27 18:14 ` Tom Tromey
2021-05-27 18:59 ` Simon Marchi
9 siblings, 1 reply; 21+ messages in thread
From: Tom Tromey @ 2021-05-27 18:14 UTC (permalink / raw)
To: Simon Marchi via Gdb-patches
>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
Simon> This all started with me finding that the update_global_location_list
Simon> function was hard to read, notably because the breakpoint iteration
Simon> macros are a bit hard to read (require multiple local variables and
Simon> pointers to pointers). So this removes all breakpoint-related iteration
Simon> macros in favor of functions that return iterable ranges.
This all looks good to me. I prefer external iteration like this when
possible, as I think it's easier to understand and write. Thank you.
Tom
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/9] gdb: add all_breakpoints_safe function
2021-05-27 17:58 ` Simon Marchi
@ 2021-05-27 18:15 ` Tom Tromey
0 siblings, 0 replies; 21+ messages in thread
From: Tom Tromey @ 2021-05-27 18:15 UTC (permalink / raw)
To: Simon Marchi via Gdb-patches; +Cc: Tom Tromey, Simon Marchi
>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
Simon> I don't think it was intentional to use all_breakpoints and not
Simon> all_breakpoints_safe here. I'm leaning towards changing it back to
Simon> all_breakpoints_safe, because it's not the goal of this patch to change
Simon> the behavior.
That seems fine as well.
Tom
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/9] Convert breakpoint iteration macros to ranges
2021-05-27 18:14 ` [PATCH 0/9] Convert breakpoint iteration macros to ranges Tom Tromey
@ 2021-05-27 18:59 ` Simon Marchi
0 siblings, 0 replies; 21+ messages in thread
From: Simon Marchi @ 2021-05-27 18:59 UTC (permalink / raw)
To: Tom Tromey, Simon Marchi via Gdb-patches
On 2021-05-27 2:14 p.m., Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi via Gdb-patches <gdb-patches@sourceware.org> writes:
>
> Simon> This all started with me finding that the update_global_location_list
> Simon> function was hard to read, notably because the breakpoint iteration
> Simon> macros are a bit hard to read (require multiple local variables and
> Simon> pointers to pointers). So this removes all breakpoint-related iteration
> Simon> macros in favor of functions that return iterable ranges.
>
> This all looks good to me. I prefer external iteration like this when
> possible, as I think it's easier to understand and write. Thank you.
>
> Tom
>
Thanks, pushed with the things you pointed out fixed.
Simon
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 8/9] gdb: remove iterate_over_breakpoints function
2021-05-27 15:35 ` [PATCH 8/9] gdb: remove iterate_over_breakpoints function Simon Marchi
@ 2021-10-21 10:20 ` Tom de Vries
2021-10-21 11:29 ` [PATCH, master + 11][gdb/tui] Fix breakpoint display functionality Tom de Vries
0 siblings, 1 reply; 21+ messages in thread
From: Tom de Vries @ 2021-10-21 10:20 UTC (permalink / raw)
To: Simon Marchi, gdb-patches; +Cc: Tom Tromey
On 5/27/21 5:35 PM, Simon Marchi via Gdb-patches wrote:
> /* Create and register solib event breakpoints. PROBES is an array
> diff --git a/gdb/tui/tui-winsource.c b/gdb/tui/tui-winsource.c
> index 738f69156485..afd51e95980c 100644
> --- a/gdb/tui/tui-winsource.c
> +++ b/gdb/tui/tui-winsource.c
> @@ -457,7 +457,7 @@ tui_source_window_base::update_breakpoint_info
> do with it. Identify enable/disabled breakpoints as well as
> those that we already hit. */
> tui_bp_flags mode = 0;
> - iterate_over_breakpoints ([&] (breakpoint *bp) -> bool
> + for (breakpoint *bp : all_breakpoints ())
> {
> if (bp == being_deleted)
> return false;
> @@ -479,7 +479,8 @@ tui_source_window_base::update_breakpoint_info
> }
> }
> return false;
> - });
> + }
> +
> if (line->break_mode != mode)
> {
> line->break_mode = mode;
> --
This changes a lambda function body to a loop body, but fails to update
the two returns. Consequently, showing breakpoints in tui is broken
(and unfortunately there's no test-case to detect that).
This works for me:
...
diff --git a/gdb/tui/tui-winsource.c b/gdb/tui/tui-winsource.c
index afd51e95980..955b68901fe 100644
--- a/gdb/tui/tui-winsource.c
+++ b/gdb/tui/tui-winsource.c
@@ -460,7 +460,7 @@ tui_source_window_base::update_breakpoint_info
for (breakpoint *bp : all_breakpoints ())
{
if (bp == being_deleted)
- return false;
+ continue;
for (bp_location *loc : bp->locations ())
{
@@ -478,7 +478,6 @@ tui_source_window_base::update_breakpoint_info
mode |= TUI_BP_HARDWARE;
}
}
- return false;
}
if (line->break_mode != mode)
...
Thanks,
- Tom
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH, master + 11][gdb/tui] Fix breakpoint display functionality
2021-10-21 10:20 ` Tom de Vries
@ 2021-10-21 11:29 ` Tom de Vries
2021-10-21 12:10 ` Tom de Vries
0 siblings, 1 reply; 21+ messages in thread
From: Tom de Vries @ 2021-10-21 11:29 UTC (permalink / raw)
To: Simon Marchi, gdb-patches; +Cc: Tom Tromey
[-- Attachment #1: Type: text/plain, Size: 1349 bytes --]
[ was: Re: [PATCH 8/9] gdb: remove iterate_over_breakpoints function ]
On 10/21/21 12:20 PM, Tom de Vries wrote:
> On 5/27/21 5:35 PM, Simon Marchi via Gdb-patches wrote:
>> /* Create and register solib event breakpoints. PROBES is an array
>> diff --git a/gdb/tui/tui-winsource.c b/gdb/tui/tui-winsource.c
>> index 738f69156485..afd51e95980c 100644
>> --- a/gdb/tui/tui-winsource.c
>> +++ b/gdb/tui/tui-winsource.c
>> @@ -457,7 +457,7 @@ tui_source_window_base::update_breakpoint_info
>> do with it. Identify enable/disabled breakpoints as well as
>> those that we already hit. */
>> tui_bp_flags mode = 0;
>> - iterate_over_breakpoints ([&] (breakpoint *bp) -> bool
>> + for (breakpoint *bp : all_breakpoints ())
>> {
>> if (bp == being_deleted)
>> return false;
>> @@ -479,7 +479,8 @@ tui_source_window_base::update_breakpoint_info
>> }
>> }
>> return false;
>> - });
>> + }
>> +
>> if (line->break_mode != mode)
>> {
>> line->break_mode = mode;
>> --
>
> This changes a lambda function body to a loop body, but fails to update
> the two returns. Consequently, showing breakpoints in tui is broken
> (and unfortunately there's no test-case to detect that).
>
> This works for me:
Now with commit log, referring to corresponding PR.
OK for trunk / 11 branch ?
Thanks,
- Tom
[-- Attachment #2: 0001-gdb-tui-Fix-breakpoint-display-functionality.patch --]
[-- Type: text/x-patch, Size: 1661 bytes --]
[gdb/tui] Fix breakpoint display functionality
In commit 81e6b8eb208 "Make tui-winsource not use breakpoint_chain", a loop
body was transformed into a lambda function body:
...
- for (bp = breakpoint_chain;
- bp != NULL;
- bp = bp->next)
+ iterate_over_breakpoints ([&] (breakpoint *bp) -> bool
...
and consequently:
- a continue was replaced by a return, and
- a final return was added.
Then in commit 240edef62f0 "gdb: remove iterate_over_breakpoints function", we
transformed back to a loop body:
...
- iterate_over_breakpoints ([&] (breakpoint *bp) -> bool
+ for (breakpoint *bp : all_breakpoints ())
...
but without reverting the changes that introduced the two returns.
Consequently, breakpoints no longer show up in the tui source window.
Fix this by reverting the changes that introduced the two returns.
Build on x86_64-linux, tested with all .exp test-cases that contain
tuiterm_env.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28483
---
gdb/tui/tui-winsource.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/gdb/tui/tui-winsource.c b/gdb/tui/tui-winsource.c
index afd51e95980..955b68901fe 100644
--- a/gdb/tui/tui-winsource.c
+++ b/gdb/tui/tui-winsource.c
@@ -460,7 +460,7 @@ tui_source_window_base::update_breakpoint_info
for (breakpoint *bp : all_breakpoints ())
{
if (bp == being_deleted)
- return false;
+ continue;
for (bp_location *loc : bp->locations ())
{
@@ -478,7 +478,6 @@ tui_source_window_base::update_breakpoint_info
mode |= TUI_BP_HARDWARE;
}
}
- return false;
}
if (line->break_mode != mode)
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH, master + 11][gdb/tui] Fix breakpoint display functionality
2021-10-21 11:29 ` [PATCH, master + 11][gdb/tui] Fix breakpoint display functionality Tom de Vries
@ 2021-10-21 12:10 ` Tom de Vries
2021-10-21 14:28 ` Simon Marchi
0 siblings, 1 reply; 21+ messages in thread
From: Tom de Vries @ 2021-10-21 12:10 UTC (permalink / raw)
To: Simon Marchi, gdb-patches; +Cc: Tom Tromey
[-- Attachment #1: Type: text/plain, Size: 1465 bytes --]
On 10/21/21 1:29 PM, Tom de Vries wrote:
> [ was: Re: [PATCH 8/9] gdb: remove iterate_over_breakpoints function ]
>
> On 10/21/21 12:20 PM, Tom de Vries wrote:
>> On 5/27/21 5:35 PM, Simon Marchi via Gdb-patches wrote:
>>> /* Create and register solib event breakpoints. PROBES is an array
>>> diff --git a/gdb/tui/tui-winsource.c b/gdb/tui/tui-winsource.c
>>> index 738f69156485..afd51e95980c 100644
>>> --- a/gdb/tui/tui-winsource.c
>>> +++ b/gdb/tui/tui-winsource.c
>>> @@ -457,7 +457,7 @@ tui_source_window_base::update_breakpoint_info
>>> do with it. Identify enable/disabled breakpoints as well as
>>> those that we already hit. */
>>> tui_bp_flags mode = 0;
>>> - iterate_over_breakpoints ([&] (breakpoint *bp) -> bool
>>> + for (breakpoint *bp : all_breakpoints ())
>>> {
>>> if (bp == being_deleted)
>>> return false;
>>> @@ -479,7 +479,8 @@ tui_source_window_base::update_breakpoint_info
>>> }
>>> }
>>> return false;
>>> - });
>>> + }
>>> +
>>> if (line->break_mode != mode)
>>> {
>>> line->break_mode = mode;
>>> --
>>
>> This changes a lambda function body to a loop body, but fails to update
>> the two returns. Consequently, showing breakpoints in tui is broken
>> (and unfortunately there's no test-case to detect that).
>>
>> This works for me:
>
> Now with commit log, referring to corresponding PR.
>
> OK for trunk / 11 branch ?
Now also included test-case.
Thanks,
- Tom
[-- Attachment #2: 0001-gdb-tui-Fix-breakpoint-display-functionality.patch --]
[-- Type: text/x-patch, Size: 3159 bytes --]
[gdb/tui] Fix breakpoint display functionality
In commit 81e6b8eb208 "Make tui-winsource not use breakpoint_chain", a loop
body was transformed into a lambda function body:
...
- for (bp = breakpoint_chain;
- bp != NULL;
- bp = bp->next)
+ iterate_over_breakpoints ([&] (breakpoint *bp) -> bool
...
and consequently:
- a continue was replaced by a return, and
- a final return was added.
Then in commit 240edef62f0 "gdb: remove iterate_over_breakpoints function", we
transformed back to a loop body:
...
- iterate_over_breakpoints ([&] (breakpoint *bp) -> bool
+ for (breakpoint *bp : all_breakpoints ())
...
but without reverting the changes that introduced the two returns.
Consequently, breakpoints no longer show up in the tui source window.
Fix this by reverting the changes that introduced the two returns.
Build on x86_64-linux, tested with all .exp test-cases that contain
tuiterm_env.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28483
---
gdb/testsuite/gdb.tui/break.exp | 37 +++++++++++++++++++++++++++++++++++++
gdb/tui/tui-winsource.c | 3 +--
2 files changed, 38 insertions(+), 2 deletions(-)
diff --git a/gdb/testsuite/gdb.tui/break.exp b/gdb/testsuite/gdb.tui/break.exp
new file mode 100644
index 00000000000..de4821c570a
--- /dev/null
+++ b/gdb/testsuite/gdb.tui/break.exp
@@ -0,0 +1,37 @@
+# Copyright 2021 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/>.
+
+# Verify that breakpoint marker is shown.
+
+tuiterm_env
+
+standard_testfile tui-layout.c
+
+if {[build_executable "failed to prepare" ${testfile} ${srcfile}] == -1} {
+ return -1
+}
+
+Term::clean_restart 24 80 $testfile
+if {![Term::enter_tui]} {
+ unsupported "TUI not supported"
+ return
+}
+
+set text [Term::get_all_lines]
+gdb_assert {![string match "No Source Available" $text]} \
+ "initial source listing"
+
+Term::command "b main"
+Term::check_contents "break marker present" "\\|b\\+"
diff --git a/gdb/tui/tui-winsource.c b/gdb/tui/tui-winsource.c
index afd51e95980..955b68901fe 100644
--- a/gdb/tui/tui-winsource.c
+++ b/gdb/tui/tui-winsource.c
@@ -460,7 +460,7 @@ tui_source_window_base::update_breakpoint_info
for (breakpoint *bp : all_breakpoints ())
{
if (bp == being_deleted)
- return false;
+ continue;
for (bp_location *loc : bp->locations ())
{
@@ -478,7 +478,6 @@ tui_source_window_base::update_breakpoint_info
mode |= TUI_BP_HARDWARE;
}
}
- return false;
}
if (line->break_mode != mode)
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH, master + 11][gdb/tui] Fix breakpoint display functionality
2021-10-21 12:10 ` Tom de Vries
@ 2021-10-21 14:28 ` Simon Marchi
0 siblings, 0 replies; 21+ messages in thread
From: Simon Marchi @ 2021-10-21 14:28 UTC (permalink / raw)
To: Tom de Vries, gdb-patches; +Cc: Tom Tromey
On 2021-10-21 8:10 a.m., Tom de Vries wrote:
> On 10/21/21 1:29 PM, Tom de Vries wrote:
>> [ was: Re: [PATCH 8/9] gdb: remove iterate_over_breakpoints function ]
>>
>> On 10/21/21 12:20 PM, Tom de Vries wrote:
>>> On 5/27/21 5:35 PM, Simon Marchi via Gdb-patches wrote:
>>>> /* Create and register solib event breakpoints. PROBES is an array
>>>> diff --git a/gdb/tui/tui-winsource.c b/gdb/tui/tui-winsource.c
>>>> index 738f69156485..afd51e95980c 100644
>>>> --- a/gdb/tui/tui-winsource.c
>>>> +++ b/gdb/tui/tui-winsource.c
>>>> @@ -457,7 +457,7 @@ tui_source_window_base::update_breakpoint_info
>>>> do with it. Identify enable/disabled breakpoints as well as
>>>> those that we already hit. */
>>>> tui_bp_flags mode = 0;
>>>> - iterate_over_breakpoints ([&] (breakpoint *bp) -> bool
>>>> + for (breakpoint *bp : all_breakpoints ())
>>>> {
>>>> if (bp == being_deleted)
>>>> return false;
>>>> @@ -479,7 +479,8 @@ tui_source_window_base::update_breakpoint_info
>>>> }
>>>> }
>>>> return false;
>>>> - });
>>>> + }
>>>> +
>>>> if (line->break_mode != mode)
>>>> {
>>>> line->break_mode = mode;
>>>> --
>>>
>>> This changes a lambda function body to a loop body, but fails to update
>>> the two returns. Consequently, showing breakpoints in tui is broken
>>> (and unfortunately there's no test-case to detect that).
>>>
>>> This works for me:
>>
>> Now with commit log, referring to corresponding PR.
>>
>> OK for trunk / 11 branch ?
>
> Now also included test-case.
>
> Thanks,
> - Tom
>
Woops, LGTM, thanks.
Simon
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2021-10-21 14:28 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-27 15:35 [PATCH 0/9] Convert breakpoint iteration macros to ranges Simon Marchi
2021-05-27 15:35 ` [PATCH 1/9] gdb: add all_breakpoints function Simon Marchi
2021-05-27 15:35 ` [PATCH 2/9] gdb: add all_breakpoints_safe function Simon Marchi
2021-05-27 17:35 ` Tom Tromey
2021-05-27 17:58 ` Simon Marchi
2021-05-27 18:15 ` Tom Tromey
2021-05-27 15:35 ` [PATCH 3/9] gdb: add all_tracepoints function Simon Marchi
2021-05-27 15:35 ` [PATCH 4/9] gdb: add breakpoint::locations method Simon Marchi
2021-05-27 15:35 ` [PATCH 5/9] gdb: make bp_locations an std::vector Simon Marchi
2021-05-27 15:35 ` [PATCH 6/9] gdb: add all_bp_locations function Simon Marchi
2021-05-27 15:35 ` [PATCH 7/9] gdb: add all_bp_locations_at_addr function Simon Marchi
2021-05-27 18:04 ` Tom Tromey
2021-05-27 18:13 ` Simon Marchi
2021-05-27 15:35 ` [PATCH 8/9] gdb: remove iterate_over_breakpoints function Simon Marchi
2021-10-21 10:20 ` Tom de Vries
2021-10-21 11:29 ` [PATCH, master + 11][gdb/tui] Fix breakpoint display functionality Tom de Vries
2021-10-21 12:10 ` Tom de Vries
2021-10-21 14:28 ` Simon Marchi
2021-05-27 15:35 ` [PATCH 9/9] gdb: remove iterate_over_bp_locations function Simon Marchi
2021-05-27 18:14 ` [PATCH 0/9] Convert breakpoint iteration macros to ranges Tom Tromey
2021-05-27 18:59 ` Simon Marchi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).