From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3272 invoked by alias); 23 Apr 2009 20:36:41 -0000 Received: (qmail 3262 invoked by uid 22791); 23 Apr 2009 20:36:40 -0000 X-SWARE-Spam-Status: No, hits=-2.4 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mx2.redhat.com (HELO mx2.redhat.com) (66.187.237.31) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 23 Apr 2009 20:36:33 +0000 Received: from int-mx2.corp.redhat.com (int-mx2.corp.redhat.com [172.16.27.26]) by mx2.redhat.com (8.13.8/8.13.8) with ESMTP id n3NKaO8i020409 for ; Thu, 23 Apr 2009 16:36:24 -0400 Received: from ns3.rdu.redhat.com (ns3.rdu.redhat.com [10.11.255.199]) by int-mx2.corp.redhat.com (8.13.1/8.13.1) with ESMTP id n3NKaMXW012380; Thu, 23 Apr 2009 16:36:23 -0400 Received: from host0.dyn.jankratochvil.net (sebastian-int.corp.redhat.com [172.16.52.221]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id n3NKaLpg015964; Thu, 23 Apr 2009 16:36:22 -0400 Received: from host0.dyn.jankratochvil.net (localhost [127.0.0.1]) by host0.dyn.jankratochvil.net (8.14.3/8.14.3) with ESMTP id n3NKaKtX009222; Thu, 23 Apr 2009 22:36:20 +0200 Received: (from jkratoch@localhost) by host0.dyn.jankratochvil.net (8.14.3/8.14.2/Submit) id n3NKaJYO009215; Thu, 23 Apr 2009 22:36:19 +0200 Date: Thu, 23 Apr 2009 20:36:00 -0000 From: Jan Kratochvil To: Tom Tromey Cc: gdb-patches@sourceware.org Subject: Re: [patch] Fix double free on error while inserting the breakpoint Message-ID: <20090423203619.GA31736@host0.dyn.jankratochvil.net> References: <20081122214006.GA22076@host0.dyn.jankratochvil.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.18 (2008-05-17) 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: 2009-04/txt/msg00663.txt.bz2 On Thu, 23 Apr 2009 01:11:25 +0200, Tom Tromey wrote: > I like this but I am unsure whether it is ok to move the call to > update_global_location_list past the call to mention. As in this case the breakpoint is pending and thus it was created by set_raw_breakpoint_without_location and so update_global_location_list is a nop for it. So I do not think it is a problem to move it. > Jan> + /* Have already BREAKPOINT_CHAIN discarded as we may get an exception while > Jan> + inserting the breakpoints which would double-free the resources both by > Jan> + BREAKPOINT_CHAIN now and during DELETE_BREAKPOINT in the future. */ > Jan> + update_global_location_list (1); > > I found this comment pretty hard to follow. I think the code would be > pretty clear without it. The ordering of discard_cleanups (breakpoint_chain); vs. update_global_location_list (1); I find important to note there. At least tried if the new comment will pass. Thanks, Jan 2009-04-23 Jan Kratochvil Fix double free on error inserting the breakpoint instruction. * breakpoint.c (create_breakpoints): Move the update_global_location_list call to ... (break_command_really): ... here together with the second local call both unified after all the cleanups. --- gdb/breakpoint.c 31 Mar 2009 16:44:17 -0000 1.390 +++ gdb/breakpoint.c 23 Apr 2009 20:13:26 -0000 @@ -5458,8 +5458,6 @@ create_breakpoints (struct symtabs_and_l cond_string, type, disposition, thread, task, ignore_count, ops, from_tty, enabled); } - - update_global_location_list (1); } /* Parse ARG which is assumed to be a SAL specification possibly @@ -5800,7 +5798,6 @@ break_command_really (char *arg, char *c b->ops = ops; b->enable_state = enabled ? bp_enabled : bp_disabled; - update_global_location_list (1); mention (b); } @@ -5812,6 +5809,9 @@ break_command_really (char *arg, char *c discard_cleanups (breakpoint_chain); /* But cleanup everything else. */ do_cleanups (old_chain); + + /* error call may happen here - have BREAKPOINT_CHAIN already discarded. */ + update_global_location_list (1); } /* Set a breakpoint.