[PATCH 5/9] gdb: make bp_locations an std::vector

Simon Marchi via Gdb-patches
2021-05-27T15:36:12Z

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