From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5303 invoked by alias); 2 Feb 2004 15:02:56 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 5293 invoked from network); 2 Feb 2004 15:02:55 -0000 Received: from unknown (HELO nevyn.them.org) (66.93.172.17) by sources.redhat.com with SMTP; 2 Feb 2004 15:02:55 -0000 Received: from drow by nevyn.them.org with local (Exim 4.30 #1 (Debian)) id 1AnfbC-0001yM-H0; Mon, 02 Feb 2004 10:02:54 -0500 Date: Mon, 02 Feb 2004 15:02:00 -0000 From: Daniel Jacobowitz To: Roland McGrath Cc: gdb-patches@sources.redhat.com Subject: Re: [PATCH] add-symbol-file-from-memory command Message-ID: <20040202150254.GA7529@nevyn.them.org> Mail-Followup-To: Roland McGrath , gdb-patches@sources.redhat.com References: <20040202040641.GA9495@nevyn.them.org> <200402020447.i124lfMl023809@magilla.sf.frob.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <200402020447.i124lfMl023809@magilla.sf.frob.com> User-Agent: Mutt/1.5.1i X-SW-Source: 2004-02/txt/msg00025.txt.bz2 On Sun, Feb 01, 2004 at 08:47:41PM -0800, Roland McGrath wrote: > > I can't approve this patch, but I have some comments anyway. > > Much appreciated. > > > There also was a segfault when the objfile is released. To reproduce, > > I could just say "file\n" after loading one. GDB will try to > > xfree(""). I see that this is fixed in the patch below, so > > if the bits were unchanged from your last post then I must have come up > > with the wrong copy. > > I won't testify to not having noticed and fixed (and since forgotten) some > small bug like this between the last time I posted and today. Probably I did. > > > I can tell you one problem with this patch, based on my backport of it: > > there's an annoying/incorrect message when a program is re-run, saying: > > "" has disappeared; keeping its symbols > > > > This is merely an annoyance, the message is harmless but should be > > fixed. > > I don't see this. AFAICT it just loses the symbols, like it should. What > exactly is the recipe for seeing this? This may be something else I forgot > I changed. It sets the OBJF_SHARED flag, which means the objfile is > dropped by objfile_purge_solibs for "run". Both of these are changes since the last time you posted the patch, so please do not insist it was unchanged. > > Mind doing this in some way that isn't gratuitously quadratic? > > Sure. I only used bfd_map_over_sections since it was said to be preferred. Thanks. > > Please remove the check and the !from_tty branch. An error is fine in > > either case, and internal errors are not appropriate for user input. > > Further down you have different error behavior on !from_tty also. Is > > there a particular inspiration for this? > > My thinking was that when there are later internal calls to this function > from target code, it would be an indication of a bug in the target code if > it ever got called for a non-ELF target, and those would be the called with > from_tty==0, hence the gdb_assert. The other errors indicate that the > thing was reasonable to attempt, but failed. I think I copied the from_tty > conditional for those errors from some other code I found similar, but I > may be misremembering. I am more than happy to have you tell me a clear > policy on what from_tty should or shouldn't affect. from_tty should generally only affect verbosity. If the user puts commands in a .gdbinit, they will be run with from_tty == 0, and should still trigger an error () when necessary. > > > + reinit_frame_cache (); /* ??? */ > > > > Yes, this is necessary if the current cached backtrace would pass > > through the newly loaded object. > > I appreciate the explanation. I've added a comment. > I'm appending my current version of the symfile.c part of the patch. > + if (!bfd_check_format (nbfd, bfd_object)) > + { > + /* FIXME: should be checking for errors from bfd_close (for one thing, > + on error it does not free all the storage associated with the > + bfd). */ > + bfd_close (nbfd); > + if (from_tty) > + error ("Got object file from memory but can't read symbols: %s.", > + bfd_errmsg (bfd_get_error ())); > + return NULL; > + } Same thing... -- Daniel Jacobowitz MontaVista Software Debian GNU/Linux Developer