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
This commit is contained in:
Simon Marchi
2021-05-27 14:58:37 -04:00
parent 40cb8ca539
commit 5d51cd5d14
2 changed files with 40 additions and 58 deletions

View File

@ -1,3 +1,11 @@
2021-05-27 Simon Marchi <simon.marchi@polymtl.ca>
* 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 <simon.marchi@polymtl.ca> 2021-05-27 Simon Marchi <simon.marchi@polymtl.ca>
* breakpoint.h (bp_locations_range): New. * breakpoint.h (bp_locations_range): New.

View File

@ -496,8 +496,8 @@ bool target_exact_watchpoints = false;
while executing the block of ALL_BP_LOCATIONS. */ while executing the block of ALL_BP_LOCATIONS. */
#define ALL_BP_LOCATIONS(B,BP_TMP) \ #define ALL_BP_LOCATIONS(B,BP_TMP) \
for (BP_TMP = bp_locations; \ for (BP_TMP = bp_locations.data (); \
BP_TMP < bp_locations + bp_locations_count && (B = *BP_TMP);\ BP_TMP < bp_locations.data () + bp_locations.size () && (B = *BP_TMP);\
BP_TMP++) BP_TMP++)
/* Iterates through locations with address ADDRESS for the currently selected /* 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, \ 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_TMP = BP_LOCP_START; \
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)->address == ADDRESS); \
BP_LOCP_TMP++) BP_LOCP_TMP++)
@ -554,11 +554,7 @@ all_tracepoints ()
/* Array is sorted by bp_location_is_less_than - primarily by the ADDRESS. */ /* Array is sorted by bp_location_is_less_than - primarily by the ADDRESS. */
static struct bp_location **bp_locations; static std::vector<bp_location *> bp_locations;
/* Number of elements of BP_LOCATIONS. */
static unsigned bp_locations_count;
/* Maximum alignment offset between bp_target_info.PLACED_ADDRESS and /* Maximum alignment offset between bp_target_info.PLACED_ADDRESS and
ADDRESS for the current elements of BP_LOCATIONS which get a valid 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. */ /* Find a close match to the first location at ADDRESS. */
locp_found = ((struct bp_location **) 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 **), sizeof (struct bp_location **),
bp_locations_compare_addrs)); 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 /* 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. */ 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 - 1))->address == address)
locp_found--; locp_found--;
@ -1582,7 +1578,7 @@ breakpoint_xfer_memory (gdb_byte *readbuf, gdb_byte *writebuf,
report higher one. */ report higher one. */
bc_l = 0; bc_l = 0;
bc_r = bp_locations_count; bc_r = bp_locations.size ();
while (bc_l + 1 < bc_r) while (bc_l + 1 < bc_r)
{ {
struct bp_location *bl; 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. */ /* 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]; struct bp_location *bl = bp_locations[bc];
@ -11831,7 +11827,6 @@ force_breakpoint_reinsertion (struct bp_location *bl)
static void static void
update_global_location_list (enum ugll_insert_mode insert_mode) update_global_location_list (enum ugll_insert_mode insert_mode)
{ {
struct bp_location **locp;
/* Last breakpoint location address that was marked for update. */ /* Last breakpoint location address that was marked for update. */
CORE_ADDR last_addr = 0; CORE_ADDR last_addr = 0;
/* Last breakpoint location program space that was marked for update. */ /* 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 /* Saved former bp_locations array which we compare against the newly
built bp_locations from the current state of ALL_BREAKPOINTS. */ built bp_locations from the current state of ALL_BREAKPOINTS. */
struct bp_location **old_locp; std::vector<bp_location *> old_locations = std::move (bp_locations);
unsigned old_locations_count; bp_locations.clear ();
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++;
bp_locations = XNEWVEC (struct bp_location *, bp_locations_count);
locp = bp_locations;
for (breakpoint *b : all_breakpoints ()) for (breakpoint *b : all_breakpoints ())
for (bp_location *loc : b->locations ()) 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 /* See if we need to "upgrade" a software breakpoint to a hardware
breakpoint. Do this before deciding whether locations are breakpoint. Do this before deciding whether locations are
duplicates. Also do this before sorting because sorting order duplicates. Also do this before sorting because sorting order
depends on location type. */ depends on location type. */
for (locp = bp_locations; for (bp_location *loc : bp_locations)
locp < bp_locations + bp_locations_count;
locp++)
{
bp_location *loc = *locp;
if (!loc->inserted && should_be_inserted (loc)) if (!loc->inserted && should_be_inserted (loc))
handle_automatic_hardware_breakpoints (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_location_is_less_than);
bp_locations_target_extensions_update (); 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 LOCP is kept in sync with OLD_LOCP, each pointing to the current
and former bp_location array state respectively. */ and former bp_location array state respectively. */
locp = bp_locations; size_t loc_i = 0;
for (old_locp = old_locations.get (); for (bp_location *old_loc : old_locations)
old_locp < old_locations.get () + old_locations_count;
old_locp++)
{ {
struct bp_location *old_loc = *old_locp;
struct bp_location **loc2p;
/* Tells if 'old_loc' is found among the new locations. If /* Tells if 'old_loc' is found among the new locations. If
not, we have to free it. */ not, we have to free it. */
int found_object = 0; 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. /* Skip LOCP entries which will definitely never be needed.
Stop either at or being the one matching OLD_LOC. */ Stop either at or being the one matching OLD_LOC. */
while (locp < bp_locations + bp_locations_count while (loc_i < bp_locations.size ()
&& (*locp)->address < old_loc->address) && bp_locations[loc_i]->address < old_loc->address)
locp++; loc_i++;
for (loc2p = locp; for (size_t loc2_i = loc_i;
(loc2p < bp_locations + bp_locations_count (loc2_i < bp_locations.size ()
&& (*loc2p)->address == old_loc->address); && bp_locations[loc2_i]->address == old_loc->address);
loc2p++) loc2_i++)
{ {
/* Check if this is a new/duplicated location or a duplicated /* Check if this is a new/duplicated location or a duplicated
location that had its condition modified. If so, we want to send location that had its condition modified. If so, we want to send
its condition to the target if evaluation of conditions is taking its condition to the target if evaluation of conditions is taking
place there. */ place there. */
if ((*loc2p)->condition_changed == condition_modified if (bp_locations[loc2_i]->condition_changed == condition_modified
&& (last_addr != old_loc->address && (last_addr != old_loc->address
|| last_pspace_num != old_loc->pspace->num)) || 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; last_pspace_num = old_loc->pspace->num;
} }
if (*loc2p == old_loc) if (bp_locations[loc2_i] == old_loc)
found_object = 1; found_object = 1;
} }
@ -11977,12 +11951,12 @@ update_global_location_list (enum ugll_insert_mode insert_mode)
/* OLD_LOC comes from existing struct breakpoint. */ /* OLD_LOC comes from existing struct breakpoint. */
if (bl_address_is_meaningful (old_loc)) if (bl_address_is_meaningful (old_loc))
{ {
for (loc2p = locp; for (size_t loc2_i = loc_i;
(loc2p < bp_locations + bp_locations_count (loc2_i < bp_locations.size ()
&& (*loc2p)->address == old_loc->address); && bp_locations[loc2_i]->address == old_loc->address);
loc2p++) loc2_i++)
{ {
struct bp_location *loc2 = *loc2p; bp_location *loc2 = bp_locations[loc2_i];
if (loc2 == old_loc) if (loc2 == old_loc)
continue; continue;
@ -12121,7 +12095,7 @@ update_global_location_list (enum ugll_insert_mode insert_mode)
awp_loc_first = NULL; awp_loc_first = NULL;
rwp_loc_first = NULL; rwp_loc_first = NULL;
bp_location *loc; bp_location *loc, **locp;
ALL_BP_LOCATIONS (loc, locp) ALL_BP_LOCATIONS (loc, locp)
{ {
/* ALL_BP_LOCATIONS bp_location has LOC->OWNER always /* ALL_BP_LOCATIONS bp_location has LOC->OWNER always