From 5d51cd5d14d12056585cf7525cd82af521e45894 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Thu, 27 May 2021 14:58:37 -0400 Subject: [PATCH] gdb: make bp_locations an std::vector 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/ChangeLog | 8 +++++ gdb/breakpoint.c | 90 +++++++++++++++++------------------------------- 2 files changed, 40 insertions(+), 58 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index a52819459fa..9add064e9ec 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,11 @@ +2021-05-27 Simon Marchi + + * 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**. + 2021-05-27 Simon Marchi * breakpoint.h (bp_locations_range): New. diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 593a7714886..f5f80794c05 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_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 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 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.30.2