From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 30530 invoked by alias); 2 Mar 2010 17:41:53 -0000 Received: (qmail 30519 invoked by uid 22791); 2 Mar 2010 17:41:52 -0000 X-SWARE-Spam-Status: No, hits=-2.5 required=5.0 tests=AWL,BAYES_00 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; Tue, 02 Mar 2010 17:41:48 +0000 Received: (qmail 25100 invoked from network); 2 Mar 2010 17:41:47 -0000 Received: from unknown (HELO caradoc.them.org) (dan@127.0.0.2) by mail.codesourcery.com with ESMTPA; 2 Mar 2010 17:41:47 -0000 Date: Tue, 02 Mar 2010 17:41:00 -0000 From: Daniel Jacobowitz To: Pedro Alves Cc: gdb-patches@sourceware.org, Doug Evans Subject: Re: [for discussion] Update inferior address spaces Message-ID: <20100302174144.GS9493@caradoc.them.org> References: <20100301213735.GA17815@caradoc.them.org> <201003021715.47165.pedro@codesourcery.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <201003021715.47165.pedro@codesourcery.com> User-Agent: Mutt/1.5.20 (2009-06-14) 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: 2010-03/txt/msg00058.txt.bz2 On Tue, Mar 02, 2010 at 05:15:47PM +0000, Pedro Alves wrote: > > Also, I believe there's a double free in the existing code, fixed in > > this patch. For the shared address space case. > > Hmm, yes, in the not-shared -> shared direction. With the patch, it's > now leaking in the shared -> not-shared direction, I think? This > function is missing one bit of info: if the previous target had a > shared address space. This would be simpler if address spaces were > refcounted. Hmm, maybe we could find a way to get rid of the need for > this function instead? Something to ponder about. Anyway, a few > leaks per session is much better than double free, so that's still an > improvement. Oh, I wasn't even thinking about changes of architecture. I was suggesting something much simpler: there's a double free when both before and after versions used shared address space, because we always free pspace->aspace, but we don't assign a shared object there. If the gdbarch changes those properties, this is going to blow up. > > This patch works around the bug, but I don't think it's right as-is. > > I think it's still good. I'd just drop the assertion, and maybe > special case gdbarch_has_global_solist like remote_add_inferior. But > I can take care of that when I get a chance of testing with > DICOS, if you prefer. I think I've been confusing the two gdbarch hooks (global solist, shared address space). They looked too much alike yesterday... Is this about right? -- Daniel Jacobowitz CodeSourcery 2010-03-02 Daniel Jacobowitz * progspace.c (update_address_spaces): Update inferior address spaces also. Index: progspace.c =================================================================== RCS file: /cvs/src/src/gdb/progspace.c,v retrieving revision 1.4 diff -u -p -r1.4 progspace.c --- progspace.c 19 Jan 2010 09:39:12 -0000 1.4 +++ progspace.c 2 Mar 2010 17:40:56 -0000 @@ -430,24 +430,30 @@ void update_address_spaces (void) { int shared_aspace = gdbarch_has_shared_address_space (target_gdbarch); - struct address_space *aspace = NULL; struct program_space *pspace; + struct inferior *inf; init_address_spaces (); - ALL_PSPACES (pspace) + if (shared_aspace) { - free_address_space (pspace->aspace); - - if (shared_aspace) - { - if (aspace == NULL) - aspace = new_address_space (); - pspace->aspace = aspace; - } - else - pspace->aspace = new_address_space (); + struct address_space *aspace = new_address_space (); + free_address_space (current_program_space->aspace); + ALL_PSPACES (pspace) + pspace->aspace = aspace; } + else + ALL_PSPACES (pspace) + { + free_address_space (pspace->aspace); + pspace->aspace = new_address_space (); + } + + for (inf = inferior_list; inf; inf = inf->next) + if (gdbarch_has_global_solist (target_gdbarch)) + inf->aspace = maybe_new_address_space (); + else + inf->aspace = inf->pspace->aspace; } /* Save the current program space so that it may be restored by a later