From patchwork Fri Sep 12 19:33:49 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 2823 Received: (qmail 6411 invoked by alias); 12 Sep 2014 19:34:03 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 6400 invoked by uid 89); 12 Sep 2014 19:34:02 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.1 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Fri, 12 Sep 2014 19:34:00 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s8CJXqDl002978 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Fri, 12 Sep 2014 15:33:52 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s8CJXoO7015476; Fri, 12 Sep 2014 15:33:50 -0400 Message-ID: <54134A9D.3010107@redhat.com> Date: Fri, 12 Sep 2014 20:33:49 +0100 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.7.0 MIME-Version: 1.0 To: Joel Brobecker CC: "Maciej W. Rozycki" , Ulrich Weigand , GDB Patches Subject: Re: [IRIX] eliminate deprecated_insert_raw_breakpoint uses References: <201409101445.s8AEjILM007935@d06av02.portsmouth.uk.ibm.com> <54106C61.7040400@redhat.com> <54107860.6060301@redhat.com> <20140910224358.GS28404@adacore.com> <5410D83E.3070401@redhat.com> <20140911032712.GT28404@adacore.com> In-Reply-To: <20140911032712.GT28404@adacore.com> On 09/11/2014 04:27 AM, Joel Brobecker wrote: >> Blah. Probably due to wiping the solib breakpoints in >> the so_ops->handle_event() handler. >> >> If you have the patience, could you try quickly commenting out >> the remove_solib_event_breakpoints call in solib-irix.c and retrying? > > Small glitch that got me kicked out of the machine, but I eventually > manage to run that experiment, and I was able to debug again. > "info shared" after starting a program gave the same list of > shared libraries. Thanks Joel. I've addressed that issue by marking the breakpoints as disp_del_at_next_stop instead of deleting them immediately. Locally, I added a call to remove_solib_event_breakpoints_at_next_stop to solib-svr4.c:svr4_handle_solib_event, to check that the breakpoint does indeed end up removed, and that we don't crash. Adding a call to remove_solib_event_breakpoints instead would crash in the same way you saw on IRIX with the previous version of this patch. I avoided exposing insert_breakpoint_locations out of breakpoint.c by adding a new create_and_insert_solib_event_breakpoint function that is like create_solib_event_breakpoint but tries to insert the breakpoint immediately. I figure that if someone wants to keep IRIX alive (either now, by bringing it back from git), this will be a better base to start from. So in interest of not having to wait for IRIX to be dropped, I've gone ahead and pushed the patch below. Thanks. --------- [IRIX] eliminate deprecated_insert_raw_breakpoint uses The IRIX support wants to set a breakpoint to be hit when the startup phase is complete, which is where shared libraries have been mapped in. AFAIU, for most IRIX ports, that location is the entry point. For MIPS IRIX however, GDB needs to set a breakpoint earlier, in __dbx_link, as explained by: #ifdef SYS_syssgi /* On mips-irix, we need to stop the inferior early enough during the startup phase in order to be able to load the shared library symbols and insert the breakpoints that are located in these shared libraries. Stopping at the program entry point is not good enough because the -init code is executed before the execution reaches that point. So what we need to do is to insert a breakpoint in the runtime loader (rld), more precisely in __dbx_link(). This procedure is called by rld once all shared libraries have been mapped, but before the -init code is executed. Unfortuantely, this is not straightforward, as rld is not part of the executable we are running, and thus we need the inferior to run until rld itself has been mapped in memory. For this, we trace all syssgi() syscall exit events. Each time we detect such an event, we iterate over each text memory maps, get its associated fd, and scan the symbol table for __dbx_link(). When found, we know that rld has been mapped, and that we can insert the breakpoint at the symbol address. Once the dbx_link() breakpoint has been inserted, the syssgi() notifications are no longer necessary, so they should be canceled. */ proc_trace_syscalls_1 (pi, SYS_syssgi, PR_SYSEXIT, FLAG_SET, 0); #endif The loop in irix_solib_create_inferior_hook then runs until whichever breakpoint is hit first, the one set by solib-irix.c or the one set by procfs.c. Note the comment in disable_break talks about __dbx_init, but I think that's a typo for __dbx_link: - /* Note that it is possible that we have stopped at a location that - is different from the location where we inserted our breakpoint. - On mips-irix, we can actually land in __dbx_init(), so we should - not check the PC against our breakpoint address here. See procfs.c - for more details. */ This looks very much like referring to the loop in irix_solib_create_inferior_hook stopping at __dbx_link instead of at the entry point. What this patch does is convert these deprecated raw breakpoints to standard solib_event breakpoints. When the first solib-event breakpoint is hit, we delete all solib-event breakpoints. We do that in the so_ops->handle_event hook. This allows getting rid of the loop in irix_solib_create_inferior_hook completely, which should allow properly handling signals and other events in the early startup phase, like in SVR4. Built on x86_64 Fedora 20 with --enable-targets=all (builds solib-irix.c). Joel tested that with an earlier version of this patch "info shared" after starting a program gave the same list of shared libraries as before. gdb/ChangeLog: 2014-09-12 Pedro Alves * breakpoint.c (remove_solib_event_breakpoints_at_next_stop) (create_and_insert_solib_event_breakpoint): New functions. * breakpoint.h (create_and_insert_solib_event_breakpoint) (remove_solib_event_breakpoints_at_next_stop): New declarations. * procfs.c (dbx_link_bpt_addr, dbx_link_bpt): Delete globals. (remove_dbx_link_breakpoint): Delete function. (insert_dbx_link_bpt_in_file): Use create_and_insert_solib_event_breakpoint instead of deprecated_insert_raw_breakpoint. (procfs_wait): Don't check whether we hit __dbx_link here. (procfs_mourn_inferior): Don't delete the __dbx_link breakpoint here. * solib-irix.c (base_breakpoint): Delete global. (disable_break): Delete function. (enable_break): Use create_solib_event_breakpoint instead of deprecated_insert_raw_breakpoint. (irix_solib_handle_event): New function. (irix_solib_create_inferior_hook): Don't run the target or disable the mapping-complete breakpoint here. (_initialize_irix_solib): Install irix_solib_handle_event as so_ops->handle_event hook. --- gdb/ChangeLog | 24 +++++++++++++ gdb/breakpoint.c | 31 ++++++++++++++++ gdb/breakpoint.h | 11 ++++++ gdb/procfs.c | 47 +++--------------------- gdb/solib-irix.c | 108 ++++++++++++++++--------------------------------------- 5 files changed, 102 insertions(+), 119 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 0ec71d4..0b9e140 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,27 @@ +2014-09-12 Pedro Alves + + * breakpoint.c (remove_solib_event_breakpoints_at_next_stop) + (create_and_insert_solib_event_breakpoint): New functions. + * breakpoint.h (create_and_insert_solib_event_breakpoint) + (remove_solib_event_breakpoints_at_next_stop): New declarations. + * procfs.c (dbx_link_bpt_addr, dbx_link_bpt): Delete globals. + (remove_dbx_link_breakpoint): Delete function. + (insert_dbx_link_bpt_in_file): Use + create_and_insert_solib_event_breakpoint instead of + deprecated_insert_raw_breakpoint. + (procfs_wait): Don't check whether we hit __dbx_link here. + (procfs_mourn_inferior): Don't delete the __dbx_link breakpoint + here. + * solib-irix.c (base_breakpoint): Delete global. + (disable_break): Delete function. + (enable_break): Use create_solib_event_breakpoint + instead of deprecated_insert_raw_breakpoint. + (irix_solib_handle_event): New function. + (irix_solib_create_inferior_hook): Don't run the target or disable + the mapping-complete breakpoint here. + (_initialize_irix_solib): Install irix_solib_handle_event as + so_ops->handle_event hook. + 2014-09-12 Edjunior Barbosa Machado Ulrich Weigand diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 683ed2b..f990d97 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -7620,6 +7620,19 @@ remove_solib_event_breakpoints (void) delete_breakpoint (b); } +/* See breakpoint.h. */ + +void +remove_solib_event_breakpoints_at_next_stop (void) +{ + struct breakpoint *b, *b_tmp; + + ALL_BREAKPOINTS_SAFE (b, b_tmp) + if (b->type == bp_shlib_event + && b->loc->pspace == current_program_space) + b->disposition = disp_del_at_next_stop; +} + struct breakpoint * create_solib_event_breakpoint (struct gdbarch *gdbarch, CORE_ADDR address) { @@ -7631,6 +7644,24 @@ create_solib_event_breakpoint (struct gdbarch *gdbarch, CORE_ADDR address) return b; } +/* See breakpoint.h. */ + +struct breakpoint * +create_and_insert_solib_event_breakpoint (struct gdbarch *gdbarch, CORE_ADDR address) +{ + struct breakpoint *b; + + b = create_solib_event_breakpoint (gdbarch, address); + if (!breakpoints_always_inserted_mode ()) + insert_breakpoint_locations (); + if (!b->loc->inserted) + { + delete_breakpoint (b); + return NULL; + } + return b; +} + /* Disable any breakpoints that are on code in shared libraries. Only apply to enabled breakpoints, disabled ones can just stay disabled. */ diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h index f6d06ce..8abb5ea 100644 --- a/gdb/breakpoint.h +++ b/gdb/breakpoint.h @@ -1419,6 +1419,13 @@ extern struct breakpoint *create_jit_event_breakpoint (struct gdbarch *, extern struct breakpoint *create_solib_event_breakpoint (struct gdbarch *, CORE_ADDR); +/* Create an solib event breakpoint at ADDRESS in the current program + space, and immediately try to insert it. Returns a pointer to the + breakpoint on success. Deletes the new breakpoint and returns NULL + if inserting the breakpoint fails. */ +extern struct breakpoint *create_and_insert_solib_event_breakpoint + (struct gdbarch *gdbarch, CORE_ADDR address); + extern struct breakpoint *create_thread_event_breakpoint (struct gdbarch *, CORE_ADDR); @@ -1426,6 +1433,10 @@ extern void remove_jit_event_breakpoints (void); extern void remove_solib_event_breakpoints (void); +/* Mark solib event breakpoints of the current program space with + delete at next stop disposition. */ +extern void remove_solib_event_breakpoints_at_next_stop (void); + extern void remove_thread_event_breakpoints (void); extern void disable_breakpoints_in_shlibs (void); diff --git a/gdb/procfs.c b/gdb/procfs.c index 3465bc5..699fcd9 100644 --- a/gdb/procfs.c +++ b/gdb/procfs.c @@ -2902,13 +2902,6 @@ static void do_detach (int signo); static void proc_trace_syscalls_1 (procinfo *pi, int syscallnum, int entry_or_exit, int mode, int from_tty); -/* On mips-irix, we need to insert a breakpoint at __dbx_link during - the startup phase. The following two variables are used to record - the address of the breakpoint, and the code that was replaced by - a breakpoint. */ -static int dbx_link_bpt_addr = 0; -static void *dbx_link_bpt; - /* Sets up the inferior to be debugged. Registers to trace signals, hardware faults, and syscalls. Note: does not set RLC flag: caller may want to customize that. Returns zero for success (note! @@ -3380,23 +3373,6 @@ syscall_is_lwp_create (procinfo *pi, int scall) return 0; } -/* Remove the breakpoint that we inserted in __dbx_link(). - Does nothing if the breakpoint hasn't been inserted or has already - been removed. */ - -static void -remove_dbx_link_breakpoint (void) -{ - if (dbx_link_bpt_addr == 0) - return; - - if (deprecated_remove_raw_breakpoint (target_gdbarch (), dbx_link_bpt) != 0) - warning (_("Unable to remove __dbx_link breakpoint.")); - - dbx_link_bpt_addr = 0; - dbx_link_bpt = NULL; -} - #ifdef SYS_syssgi /* Return the address of the __dbx_link() function in the file refernced by ABFD by scanning its symbol table. Return 0 if @@ -3461,10 +3437,12 @@ insert_dbx_link_bpt_in_file (int fd, CORE_ADDR ignored) sym_addr = dbx_link_addr (abfd); if (sym_addr != 0) { + struct breakpoint *dbx_link_bpt; + /* Insert the breakpoint. */ - dbx_link_bpt_addr = sym_addr; - dbx_link_bpt = deprecated_insert_raw_breakpoint (target_gdbarch (), NULL, - sym_addr); + dbx_link_bpt + = create_and_insert_solib_event_breakpoint (target_gdbarch (), + sym_addr); if (dbx_link_bpt == NULL) { warning (_("Failed to insert dbx_link breakpoint.")); @@ -3894,14 +3872,6 @@ wait_again: #if (FLTTRACE != FLTBPT) /* Avoid "duplicate case" error. */ case FLTTRACE: #endif - /* If we hit our __dbx_link() internal breakpoint, - then remove it. See comments in procfs_init_inferior() - for more details. */ - if (dbx_link_bpt_addr != 0 - && dbx_link_bpt_addr - == regcache_read_pc (get_current_regcache ())) - remove_dbx_link_breakpoint (); - wstat = (SIGTRAP << 8) | 0177; break; case FLTSTACK: @@ -4340,13 +4310,6 @@ procfs_mourn_inferior (struct target_ops *ops) generic_mourn_inferior (); - if (dbx_link_bpt != NULL) - { - deprecated_remove_raw_breakpoint (target_gdbarch (), dbx_link_bpt); - dbx_link_bpt_addr = 0; - dbx_link_bpt = NULL; - } - inf_child_maybe_unpush_target (ops); } diff --git a/gdb/solib-irix.c b/gdb/solib-irix.c index 12ed766..00236bf 100644 --- a/gdb/solib-irix.c +++ b/gdb/solib-irix.c @@ -240,8 +240,6 @@ fetch_lm_info (CORE_ADDR addr) /* The symbol which starts off the list of shared libraries. */ #define DEBUG_BASE "__rld_obj_head" -static void *base_breakpoint; - static CORE_ADDR debug_base; /* Base of dynamic linker structures. */ /* Locate the base address of dynamic linker structs. @@ -293,34 +291,6 @@ locate_base (void) return (address); } -/* Remove the "mapping changed" breakpoint. - - Removes the breakpoint that gets hit when the dynamic linker - completes a mapping change. */ - -static int -disable_break (void) -{ - int status = 1; - - /* Note that breakpoint address and original contents are in our address - space, so we just need to write the original contents back. */ - - if (deprecated_remove_raw_breakpoint (target_gdbarch (), base_breakpoint) != 0) - { - status = 0; - } - - base_breakpoint = NULL; - - /* Note that it is possible that we have stopped at a location that - is different from the location where we inserted our breakpoint. - On mips-irix, we can actually land in __dbx_init(), so we should - not check the PC against our breakpoint address here. See procfs.c - for more details. */ - - return (status); -} /* Arrange for dynamic linker to hit breakpoint. @@ -332,23 +302,39 @@ enable_break (void) { if (symfile_objfile != NULL && has_stack_frames ()) { - struct frame_info *frame = get_current_frame (); - struct address_space *aspace = get_frame_address_space (frame); CORE_ADDR entry_point; - if (!entry_point_address_query (&entry_point)) - return 0; - - base_breakpoint = deprecated_insert_raw_breakpoint (target_gdbarch (), - aspace, entry_point); - - if (base_breakpoint != NULL) - return 1; + if (entry_point_address_query (&entry_point)) + { + create_solib_event_breakpoint (target_gdbarch (), entry_point); + return 1; + } } return 0; } +/* Implement the "handle_event" target_solib_ops method. */ + +static void +irix_solib_handle_event (void) +{ + /* We are now at the "mapping complete" breakpoint, we no longer + need it. Note that it is possible that we have stopped at a + location that is different from the location where we inserted + our breakpoint: On mips-irix, we can actually land in + __dbx_link(), so we should not check the PC against our + breakpoint address here. See procfs.c for more details. Note + we're being called by the bpstat handling code, and so can't + delete the breakpoint immediately. Mark it for later deletion, + which has the same effect (it'll be removed before we next resume + or if we're stopping). */ + remove_solib_event_breakpoints_at_next_stop (); + + /* The caller calls solib_add, which will add any shared libraries + that were mapped in. */ +} + /* Implement the "create_inferior_hook" target_solib_ops method. For SunOS executables, this first instruction is typically the @@ -409,43 +395,10 @@ irix_solib_create_inferior_hook (int from_tty) return; } - /* Now run the target. It will eventually hit the breakpoint, at - which point all of the libraries will have been mapped in and we - can go groveling around in the dynamic linker structures to find - out what we need to know about them. */ - - tp = inferior_thread (); - - clear_proceed_status (0); - - inf->control.stop_soon = STOP_QUIETLY; - tp->suspend.stop_signal = GDB_SIGNAL_0; - - do - { - target_resume (pid_to_ptid (-1), 0, tp->suspend.stop_signal); - wait_for_inferior (); - } - while (tp->suspend.stop_signal != GDB_SIGNAL_TRAP); - - /* We are now either at the "mapping complete" breakpoint (or somewhere - else, a condition we aren't prepared to deal with anyway), so adjust - the PC as necessary after a breakpoint, disable the breakpoint, and - add any shared libraries that were mapped in. */ - - if (!disable_break ()) - { - warning (_("shared library handler failed to disable breakpoint")); - } - - /* solib_add will call reinit_frame_cache. - But we are stopped in the startup code and we might not have symbols - for the startup code, so heuristic_proc_start could be called - and will put out an annoying warning. - Delaying the resetting of stop_soon until after symbol loading - suppresses the warning. */ - solib_add ((char *) 0, 0, (struct target_ops *) 0, auto_solib_add); - inf->control.stop_soon = NO_STOP_QUIETLY; + /* The target will eventually hit the breakpoint, at which point all + of the libraries will have been mapped in and we can go groveling + around in the dynamic linker structures to find out what we need + to know about them. */ } /* Implement the "current_sos" target_so_ops method. */ @@ -653,4 +606,5 @@ _initialize_irix_solib (void) irix_so_ops.open_symbol_file_object = irix_open_symbol_file_object; irix_so_ops.in_dynsym_resolve_code = irix_in_dynsym_resolve_code; irix_so_ops.bfd_open = solib_bfd_open; + irix_so_ops.handle_event = irix_solib_handle_event; }