Re: [patch 4/8] Download tracepoint locations and track its status

Yao Qi
2011-11-09T03:47:47Z

[-- Attachment #1: Type: text/plain, Size: 132 bytes --]

This is a updated version to reflect the change in return value of
to_download_tracepoint_loc in patch 1/8.

--
Yao (é½å°§)

[-- Attachment #2: 0004-tracepoint-change-loc.patch --]
[-- Type: text/x-patch, Size: 9744 bytes --]

* breakpoint.h (struct bp_location): Add comment on field
`duplicate'.
* breakpoint.c (should_be_inserted): Don't differentiate breakpoint
and tracepoint.
(remove_breakpoints): Don't remove tracepoints.
(disable_breakpoints_in_unloaded_shlib): Handle tracepoint.
(download_tracepoint_locations): New.
(update_global_location_list): Call it. Consider bp's type when
swap bp_location.
* tracepoint.c (find_matching_tracepoint): Delete.
(find_matching_tracepoint_location): Renamed from
find_matching_tracepoint. Return bp_location rather than
tracepoint.
(merge_uploaded_tracepoints): Set `inserted' field to 1 if
tracepoint is found.
---
gdb/breakpoint.c | 89 +++++++++++++++++++++++++++++++++++++++++++----------
gdb/breakpoint.h | 8 ++++-
gdb/tracepoint.c | 39 ++++++++++++++++-------
3 files changed, 106 insertions(+), 30 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 8c98bef..7444b38 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -1555,7 +1555,10 @@ in which its expression is valid.\n"),

/* Returns 1 iff breakpoint location should be
- inserted in the inferior. */
+ inserted in the inferior. We don't differentiate the type of BL's owner
+ (breakpoint vs. tracepoint), although insert_location in tracepoint's
+ breakpoint_ops is not defined, because in insert_bp_location,
+ tracepoint's insert_location will not be called. */
static int
should_be_inserted (struct bp_location *bl)
{
@@ -1579,11 +1582,6 @@ should_be_inserted (struct bp_location *bl)
if (bl->pspace->breakpoints_not_allowed)
return 0;

- /* Tracepoints are inserted by the target at a time of its choosing,
- not by us. */
- if (is_tracepoint (bl->owner))
- return 0;
-
return 1;
}

@@ -2049,7 +2047,7 @@ remove_breakpoints (void)

ALL_BP_LOCATIONS (bl, blp_tmp)
{
- if (bl->inserted)
+ if (bl->inserted && !is_tracepoint (bl->owner))
val |= remove_breakpoint (bl, mark_uninserted);
}
return val;
@@ -6071,8 +6069,8 @@ disable_breakpoints_in_shlibs (void)
}
}

-/* Disable any breakpoints that are in an unloaded shared library.
- Only apply to enabled breakpoints, disabled ones can just stay
+/* Disable any breakpoints and tracepoints that are in an unloaded shared
+ library. Only apply to enabled breakpoints, disabled ones can just stay
disabled. */

static void
@@ -6094,13 +6092,14 @@ disable_breakpoints_in_unloaded_shlib (struct so_list *solib)
/* ALL_BP_LOCATIONS bp_location has LOC->OWNER always non-NULL. */
struct breakpoint *b = loc->owner;

- if ((loc->loc_type == bp_loc_hardware_breakpoint
- || loc->loc_type == bp_loc_software_breakpoint)
- && solib->pspace == loc->pspace
+ if (solib->pspace == loc->pspace
&& !loc->shlib_disabled
- && (b->type == bp_breakpoint
- || b->type == bp_jit_event
- || b->type == bp_hardware_breakpoint)
+ && (((b->type == bp_breakpoint
+ || b->type == bp_jit_event
+ || b->type == bp_hardware_breakpoint)
+ && (loc->loc_type == bp_loc_hardware_breakpoint
+ || loc->loc_type == bp_loc_software_breakpoint))
+ || is_tracepoint (b))
&& solib_contains_address_p (solib, loc->address))
{
loc->shlib_disabled = 1;
@@ -10325,6 +10324,49 @@ bp_location_target_extensions_update (void)
}
}

+/* Download tracepoint locations if they haven't been. */
+
+static void
+download_tracepoint_locations (void)
+{
+ struct bp_location *bl, **blp_tmp;
+ struct cleanup *old_chain;
+
+ if (!target_can_download_tracepoint_loc ())
+ return;
+
+ old_chain = save_current_space_and_thread ();
+
+ ALL_BP_LOCATIONS (bl, blp_tmp)
+ {
+ struct tracepoint *t;
+
+ if (!is_tracepoint (bl->owner))
+ continue;
+
+ if ((bl->owner->type == bp_fast_tracepoint
+ ? !may_insert_fast_tracepoints
+ : !may_insert_tracepoints))
+ continue;
+
+ /* In tracepoint, locations are _never_ duplicated, so
+ should_be_inserted is equivalent to
+ unduplicated_should_be_inserted. */
+ if (!should_be_inserted (bl) || bl->inserted)
+ continue;
+
+ switch_to_program_space_and_thread (bl->pspace);
+
+ target_download_tracepoint_loc (bl);
+
+ bl->inserted = 1;
+ t = (struct tracepoint *) bl->owner;
+ t->number_on_target = bl->owner->number;
+ }
+
+ do_cleanups (old_chain);
+}
+
/* Swap the insertion/duplication state between two locations. */

