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

Daniel Jacobowitz
2004-02-02T15:02:56Z

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