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

Roland McGrath
2004-02-02T04:47:47Z

> 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".

> 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.

> 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.

> > + 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.

Thanks,
Roland

Index: symfile.c
===================================================================
RCS file: /cvs/src/src/gdb/symfile.c,v
retrieving revision 1.117
diff -u -b -p -r1.117 symfile.c
--- symfile.c 23 Jan 2004 17:56:46 -0000 1.117
+++ symfile.c 2 Feb 2004 04:41:50 -0000
@@ -799,6 +799,10 @@ new_symfile_objfile (struct objfile *obj
NAME is the file name (which will be tilde-expanded and made
absolute herein) (but we don't free or modify NAME itself).

+ If ABFD is not null, it's already open and should be used instead of
+ opening the file by name. This BFD will be closed on error, and
+ is always consumed by this function.
+
FROM_TTY says how verbose to be.

MAINLINE specifies whether this is the main symbol file, or whether
@@ -811,7 +815,7 @@ new_symfile_objfile (struct objfile *obj
Upon success, returns a pointer to the objfile that was added.
Upon failure, jumps back to command level (never returns). */
static struct objfile *
-symbol_file_add_with_addrs_or_offsets (char *name, int from_tty,
+symbol_file_add_with_addrs_or_offsets (char *name, bfd *abfd, int from_tty,
struct section_addr_info *addrs,
struct section_offsets *offsets,
int num_offsets,
@@ -820,15 +824,17 @@ symbol_file_add_with_addrs_or_offsets (c
struct objfile *objfile;
struct partial_symtab *psymtab;
char *debugfile;
- bfd *abfd;
struct section_addr_info *orig_addrs;
struct cleanup *my_cleanups;

/* Open a bfd for the file, and give user a chance to burp if we'd be
interactively wiping out any existing symbols. */

+ if (abfd == NULL)
abfd = symfile_bfd_open (name);

+ my_cleanups = make_cleanup_bfd_close (abfd);
+
if ((have_full_symbols () || have_partial_symbols ())
&& mainline
&& from_tty
@@ -836,6 +842,7 @@ symbol_file_add_with_addrs_or_offsets (c
error ("Not confirmed.");

objfile = allocate_objfile (abfd, flags);
+ discard_cleanups (my_cleanups);

orig_addrs = alloc_section_addr_info (bfd_count_sections (abfd));
my_cleanups = make_cleanup (xfree, orig_addrs);
@@ -952,7 +959,8 @@ struct objfile *
symbol_file_add (char *name, int from_tty, struct section_addr_info *addrs,
int mainline, int flags)
{
- return symbol_file_add_with_addrs_or_offsets (name, from_tty, addrs, 0, 0,
+ return symbol_file_add_with_addrs_or_offsets (name, NULL, from_tty,
+ addrs, 0, 0,
mainline, flags);
}

@@ -1802,6 +1810,90 @@ add_shared_symbol_files_command (char *a
#endif
}
\f
+/* Read inferior memory at ADDR to find the header of a loaded object file
+ and read its in-core symbols out of inferior memory. TEMPL is a bfd
+ representing the target's format. */
+struct objfile *
+symbol_file_add_from_memory (bfd *templ, CORE_ADDR addr, int from_tty)
+{
+ struct objfile *objf;
+ bfd *nbfd;
+ asection *sec;
+ bfd_vma loadbase;
+ struct section_addr_info *sai;
+ unsigned int i;
+
+ if (bfd_get_flavour (templ) != bfd_target_elf_flavour)
+ error ("add-symbol-file-from-memory not supported for this target");
+
+ nbfd = bfd_elf_bfd_from_remote_memory (templ, addr, &loadbase,
+ target_read_memory);
+ if (nbfd == NULL)
+ {
+ if (from_tty)
+ error ("Failed to read a valid object file image from memory.");
+ return NULL;
+ }
+
+ nbfd->filename = xstrdup ("shared object read from target memory");
+
+ 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;
+ }
+
+ sai = alloc_section_addr_info (bfd_count_sections (nbfd));
+ make_cleanup (xfree, sai);
+ i = 0;
+ for (sec = nbfd->sections; sec != NULL; sec = sec->next)
+ if ((bfd_get_section_flags (nbfd, sec) & (SEC_ALLOC|SEC_LOAD)) != 0)
+ {
+ sai->other[i].addr = bfd_get_section_vma (nbfd, sec) + loadbase;
+ sai->other[i].name = (char *) bfd_get_section_name (nbfd, sec);
+ sai->other[i].sectindex = sec->index;
+ ++i;
+ }
+
+ objf = symbol_file_add_with_addrs_or_offsets ((char *) nbfd->filename, nbfd,
+ from_tty, sai, NULL, 0, 0,
+ OBJF_SHARED);
+
+ /* This might change our ideas about frames already looked at. */
+ reinit_frame_cache ();
+
+ return objf;
+}
+
+static void
+add_symbol_file_from_memory_command (char *args, int from_tty)
+{
+ CORE_ADDR addr;
+ bfd *templ;
+
+ if (args == NULL)
+ error ("add-symbol-file-from-memory requires an expression argument");
+
+ addr = parse_and_eval_address (args);
+
+ /* We need some representative bfd to know the target we are looking at. */
+ if (symfile_objfile != NULL)
+ templ = symfile_objfile->obfd;
+ else
+ templ = exec_bfd;
+ if (templ == NULL)
+ error ("\
+Must use symbol-file or exec-file before add-symbol-file-from-memory.");
+
+ (void) symbol_file_add_from_memory (templ, addr, from_tty);
+}
+\f
/* Re-read symbols if a symbol-file has changed. */
void
reread_symbols (void)
@@ -2063,7 +2155,7 @@ reread_separate_symbols (struct objfile
Preserve the flags from objfile that make sense. */
objfile->separate_debug_objfile
= (symbol_file_add_with_addrs_or_offsets
- (debug_file,
+ (debug_file, NULL,
info_verbose, /* from_tty: Don't override the default. */
0, /* No addr table. */
objfile->section_offsets, objfile->num_sections,
@@ -3566,6 +3658,13 @@ with the text. SECT is a section name t
&cmdlist);
set_cmd_completer (c, filename_completer);

+ c = add_cmd ("add-symbol-file-from-memory", class_files,
+ add_symbol_file_from_memory_command,
+ "Usage: add-symbol-file-from-memory ADDR\n\
+Load the symbols out of memory from a dynamically loaded object file.\n\
+ADDR is the starting address of the file's shared object file header.",
+ &cmdlist);
+
c = add_cmd ("add-shared-symbol-files", class_files,
add_shared_symbol_files_command,
"Load the symbols from shared objects in the dynamic linker's link map.",