From patchwork Mon Oct 9 14:30:35 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Marchi X-Patchwork-Id: 23409 Received: (qmail 63272 invoked by alias); 9 Oct 2017 14:30:53 -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 63032 invoked by uid 89); 9 Oct 2017 14:30:50 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.2 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_SOFTFAIL autolearn=ham version=3.3.2 spammy=events, Killing, 8737, perspective X-HELO: barracuda.ebox.ca Received: from barracuda.ebox.ca (HELO barracuda.ebox.ca) (96.127.255.19) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 09 Oct 2017 14:30:46 +0000 X-ASG-Debug-ID: 1507559440-0c856e65d53651f20001-fS2M51 Received: from smtp.electronicbox.net (smtp.electronicbox.net [96.127.255.82]) by barracuda.ebox.ca with ESMTP id h3sVghVjwkvsgLjq (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 09 Oct 2017 10:30:40 -0400 (EDT) X-Barracuda-Envelope-From: simon.marchi@polymtl.ca X-Barracuda-RBL-Trusted-Forwarder: 96.127.255.82 Received: from simark.lan (cable-192.222.251.162.electronicbox.net [192.222.251.162]) by smtp.electronicbox.net (Postfix) with ESMTP id 7FB63441B21; Mon, 9 Oct 2017 10:30:40 -0400 (EDT) From: Simon Marchi X-Barracuda-Effective-Source-IP: cable-192.222.251.162.electronicbox.net[192.222.251.162] X-Barracuda-Apparent-Source-IP: 192.222.251.162 X-Barracuda-RBL-IP: 192.222.251.162 To: gdb-patches@sourceware.org Cc: Simon Marchi Subject: [PATCH 2/3] gdbserver: Use std::list for all_processes Date: Mon, 9 Oct 2017 10:30:35 -0400 X-ASG-Orig-Subj: [PATCH 2/3] gdbserver: Use std::list for all_processes Message-Id: <20171009143036.10215-3-simon.marchi@polymtl.ca> In-Reply-To: <20171009143036.10215-1-simon.marchi@polymtl.ca> References: <20171009143036.10215-1-simon.marchi@polymtl.ca> X-Barracuda-Connect: smtp.electronicbox.net[96.127.255.82] X-Barracuda-Start-Time: 1507559440 X-Barracuda-Encrypted: DHE-RSA-AES256-SHA X-Barracuda-URL: https://96.127.255.19:443/cgi-mod/mark.cgi X-Barracuda-Scan-Msg-Size: 21868 X-Barracuda-BRTS-Status: 1 X-Barracuda-Spam-Score: 0.00 X-Barracuda-Spam-Status: No, SCORE=0.00 using global scores of TAG_LEVEL=1000.0 QUARANTINE_LEVEL=1000.0 KILL_LEVEL=8.0 tests= X-Barracuda-Spam-Report: Code version 3.2, rules version 3.2.3.43737 Rule breakdown below pts rule name description ---- ---------------------- -------------------------------------------------- X-IsSubscribed: yes Remove the usage of inferior_list for the all_processes list in gdbserver, replace it with an std::list. The entry field in process_info is removed, and replaced by a simple pid field. The pid_of macro, used for both processes and threads, is replaced with separate functions. For completeness, I changed ptid_of and lwpid_of to functions as well. gdb/gdbserver/ChangeLog: * gdbthread.h (ptid_of, pid_of, lwpid_of): New functions. * inferiors.h: Include . (struct process_info) : Remove field. : New field. (pid_of): Change macro to function. (ptid_of, lwpid_of): Remove macro. (all_processes): Change type to std::list. (ALL_PROCESSES): Remove macro. (for_each_process, find_process): New function. * inferiors.c (all_processes): Change type to std::list. (find_thread_process): Adjust. (add_process): Likewise. (remove_process): Likewise. (find_process_pid): Likewise. (get_first_process): Likewise. (started_inferior_callback): Remove. (have_started_inferiors_p): Adjust. (attached_inferior_callback): Remove. (have_attached_inferiors_p): Adjust. * linux-low.c (check_zombie_leaders): Likewise. * linux-x86-low.c (x86_arch_setup_process_callback): Remove. (x86_linux_update_xmltarget): Adjust. * server.c (handle_query): Likewise. (gdb_reattached_process): Remove. (handle_status): Adjust. (kill_inferior_callback): Likewise. (detach_or_kill_inferior): Remove. (print_started_pid): Likewise. (print_attached_pid): Likewise. (detach_or_kill_for_exit): Update. (process_serial_event): Likewise. * linux-arm-low.c (arm_new_fork): Likewise. --- gdb/gdbserver/gdbthread.h | 24 +++++++++ gdb/gdbserver/inferiors.c | 57 ++++++++-------------- gdb/gdbserver/inferiors.h | 60 +++++++++++++++++++---- gdb/gdbserver/linux-arm-low.c | 2 +- gdb/gdbserver/linux-low.c | 111 ++++++++++++++++++++---------------------- gdb/gdbserver/linux-x86-low.c | 23 +++------ gdb/gdbserver/server.c | 102 ++++++++++++-------------------------- 7 files changed, 189 insertions(+), 190 deletions(-) diff --git a/gdb/gdbserver/gdbthread.h b/gdb/gdbserver/gdbthread.h index a864f95884..93688a36e4 100644 --- a/gdb/gdbserver/gdbthread.h +++ b/gdb/gdbserver/gdbthread.h @@ -88,6 +88,30 @@ struct thread_info *find_any_thread_of_pid (int pid); /* Get current thread ID (Linux task ID). */ #define current_ptid (current_thread->entry.id) +/* Get the ptid of THREAD. */ + +static inline ptid_t +ptid_of (const thread_info *thread) +{ + return thread->entry.id; +} + +/* Get the pid of THREAD. */ + +static inline int +pid_of (const thread_info *thread) +{ + return thread->entry.id.pid (); +} + +/* Get the lwp of THREAD. */ + +static inline long +lwpid_of (const thread_info *thread) +{ + return thread->entry.id.lwp (); +} + /* Create a cleanup to restore current_thread. */ struct cleanup *make_cleanup_restore_current_thread (void); diff --git a/gdb/gdbserver/inferiors.c b/gdb/gdbserver/inferiors.c index 154c167f4c..13ee8c97eb 100644 --- a/gdb/gdbserver/inferiors.c +++ b/gdb/gdbserver/inferiors.c @@ -22,7 +22,7 @@ #include "gdbthread.h" #include "dll.h" -struct inferior_list all_processes; +std::list all_processes; struct inferior_list all_threads; struct thread_info *current_thread; @@ -144,7 +144,7 @@ find_thread_ptid (ptid_t ptid) static struct thread_info * find_thread_process (const struct process_info *const process) { - return find_any_thread_of_pid (process->entry.id.pid ()); + return find_any_thread_of_pid (process->pid); } /* Helper for find_any_thread_of_pid. Returns true if a thread @@ -336,10 +336,10 @@ add_process (int pid, int attached) { struct process_info *process = XCNEW (struct process_info); - process->entry.id = pid_to_ptid (pid); + process->pid = pid; process->attached = attached; - add_inferior_to_list (&all_processes, &process->entry); + all_processes.push_back (process); return process; } @@ -354,35 +354,28 @@ remove_process (struct process_info *process) clear_symbol_cache (&process->symbol_cache); free_all_breakpoints (process); gdb_assert (find_thread_process (process) == NULL); - remove_inferior (&all_processes, &process->entry); + all_processes.remove (process); VEC_free (int, process->syscalls_to_catch); free (process); } -struct process_info * +process_info * find_process_pid (int pid) { - return (struct process_info *) - find_inferior_id (&all_processes, pid_to_ptid (pid)); + return find_process ([&] (process_info *process) { + return process->pid == pid; + }); } -/* Wrapper around get_first_inferior to return a struct process_info *. */ +/* Get the first process in the process list, or NULL if the list is empty. */ -struct process_info * +process_info * get_first_process (void) { - return (struct process_info *) get_first_inferior (&all_processes); -} - -/* Return non-zero if INF, a struct process_info, was started by us, - i.e. not attached to. */ - -static int -started_inferior_callback (struct inferior_list_entry *entry, void *args) -{ - struct process_info *process = (struct process_info *) entry; - - return ! process->attached; + if (!all_processes.empty ()) + return all_processes.front (); + else + return NULL; } /* Return non-zero if there are any inferiors that we have created @@ -391,18 +384,9 @@ started_inferior_callback (struct inferior_list_entry *entry, void *args) int have_started_inferiors_p (void) { - return (find_inferior (&all_processes, started_inferior_callback, NULL) - != NULL); -} - -/* Return non-zero if INF, a struct process_info, was attached to. */ - -static int -attached_inferior_callback (struct inferior_list_entry *entry, void *args) -{ - struct process_info *process = (struct process_info *) entry; - - return process->attached; + return find_process ([] (process_info *process) { + return !process->attached; + }) != NULL; } /* Return non-zero if there are any inferiors that we have attached to. */ @@ -410,8 +394,9 @@ attached_inferior_callback (struct inferior_list_entry *entry, void *args) int have_attached_inferiors_p (void) { - return (find_inferior (&all_processes, attached_inferior_callback, NULL) - != NULL); + return find_process ([] (process_info *process) { + return process->attached; + }) != NULL; } struct process_info * diff --git a/gdb/gdbserver/inferiors.h b/gdb/gdbserver/inferiors.h index aef8433980..674b8c6dcb 100644 --- a/gdb/gdbserver/inferiors.h +++ b/gdb/gdbserver/inferiors.h @@ -20,6 +20,7 @@ #define INFERIORS_H #include "gdb_vecs.h" +#include /* Generic information for tracking a list of ``inferiors'' - threads, processes, etc. */ @@ -45,9 +46,8 @@ struct process_info_private; struct process_info { - /* This must appear first. - The list iterator functions assume it. */ - struct inferior_list_entry entry; + /* This process' pid. */ + int pid; /* Nonzero if this child process was attached rather than spawned. */ @@ -79,9 +79,13 @@ struct process_info struct process_info_private *priv; }; -#define ptid_of(inf) ((inf)->entry.id) -#define pid_of(inf) ptid_get_pid ((inf)->entry.id) -#define lwpid_of(inf) ptid_get_lwp ((inf)->entry.id) +/* Get the pid of PROC. */ + +static inline int +pid_of (const process_info *proc) +{ + return proc->pid; +} /* Return a pointer to the process that corresponds to the current thread (current_thread). It is an error to call this if there is @@ -90,7 +94,7 @@ struct process_info struct process_info *current_process (void); struct process_info *get_thread_process (const struct thread_info *); -extern struct inferior_list all_processes; +extern std::list all_processes; void add_inferior_to_list (struct inferior_list *list, struct inferior_list_entry *new_inferior); @@ -124,9 +128,45 @@ int one_inferior_p (struct inferior_list *list); #define ALL_INFERIORS(list, cur, tmp) \ ALL_INFERIORS_TYPE (struct inferior_list_entry, list, cur, tmp) -/* Iterate over all processes, open loop style. */ -#define ALL_PROCESSES(cur, tmp) \ - ALL_INFERIORS_TYPE (struct process_info, &all_processes, cur, tmp) +/* Invoke FUNC for each process. */ + +template +static void +for_each_process (Func func) +{ + std::list::iterator next, cur = all_processes.begin (); + + while (cur != all_processes.end ()) + { + next = cur; + next++; + func (*cur); + cur = next; + } +} + +/* Find the first process for which FUNC returns true. Return NULL if no + process satisfying FUNC is found. */ + +template +static process_info * +find_process (Func func) +{ + std::list::iterator next, cur = all_processes.begin (); + + while (cur != all_processes.end ()) + { + next = cur; + next++; + + if (func (*cur)) + return *cur; + + cur = next; + } + + return NULL; +} extern struct thread_info *current_thread; void remove_inferior (struct inferior_list *list, diff --git a/gdb/gdbserver/linux-arm-low.c b/gdb/gdbserver/linux-arm-low.c index 5a3f465dee..cb59c2a130 100644 --- a/gdb/gdbserver/linux-arm-low.c +++ b/gdb/gdbserver/linux-arm-low.c @@ -691,7 +691,7 @@ arm_new_fork (struct process_info *parent, struct process_info *child) /* Mark all the hardware breakpoints and watchpoints as changed to make sure that the registers will be updated. */ - child_lwp = find_lwp_pid (ptid_of (child)); + child_lwp = find_lwp_pid (ptid_t (child->pid)); child_lwp_info = child_lwp->arch_private; for (i = 0; i < MAX_BPTS; i++) child_lwp_info->bpts_changed[i] = 1; diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c index 54005c1f1d..dc0ad85734 100644 --- a/gdb/gdbserver/linux-low.c +++ b/gdb/gdbserver/linux-low.c @@ -1936,63 +1936,60 @@ iterate_over_lwps (ptid_t filter, static void check_zombie_leaders (void) { - struct process_info *proc, *tmp; - - ALL_PROCESSES (proc, tmp) - { - pid_t leader_pid = pid_of (proc); - struct lwp_info *leader_lp; - - leader_lp = find_lwp_pid (pid_to_ptid (leader_pid)); - - if (debug_threads) - debug_printf ("leader_pid=%d, leader_lp!=NULL=%d, " - "num_lwps=%d, zombie=%d\n", - leader_pid, leader_lp!= NULL, num_lwps (leader_pid), - linux_proc_pid_is_zombie (leader_pid)); - - 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) - && linux_proc_pid_is_zombie (leader_pid)) - { - /* A leader zombie can mean one of two things: - - - It exited, and there's an exit status pending - available, or only the leader exited (not the whole - program). In the latter case, we can't waitpid the - leader's exit status until all other threads are gone. - - - There are 3 or more threads in the group, and a thread - other than the leader exec'd. On an exec, the Linux - kernel destroys all other threads (except the execing - one) in the thread group, and resets the execing thread's - tid to the tgid. No exit notification is sent for the - execing thread -- from the ptracer's perspective, it - appears as though the execing thread just vanishes. - Until we reap all other threads except the leader and the - execing thread, the leader will be zombie, and the - execing thread will be in `D (disc sleep)'. As soon as - all other threads are reaped, the execing thread changes - it's tid to the tgid, and the previous (zombie) leader - vanishes, giving place to the "new" leader. We could try - distinguishing the exit and exec cases, by waiting once - more, and seeing if something comes out, but it doesn't - sound useful. The previous leader _does_ go away, and - we'll re-add the new one once we see the exec event - (which is just the same as what would happen if the - previous leader did exit voluntarily before some other - thread execs). */ - - if (debug_threads) - debug_printf ("CZL: Thread group leader %d zombie " - "(it exited, or another thread execd).\n", - leader_pid); - - delete_lwp (leader_lp); - } - } + for_each_process ([] (process_info *proc) { + pid_t leader_pid = pid_of (proc); + struct lwp_info *leader_lp; + + leader_lp = find_lwp_pid (pid_to_ptid (leader_pid)); + + if (debug_threads) + debug_printf ("leader_pid=%d, leader_lp!=NULL=%d, " + "num_lwps=%d, zombie=%d\n", + leader_pid, leader_lp!= NULL, num_lwps (leader_pid), + linux_proc_pid_is_zombie (leader_pid)); + + 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) + && linux_proc_pid_is_zombie (leader_pid)) + { + /* A leader zombie can mean one of two things: + + - It exited, and there's an exit status pending + available, or only the leader exited (not the whole + program). In the latter case, we can't waitpid the + leader's exit status until all other threads are gone. + + - There are 3 or more threads in the group, and a thread + other than the leader exec'd. On an exec, the Linux + kernel destroys all other threads (except the execing + one) in the thread group, and resets the execing thread's + tid to the tgid. No exit notification is sent for the + execing thread -- from the ptracer's perspective, it + appears as though the execing thread just vanishes. + Until we reap all other threads except the leader and the + execing thread, the leader will be zombie, and the + execing thread will be in `D (disc sleep)'. As soon as + all other threads are reaped, the execing thread changes + it's tid to the tgid, and the previous (zombie) leader + vanishes, giving place to the "new" leader. We could try + distinguishing the exit and exec cases, by waiting once + more, and seeing if something comes out, but it doesn't + sound useful. The previous leader _does_ go away, and + we'll re-add the new one once we see the exec event + (which is just the same as what would happen if the + previous leader did exit voluntarily before some other + thread execs). */ + + if (debug_threads) + debug_printf ("CZL: Thread group leader %d zombie " + "(it exited, or another thread execd).\n", + leader_pid); + + delete_lwp (leader_lp); + } + }); } /* Callback for `find_inferior'. Returns the first LWP that is not diff --git a/gdb/gdbserver/linux-x86-low.c b/gdb/gdbserver/linux-x86-low.c index 49f6b2d489..0e5c8d48b7 100644 --- a/gdb/gdbserver/linux-x86-low.c +++ b/gdb/gdbserver/linux-x86-low.c @@ -846,20 +846,6 @@ x86_linux_read_description (void) gdb_assert_not_reached ("failed to return tdesc"); } -/* Callback for for_each_inferior. Calls the arch_setup routine for - each process. */ - -static void -x86_arch_setup_process_callback (struct inferior_list_entry *entry) -{ - int pid = ptid_get_pid (entry->id); - - /* Look up any thread of this processes. */ - current_thread = find_any_thread_of_pid (pid); - - the_low_target.arch_setup (); -} - /* Update all the target description of all processes; a new GDB connected, and it may or not support xml target descriptions. */ @@ -873,7 +859,14 @@ x86_linux_update_xmltarget (void) release the current regcache objects. */ regcache_release (); - for_each_inferior (&all_processes, x86_arch_setup_process_callback); + for_each_process ([] (process_info *proc) { + int pid = proc->pid; + + /* Look up any thread of this process. */ + current_thread = find_any_thread_of_pid (pid); + + the_low_target.arch_setup (); + }); current_thread = saved_thread; } diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c index 3d2c2ff5a0..14811c0390 100644 --- a/gdb/gdbserver/server.c +++ b/gdb/gdbserver/server.c @@ -2615,8 +2615,7 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p) if (own_buf[sizeof ("qAttached") - 1]) { int pid = strtoul (own_buf + sizeof ("qAttached:") - 1, NULL, 16); - process = (struct process_info *) - find_inferior_id (&all_processes, pid_to_ptid (pid)); + process = find_process_pid (pid); } else { @@ -3305,16 +3304,6 @@ gdb_wants_all_threads_stopped (void) for_each_inferior (&all_threads, gdb_wants_thread_stopped); } -/* Clear the gdb_detached flag of every process. */ - -static void -gdb_reattached_process (struct inferior_list_entry *entry) -{ - struct process_info *process = (struct process_info *) entry; - - process->gdb_detached = 0; -} - /* Callback for for_each_inferior. Clear the thread's pending status flag. */ @@ -3363,7 +3352,9 @@ static void handle_status (char *own_buf) { /* GDB is connected, don't forward events to the target anymore. */ - for_each_inferior (&all_processes, gdb_reattached_process); + for_each_process ([] (process_info *process) { + process->gdb_detached = 0; + }); /* In non-stop mode, we must send a stop reply for each stopped thread. In all-stop mode, just send one for the first stopped @@ -3517,64 +3508,14 @@ gdbserver_show_disableable (FILE *stream) } static void -kill_inferior_callback (struct inferior_list_entry *entry) +kill_inferior_callback (process_info *process) { - struct process_info *process = (struct process_info *) entry; - int pid = ptid_get_pid (process->entry.id); + int pid = process->pid; kill_inferior (pid); discard_queued_stop_replies (pid_to_ptid (pid)); } -/* Callback for for_each_inferior to detach or kill the inferior, - depending on whether we attached to it or not. - We inform the user whether we're detaching or killing the process - as this is only called when gdbserver is about to exit. */ - -static void -detach_or_kill_inferior_callback (struct inferior_list_entry *entry) -{ - struct process_info *process = (struct process_info *) entry; - int pid = ptid_get_pid (process->entry.id); - - if (process->attached) - detach_inferior (pid); - else - kill_inferior (pid); - - discard_queued_stop_replies (pid_to_ptid (pid)); -} - -/* for_each_inferior callback for detach_or_kill_for_exit to print - the pids of started inferiors. */ - -static void -print_started_pid (struct inferior_list_entry *entry) -{ - struct process_info *process = (struct process_info *) entry; - - if (! process->attached) - { - int pid = ptid_get_pid (process->entry.id); - fprintf (stderr, " %d", pid); - } -} - -/* for_each_inferior callback for detach_or_kill_for_exit to print - the pids of attached inferiors. */ - -static void -print_attached_pid (struct inferior_list_entry *entry) -{ - struct process_info *process = (struct process_info *) entry; - - if (process->attached) - { - int pid = ptid_get_pid (process->entry.id); - fprintf (stderr, " %d", pid); - } -} - /* Call this when exiting gdbserver with possible inferiors that need to be killed or detached from. */ @@ -3589,19 +3530,37 @@ detach_or_kill_for_exit (void) if (have_started_inferiors_p ()) { fprintf (stderr, "Killing process(es):"); - for_each_inferior (&all_processes, print_started_pid); + + for_each_process ([] (process_info *process) { + if (!process->attached) + fprintf (stderr, " %d", process->pid); + }); + fprintf (stderr, "\n"); } if (have_attached_inferiors_p ()) { fprintf (stderr, "Detaching process(es):"); - for_each_inferior (&all_processes, print_attached_pid); + + for_each_process ([] (process_info *process) { + if (process->attached) + fprintf (stderr, " %d", process->pid); + }); + fprintf (stderr, "\n"); } /* Now we can kill or detach the inferiors. */ + for_each_process ([] (process_info *process) { + int pid = process->pid; + + if (process->attached) + detach_inferior (pid); + else + kill_inferior (pid); - for_each_inferior (&all_processes, detach_or_kill_inferior_callback); + discard_queued_stop_replies (pid_to_ptid (pid)); + }); } /* Value that will be passed to exit(3) when gdbserver exits. */ @@ -4332,7 +4291,8 @@ process_serial_event (void) return 0; fprintf (stderr, "Killing all inferiors\n"); - for_each_inferior (&all_processes, kill_inferior_callback); + + for_each_process (kill_inferior_callback); /* When using the extended protocol, we wait with no program running. The traditional protocol will exit instead. */ @@ -4370,8 +4330,8 @@ process_serial_event (void) if (extended_protocol) { if (target_running ()) - for_each_inferior (&all_processes, - kill_inferior_callback); + for_each_process (kill_inferior_callback); + fprintf (stderr, "GDBserver restarting\n"); /* Wait till we are at 1st instruction in prog. */