This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch v6 1/3] New remove-symbol-file command.
- From: Luis Machado <lgustavo at codesourcery dot com>
- To: Nicolas Blanc <nicolas dot blanc at intel dot com>
- Cc: gdb-patches at sourceware dot org, palves at redhat dot com, tromey at redhat dot com, eliz at gnu dot org, yao at codesourcery dot com, lgustavo at codesourcery dot com, dje at google dot com
- Date: Thu, 06 Jun 2013 17:43:54 +0200
- Subject: Re: [patch v6 1/3] New remove-symbol-file command.
- References: <1370459252-24643-1-git-send-email-nicolas dot blanc at intel dot com> <1370459252-24643-2-git-send-email-nicolas dot blanc at intel dot com>
- Reply-to: lgustavo at codesourcery dot com
Thanks.
It is looking good. A few more comments below.
On 06/05/2013 09:07 PM, Nicolas Blanc wrote:
New command for removing symbol files added via
the add-symbol-file command.
2013-18-03 Nicolas Blanc <nicolas.blanc@intel.com>
* breakpoint.c (disable_breakpoints_in_freed_objfile): New function for
disabling breakpoints in objfiles upon free_objfile notifications.
* objfiles.c (free_objfile): Notify free_objfile.
(is_addr_in_objfile): New query function.
* objfiles.h (is_addr_in_objfile): New declaration.
* printcmd.c (clear_dangling_display_expressions): Act upon free_objfile
events instead of solib_unloaded events.
(_initialize_printcmd): Register observer for free_objfile instead
of solib_unloaded notifications.
* solib.c (remove_user_added_objfile): New function for removing
dangling references upon notification of free_objfile.
* symfile.c (remove_symbol_file_command): New command for removing symbol
files.
(_initialize_symfile): Add remove-symbol-file.
gdb/doc
* observer.texi: New free_objfile event.
For new functions, "New function" should do. No need to explain the
functions in the ChangeLog entry.
Signed-off-by: Nicolas Blanc <nicolas.blanc@intel.com>
---
gdb/breakpoint.c | 66 ++++++++++++++++++++++++++++++++++++--
gdb/doc/observer.texi | 4 ++
gdb/objfiles.c | 23 +++++++++++++
gdb/objfiles.h | 2 +
gdb/printcmd.c | 15 +++++---
gdb/solib.c | 22 +++++++++++++
gdb/symfile.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++++
7 files changed, 207 insertions(+), 9 deletions(-)
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index d4ccc81..9446918 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -7403,9 +7403,9 @@ disable_breakpoints_in_shlibs (void)
}
}
-/* Disable any breakpoints and tracepoints that are in an unloaded shared
- library. Only apply to enabled breakpoints, disabled ones can just stay
- disabled. */
+/* Disable any breakpoints and tracepoints that are in SOLIB upon
+ notification of unloaded_shlib. Only apply to enabled breakpoints,
+ disabled ones can just stay disabled. */
static void
disable_breakpoints_in_unloaded_shlib (struct so_list *solib)
@@ -7457,6 +7457,65 @@ disable_breakpoints_in_unloaded_shlib (struct so_list *solib)
}
}
+/* Disable any breakpoints and tracepoints in OBJFILE upon
+ notification of free_objfile. Only apply to enabled breakpoints,
+ disabled ones can just stay disabled. */
+
+static void
+disable_breakpoints_in_freed_objfile (struct objfile * objfile)
+{
+ struct breakpoint *b;
+
+ if (objfile == NULL)
+ return;
+
+ /* If the file is a shared library not loaded by the user then
+ solib_unloaded was notified and disable_breakpoints_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_BREAKPOINTS (b)
+ {
+ struct bp_location *loc;
+ int bp_modified = 0;
+ int is_no_tracepoint = !is_tracepoint (b);
+
+ if (is_no_tracepoint
+ && b->type != bp_breakpoint
+ && b->type != bp_jit_event
+ && b->type != bp_hardware_breakpoint)
+ continue;
Sorry i didn't notice this, but, does it make sense to use the
is_breakpoint function here? It checks for bp_breakpoint,
bp_hardware_breakpoint and bp_dprintf.
+
+ for (loc = b->loc; loc != NULL; loc = loc->next)
+ {
+ CORE_ADDR loc_addr = loc->address;
+
+ if (is_no_tracepoint
+ && loc->loc_type != bp_loc_hardware_breakpoint
+ && loc->loc_type != bp_loc_software_breakpoint)
+ continue;
+
+ if (loc->shlib_disabled != 0)
+ continue;
+
+ if (objfile->pspace != loc->pspace)
+ continue;
+
+ if (is_addr_in_objfile (loc_addr, objfile))
+ {
+ loc->shlib_disabled = 1;
+ loc->inserted = 0;
+ bp_modified = 1;
+ }
+ }
+
+ if (bp_modified)
+ observer_notify_breakpoint_modified (b);
+ }
+
+}
+
Since we are disabling breakpoints that are related to a specific
objfile or shared library, i think we should call either
mark_breakpoint_modified or mark_breakpoint_location_modified to make
sure things still work OK in case any of these breakpoints are carrying
conditions and the evaluation of these conditions is taking place on the
target's side.
In fact, i think we may be missing calls to these functions in a couple
other places, but you don't need to address those.
diff --git a/gdb/solib.c b/gdb/solib.c
index c987fe5..987d510 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -1478,6 +1478,26 @@ gdb_bfd_lookup_symbol (bfd *abfd,
return symaddr;
}
+/* SO_LIST_HEAD may contain user-loaded object files that can be removed
+ out-of-band by the user. So upon notification of free_objfile remove
+ any reference to any user-loaded file that is about to be freed. */
+
+static void
+remove_user_added_objfile (struct objfile *objfile)
+{
+ struct so_list *so;
+
+ if (!objfile)
+ return;
+
Just a nit. Keep everything consistent in the code. Above you used
"objfile == NULL" instead of "!objfile".