From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8770 invoked by alias); 10 Oct 2011 11:58:35 -0000 Received: (qmail 8762 invoked by uid 22791); 10 Oct 2011 11:58:34 -0000 X-SWARE-Spam-Status: No, hits=-1.0 required=5.0 tests=AWL,BAYES_00,SARE_SUB_IMPROVE,SPF_FAIL X-Spam-Check-By: sourceware.org Received: from gbenson.demon.co.uk (HELO gbenson.demon.co.uk) (80.177.220.214) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 10 Oct 2011 11:58:15 +0000 Date: Mon, 10 Oct 2011 11:58:00 -0000 From: Gary Benson To: Pedro Alves Cc: gdb-patches@sourceware.org Subject: Re: PING: [RFA] Improve performance with lots of shared libraries Message-ID: <20111010115811.GB2804@redhat.com> Mail-Followup-To: Pedro Alves , gdb-patches@sourceware.org References: <20110922171206.GB5874@redhat.com> <20111004102517.GA2679@redhat.com> <201110071601.57025.pedro@codesourcery.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <201110071601.57025.pedro@codesourcery.com> 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-10/txt/msg00257.txt.bz2 Hi Pedro, Pedro Alves wrote: > On Tuesday 04 October 2011 11:25:18, Gary Benson wrote: > > 2011-09-22 Gary Benson > > > > * infrun.c (stopped_at_solib_event_breakpoint): New function. > > (stopped_at_solib_event_breakpoint_helper): Likewise. > > (handle_inferior_event): Avoid calling skip_inline_frames > > when at the solib event breakpoint. > > > > diff --git a/gdb/infrun.c b/gdb/infrun.c > > index 225034c..2e49470 100644 > > --- a/gdb/infrun.c > > +++ b/gdb/infrun.c > > @@ -3102,6 +3102,39 @@ fill_in_stop_func (struct gdbarch *gdbarch, > > } > > } > > > > +/* Helper for stopped_at_solib_event_breakpoint. */ > > + > > +static int > > +stopped_at_solib_event_breakpoint_helper (struct breakpoint *b, void *arg) > > As I mentioned before, better would for this to be a > property/function of the breakpoint itself (and its locations) -- > meaning "I am / am not an inlined location". There's no reason to > limit this just to the solib event breakpoint. An obvious example > without even that generalization is the thread event breakpoint. Are you talking about a function like this: int non_inline_function (struct breakpoint *b) { return b->type == bp_shlib_event; } or something more involved? > Even if you don't do that, please add this new function (that checks > whether we stopped at an address that can't be inlined) to > breakpoint.c instead, and call it `stopped_at_non_inline_function' > or something along those lines. We can later rework its internals > keeping its interface. Ok. > > +{ > > + struct execution_control_state *ecs > > + = (struct execution_control_state *) arg; > > + > > You need to check that the breakpoint is enabled and installed. Does it matter if it isn't? My thinking was that we are trying to state that there cannot be inlined functions at the stop address, and the existence of the shlib event breakpoint there allows you to state that whether it is enabled or not. (Am I correct in thinking "installed" means "not-pending"?) > > + if (b->type == bp_shlib_event) > > + { > > + CORE_ADDR prev_pc = ecs->event_thread->prev_pc; > > + struct bp_location *loc; > > + > > + for (loc = b->loc; loc; loc = loc->next) > > + { > > + if (loc->pspace == current_program_space > > + && (loc->address == stop_pc || loc->address == prev_pc)) > > I don't follow the prev_pc check. What does that intend to do? You end up in handle_inferior_event twice for every breakpoint, once when you hit it and once when you've single-stepped over it. The prev_pc check catches the latter. > When working in the direction of matching from an event back to our > tables and symbolic info, you start from the address_space instead. > Get at the address space of the current PC using > `get_regcache_aspace (get_thread_regcache (ecs->ptid))'. But better > yet, since you'd end up rewritting bkpt_breakpoint_hit, it's better > instead to reuse bpstat_check_location (despite the name, it doesn't > work with a bpstat). See > bpstop_status->bpstat_check_location->bkpt_breakpoint_hit, and > factor out the necessary bits. Ok, I will look into this. Thank you, Gary -- http://gbenson.net/