From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 17824 invoked by alias); 9 Nov 2011 03:47:47 -0000 Received: (qmail 17808 invoked by uid 22791); 9 Nov 2011 03:47:45 -0000 X-SWARE-Spam-Status: No, hits=-1.7 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 09 Nov 2011 03:47:30 +0000 Received: from nat-jpt.mentorg.com ([192.94.33.2] helo=PR1-MAIL.mgc.mentorg.com) by relay1.mentorg.com with esmtp id 1RNz8L-00029y-HQ from Yao_Qi@mentor.com for gdb-patches@sourceware.org; Tue, 08 Nov 2011 19:47:29 -0800 Received: from [127.0.0.1] ([172.16.63.104]) by PR1-MAIL.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.1830); Wed, 9 Nov 2011 12:47:27 +0900 Message-ID: <4EB9F7BF.7000005@codesourcery.com> Date: Wed, 09 Nov 2011 03:47:00 -0000 From: Yao Qi User-Agent: Mozilla/5.0 (X11; Linux i686; rv:7.0.1) Gecko/20110930 Thunderbird/7.0.1 MIME-Version: 1.0 To: gdb-patches@sourceware.org Subject: Re: [patch 4/8] Download tracepoint locations and track its status References: <4EB8C551.9090609@codesourcery.com> <4EB8CC79.4010100@codesourcery.com> In-Reply-To: <4EB8CC79.4010100@codesourcery.com> Content-Type: multipart/mixed; boundary="------------020707060400030501070000" X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2011-11/txt/msg00219.txt.bz2 This is a multi-part message in MIME format. --------------020707060400030501070000 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Content-length: 126 This is a updated version to reflect the change in return value of to_download_tracepoint_loc in patch 1/8. -- Yao (齐尧) --------------020707060400030501070000 Content-Type: text/x-patch; name="0004-tracepoint-change-loc.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="0004-tracepoint-change-loc.patch" Content-length: 9744 * 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 --------------020707060400030501070000--