This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 3/7] New commands for loading and unloading a reader.
>>>>> "Sanjoy" == Sanjoy Das <sanjoy@playingwithpointers.com> writes:
Sanjoy> Introduces two new GDB commands - `load-jit-reader' and
Sanjoy> `unload-jit-reader'.
I am not that fond of these command names.
I'm not sure what else I would propose... maybe a "jit" prefix command,
with "load" and "unload" subcommands?
Do we actually need the ability to unload?
Sanjoy> +#define GDB_READER_DIR (LIBDIR "/gdb/")
This should go through the relocation process done for other
directories. See main.c.
Sanjoy> +/* One reader that has been loaded successfully, and can potentially be used to
Sanjoy> + parse debug info. */
Sanjoy> +struct jit_reader
Sanjoy> +{
Sanjoy> + struct gdb_reader_funcs *functions;
Sanjoy> +} *loaded_jit_reader = NULL;
Can this be static?
Sanjoy> +typedef struct gdb_reader_funcs * (reader_init_fn_type) (void);
Sanjoy> +const char *reader_init_fn_sym = "gdb_init_reader";
Likewise.
Sanjoy> +/* Try to load FILE_NAME as a JIT debug info reader. Set ERROR_STRING
Sanjoy> + in case of an error (and return NULL), else return a correcly
Sanjoy> + formed struct jit_reader. */
Sanjoy> +static struct jit_reader *
Sanjoy> +jit_reader_load (const char *file_name, char **error_string)
It is more normal in gdb to simply call error when an error occurs.
Then you don't need to worry about the return value, or passing in the
error_string pointer.
Sanjoy> + so = gdb_dlopen (file_name);
Sanjoy> +
Sanjoy> + if (!so)
Sanjoy> + {
Sanjoy> + *error_string = _("could not open reader file");
Sanjoy> + return NULL;
Sanjoy> + }
It might be nicer if gdb_dlopen called error, so that the different
ports could give better error messages, say via dlerror.
Sanjoy> + if (so)
Sanjoy> + gdb_dlclose(so);
With the error approach, you would install a cleanup that calls
gdb_dlclose, then discard_cleanups at the point of normal return.
Sanjoy> + strncat (so_name, args, PATH_MAX - sizeof(GDB_READER_DIR) - 4);
Sanjoy> + strcat (so_name, ".so");
It seems to me that you want a directory separator in here.
Sanjoy> + free (loaded_jit_reader);
xfree.
Sanjoy> @@ -510,10 +610,10 @@ jit_event_handler (struct gdbarch *gdbarch)
Sanjoy> struct jit_code_entry code_entry;
Sanjoy> CORE_ADDR entry_addr;
Sanjoy> struct objfile *objf;
Sanjoy> + struct jit_inferior_data *inf_data = get_jit_inferior_data ();
Sanjoy> /* Read the descriptor from remote memory. */
Sanjoy> - jit_read_descriptor (gdbarch, &descriptor,
Sanjoy> - get_jit_inferior_data ()->descriptor_addr);
Sanjoy> + jit_read_descriptor (gdbarch, &descriptor, inf_data->descriptor_addr);
Sanjoy> entry_addr = descriptor.relevant_entry;
Sanjoy> /* Do the corresponding action. */
Sanjoy> @@ -526,15 +626,16 @@ jit_event_handler (struct gdbarch *gdbarch)
Sanjoy> jit_register_code (gdbarch, entry_addr, &code_entry);
Sanjoy> break;
Sanjoy> case JIT_UNREGISTER:
Sanjoy> - objf = jit_find_objf_with_entry_addr (entry_addr);
Sanjoy> - if (objf == NULL)
Sanjoy> - printf_unfiltered (_("Unable to find JITed code "
Sanjoy> - "entry at address: %s\n"),
Sanjoy> - paddress (gdbarch, entry_addr));
Sanjoy> - else
Sanjoy> - jit_unregister_code (objf);
Sanjoy> -
Sanjoy> - break;
Sanjoy> + {
Sanjoy> + objf = jit_find_objf_with_entry_addr (entry_addr);
Sanjoy> + if (objf == NULL)
Sanjoy> + printf_unfiltered (_("Unable to find JITed code "
Sanjoy> + "entry at address: %s\n"),
Sanjoy> + paddress (gdbarch, entry_addr));
Sanjoy> + else
Sanjoy> + jit_unregister_code (objf);
Sanjoy> + break;
Sanjoy> + }
These hunks seem to be just formatting changes.
Please drop.
Tom