static void
@@ -10334,6 +10376,12 @@ swap_insertion (struct bp_location *left, struct bp_location *right)
const int left_duplicate = left->duplicate;
const struct bp_target_info left_target_info = left->target_info;

+ /* Locations of tracepoints never be duplicated. */
+ if (is_tracepoint (left->owner))
+ gdb_assert (left->duplicate == 0);
+ if (is_tracepoint (right->owner))
+ gdb_assert (right->duplicate == 0);
+
left->inserted = right->inserted;
left->duplicate = right->duplicate;
left->target_info = right->target_info;
@@ -10424,7 +10472,7 @@ update_global_location_list (int should_insert)
int keep_in_target = 0;
int removed = 0;

- /* 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. */
while (locp < bp_location + bp_location_count
&& (*locp)->address < old_loc->address)
@@ -10491,7 +10539,9 @@ update_global_location_list (int should_insert)
if it should be inserted in case it will be
unduplicated. */
if (loc2 != old_loc
- && unduplicated_should_be_inserted (loc2))
+ && unduplicated_should_be_inserted (loc2)
+ && (is_tracepoint (old_loc->owner)
+ == is_tracepoint (loc2->owner)))
{
swap_insertion (old_loc, loc2);
keep_in_target = 1;
@@ -10613,6 +10663,9 @@ update_global_location_list (int should_insert)
|| !loc->enabled
|| loc->shlib_disabled
|| !breakpoint_address_is_meaningful (b)
+ /* Don't detect duplicate for tracepoint locations because they are
+ never duplicated. See the comments in field `duplicate' of
+ `struct bp_location'. */
|| is_tracepoint (b))
continue;

@@ -10660,6 +10713,8 @@ update_global_location_list (int should_insert)
|| (gdbarch_has_global_breakpoints (target_gdbarch))))
insert_breakpoint_locations ();

+ download_tracepoint_locations ();
+
do_cleanups (cleanups);
}

diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index fe381df..a1aa9ea 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -335,7 +335,13 @@ struct bp_location
char inserted;

/* Nonzero if this is not the first breakpoint in the list
- for the given address. */
+ for the given address. In current implementation, location
+ of tracepoint _never_ be duplicated with other locations of
+ tracepoints and other kinds of breakpoints, because two locations
+ at the same address may have different actions, so both of
+ these locations should be downloaded. It is possible that
+ two locations have the same address and actions, and they can
+ be treated as `duplicated', but we didn't do it in code. */
char duplicate;

/* If we someday support real thread-specific breakpoints, then
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index 493d324..98f1ade 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -1711,7 +1711,15 @@ start_tracing (void)
t->number_on_target = 0;

for (loc = b->loc; loc; loc = loc->next)
- target_download_tracepoint_loc (loc);
+ {
+ /* Since tracepoint locations are never duplicated, `inserted'
+ flag should be zero. */
+ gdb_assert (loc->inserted == 0);
+
+ target_download_tracepoint_loc (loc);
+
+ loc->inserted = 1;
+ }

t->number_on_target = b->number;
}
@@ -3203,10 +3211,10 @@ cond_string_is_same (char *str1, char *str2)
/* Look for an existing tracepoint that seems similar enough to the
uploaded one. Enablement isn't compared, because the user can
toggle that freely, and may have done so in anticipation of the
- next trace run. */
+ next trace run. Return the location of matched tracepoint. */

-struct tracepoint *
-find_matching_tracepoint (struct uploaded_tp *utp)
+struct bp_location *
+find_matching_tracepoint_location (struct uploaded_tp *utp)
{
VEC(breakpoint_p) *tp_vec = all_tracepoints ();
int ix;
@@ -3228,7 +3236,7 @@ find_matching_tracepoint (struct uploaded_tp *utp)
for (loc = b->loc; loc; loc = loc->next)
{
if (loc->address == utp->addr)
- return t;
+ return loc;
}
}
}
@@ -3243,17 +3251,24 @@ void
merge_uploaded_tracepoints (struct uploaded_tp **uploaded_tps)
{
struct uploaded_tp *utp;
- struct tracepoint *t;

/* Look for GDB tracepoints that match up with our uploaded versions. */
for (utp = *uploaded_tps; utp; utp = utp->next)
{
- t = find_matching_tracepoint (utp);
- if (t)
- printf_filtered (_("Assuming tracepoint %d is same "
- "as target's tracepoint %d at %s.\n"),
- t->base.number, utp->number,
- paddress (get_current_arch (), utp->addr));
+ struct bp_location *loc;
+ struct tracepoint *t;
+
+ loc = find_matching_tracepoint_location (utp);
+ if (loc)
+ {
+ /* Mark this location as already inserted. */
+ loc->inserted = 1;
+ t = (struct tracepoint *) loc->owner;
+ printf_filtered (_("Assuming tracepoint %d is same "
+ "as target's tracepoint %d at %s.\n"),
+ loc->owner->number, utp->number,
+ paddress (loc->gdbarch, utp->addr));
+ }
else
{
t = create_tracepoint_from_upload (utp);
--
1.7.0.4