This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [gdbserver] Tweak a target resume interface a bit.


On Wednesday 11 March 2009 01:15:19, Pedro Alves wrote:
> When adding non-stop mode to gdbserver, I found out that a new
> "stop thread" resumption kind (for vCont;t) conflicts a bit with
> the "leave_stopped" field in struct thread_resume.  I also noticed
> the global "resume_ptr" in linux-low.c that we could do without.  We
> can eliminate both of these if we adjust the target_ops->resume interface
> to take the length of the thread_resume array that is passed, and, if
> we use "process->resumption == NULL" instead
> of "process->resumption->leave_stopped" to mean "leave thread stopped".
> In non-stop mode "process->resumption == NULL" naturally means "leave
> thread doing whatever it is doing now, either stopped or running".
> 
> Tested against x86_64-linux gdbserver and i686-cygwin gdbserver.
> 
> OK to apply?
> 

Here's the updated patch needed due to the s/process/lwp/g changes.

Note that this patch applies on top of:

 http://sourceware.org/ml/gdb-patches/2009-03/msg00573.html

-- 
Pedro Alves

2009-03-25  Pedro Alves  <pedro@codesourcery.com>

	* target.h (struct thread_resume): Delete leave_stopped member.
	(struct target_ops): Add a `n' argument to the `resume' callback.
	* server.c (start_inferior): Adjust.
	(handle_v_cont, myresume): Adjust.
	* linux-low.c (check_removed_breakpoint): Adjust to resume
	interface change, and to removed leave_stopped field.
	(resume_ptr): Delete.
	(struct thread_resume_array): New.
	(linux_set_resume_request): Add new `arg' parameter.  Adjust to
	resume interface change.
	(linux_continue_one_thread, linux_queue_one_thread)
	(resume_status_pending_p): Check if the resume field is NULL
	instead of checking the leave_stopped member.
	(linux_resume): Adjust to the target resume interface change.
	* spu-low.c (spu_resume): Adjust to the target resume interface
	change.
	* win32-low.c (win32_detach, win32_resume): Ditto.

---
 gdb/gdbserver/linux-low.c |   46 ++++++++++++++++++++++++++++------------------
 gdb/gdbserver/server.c    |   42 +++++++++++++++++++-----------------------
 gdb/gdbserver/spu-low.c   |   17 ++++++++++-------
 gdb/gdbserver/target.h    |   13 +++++--------
 gdb/gdbserver/win32-low.c |   11 +++++------
 5 files changed, 67 insertions(+), 62 deletions(-)

Index: src/gdb/gdbserver/target.h
===================================================================
--- src.orig/gdb/gdbserver/target.h	2009-03-23 00:23:21.000000000 +0000
+++ src/gdb/gdbserver/target.h	2009-03-23 00:24:03.000000000 +0000
@@ -22,18 +22,15 @@
 #ifndef TARGET_H
 #define TARGET_H
 
-/* This structure describes how to resume a particular thread (or
-   all threads) based on the client's request.  If thread is -1, then
-   this entry applies to all threads.  These are generally passed around
-   as an array, and terminated by a thread == -1 entry.  */
+/* This structure describes how to resume a particular thread (or all
+   threads) based on the client's request.  If thread is -1, then this
+   entry applies to all threads.  These are passed around as an
+   array.  */
 
 struct thread_resume
 {
   unsigned long thread;
 
-  /* If non-zero, leave this thread stopped.  */
-  int leave_stopped;
-
   /* If non-zero, we want to single-step.  */
   int step;
 
@@ -83,7 +80,7 @@ struct target_ops
 
   /* Resume the inferior process.  */
 
-  void (*resume) (struct thread_resume *resume_info);
+  void (*resume) (struct thread_resume *resume_info, size_t n);
 
   /* Wait for the inferior process to change state.
 
Index: src/gdb/gdbserver/server.c
===================================================================
--- src.orig/gdb/gdbserver/server.c	2009-03-23 00:23:21.000000000 +0000
+++ src/gdb/gdbserver/server.c	2009-03-23 00:24:03.000000000 +0000
@@ -146,7 +146,6 @@ start_inferior (char **argv, char *statu
       resume_info.thread = -1;
       resume_info.step = 0;
       resume_info.sig = 0;
-      resume_info.leave_stopped = 0;
 
       sig = mywait (statusptr, 0);
       if (*statusptr != 'T')
@@ -154,7 +153,7 @@ start_inferior (char **argv, char *statu
 
       do
 	{
-	  (*the_target->resume) (&resume_info);
+	  (*the_target->resume) (&resume_info, 1);
 
 	  sig = mywait (statusptr, 0);
 	  if (*statusptr != 'T')
@@ -1067,26 +1066,16 @@ handle_v_cont (char *own_buf, char *stat
       p++;
       p = strchr (p, ';');
     }
-  /* Allocate room for one extra action, for the default remain-stopped
-     behavior; if no default action is in the list, we'll need the extra
-     slot.  */
-  resume_info = malloc ((n + 1) * sizeof (resume_info[0]));
+
+  resume_info = malloc (n * sizeof (resume_info[0]));
   if (resume_info == NULL)
     goto err;
 
-  default_action.thread = -1;
-  default_action.leave_stopped = 1;
-  default_action.step = 0;
-  default_action.sig = 0;
-
   p = &own_buf[5];
-  i = 0;
   while (*p)
     {
       p++;
 
-      resume_info[i].leave_stopped = 0;
-
       if (p[0] == 's' || p[0] == 'S')
 	resume_info[i].step = 1;
       else if (p[0] == 'c' || p[0] == 'C')
@@ -1141,7 +1130,8 @@ handle_v_cont (char *own_buf, char *stat
 	}
     }
 
-  resume_info[i] = default_action;
+  if (i < n)
+    resume_info[i] = default_action;
 
   /* Still used in occasional places in the backend.  */
   if (n == 1 && resume_info[0].thread != -1)
@@ -1151,7 +1141,7 @@ handle_v_cont (char *own_buf, char *stat
   set_desired_inferior (0);
 
   enable_async_io ();
-  (*the_target->resume) (resume_info);
+  (*the_target->resume) (resume_info, n);
 
   free (resume_info);
 
@@ -1333,25 +1323,31 @@ myresume (char *own_buf, int step, int *
   struct thread_resume resume_info[2];
   int n = 0;
   int sig = *signalp;
+  int valid_cont_thread;
 
   set_desired_inferior (0);
 
-  if (step || sig || (cont_thread != 0 && cont_thread != -1))
+  valid_cont_thread = (cont_thread != 0 && cont_thread != -1);
+
+  if (step || sig || valid_cont_thread)
     {
       resume_info[0].thread
 	= ((struct inferior_list_entry *) current_inferior)->id;
       resume_info[0].step = step;
       resume_info[0].sig = sig;
-      resume_info[0].leave_stopped = 0;
       n++;
     }
-  resume_info[n].thread = -1;
-  resume_info[n].step = 0;
-  resume_info[n].sig = 0;
-  resume_info[n].leave_stopped = (cont_thread != 0 && cont_thread != -1);
+
+  if (!valid_cont_thread)
+    {
+      resume_info[n].thread = -1;
+      resume_info[n].step = 0;
+      resume_info[n].sig = 0;
+      n++;
+    }
 
   enable_async_io ();
-  (*the_target->resume) (resume_info);
+  (*the_target->resume) (resume_info, n);
   *signalp = mywait (statusp, 1);
   prepare_resume_reply (own_buf, *statusp, *signalp);
   disable_async_io ();
Index: src/gdb/gdbserver/linux-low.c
===================================================================
--- src.orig/gdb/gdbserver/linux-low.c	2009-03-23 00:24:03.000000000 +0000
+++ src/gdb/gdbserver/linux-low.c	2009-03-23 00:24:03.000000000 +0000
@@ -116,7 +116,7 @@ static int new_inferior;
 
 static void linux_resume_one_lwp (struct inferior_list_entry *entry,
 				  int step, int signal, siginfo_t *info);
-static void linux_resume (struct thread_resume *resume_info);
+static void linux_resume (struct thread_resume *resume_info, size_t n);
 static void stop_all_lwps (void);
 static int linux_wait_for_event (struct thread_info *child);
 static int check_removed_breakpoint (struct lwp_info *event_child);
@@ -952,8 +952,8 @@ retry:
 	{
 	  struct thread_resume resume_info;
 	  resume_info.thread = -1;
-	  resume_info.step = resume_info.sig = resume_info.leave_stopped = 0;
-	  linux_resume (&resume_info);
+	  resume_info.step = resume_info.sig = 0;
+	  linux_resume (&resume_info, 1);
 	}
     }
 
@@ -1247,7 +1247,11 @@ linux_resume_one_lwp (struct inferior_li
     }
 }
 
-static struct thread_resume *resume_ptr;
+struct thread_resume_array
+{
+  struct thread_resume *resume;
+  size_t n;
+};
 
 /* This function is called once per thread.  We look up the thread
    in RESUME_PTR, and mark the thread with a pointer to the appropriate
@@ -1256,21 +1260,29 @@ static struct thread_resume *resume_ptr;
    This algorithm is O(threads * resume elements), but resume elements
    is small (and will remain small at least until GDB supports thread
    suspension).  */
-static void
-linux_set_resume_request (struct inferior_list_entry *entry)
+static int
+linux_set_resume_request (struct inferior_list_entry *entry, void *arg)
 {
   struct lwp_info *lwp;
   struct thread_info *thread;
   int ndx;
+  struct thread_resume_array *r;
 
   thread = (struct thread_info *) entry;
   lwp = get_thread_lwp (thread);
+  r = arg;
 
-  ndx = 0;
-  while (resume_ptr[ndx].thread != -1 && resume_ptr[ndx].thread != entry->id)
-    ndx++;
+  for (ndx = 0; ndx < r->n; ndx++)
+    if (r->resume[ndx].thread == -1 || r->resume[ndx].thread == entry->id)
+      {
+	lwp->resume = &r->resume[ndx];
+	return 0;
+      }
+
+  /* No resume action for this thread.  */
+  lwp->resume = NULL;
 
-  lwp->resume = &resume_ptr[ndx];
+  return 0;
 }
 
 /* This function is called once per thread.  We check the thread's resume
@@ -1289,7 +1301,7 @@ linux_continue_one_thread (struct inferi
   thread = (struct thread_info *) entry;
   lwp = get_thread_lwp (thread);
 
-  if (lwp->resume->leave_stopped)
+  if (lwp->resume == NULL)
     return;
 
   if (lwp->resume->thread == -1
@@ -1320,7 +1332,7 @@ linux_queue_one_thread (struct inferior_
   thread = (struct thread_info *) entry;
   lwp = get_thread_lwp (thread);
 
-  if (lwp->resume->leave_stopped)
+  if (lwp->resume == NULL)
     return;
 
   /* If we have a new signal, enqueue the signal.  */
@@ -1354,7 +1366,7 @@ resume_status_pending_p (struct inferior
 
   /* Processes which will not be resumed are not interesting, because
      we might not wait for them next time through linux_wait.  */
-  if (lwp->resume->leave_stopped)
+  if (lwp->resume == NULL)
     return 0;
 
   /* If this thread has a removed breakpoint, we won't have any
@@ -1375,14 +1387,12 @@ resume_status_pending_p (struct inferior
 }
 
 static void
-linux_resume (struct thread_resume *resume_info)
+linux_resume (struct thread_resume *resume_info, size_t n)
 {
   int pending_flag;
+  struct thread_resume_array array = { resume_info, n };
 
-  /* Yes, the use of a global here is rather ugly.  */
-  resume_ptr = resume_info;
-
-  for_each_inferior (&all_threads, linux_set_resume_request);
+  find_inferior (&all_threads, linux_set_resume_request, &array);
 
   /* If there is a thread which would otherwise be resumed, which
      has a pending status, then don't resume any threads - we can just
Index: src/gdb/gdbserver/spu-low.c
===================================================================
--- src.orig/gdb/gdbserver/spu-low.c	2009-03-23 00:23:21.000000000 +0000
+++ src/gdb/gdbserver/spu-low.c	2009-03-23 00:24:03.000000000 +0000
@@ -345,24 +345,27 @@ spu_thread_alive (unsigned long tid)
 
 /* Resume process.  */
 static void
-spu_resume (struct thread_resume *resume_info)
+spu_resume (struct thread_resume *resume_info, size_t n)
 {
-  while (resume_info->thread != -1
-	 && resume_info->thread != current_tid)
-    resume_info++;
+  size_t i;
 
-  if (resume_info->leave_stopped)
+  for (i = 0; i < n; i++)
+    if (resume_info[i].thread == -1
+	|| resume_info[i].thread == current_tid)
+      break;
+
+  if (i == n)
     return;
 
   /* We don't support hardware single-stepping right now, assume
      GDB knows to use software single-stepping.  */
-  if (resume_info->step)
+  if (resume_info[i].step)
     fprintf (stderr, "Hardware single-step not supported.\n");
 
   regcache_invalidate ();
 
   errno = 0;
-  ptrace (PTRACE_CONT, current_tid, 0, resume_info->sig);
+  ptrace (PTRACE_CONT, current_tid, 0, resume_info[i].sig);
   if (errno)
     perror_with_name ("ptrace");
 }
Index: src/gdb/gdbserver/win32-low.c
===================================================================
--- src.orig/gdb/gdbserver/win32-low.c	2009-03-23 00:23:21.000000000 +0000
+++ src/gdb/gdbserver/win32-low.c	2009-03-23 00:24:03.000000000 +0000
@@ -90,7 +90,7 @@ typedef BOOL WINAPI (*winapi_DebugSetPro
 typedef BOOL WINAPI (*winapi_DebugBreakProcess) (HANDLE);
 typedef BOOL WINAPI (*winapi_GenerateConsoleCtrlEvent) (DWORD, DWORD);
 
-static void win32_resume (struct thread_resume *resume_info);
+static void win32_resume (struct thread_resume *resume_info, size_t n);
 
 /* Get the thread ID from the current selected inferior (the current
    thread).  */
@@ -726,8 +726,7 @@ win32_detach (void)
     resume.thread = -1;
     resume.step = 0;
     resume.sig = 0;
-    resume.leave_stopped = 0;
-    win32_resume (&resume);
+    win32_resume (&resume, 1);
   }
 
   if (!DebugActiveProcessStop (current_process_id))
@@ -771,7 +770,7 @@ win32_thread_alive (unsigned long tid)
 /* Resume the inferior process.  RESUME_INFO describes how we want
    to resume.  */
 static void
-win32_resume (struct thread_resume *resume_info)
+win32_resume (struct thread_resume *resume_info, size_t n)
 {
   DWORD tid;
   enum target_signal sig;
@@ -782,9 +781,9 @@ win32_resume (struct thread_resume *resu
   /* This handles the very limited set of resume packets that GDB can
      currently produce.  */
 
-  if (resume_info[0].thread == -1)
+  if (n == 1 && resume_info[0].thread == -1)
     tid = -1;
-  else if (resume_info[1].thread == -1 && !resume_info[1].leave_stopped)
+  else if (n > 1)
     tid = -1;
   else
     /* Yes, we're ignoring resume_info[0].thread.  It'd be tricky to make


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]