From patchwork Thu Sep 10 22:56:39 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Don Breazeal X-Patchwork-Id: 8628 Received: (qmail 112177 invoked by alias); 10 Sep 2015 22:56:54 -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 112157 invoked by uid 89); 10 Sep 2015 22:56:54 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.2 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 10 Sep 2015 22:56:48 +0000 Received: from svr-orw-fem-04.mgc.mentorg.com ([147.34.97.41]) by relay1.mentorg.com with esmtp id 1ZaAlp-0001Is-6J from Don_Breazeal@mentor.com ; Thu, 10 Sep 2015 15:56:45 -0700 Received: from [172.30.14.130] (147.34.91.1) by SVR-ORW-FEM-04.mgc.mentorg.com (147.34.97.41) with Microsoft SMTP Server (TLS) id 14.3.224.2; Thu, 10 Sep 2015 15:56:44 -0700 Subject: Re: [PATCH v3 1/4] Extended-remote follow exec To: Pedro Alves , "gdb-patches@sourceware.org" References: <1438298360-29594-1-git-send-email-donb@codesourcery.com> <1441839937-22251-1-git-send-email-donb@codesourcery.com> <1441839937-22251-2-git-send-email-donb@codesourcery.com> <55F17AFA.5080102@redhat.com> From: Don Breazeal Message-ID: <55F20AA7.8050301@codesourcery.com> Date: Thu, 10 Sep 2015 15:56:39 -0700 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 MIME-Version: 1.0 In-Reply-To: <55F17AFA.5080102@redhat.com> X-IsSubscribed: yes On 9/10/2015 5:43 AM, Pedro Alves wrote: > On 09/10/2015 12:05 AM, Don Breazeal wrote: >> Hi Pedro, >> >> This is an updated version of the patch previously submitted here: >> https://sourceware.org/ml/gdb-patches/2015-07/msg00924.html. Changes >> from the previous version include: >> >> * In gdbserver, when an exec event occurs, gdbserver deletes all >> of the data (inferior, lwps, threads) associated with the execing >> process and replaces it with a new set of data. >> >> * In GDB, the remote exec-file is now stored per-inferior in the >> inferior's program space as a REGISTRY field. >> >> * In GDB, a new target hook, target_follow_exec, is used to enable >> storing the remote exec-file as per-inferior data. >> >> * In GDB, follow_exec now calls add_inferior_with_spaces for mode >> "new" in place of add_inferior and the calls to set up the program >> and address spaces. >> >> Some of the things that were part of the previous patchset were >> eliminated as a result of these changes, including: >> >> * Deleting "vanished" lwps in gdbserver/linux-low.c:send_sigstop. >> >> * Fiddling with the regcache and r_debug in >> gdbserver/linux-low.c:handle_extended_wait. >> >> * Fiddling with the inferior's architecture in >> remote.c:remote_parse_stop_reply. >> >> A couple of your questions about the previous version of the patch still >> apply, in spite of the rework. Regarding the handling of the exec event >> in linux-low.c:handle_extended_wait: >> >>>> + /* Mark the exec status as pending. */ >>>> + event_lwp->stopped = 1; >>>> + event_lwp->status_pending_p = 1; >>>> + event_lwp->status_pending = wstat; >>>> + event_thr->last_resume_kind = resume_stop; >>> >>> Shouldn't this be resume_continue? >> >> My thinking here is that as far as gdbserver is concerned, we *do* want >> to use resume_stop, so that we stop and report the event to GDB. It will >> be up to GDB whether to continue from this point. Does that make sense? > > Not really -- putting exec events out of the picture, consider: > > If you simply continue a thread (vCont;c) and it hits a breakpoint, it'll > have last_resume_kind==resume_continue, and we still report the event > to gdb, and it's still up to GDB whether to continue past the breakpoint. > > So if you set last_resume_kind to resume_continue, and drop this hunk: > >> @@ -3373,7 +3463,8 @@ linux_wait_1 (ptid_t ptid, >> ourstatus->value.sig = GDB_SIGNAL_0; >> } >> else if (current_thread->last_resume_kind == resume_stop >> - && WSTOPSIG (w) != SIGSTOP) >> + && WSTOPSIG (w) != SIGSTOP >> + && ourstatus->kind != TARGET_WAITKIND_EXECD) >> { >> /* A thread that has been requested to stop by GDB with vCont;t, >> but, it stopped for other reasons. */ >> @@ -5801,6 +5892,14 @@ linux_supports_vfork_events (void) >> return linux_supports_tracefork (); >> } > > ... what doesn't work? That works just fine. I've made that change. Clearly I didn't understand the purpose of resume_stop. Is that only used when GDB requests a stop, and/or when an inferior is just starting up or being attached? As opposed to when the inferior is stopped by an event? > > >> @@ -571,6 +598,50 @@ handle_extended_wait (struct lwp_info *event_lwp, int wstat) >> /* Report the event. */ >> return 0; >> } >> + else if (event == PTRACE_EVENT_EXEC && report_exec_events) >> + { >> + struct process_info *proc; >> + ptid_t event_ptid; >> + pid_t event_pid; >> + >> + if (debug_threads) >> + { >> + debug_printf ("HEW: Got exec event from LWP %ld\n", >> + lwpid_of (event_thr)); >> + } >> + >> + /* Get the event ptid. */ >> + event_ptid = event_thr->entry.id; > > event_ptid = ptid_of (event_thr); > Made this change. > >> + event_pid = ptid_get_pid (event_ptid); >> + >> + /* Delete the execing process and all its threads. */ >> + proc = get_thread_process (event_thr); >> + linux_mourn (proc); >> + current_thread = NULL; >> + >> + /* Create a new process/lwp/thread. */ >> + proc = linux_add_process (event_pid, 0); >> + event_lwp = add_lwp (event_ptid); >> + event_thr = get_lwp_thread (event_lwp); >> + gdb_assert (current_thread == event_thr); >> + linux_arch_setup_thread (event_thr); >> + >> + /*set the event status*/ > > Uppercase, period at end, double space. Done. > >> + event_lwp->waitstatus.kind = TARGET_WAITKIND_EXECD; >> + event_lwp->waitstatus.value.execd_pathname >> + = xstrdup (linux_proc_pid_to_exec_file (lwpid_of (event_thr))); >> + >> + /* Mark the exec status as pending. */ >> + event_lwp->stopped = 1; >> + event_lwp->status_pending_p = 1; >> + event_lwp->status_pending = wstat; >> + event_thr->last_resume_kind = resume_stop; >> + event_thr->last_status.kind = TARGET_WAITKIND_IGNORE; >> + >> + /* Report the event. */ >> + *orig_event_lwp = event_lwp; >> + return 0; >> + } >> >> internal_error (__FILE__, __LINE__, _("unknown ptrace event %d"), event); >> } > > > >> @@ -1134,6 +1135,25 @@ prepare_resume_reply (char *buf, ptid_t ptid, >> buf = write_ptid (buf, status->value.related_pid); >> strcat (buf, ";"); >> } >> + else if (status->kind == TARGET_WAITKIND_EXECD && multi_process) >> + { >> + enum gdb_signal signal = GDB_SIGNAL_TRAP; >> + const char *event = "exec"; >> + char hexified_pathname[PATH_MAX*2]; > > Spaces around *. Done. > >> + >> + sprintf (buf, "T%02x%s:", signal, event); >> + buf += strlen (buf); >> + >> + /* Encode pathname to hexified format. */ >> + bin2hex ((const gdb_byte *) status->value.execd_pathname, >> + hexified_pathname, >> + strlen (status->value.execd_pathname)); >> + >> + sprintf (buf, "%s;", hexified_pathname); >> + xfree (status->value.execd_pathname); >> + status->value.execd_pathname = NULL; >> + buf += strlen (buf); >> + } >> else >> sprintf (buf, "T%02x", status->value.sig); >> > > > > >> @@ -619,6 +622,62 @@ get_remote_state (void) >> return get_remote_state_raw (); >> } >> >> +/* Cleanup routine for the remote module's pspace data. */ >> + >> +static void >> +remote_pspace_data_cleanup (struct program_space *pspace, void *arg) >> +{ >> + char *remote_exec_file = arg; >> + >> + if (remote_exec_file != NULL) >> + xfree (remote_exec_file); > > No need to check for NULL before calling xfree. Done. > >> +} >> + >> +/* Fetch the remote exec-file from the current program space. */ >> + >> +static char * > > Can this be const char * ? > >> +get_remote_exec_file (void) >> +{ >> + char *remote_exec_file; >> + >> + remote_exec_file = program_space_data (current_program_space, >> + remote_pspace_data); > > How about adding: > > if (remote_exec_file == NULL) > return ""; > >> + return remote_exec_file; > > avoiding callers having to do it. > Done. >> +} >> + >> +/* Set the remote exec file for the current program space. */ >> + >> +static void >> +set_remote_exec_file_1 (char *remote_exec_file) >> +{ >> + char *old_file = get_remote_exec_file (); >> + > > Here's you'd use > old_file = program_space_data (current_program_space, > remote_pspace_data); > > directly. It would seem super fine to me given the > set_program_space_data just below. Done. > >> + if (old_file != NULL) >> + xfree (old_file); > > No need for NULL check. Done. > >> + >> + set_program_space_data (current_program_space, remote_pspace_data, >> + xstrdup (remote_exec_file)); >> +} >> + >> +/* The "set/show remote exec-file" set hook. */ >> + >> +static void >> +set_remote_exec_file (char *ignored, int from_tty, >> + struct cmd_list_element *c) >> +{ >> + gdb_assert (*(char **) c->var != NULL); >> + set_remote_exec_file_1 (*(char **) c->var); > > Use temp var please. E.g.: > > char *file = *(char **) c->var; > > gdb_assert (file != NULL); > set_remote_exec_file_1 (file); > > Or see another suggestion below. > >> +} >> + >> +/* The "set/show remote exec-file" show hook. */ >> + >> +static void >> +show_remote_exec_file (struct ui_file *file, int from_tty, >> + struct cmd_list_element *cmd, const char *value) >> +{ >> + fprintf_filtered (file, "%s\n", get_remote_exec_file ()); >> +} >> + >> static int >> compare_pnums (const void *lhs_, const void *rhs_) >> { > > > > > >> @@ -4779,6 +4842,28 @@ remote_follow_fork (struct target_ops *ops, int follow_child, >> return 0; >> } >> >> +/* Target follow-exec function for remote targets. Save EXECD_PATHNAME >> + in the program space of the new inferior. On entry and at return the >> + current inferior is the exec'ing inferior. INF is the new exec'd >> + inferior, which may be the same as the exec'ing inferior unless >> + follow-exec-mode is "new". */ >> + >> +static void >> +remote_follow_exec (struct target_ops *ops, >> + struct inferior *inf, char *execd_pathname) >> +{ >> + struct cleanup *old_chain = save_current_program_space (); >> + >> + /* We know that this is a target file name, so if it has the "target:" >> + prefix we strip it off before saving it in the program space. */ >> + if (is_target_filename (execd_pathname)) >> + execd_pathname += strlen (TARGET_SYSROOT_PREFIX); >> + >> + set_current_program_space (inf->pspace); > > Why not pass down the pspace as parameter to set_remote_exec_file_1 > etc., avoiding this? > Done. >> + set_remote_exec_file_1 (execd_pathname); >> + do_cleanups (old_chain); >> +} >> + >> /* Same as remote_detach, but don't send the "D" packet; just disconnect. */ >> >> static void >> @@ -5977,6 +6062,7 @@ remote_parse_stop_reply (char *buf, struct stop_reply *event) >> struct remote_arch_state *rsa = get_remote_arch_state (); >> ULONGEST addr; >> char *p; >> + int skipregs = 0; >> >> event->ptid = null_ptid; >> event->rs = get_remote_state (); > > > > >> @@ -13340,12 +13479,21 @@ Transfer files to and from the remote target system."), >> _("Delete a remote file."), >> &remote_cmdlist); >> >> - remote_exec_file = xstrdup (""); >> - add_setshow_string_noescape_cmd ("exec-file", class_files, >> - &remote_exec_file, _("\ >> + { >> + /* Pass a NULL (by reference) as the 'var' argument, since we do >> + not have a single variable in which to store the value. The >> + value is set per-inferior, stored in the program space. */ >> + char *nullptr = NULL; > > Passing the address of a local variable can't be good. In: > >> +static void >> +set_remote_exec_file (char *ignored, int from_tty, >> + struct cmd_list_element *c) >> +{ >> + gdb_assert (*(char **) c->var != NULL); >> + set_remote_exec_file_1 (*(char **) c->var); > > c->var points to nullptr. So this set command ends up poking > memory to something random on the stack. Oh, ugh. :-P > > I'd prefer leaving the old command variable global, but rename it > remote_exec_file_var, like: > > /* The variable registered as control variable the set/show > remote exec-file commands. Necessary because ... */ > static char *remote_exec_file_var = ""; > > There's some precedent for that, in e.g., record_insn_history_size_setshow_var > and history_size_setshow_var. > > Note that that way, referencing remote_exec_file_var directly in > set_remote_exec_file avoids the casts. I've made these changes. > >> + >> + add_setshow_string_noescape_cmd ("exec-file", class_files, >> + &nullptr, _("\ >> Set the remote pathname for \"run\""), _("\ >> -Show the remote pathname for \"run\""), NULL, NULL, NULL, >> - &remote_set_cmdlist, &remote_show_cmdlist); >> +Show the remote pathname for \"run\""), NULL, >> + set_remote_exec_file, >> + show_remote_exec_file, >> + &remote_set_cmdlist, >> + &remote_show_cmdlist); >> + } Thanks for the review, Pedro. An updated patch follows below. I tested the changes by running the exec-related tests on Linux x86_64 with extended-remote. How does this look? --Don --- gdb/gdbserver/linux-low.c | 147 ++++++++++++++++++++++++++++++++++++------- gdb/gdbserver/lynx-low.c | 1 + gdb/gdbserver/remote-utils.c | 20 ++++++ gdb/gdbserver/server.c | 11 ++++ gdb/gdbserver/server.h | 1 + gdb/gdbserver/target.h | 7 +++ gdb/gdbserver/win32-low.c | 1 + gdb/infrun.c | 17 +++-- gdb/nat/linux-ptrace.c | 11 ++++ gdb/nat/linux-ptrace.h | 1 + gdb/remote.c | 147 ++++++++++++++++++++++++++++++++++++++++--- gdb/target-debug.h | 2 + gdb/target-delegates.c | 30 +++++++++ gdb/target.c | 8 +++ gdb/target.h | 7 +++ 15 files changed, 370 insertions(+), 41 deletions(-) catchpoint for such events. They return 0 for success, 1 if the diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c index 4256bc5..aa4c868 100644 --- a/gdb/gdbserver/linux-low.c +++ b/gdb/gdbserver/linux-low.c @@ -265,6 +265,7 @@ static int linux_wait_for_event_filtered (ptid_t wait_ptid, ptid_t filter_ptid, int *wstat, int options); static int linux_wait_for_event (ptid_t ptid, int *wstat, int options); static struct lwp_info *add_lwp (ptid_t ptid); +static void linux_mourn (struct process_info *process); static int linux_stopped_by_watchpoint (void); static void mark_lwp_dead (struct lwp_info *lwp, int wstat); static int lwp_is_marked_dead (struct lwp_info *lwp); @@ -419,13 +420,39 @@ linux_add_process (int pid, int attached) static CORE_ADDR get_pc (struct lwp_info *lwp); -/* Handle a GNU/Linux extended wait response. If we see a clone - event, we need to add the new LWP to our list (and return 0 so as - not to report the trap to higher layers). */ +/* Implement the arch_setup target_ops method. */ + +static void +linux_arch_setup (void) +{ + the_low_target.arch_setup (); +} + +/* Call the target arch_setup function on THREAD. */ + +static void +linux_arch_setup_thread (struct thread_info *thread) +{ + struct thread_info *saved_thread; + + saved_thread = current_thread; + current_thread = thread; + + linux_arch_setup (); + + current_thread = saved_thread; +} + +/* Handle a GNU/Linux extended wait response. If we see a clone, + fork, or vfork event, we need to add the new LWP to our list + (and return 0 so as not to report the trap to higher layers). + If we see an exec event, we will modify ORIG_EVENT_LWP to point + to a new LWP representing the new program. */ static int -handle_extended_wait (struct lwp_info *event_lwp, int wstat) +handle_extended_wait (struct lwp_info **orig_event_lwp, int wstat) { + struct lwp_info *event_lwp = *orig_event_lwp; int event = linux_ptrace_get_extended_event (wstat); struct thread_info *event_thr = get_lwp_thread (event_lwp); struct lwp_info *new_lwp; @@ -571,6 +598,50 @@ handle_extended_wait (struct lwp_info *event_lwp, int wstat) /* Report the event. */ return 0; } + else if (event == PTRACE_EVENT_EXEC && report_exec_events) + { + struct process_info *proc; + ptid_t event_ptid; + pid_t event_pid; + + if (debug_threads) + { + debug_printf ("HEW: Got exec event from LWP %ld\n", + lwpid_of (event_thr)); + } + + /* Get the event ptid. */ + event_ptid = ptid_of (event_thr); + event_pid = ptid_get_pid (event_ptid); + + /* Delete the execing process and all its threads. */ + proc = get_thread_process (event_thr); + linux_mourn (proc); + current_thread = NULL; + + /* Create a new process/lwp/thread. */ + proc = linux_add_process (event_pid, 0); + event_lwp = add_lwp (event_ptid); + event_thr = get_lwp_thread (event_lwp); + gdb_assert (current_thread == event_thr); + linux_arch_setup_thread (event_thr); + + /* Set the event status. */ + event_lwp->waitstatus.kind = TARGET_WAITKIND_EXECD; + event_lwp->waitstatus.value.execd_pathname + = xstrdup (linux_proc_pid_to_exec_file (lwpid_of (event_thr))); + + /* Mark the exec status as pending. */ + event_lwp->stopped = 1; + event_lwp->status_pending_p = 1; + event_lwp->status_pending = wstat; + event_thr->last_resume_kind = resume_continue; + event_thr->last_status.kind = TARGET_WAITKIND_IGNORE; + + /* Report the event. */ + *orig_event_lwp = event_lwp; + return 0; + } internal_error (__FILE__, __LINE__, _("unknown ptrace event %d"), event); } @@ -839,14 +910,6 @@ linux_create_inferior (char *program, char **allargs) return pid; } -/* Implement the arch_setup target_ops method. */ - -static void -linux_arch_setup (void) -{ - the_low_target.arch_setup (); -} - /* Attach to an inferior process. Returns 0 on success, ERRNO on error. */ @@ -1639,7 +1702,7 @@ check_zombie_leaders (void) leader_pid, leader_lp!= NULL, num_lwps (leader_pid), linux_proc_pid_is_zombie (leader_pid)); - if (leader_lp != NULL + if (leader_lp != NULL && !leader_lp->stopped /* Check if there are other threads in the group, as we may have raced with the inferior simply exiting. */ && !last_thread_of_process_p (leader_pid) @@ -2098,6 +2161,9 @@ linux_low_ptrace_options (int attached) if (report_vfork_events) options |= (PTRACE_O_TRACEVFORK | PTRACE_O_TRACEVFORKDONE); + if (report_exec_events) + options |= PTRACE_O_TRACEEXEC; + return options; } @@ -2114,6 +2180,38 @@ linux_low_filter_event (int lwpid, int wstat) child = find_lwp_pid (pid_to_ptid (lwpid)); + /* Check for stop events reported by a process we didn't already + know about - anything not already in our LWP list. + + If we're expecting to receive stopped processes after + fork, vfork, and clone events, then we'll just add the + new one to our list and go back to waiting for the event + to be reported - the stopped process might be returned + from waitpid before or after the event is. + + But note the case of a non-leader thread exec'ing after the + leader having exited, and gone from our lists (because + check_zombie_leaders deleted it). The non-leader thread + changes its tid to the tgid. */ + + if (WIFSTOPPED (wstat) && child == NULL && WSTOPSIG (wstat) == SIGTRAP + && linux_ptrace_get_extended_event (wstat) == PTRACE_EVENT_EXEC) + { + ptid_t child_ptid; + + /* A multi-thread exec after we had seen the leader exiting. */ + if (debug_threads) + { + debug_printf ("LLW: Re-adding thread group leader LWP %d" + "after exec.\n", lwpid); + } + + child_ptid = ptid_build (lwpid, lwpid, 0); + child = add_lwp (child_ptid); + child->stopped = 1; + current_thread = child->thread; + } + /* If we didn't find a process, one of two things presumably happened: - A process we started and then detached from has exited. Ignore it. - A process we are controlling has forked and the new child's stop @@ -2171,17 +2269,10 @@ linux_low_filter_event (int lwpid, int wstat) { if (proc->attached) { - struct thread_info *saved_thread; - /* This needs to happen after we have attached to the inferior and it is stopped for the first time, but before we access any inferior registers. */ - saved_thread = current_thread; - current_thread = thread; - - the_low_target.arch_setup (); - - current_thread = saved_thread; + linux_arch_setup_thread (thread); } else { @@ -2210,7 +2301,7 @@ linux_low_filter_event (int lwpid, int wstat) && linux_is_extended_waitstatus (wstat)) { child->stop_pc = get_pc (child); - if (handle_extended_wait (child, wstat)) + if (handle_extended_wait (&child, wstat)) { /* The event has been handled, so just return without reporting it. */ @@ -2419,8 +2510,7 @@ linux_wait_for_event_filtered (ptid_t wait_ptid, ptid_t filter_ptid, - When a non-leader thread execs, that thread just vanishes without reporting an exit (so we'd hang if we waited for it explicitly in that case). The exec event is reported to - the TGID pid (although we don't currently enable exec - events). */ + the TGID pid. */ errno = 0; ret = my_waitpid (-1, wstatp, options | WNOHANG); @@ -5801,6 +5891,14 @@ linux_supports_vfork_events (void) return linux_supports_tracefork (); } +/* Check if exec events are supported. */ + +static int +linux_supports_exec_events (void) +{ + return linux_supports_traceexec (); +} + /* Callback for 'find_inferior'. Set the (possibly changed) ptrace options for the specified lwp. */ @@ -6891,6 +6989,7 @@ static struct target_ops linux_target_ops = { linux_supports_multi_process, linux_supports_fork_events, linux_supports_vfork_events, + linux_supports_exec_events, linux_handle_new_gdb_connection, #ifdef USE_THREAD_DB thread_db_handle_monitor_command, diff --git a/gdb/gdbserver/lynx-low.c b/gdb/gdbserver/lynx-low.c index 1a187c8..b722930 100644 --- a/gdb/gdbserver/lynx-low.c +++ b/gdb/gdbserver/lynx-low.c @@ -765,6 +765,7 @@ static struct target_ops lynx_target_ops = { NULL, /* supports_multi_process */ NULL, /* supports_fork_events */ NULL, /* supports_vfork_events */ + NULL, /* supports_exec_events */ NULL, /* handle_new_gdb_connection */ NULL, /* handle_monitor_command */ }; diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c index 0c4a693..ac86dd5 100644 --- a/gdb/gdbserver/remote-utils.c +++ b/gdb/gdbserver/remote-utils.c @@ -1117,6 +1117,7 @@ prepare_resume_reply (char *buf, ptid_t ptid, case TARGET_WAITKIND_STOPPED: case TARGET_WAITKIND_FORKED: case TARGET_WAITKIND_VFORKED: + case TARGET_WAITKIND_EXECD: { struct thread_info *saved_thread; const char **regp; @@ -1134,6 +1135,25 @@ prepare_resume_reply (char *buf, ptid_t ptid, buf = write_ptid (buf, status->value.related_pid); strcat (buf, ";"); } + else if (status->kind == TARGET_WAITKIND_EXECD && multi_process) + { + enum gdb_signal signal = GDB_SIGNAL_TRAP; + const char *event = "exec"; + char hexified_pathname[PATH_MAX * 2]; + + sprintf (buf, "T%02x%s:", signal, event); + buf += strlen (buf); + + /* Encode pathname to hexified format. */ + bin2hex ((const gdb_byte *) status->value.execd_pathname, + hexified_pathname, + strlen (status->value.execd_pathname)); + + sprintf (buf, "%s;", hexified_pathname); + xfree (status->value.execd_pathname); + status->value.execd_pathname = NULL; + buf += strlen (buf); + } else sprintf (buf, "T%02x", status->value.sig); diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c index c52cf16..9aa8a3f 100644 --- a/gdb/gdbserver/server.c +++ b/gdb/gdbserver/server.c @@ -59,6 +59,7 @@ int run_once; int multi_process; int report_fork_events; int report_vfork_events; +int report_exec_events; int non_stop; int swbreak_feature; int hwbreak_feature; @@ -2111,6 +2112,12 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p) if (target_supports_vfork_events ()) report_vfork_events = 1; } + if (strcmp (p, "exec-events+") == 0) + { + /* GDB supports and wants exec events if possible. */ + if (target_supports_exec_events ()) + report_exec_events = 1; + } else target_process_qsupported (p); @@ -2167,6 +2174,9 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p) if (target_supports_vfork_events ()) strcat (own_buf, ";vfork-events+"); + if (target_supports_exec_events ()) + strcat (own_buf, ";exec-events+"); + if (target_supports_non_stop ()) strcat (own_buf, ";QNonStop+"); @@ -3544,6 +3554,7 @@ captured_main (int argc, char *argv[]) multi_process = 0; report_fork_events = 0; report_vfork_events = 0; + report_exec_events = 0; /* Be sure we're out of tfind mode. */ current_traceframe = -1; cont_thread = null_ptid; diff --git a/gdb/gdbserver/server.h b/gdb/gdbserver/server.h index 6020d72..96ad4fa 100644 --- a/gdb/gdbserver/server.h +++ b/gdb/gdbserver/server.h @@ -86,6 +86,7 @@ extern int run_once; extern int multi_process; extern int report_fork_events; extern int report_vfork_events; +extern int report_exec_events; extern int non_stop; extern int extended_protocol; diff --git a/gdb/gdbserver/target.h b/gdb/gdbserver/target.h index 3e3b80f..aea3d15 100644 --- a/gdb/gdbserver/target.h +++ b/gdb/gdbserver/target.h @@ -290,6 +290,9 @@ struct target_ops /* Returns true if vfork events are supported. */ int (*supports_vfork_events) (void); + /* Returns true if exec events are supported. */ + int (*supports_exec_events) (void); + /* Allows target to re-initialize connection-specific settings. */ void (*handle_new_gdb_connection) (void); @@ -468,6 +471,10 @@ int kill_inferior (int); (the_target->supports_vfork_events ? \ (*the_target->supports_vfork_events) () : 0) +#define target_supports_exec_events() \ + (the_target->supports_exec_events ? \ + (*the_target->supports_exec_events) () : 0) + #define target_handle_new_gdb_connection() \ do \ { \ diff --git a/gdb/gdbserver/win32-low.c b/gdb/gdbserver/win32-low.c index 86386ce..85cc040 100644 --- a/gdb/gdbserver/win32-low.c +++ b/gdb/gdbserver/win32-low.c @@ -1832,6 +1832,7 @@ static struct target_ops win32_target_ops = { NULL, /* supports_multi_process */ NULL, /* supports_fork_events */ NULL, /* supports_vfork_events */ + NULL, /* supports_exec_events */ NULL, /* handle_new_gdb_connection */ NULL, /* handle_monitor_command */ NULL, /* core_of_thread */ diff --git a/gdb/infrun.c b/gdb/infrun.c index e89e02a..84890b4 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -1095,6 +1095,7 @@ follow_exec (ptid_t ptid, char *execd_pathname) struct thread_info *th, *tmp; struct inferior *inf = current_inferior (); int pid = ptid_get_pid (ptid); + ptid_t process_ptid; /* This is an exec event that we actually wish to pay attention to. Refresh our symbol table to the newly exec'd program, remove any @@ -1161,8 +1162,9 @@ follow_exec (ptid_t ptid, char *execd_pathname) update_breakpoints_after_exec (); /* What is this a.out's name? */ + process_ptid = pid_to_ptid (pid); printf_unfiltered (_("%s is executing new program: %s\n"), - target_pid_to_str (inferior_ptid), + target_pid_to_str (process_ptid), execd_pathname); /* We've followed the inferior through an exec. Therefore, the @@ -1191,8 +1193,6 @@ follow_exec (ptid_t ptid, char *execd_pathname) if (follow_exec_mode_string == follow_exec_mode_new) { - struct program_space *pspace; - /* The user wants to keep the old inferior and program spaces around. Create a new fresh one, and switch to it. */ @@ -1201,14 +1201,13 @@ follow_exec (ptid_t ptid, char *execd_pathname) the same ptid, which can confuse find_inferior_ptid. */ exit_inferior_num_silent (current_inferior ()->num); - inf = add_inferior (pid); - pspace = add_program_space (maybe_new_address_space ()); - inf->pspace = pspace; - inf->aspace = pspace->aspace; - add_thread (ptid); + inf = add_inferior_with_spaces (); + inf->pid = pid; + target_follow_exec (inf, execd_pathname); set_current_inferior (inf); - set_current_program_space (pspace); + set_current_program_space (inf->pspace); + add_thread (ptid); } else { diff --git a/gdb/nat/linux-ptrace.c b/gdb/nat/linux-ptrace.c index f097c8a..4222df5 100644 --- a/gdb/nat/linux-ptrace.c +++ b/gdb/nat/linux-ptrace.c @@ -538,6 +538,17 @@ linux_supports_tracefork (void) return ptrace_supports_feature (PTRACE_O_TRACEFORK); } +/* Returns non-zero if PTRACE_EVENT_EXEC is supported by ptrace, + 0 otherwise. Note that if PTRACE_EVENT_FORK is supported so is + PTRACE_EVENT_CLONE, PTRACE_EVENT_FORK and PTRACE_EVENT_VFORK, + since they were all added to the kernel at the same time. */ + +int +linux_supports_traceexec (void) +{ + return ptrace_supports_feature (PTRACE_O_TRACEEXEC); +} + /* Returns non-zero if PTRACE_EVENT_CLONE is supported by ptrace, 0 otherwise. Note that if PTRACE_EVENT_CLONE is supported so is PTRACE_EVENT_FORK, PTRACE_EVENT_EXEC and PTRACE_EVENT_VFORK, diff --git a/gdb/nat/linux-ptrace.h b/gdb/nat/linux-ptrace.h index 8bff908..1be38fe 100644 --- a/gdb/nat/linux-ptrace.h +++ b/gdb/nat/linux-ptrace.h @@ -168,6 +168,7 @@ extern void linux_check_ptrace_features (void); extern void linux_enable_event_reporting (pid_t pid, int attached); extern void linux_disable_event_reporting (pid_t pid); extern int linux_supports_tracefork (void); +extern int linux_supports_traceexec (void); extern int linux_supports_traceclone (void); extern int linux_supports_tracevforkdone (void); extern int linux_supports_tracesysgood (void); diff --git a/gdb/remote.c b/gdb/remote.c index e4d3edf..25def33 100644 --- a/gdb/remote.c +++ b/gdb/remote.c @@ -75,6 +75,14 @@ static char *target_buf; static long target_buf_size; +/* Per-program-space data key. */ +static const struct program_space_data *remote_pspace_data; + +/* The variable registered as the control variable used by the + remote exec-file commands. Used by the set/show machinery + as the location of the remote exec-file value. */ +static char *remote_exec_file_var; + /* The size to align memory write packets, when practical. The protocol does not guarantee any alignment, and gdb will generate short writes and unaligned writes, but even as a best-effort attempt this @@ -619,6 +627,63 @@ get_remote_state (void) return get_remote_state_raw (); } +/* Cleanup routine for the remote module's pspace data. */ + +static void +remote_pspace_data_cleanup (struct program_space *pspace, void *arg) +{ + char *remote_exec_file = arg; + + xfree (remote_exec_file); +} + +/* Fetch the remote exec-file from the current program space. */ + +static const char * +get_remote_exec_file (void) +{ + char *remote_exec_file; + + remote_exec_file = program_space_data (current_program_space, + remote_pspace_data); + if (remote_exec_file == NULL) + return ""; + + return remote_exec_file; +} + +/* Set the remote exec file for the current program space. */ + +static void +set_remote_exec_file_1 (struct program_space *pspace, + char *remote_exec_file) +{ + char *old_file = program_space_data (pspace, remote_pspace_data); + + xfree (old_file); + set_program_space_data (pspace, remote_pspace_data, + xstrdup (remote_exec_file)); +} + +/* The "set/show remote exec-file" set command hook. */ + +static void +set_remote_exec_file (char *ignored, int from_tty, + struct cmd_list_element *c) +{ + gdb_assert (remote_exec_file_var != NULL); + set_remote_exec_file_1 (current_program_space, remote_exec_file_var); +} + +/* The "set/show remote exec-file" show command hook. */ + +static void +show_remote_exec_file (struct ui_file *file, int from_tty, + struct cmd_list_element *cmd, const char *value) +{ + fprintf_filtered (file, "%s\n", remote_exec_file_var); +} + static int compare_pnums (const void *lhs_, const void *rhs_) { @@ -901,10 +966,6 @@ static unsigned int remote_address_size; static int remote_async_terminal_ours_p; -/* The executable file to use for "run" on the remote side. */ - -static char *remote_exec_file = ""; - /* User configurable variables for the number of characters in a memory read/write packet. MIN (rsa->remote_packet_size, @@ -1401,6 +1462,9 @@ enum { /* Support for the Qbtrace-conf:pt:size packet. */ PACKET_Qbtrace_conf_pt_size, + /* Support for exec events. */ + PACKET_exec_event_feature, + PACKET_MAX }; @@ -4279,6 +4343,8 @@ static const struct protocol_feature remote_protocol_features[] = { PACKET_fork_event_feature }, { "vfork-events", PACKET_DISABLE, remote_supported_packet, PACKET_vfork_event_feature }, + { "exec-events", PACKET_DISABLE, remote_supported_packet, + PACKET_exec_event_feature }, { "Qbtrace-conf:pt:size", PACKET_DISABLE, remote_supported_packet, PACKET_Qbtrace_conf_pt_size } }; @@ -4368,6 +4434,9 @@ remote_query_supported (void) if (packet_set_cmd_state (PACKET_vfork_event_feature) != AUTO_BOOLEAN_FALSE) q = remote_query_supported_append (q, "vfork-events+"); + if (packet_set_cmd_state (PACKET_exec_event_feature) + != AUTO_BOOLEAN_FALSE) + q = remote_query_supported_append (q, "exec-events+"); } q = reconcat (q, "qSupported:", q, (char *) NULL); @@ -4779,6 +4848,24 @@ remote_follow_fork (struct target_ops *ops, int follow_child, return 0; } +/* Target follow-exec function for remote targets. Save EXECD_PATHNAME + in the program space of the new inferior. On entry and at return the + current inferior is the exec'ing inferior. INF is the new exec'd + inferior, which may be the same as the exec'ing inferior unless + follow-exec-mode is "new". */ + +static void +remote_follow_exec (struct target_ops *ops, + struct inferior *inf, char *execd_pathname) +{ + /* We know that this is a target file name, so if it has the "target:" + prefix we strip it off before saving it in the program space. */ + if (is_target_filename (execd_pathname)) + execd_pathname += strlen (TARGET_SYSROOT_PREFIX); + + set_remote_exec_file_1 (inf->pspace, execd_pathname); +} + /* Same as remote_detach, but don't send the "D" packet; just disconnect. */ static void @@ -5977,6 +6064,7 @@ remote_parse_stop_reply (char *buf, struct stop_reply *event) struct remote_arch_state *rsa = get_remote_arch_state (); ULONGEST addr; char *p; + int skipregs = 0; event->ptid = null_ptid; event->rs = get_remote_state (); @@ -6089,11 +6177,42 @@ Packet: '%s'\n"), event->ws.kind = TARGET_WAITKIND_VFORK_DONE; p = skip_to_semicolon (p1 + 1); } + else if (strncmp (p, "exec", p1 - p) == 0) + { + ULONGEST ignored; + char pathname[PATH_MAX]; + int pathlen; + + /* Determine the length of the execd pathname. */ + p = unpack_varlen_hex (++p1, &ignored); + pathlen = (p - p1) / 2; + + /* Save the pathname for event reporting and for + the next run command. */ + hex2bin (p1, (gdb_byte *) pathname, pathlen); + pathname[pathlen] = '\0'; + + /* This is freed during event handling. */ + event->ws.value.execd_pathname = xstrdup (pathname); + event->ws.kind = TARGET_WAITKIND_EXECD; + + /* Skip the registers included in this packet, since + they may be for an architecture different from the + one used by the original program. */ + skipregs = 1; + } else { ULONGEST pnum; char *p_temp; + if (skipregs) + { + p = skip_to_semicolon (p1 + 1); + p++; + continue; + } + /* Maybe a real ``P'' register number. */ p_temp = unpack_varlen_hex (p, &pnum); /* If the first invalid character is the colon, we got a @@ -8593,6 +8712,7 @@ extended_remote_run (char *args) { struct remote_state *rs = get_remote_state (); int len; + const char *remote_exec_file = get_remote_exec_file (); /* If the user has disabled vRun support, or we have detected that support is not available, do not try it. */ @@ -8665,6 +8785,7 @@ extended_remote_create_inferior (struct target_ops *ops, int run_worked; char *stop_reply; struct remote_state *rs = get_remote_state (); + const char *remote_exec_file = get_remote_exec_file (); /* If running asynchronously, register the target file descriptor with the event loop. */ @@ -12662,6 +12783,7 @@ Specify the serial device it is connected to (e.g. /dev/ttya)."; extended_remote_ops.to_supports_disable_randomization = extended_remote_supports_disable_randomization; extended_remote_ops.to_follow_fork = remote_follow_fork; + extended_remote_ops.to_follow_exec = remote_follow_exec; extended_remote_ops.to_insert_fork_catchpoint = remote_insert_fork_catchpoint; extended_remote_ops.to_remove_fork_catchpoint @@ -12893,6 +13015,10 @@ _initialize_remote (void) remote_g_packet_data_handle = gdbarch_data_register_pre_init (remote_g_packet_data_init); + remote_pspace_data + = register_program_space_data_with_cleanup (NULL, + remote_pspace_data_cleanup); + /* Initialize the per-target state. At the moment there is only one of these, not one per target. Only one target is active at a time. */ @@ -13272,6 +13398,9 @@ Show the maximum size of the address (in bits) in a memory packet."), NULL, add_packet_config_cmd (&remote_protocol_packets[PACKET_Qbtrace_conf_pt_size], "Qbtrace-conf:pt:size", "btrace-conf-pt-size", 0); + add_packet_config_cmd (&remote_protocol_packets[PACKET_exec_event_feature], + "exec-event-feature", "exec-event-feature", 0); + /* Assert that we've registered "set remote foo-packet" commands for all packet configs. */ { @@ -13340,12 +13469,14 @@ Transfer files to and from the remote target system."), _("Delete a remote file."), &remote_cmdlist); - remote_exec_file = xstrdup (""); add_setshow_string_noescape_cmd ("exec-file", class_files, - &remote_exec_file, _("\ + &remote_exec_file_var, _("\ Set the remote pathname for \"run\""), _("\ -Show the remote pathname for \"run\""), NULL, NULL, NULL, - &remote_set_cmdlist, &remote_show_cmdlist); +Show the remote pathname for \"run\""), NULL, + set_remote_exec_file, + show_remote_exec_file, + &remote_set_cmdlist, + &remote_show_cmdlist); add_setshow_boolean_cmd ("range-stepping", class_run, &use_range_stepping, _("\ diff --git a/gdb/target-debug.h b/gdb/target-debug.h index ddbdfd1..470d6f3 100644 --- a/gdb/target-debug.h +++ b/gdb/target-debug.h @@ -156,6 +156,8 @@ target_debug_do_print (plongest (X)) #define target_debug_print_enum_bptype(X) \ target_debug_do_print (plongest (X)) +#define target_debug_print_struct_inferior_p(X) \ + target_debug_do_print (host_address_to_string (X)) static void target_debug_print_struct_target_waitstatus_p (struct target_waitstatus *status) diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c index 8d51b6c..87197f8 100644 --- a/gdb/target-delegates.c +++ b/gdb/target-delegates.c @@ -1208,6 +1208,32 @@ debug_remove_exec_catchpoint (struct target_ops *self, int arg1) return result; } +static void +delegate_follow_exec (struct target_ops *self, struct inferior *arg1, char *arg2) +{ + self = self->beneath; + self->to_follow_exec (self, arg1, arg2); +} + +static void +tdefault_follow_exec (struct target_ops *self, struct inferior *arg1, char *arg2) +{ +} + +static void +debug_follow_exec (struct target_ops *self, struct inferior *arg1, char *arg2) +{ + fprintf_unfiltered (gdb_stdlog, "-> %s->to_follow_exec (...)\n", debug_target.to_shortname); + debug_target.to_follow_exec (&debug_target, arg1, arg2); + fprintf_unfiltered (gdb_stdlog, "<- %s->to_follow_exec (", debug_target.to_shortname); + target_debug_print_struct_target_ops_p (&debug_target); + fputs_unfiltered (", ", gdb_stdlog); + target_debug_print_struct_inferior_p (arg1); + fputs_unfiltered (", ", gdb_stdlog); + target_debug_print_char_p (arg2); + fputs_unfiltered (")\n", gdb_stdlog); +} + static int delegate_set_syscall_catchpoint (struct target_ops *self, int arg1, int arg2, int arg3, int arg4, int *arg5) { @@ -4032,6 +4058,8 @@ install_delegators (struct target_ops *ops) ops->to_insert_exec_catchpoint = delegate_insert_exec_catchpoint; if (ops->to_remove_exec_catchpoint == NULL) ops->to_remove_exec_catchpoint = delegate_remove_exec_catchpoint; + if (ops->to_follow_exec == NULL) + ops->to_follow_exec = delegate_follow_exec; if (ops->to_set_syscall_catchpoint == NULL) ops->to_set_syscall_catchpoint = delegate_set_syscall_catchpoint; if (ops->to_has_exited == NULL) @@ -4285,6 +4313,7 @@ install_dummy_methods (struct target_ops *ops) ops->to_follow_fork = default_follow_fork; ops->to_insert_exec_catchpoint = tdefault_insert_exec_catchpoint; ops->to_remove_exec_catchpoint = tdefault_remove_exec_catchpoint; + ops->to_follow_exec = tdefault_follow_exec; ops->to_set_syscall_catchpoint = tdefault_set_syscall_catchpoint; ops->to_has_exited = tdefault_has_exited; ops->to_mourn_inferior = default_mourn_inferior; @@ -4436,6 +4465,7 @@ init_debug_target (struct target_ops *ops) ops->to_follow_fork = debug_follow_fork; ops->to_insert_exec_catchpoint = debug_insert_exec_catchpoint; ops->to_remove_exec_catchpoint = debug_remove_exec_catchpoint; + ops->to_follow_exec = debug_follow_exec; ops->to_set_syscall_catchpoint = debug_set_syscall_catchpoint; ops->to_has_exited = debug_has_exited; ops->to_mourn_inferior = debug_mourn_inferior; diff --git a/gdb/target.c b/gdb/target.c index 3da984e..f425fbc 100644 --- a/gdb/target.c +++ b/gdb/target.c @@ -2304,6 +2304,14 @@ target_follow_fork (int follow_child, int detach_fork) follow_child, detach_fork); } +/* Target wrapper for follow exec hook. */ + +void +target_follow_exec (struct inferior *inf, char *execd_pathname) +{ + current_target.to_follow_exec (¤t_target, inf, execd_pathname); +} + static void default_mourn_inferior (struct target_ops *self) { diff --git a/gdb/target.h b/gdb/target.h index da18f99..5f05b56 100644 --- a/gdb/target.h +++ b/gdb/target.h @@ -596,6 +596,8 @@ struct target_ops TARGET_DEFAULT_RETURN (1); int (*to_remove_exec_catchpoint) (struct target_ops *, int) TARGET_DEFAULT_RETURN (1); + void (*to_follow_exec) (struct target_ops *, struct inferior *, char *) + TARGET_DEFAULT_IGNORE (); int (*to_set_syscall_catchpoint) (struct target_ops *, int, int, int, int, int *) TARGET_DEFAULT_RETURN (1); @@ -1577,6 +1579,11 @@ extern void target_load (const char *arg, int from_tty); int target_follow_fork (int follow_child, int detach_fork); +/* Handle the target-specific bookkeeping required when the inferior + makes an exec call. INF is the exec'd inferior. */ + +void target_follow_exec (struct inferior *inf, char *execd_pathname); + /* On some targets, we can catch an inferior exec event when it occurs. These functions insert/remove an already-created