Patchwork [2/3] gdbserver: Use std::list for all_processes

login
register
mail settings
Submitter Simon Marchi
Date Oct. 9, 2017, 2:30 p.m.
Message ID <20171009143036.10215-3-simon.marchi@polymtl.ca>
Download mbox | patch
Permalink /patch/23409/
State New
Headers show

Comments

Simon Marchi - Oct. 9, 2017, 2:30 p.m.
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 <list>.
	(struct process_info) <entry>: Remove field.
	<pid>: New field.
	(pid_of): Change macro to function.
	(ptid_of, lwpid_of): Remove macro.
	(all_processes): Change type to std::list<process_info *>.
	(ALL_PROCESSES): Remove macro.
	(for_each_process, find_process): New function.
	* inferiors.c (all_processes): Change type to
	std::list<process_info *>.
	(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(-)

Patch

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<process_info *> 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 <list>
 
 /* 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<process_info *> 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 <typename Func>
+static void
+for_each_process (Func func)
+{
+  std::list<process_info *>::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 <typename Func>
+static process_info *
+find_process (Func func)
+{
+  std::list<process_info *>::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.  */