From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 20925 invoked by alias); 27 Jan 2011 12:44:19 -0000 Received: (qmail 20914 invoked by uid 22791); 27 Jan 2011 12:44:18 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL,BAYES_00,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 27 Jan 2011 12:44:13 +0000 Received: (qmail 7943 invoked from network); 27 Jan 2011 12:44:11 -0000 Received: from unknown (HELO scottsdale.localnet) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 27 Jan 2011 12:44:11 -0000 From: Pedro Alves To: Paul Pluzhnikov Subject: Re: [patch] Fix leak of bp_jit_event breakpoints Date: Thu, 27 Jan 2011 14:15:00 -0000 User-Agent: KMail/1.13.5 (Linux/2.6.35-24-generic; KDE/4.5.1; x86_64; ; ) Cc: gdb-patches@sourceware.org, Yao Qi References: <20110119204315.0A235190C48@elbrus2.mtv.corp.google.com> In-Reply-To: MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-15" Content-Transfer-Encoding: 7bit Message-Id: <201101271244.09767.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-01/txt/msg00518.txt.bz2 Hi Paul, sorry for the delay in getting back to this. On Thursday 27 January 2011 23:27:39, Paul Pluzhnikov wrote: > > +struct jit_inferior_data { Put the { on it's own line, please. > + struct breakpoint *breakpoint; > + CORE_ADDR breakpoint_addr; > + CORE_ADDR descriptor_addr; > +}; > +static struct jit_inferior_data * > +get_jit_inferior_data (void) Please add a small describing blurb over functions. > + if (inf_data->breakpoint != NULL) > + { > + if (inf_data->breakpoint_addr == inf_data->breakpoint->loc->address) > + /* Same location as on last run. Existing breakpoint is good. */ > + return 0; I'm a little warry about this optimization. For example, we should probably also compare other things, like gdbarch and location's pspace|aspace. Is it a significant difference if we always delete the breakpoint (here or perhaps on inferior exit?) There's at least one problem to solve: on "exec", update_breakpoints_after_exec deletes bp_jit_event breakpoints, effectively making it so that your inf_data->breakpoint pointer becomes stale. There may be other paths that delete the breakpoint behind jit.c's back. One solution would be to get rid of the breakpoint pointer in jit.c, and add a remove_jit_event_breakpoints function, modelled on remove_solib_event_breakpoints. But if you want to come up with other solutions, I'd be happy to consider them. I'm thinking that we should delete the jit breakpoint (and perhaps more) whenever the executable changes (say, the "file" command), which is kind of a similar case of an "exec", so maybe we should install an executable_changed observer as well. Not sure that covers all we need. > + > + /* Location has changed since last run. */ > + delete_breakpoint (inf_data->breakpoint); > + } -- Pedro Alves