Re: [PATCH] add-symbol-file-from-memory command

[Daniel Jacobowitz <drow at mvista dot com>]

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("<in-memory>").  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:
> >    "<in-memory>" 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