From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8372 invoked by alias); 23 Aug 2004 21:33:28 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 8362 invoked from network); 23 Aug 2004 21:33:27 -0000 Received: from unknown (HELO mx1.redhat.com) (66.187.233.31) by sourceware.org with SMTP; 23 Aug 2004 21:33:27 -0000 Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.12.10/8.12.10) with ESMTP id i7NLXRe1020017 for ; Mon, 23 Aug 2004 17:33:27 -0400 Received: from pobox.toronto.redhat.com (pobox.toronto.redhat.com [172.16.14.4]) by int-mx1.corp.redhat.com (8.11.6/8.11.6) with ESMTP id i7NLXRa25396; Mon, 23 Aug 2004 17:33:27 -0400 Received: from touchme.toronto.redhat.com (IDENT:postfix@touchme.toronto.redhat.com [172.16.14.9]) by pobox.toronto.redhat.com (8.12.8/8.12.8) with ESMTP id i7NLXQse028995; Mon, 23 Aug 2004 17:33:26 -0400 Received: from redhat.com (toocool.toronto.redhat.com [172.16.14.72]) by touchme.toronto.redhat.com (Postfix) with ESMTP id 5A4C7800025; Mon, 23 Aug 2004 17:33:26 -0400 (EDT) Message-ID: <412A62A6.3010806@redhat.com> Date: Mon, 23 Aug 2004 21:33:00 -0000 From: Jeff Johnston User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.4) Gecko/20030624 Netscape/7.1 MIME-Version: 1.0 To: Jeff Johnston Cc: Daniel Jacobowitz , gdb-patches@sources.redhat.com, eliz@gnu.org Subject: Re: [RFA]: Fix for pending breakpoints in manually loaded/unloaded shlibs References: <41191D71.60204@redhat.com> <20040811171203.GA4152@nevyn.them.org> <411A7D97.50104@redhat.com> <20040818135621.GA26257@nevyn.them.org> <4123AC6E.8000300@redhat.com> <20040818193952.GA27639@nevyn.them.org> <4123B62C.6060703@redhat.com> In-Reply-To: <4123B62C.6060703@redhat.com> Content-Type: multipart/mixed; boundary="------------070008020603080501020101" X-SW-Source: 2004-08/txt/msg00632.txt.bz2 This is a multi-part message in MIME format. --------------070008020603080501020101 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit Content-length: 3394 The updated patch is attached. The testcase had to be slightly modified to match the new warning message format. 2004-08-23 Jeff Johnston * observer.sh: Add struct so_list declaration. * Makefile.in: Add dependencies on observer.h for solib.c and breakpoint.c. * breakpoint.c (disable_breakpoints_in_unloaded_shlib): New function. (_initialize_breakpoint): Register disable_breakpoints_in_unloaded_shlib as an observer of the "solib unloaded" observation event. (re_enable_breakpoints_in_shlibs): For bp_shlib_disabled breakpoints, call decode_line_1 so unfound breakpoint errors are silent. * solib.c (update_solib_list): When a solib is discovered to have been unloaded by the program, notify all observers of the "solib unloaded" observation event. 2004-08-23 Jeff Johnston * gdb.base/unload.exp: Fix expected warning message to match latest format. Ok to commit? -- Jeff J. Jeff Johnston wrote: > Daniel Jacobowitz wrote: > >> On Wed, Aug 18, 2004 at 03:22:22PM -0400, Jeff Johnston wrote: >> >>> Daniel Jacobowitz wrote: >>> >>>> On Wed, Aug 11, 2004 at 04:12:07PM -0400, Jeff Johnston wrote: >>>> >>>>> + if (so_name + && !strcmp (so_name, solib->so_name)) >>>>> + { >>>>> + b->enable_state = bp_shlib_disabled; >>>>> + /* At this point, we cannot rely on remove_breakpoint >>>>> + succeeding so we must mark the breakpoint as not inserted >>>>> + to prevent future errors occurring in >>>>> remove_breakpoints. */ >>>>> + b->loc->inserted = 0; >>>>> + if (!disabled_shlib_breaks) >>>>> + { >>>>> + target_terminal_ours_for_output (); >>>>> + warning ("Temporarily disabling unloaded shared library >>>>> breakpoints:"); >>>>> + } >>>>> + disabled_shlib_breaks = 1; >>>>> + warning ("breakpoint #%d ", b->number); >>>> >>>> >>>> >>>> I think you're missing a space after the colon, in the first warning. >>>> Also, this use of multiple warning() statements is neither i18n >>>> friendly nor MI/GUI friendly - you may get a separate dialog box for >>>> each. I believe other places do this with sprintf; still not 100% i18n >>>> friendly, but avoids the MI/GUI problems. I can't find an example >>>> offhand. >>>> >>> >>> What you do want to see so I don't waste my time on this. As you >>> already know, this routine was copied from the routine which disables >>> shared library breakpoints in breakpoint.c. Is it sufficient to just >>> issue the warning that I am temporarily disabling unloaded shared >>> library breakpoints and not spell out each breakpoint in turn? I can >>> see this as really annoying and pointless to an end-user if there are >>> hundreds or thousands of breakpoints. >> >> >> >> That's a good idea. How about this? >> >> target_terminal_ours_for_output (); >> warning ("Temporarily disabling breakpoints for unloaded shared >> library \"%s\", >> so_name); >> > > Yes, that's exactly what I had in mind. Consider it done plus your > other comments. Eli, in light of what Daniel and Andrew have said > regarding the value of having an observer, may I repost with changes and > check the code in? > > -- Jeff J. > > --------------070008020603080501020101 Content-Type: text/plain; name="unload.patch2" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="unload.patch2" Content-length: 6724 Index: doc/observer.texi =================================================================== RCS file: /cvs/src/src/gdb/doc/observer.texi,v retrieving revision 1.7 diff -u -p -r1.7 observer.texi --- doc/observer.texi 21 May 2004 16:04:03 -0000 1.7 +++ doc/observer.texi 23 Aug 2004 21:27:18 -0000 @@ -90,3 +90,8 @@ at the entry-point instruction. For @sa @value{GDBN} calls this observer immediately after connecting to the inferior, and before any information on the inferior has been printed. @end deftypefun + +@deftypefun void solib_unloaded (struct so_list *@var{solib}) +The specified shared library has been discovered to be unloaded. +@end deftypefun + Index: Makefile.in =================================================================== RCS file: /cvs/src/src/gdb/Makefile.in,v retrieving revision 1.607 diff -u -p -r1.607 Makefile.in --- Makefile.in 8 Aug 2004 19:27:09 -0000 1.607 +++ Makefile.in 23 Aug 2004 21:27:19 -0000 @@ -1718,7 +1718,7 @@ breakpoint.o: breakpoint.c $(defs_h) $(s $(gdb_string_h) $(demangle_h) $(annotate_h) $(symfile_h) \ $(objfiles_h) $(source_h) $(linespec_h) $(completer_h) $(gdb_h) \ $(ui_out_h) $(cli_script_h) $(gdb_assert_h) $(block_h) \ - $(gdb_events_h) + $(gdb_events_h) $(observer_h) $(solist_h) bsd-kvm.o: bsd-kvm.c $(defs_h) $(cli_cmds_h) $(command_h) $(frame_h) \ $(regcache_h) $(target_h) $(value_h) $(gdb_assert_h) $(readline_h) \ $(bsd_kvm_h) @@ -2450,7 +2450,8 @@ solib-aix5.o: solib-aix5.c $(defs_h) $(g solib.o: solib.c $(defs_h) $(gdb_string_h) $(symtab_h) $(bfd_h) $(symfile_h) \ $(objfiles_h) $(gdbcore_h) $(command_h) $(target_h) $(frame_h) \ $(gdb_regex_h) $(inferior_h) $(environ_h) $(language_h) $(gdbcmd_h) \ - $(completer_h) $(filenames_h) $(exec_h) $(solist_h) $(readline_h) + $(completer_h) $(filenames_h) $(exec_h) $(solist_h) $(readline_h) \ + $(observer_h) solib-frv.o: solib-frv.c $(defs_h) $(gdb_string_h) $(inferior_h) \ $(gdbcore_h) $(solist_h) $(frv_tdep_h) $(objfiles_h) $(symtab_h) \ $(language_h) $(command_h) $(gdbcmd_h) $(elf_frv_h) Index: breakpoint.c =================================================================== RCS file: /cvs/src/src/gdb/breakpoint.c,v retrieving revision 1.179 diff -u -p -r1.179 breakpoint.c --- breakpoint.c 28 Jul 2004 17:26:26 -0000 1.179 +++ breakpoint.c 23 Aug 2004 21:27:19 -0000 @@ -49,6 +49,8 @@ #include "cli/cli-script.h" #include "gdb_assert.h" #include "block.h" +#include "solist.h" +#include "observer.h" #include "gdb-events.h" @@ -4388,6 +4390,46 @@ disable_breakpoints_in_shlibs (int silen } } +/* Disable any breakpoints that are in in an unloaded shared library. Only + apply to enabled breakpoints, disabled ones can just stay disabled. */ + +void +disable_breakpoints_in_unloaded_shlib (struct so_list *solib) +{ + struct breakpoint *b; + int disabled_shlib_breaks = 0; + +#if defined (PC_SOLIB) + /* See also: insert_breakpoints, under DISABLE_UNSETTABLE_BREAK. */ + ALL_BREAKPOINTS (b) + { + if ((b->loc->loc_type == bp_loc_hardware_breakpoint + || b->loc->loc_type == bp_loc_software_breakpoint) + && breakpoint_enabled (b) + && !b->loc->duplicate) + { + char *so_name = PC_SOLIB (b->loc->address); + if (so_name + && !strcmp (so_name, solib->so_name)) + { + b->enable_state = bp_shlib_disabled; + /* At this point, we cannot rely on remove_breakpoint + succeeding so we must mark the breakpoint as not inserted + to prevent future errors occurring in remove_breakpoints. */ + b->loc->inserted = 0; + if (!disabled_shlib_breaks) + { + target_terminal_ours_for_output (); + warning ("Temporarily disabling breakpoints for unloaded shared library \"%s\"", + so_name); + } + disabled_shlib_breaks = 1; + } + } + } +#endif +} + /* Try to reenable any breakpoints in shared libraries. */ void re_enable_breakpoints_in_shlibs (void) @@ -7100,6 +7142,8 @@ breakpoint_re_set_one (void *bint) struct breakpoint *b = (struct breakpoint *) bint; struct value *mark; int i; + int not_found; + int *not_found_ptr = NULL; struct symtabs_and_lines sals; char *s; enum enable_state save_enable; @@ -7150,11 +7194,19 @@ breakpoint_re_set_one (void *bint) save_enable = b->enable_state; if (b->enable_state != bp_shlib_disabled) b->enable_state = bp_disabled; + else + /* If resetting a shlib-disabled breakpoint, we don't want to + see an error message if it is not found since we will expect + this to occur until the shared library is finally reloaded. + We accomplish this by giving decode_line_1 a pointer to use + for silent notification that the symbol is not found. */ + not_found_ptr = ¬_found; set_language (b->language); input_radix = b->input_radix; s = b->addr_string; - sals = decode_line_1 (&s, 1, (struct symtab *) NULL, 0, (char ***) NULL, NULL); + sals = decode_line_1 (&s, 1, (struct symtab *) NULL, 0, (char ***) NULL, + not_found_ptr); for (i = 0; i < sals.nelts; i++) { resolve_sal_pc (&sals.sals[i]); @@ -7755,6 +7807,10 @@ _initialize_breakpoint (void) static struct cmd_list_element *breakpoint_show_cmdlist; struct cmd_list_element *c; +#ifdef SOLIB_ADD + observer_attach_solib_unloaded (disable_breakpoints_in_unloaded_shlib); +#endif + breakpoint_chain = 0; /* Don't bother to call set_breakpoint_count. $bpnum isn't useful before a breakpoint is set. */ Index: observer.sh =================================================================== RCS file: /cvs/src/src/gdb/observer.sh,v retrieving revision 1.3 diff -u -p -r1.3 observer.sh --- observer.sh 7 May 2004 22:51:52 -0000 1.3 +++ observer.sh 23 Aug 2004 21:27:19 -0000 @@ -52,6 +52,7 @@ case $lang in struct observer; struct bpstats; +struct so_list; EOF ;; esac Index: solib.c =================================================================== RCS file: /cvs/src/src/gdb/solib.c,v retrieving revision 1.66 diff -u -p -r1.66 solib.c --- solib.c 30 Jul 2004 19:17:19 -0000 1.66 +++ solib.c 23 Aug 2004 21:27:19 -0000 @@ -42,6 +42,7 @@ #include "filenames.h" /* for DOSish file names */ #include "exec.h" #include "solist.h" +#include "observer.h" #include "readline/readline.h" /* external data declarations */ @@ -478,6 +479,10 @@ update_solib_list (int from_tty, struct /* If it's not on the inferior's list, remove it from GDB's tables. */ else { + /* Notify any observer that the SO has been unloaded + before we remove it from the gdb tables. */ + observer_notify_solib_unloaded (gdb); + *gdb_link = gdb->next; /* Unless the user loaded it explicitly, free SO's objfile. */ --------------070008020603080501020101 Content-Type: text/plain; name="unloadtest.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="unloadtest.patch" Content-length: 1138 Index: gdb.base/unload.exp =================================================================== RCS file: /cvs/src/src/gdb/testsuite/gdb.base/unload.exp,v retrieving revision 1.1 diff -u -p -r1.1 unload.exp --- gdb.base/unload.exp 12 Aug 2004 20:22:59 -0000 1.1 +++ gdb.base/unload.exp 23 Aug 2004 21:30:34 -0000 @@ -126,7 +126,7 @@ Breakpoint.*, shrfunc1 \\\(x=3\\\).*unlo "running program" gdb_test "continue" \ -"Continuing.*y is 7.*warning: Temporarily disabling unloaded shared library breakpoints.*warning: breakpoint #.*Program exited normally." \ +"Continuing.*y is 7.*warning: Temporarily disabling breakpoints for.*unloadshr.sl.*Program exited normally." \ "continuing to end of program" # @@ -138,5 +138,6 @@ gdb_test "run" \ "rerun to shared library breakpoint" gdb_test "continue" \ -"Continuing.*y is 7.*warning: Temporarily disabling unloaded shared library breakpoints.*warning: breakpoint #.*Program exited normally." \ -"continuing to end of program second time" +"Continuing.*y is 7.*warning: Temporarily disabling breakpoints for.*unloadshr.sl.*Program exited normally." \ +"continuing to end of program" + --------------070008020603080501020101--