[PATCH 1/3] Added command remove-symbol-file.
Yao Qi
yao@codesourcery.com
Mon Apr 22 13:32:00 GMT 2013
On 04/16/2013 03:51 PM, Nicolas Blanc wrote:
> Added command remove-symbol-file for removing
> symbol files added via the add-symbol-file command.
>
Nicolas,
The patch looks good to me, but I am not the people to approve it. Some
nits below,
> 2013-18-03 Nicolas Blanc <nicolas.blanc@intel.com>
>
> * breakpoint.c (disable_breakpoints_in_free_objfile): Created
> function for disabling breakoints in objfiles upon FREE_OBJFILE
> notifications.
> * doc/observer.text: Created FREE_OBJFILE event.
Change to doc/observer.texi should be moved to a separate changelog entry.
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index f374368..d5c7b21 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
>
> +/* Disable any breakpoints and tracepoints in OBJFILE upon
> + notification of FREE_OBJFILE. Only apply to enabled breakpoints,
^^ Miss one space after period.
> + disabled ones can just stay disabled. */
> +
> +static void
> +disable_breakpoints_in_free_objfile (struct objfile * objfile)
> +{
> + struct bp_location *loc, **locp_tmp;
> +
> + if (objfile == NULL)
> + return;
> +
> + /* If the file is a shared library not loaded by the user then
> + SOLIB_UNLOADED was notified and DISABLE_BREAKPIONTS_IN_UNLOADED_SHLIB
> + was called. In that case there is no need to take action again. */
> + if ((objfile->flags & OBJF_SHARED) && !(objfile->flags & OBJF_USERLOADED))
> + return;
> +
> + ALL_BP_LOCATIONS (loc, locp_tmp)
> + {
> + struct obj_section *osect;
> + CORE_ADDR loc_addr = loc->address;
> +
> + /* ALL_BP_LOCATIONS bp_location has LOC->OWNER always non-NULL. */
> + struct breakpoint *b = loc->owner;
> +
> + if (loc->shlib_disabled != 0)
> + continue;
> +
> + if (objfile->pspace != loc->pspace)
> + continue;
> +
> + if (!is_tracepoint(b))
> + {
> + if (b->type != bp_breakpoint
> + && b->type != bp_jit_event
> + && b->type != bp_hardware_breakpoint)
> + continue;
> +
> + if (loc->loc_type != bp_loc_hardware_breakpoint
> + && loc->loc_type != bp_loc_software_breakpoint)
> + continue;
> + }
> +
> + ALL_OBJFILE_OSECTIONS (objfile, osect)
> + {
> + if (obj_section_addr (osect) <= loc_addr
> + && loc_addr < obj_section_endaddr (osect))
> + {
> + loc->shlib_disabled = 1;
> + loc->inserted = 0;
> + observer_notify_breakpoint_modified (loc->owner);
Nit: this observer will be called multiple times for a single breakpoint
LOC->owner, if it has multiple breakpoint locations in this objfile. It
is ideal to call observer only once for a breakpoint. Your patch is OK
to me as well.
> diff --git a/gdb/doc/observer.texi b/gdb/doc/observer.texi
> index adb7085..7b420ea 100644
> --- a/gdb/doc/observer.texi
> +++ b/gdb/doc/observer.texi
> @@ -138,6 +138,10 @@ Called with @var{objfile} equal to @code{NULL} to indicate
> previously loaded symbol table data has now been invalidated.
> @end deftypefun
>
> +@deftypefun void free_objfile (struct objfile *@var{objfile})
> +The symbol file specified by @var{objfile} is about to be freed.
I feel that "symbol file" is not accurate. Maybe "object file"?
> +@end deftypefun
> +
> @deftypefun void new_thread (struct thread_info *@var{t})
> The thread specified by @var{t} has been created.
> @end deftypefun
> index 2cc33d0..e6f62ba 100644
> --- a/gdb/printcmd.c
> +++ b/gdb/printcmd.c
> @@ -1928,21 +1928,22 @@ disable_display_command (char *args, int from_tty)
> an item by re-parsing .exp_string field in the new execution context. */
>
> static void
> -clear_dangling_display_expressions (struct so_list *solib)
> +clear_dangling_display_expressions (struct objfile *objfile)
> {
> - struct objfile *objfile = solib->objfile;
> struct display *d;
> + struct program_space *pspace;
>
> /* With no symbol file we cannot have a block or expression from it. */
> if (objfile == NULL)
> return;
> + pspace = objfile->pspace;
> if (objfile->separate_debug_objfile_backlink)
> objfile = objfile->separate_debug_objfile_backlink;
> - gdb_assert (objfile->pspace == solib->pspace);
> + gdb_assert (objfile->pspace == pspace);
>
This assert is always true. Probably, we need to move this assert to
the callers of "free_objfile" when argument is solib->objfile.
> for (d = display_chain; d != NULL; d = d->next)
> {
> - if (d->pspace != solib->pspace)
> + if (d->pspace != pspace)
> continue;
>
> if (lookup_objfile_from_block (d->block) == objfile
> @@ -2474,7 +2475,7 @@ _initialize_printcmd (void)
>
> current_display_number = -1;
>
> - observer_attach_solib_unloaded (clear_dangling_display_expressions);
> + observer_attach_free_objfile (clear_dangling_display_expressions);
>
> add_info ("address", address_info,
> _("Describe where symbol SYM is stored."));
> diff --git a/gdb/solib.c b/gdb/solib.c
> index 6978677..b0c2f04 100644
> --- a/gdb/solib.c
> +++ b/gdb/solib.c
> @@ -1442,6 +1442,30 @@ gdb_bfd_lookup_symbol (bfd *abfd,
> return symaddr;
> }
>
> +/* Upon notification of FREE_OBJFILE remove any reference
> + to any user-added file that is about to be freed. */
A blank line is needed here.
> +static void
> +remove_user_added_objfile (struct objfile *objfile)
> +{
> + struct so_list *gdb;
> +
> + if (!objfile)
> + return;
> +
> + if (!(objfile->flags & OBJF_USERLOADED)
> + || !(objfile->flags & OBJF_SHARED))
> + return;
> +
> + gdb = so_list_head;
> + while (gdb)
> + {
> + if (gdb->objfile == objfile)
> + gdb->objfile = NULL;
> + gdb = gdb->next;
> + }
> +}
> +
> diff --git a/gdb/symfile.c b/gdb/symfile.c
> index 3e66bd1..273c07a 100644
> --- a/gdb/symfile.c
> +++ b/gdb/symfile.c
> @@ -2364,6 +2372,56 @@ add_symbol_file_command (char *args, int from_tty)
> }
>
>
> +
> +/* This function removes a symbol file that was added via add-symbol-file. */
A blank line is needed here.
> +static void
> +remove_symbol_file_command (char *args, int from_tty)
> +{
--
Yao (é½å°§)
More information about the Gdb-patches
mailing list