[binutils-gdb] gdbserver:prepare_access_memory: pick another thread

[Pedro Alves <palves at sourceware dot org>]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=a67a9faef0e32886c83611cc7a0ba61e91123063

commit a67a9faef0e32886c83611cc7a0ba61e91123063
Author: Pedro Alves <palves@redhat.com>
Date:   Mon Nov 30 16:05:26 2015 +0000

    gdbserver:prepare_access_memory: pick another thread
    
    Say GDB wants to access the inferior process's memory.  The current
    remote general thread is 3, but GDB's switched to thread 2.  Because
    both threads are of the same process, GDB skips making the remote
    thread be thread 2 as well (sending an Hg packet) before accessing
    memory (remote.c:set_general_process).  However, if thread 3 has
    exited meanwhile, thread 3 no longer exists on the server and
    gdbserver points current_thread to NULL.  The result is the memory
    access fails, even through the process still exists.
    
    Fix this by making prepare_to_access memory select the thread to
    access memory through.
    
    gdb/gdbserver/ChangeLog:
    2015-11-30  Pedro Alves  <palves@redhat.com>
    
    	* mem-break.c (check_gdb_bp_preconditions): Remove current_thread
    	check.
    	(set_gdb_breakpoint): If prepare_to_access_memory fails, set *ERR
    	to -1.
    	* target.c (struct thread_search): New structure.
    	(thread_search_callback): New function.
    	(prev_general_thread): New global.
    	(prepare_to_access_memory, done_accessing_memory): New functions.
    	* target.h (prepare_to_access_memory, done_accessing_memory):
    	Replace macros with function declarations.

Diff:
---
 gdb/gdbserver/ChangeLog   |  13 ++++++
 gdb/gdbserver/mem-break.c |  17 +++-----
 gdb/gdbserver/target.c    | 107 ++++++++++++++++++++++++++++++++++++++++++++++
 gdb/gdbserver/target.h    |  14 ++----
 4 files changed, 131 insertions(+), 20 deletions(-)

diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
index dbe7dc4..86ca736 100644
--- a/gdb/gdbserver/ChangeLog
+++ b/gdb/gdbserver/ChangeLog
@@ -1,5 +1,18 @@
 2015-11-30  Pedro Alves  <palves@redhat.com>
 
+	* mem-break.c (check_gdb_bp_preconditions): Remove current_thread
+	check.
+	(set_gdb_breakpoint): If prepare_to_access_memory fails, set *ERR
+	to -1.
+	* target.c (struct thread_search): New structure.
+	(thread_search_callback): New function.
+	(prev_general_thread): New global.
+	(prepare_to_access_memory, done_accessing_memory): New functions.
+	* target.h (prepare_to_access_memory, done_accessing_memory):
+	Replace macros with function declarations.
+
+2015-11-30  Pedro Alves  <palves@redhat.com>
+
 	PR 14618
 	* linux-low.c (linux_wait_1): If the last resumed thread is gone,
 	report TARGET_WAITKIND_NO_RESUMED.
diff --git a/gdb/gdbserver/mem-break.c b/gdb/gdbserver/mem-break.c
index c808a84..11c21db 100644
--- a/gdb/gdbserver/mem-break.c
+++ b/gdb/gdbserver/mem-break.c
@@ -1023,13 +1023,8 @@ check_gdb_bp_preconditions (char z_type, int *err)
       *err = 1;
       return 0;
     }
-  else if (current_thread == NULL)
-    {
-      *err = -1;
-      return 0;
-    }
-  else
-    return 1;
+
+  return 1;
 }
 
 /* See mem-break.h.  This is a wrapper for set_gdb_breakpoint_1 that
@@ -1047,9 +1042,11 @@ set_gdb_breakpoint (char z_type, CORE_ADDR addr, int kind, int *err)
      access memory.  */
   if (z_type == Z_PACKET_SW_BP)
     {
-      *err = prepare_to_access_memory ();
-      if (*err != 0)
-	return NULL;
+      if (prepare_to_access_memory () != 0)
+	{
+	  *err = -1;
+	  return NULL;
+	}
     }
 
   bp = set_gdb_breakpoint_1 (z_type, addr, kind, err);
diff --git a/gdb/gdbserver/target.c b/gdb/gdbserver/target.c
index 80512d86..b376ce8 100644
--- a/gdb/gdbserver/target.c
+++ b/gdb/gdbserver/target.c
@@ -37,6 +37,113 @@ set_desired_thread (int use_general)
   return (current_thread != NULL);
 }
 
+/* Structure used to look up a thread to use as current when accessing
+   memory.  */
+
+struct thread_search
+{
+  /* The PTID of the current general thread.  This is an input
+     parameter.  */
+  ptid_t current_gen_ptid;
+
+  /* The first thread found.  */
+  struct thread_info *first;
+
+  /* The first stopped thread found.  */
+  struct thread_info *stopped;
+
+  /* The current general thread, if found.  */
+  struct thread_info *current;
+};
+
+/* Callback for find_inferior.  Search for a thread to use as current
+   when accessing memory.  */
+
+static int
+thread_search_callback (struct inferior_list_entry *entry, void *args)
+{
+  struct thread_info *thread = (struct thread_info *) entry;
+  struct thread_search *s = (struct thread_search *) args;
+
+  if (ptid_get_pid (entry->id) == ptid_get_pid (s->current_gen_ptid)
+      && mythread_alive (ptid_of (thread)))
+    {
+      if (s->stopped == NULL && thread_stopped (thread))
+	s->stopped = thread;
+
+      if (s->first == NULL)
+	s->first = thread;
+
+      if (s->current == NULL && ptid_equal (s->current_gen_ptid, entry->id))
+	s->current = thread;
+    }
+
+  return 0;
+}
+
+/* The thread that was current before prepare_to_access_memory was
+   called.  done_accessing_memory uses this to restore the previous
+   selected thread.  */
+static ptid_t prev_general_thread;
+
+/* See target.h.  */
+
+int
+prepare_to_access_memory (void)
+{
+  struct thread_search search;
+  struct thread_info *thread;
+
+  memset (&search, 0, sizeof (search));
+  search.current_gen_ptid = general_thread;
+  prev_general_thread = general_thread;
+
+  if (the_target->prepare_to_access_memory != NULL)
+    {
+      int res;
+
+      res = the_target->prepare_to_access_memory ();
+      if (res != 0)
+	return res;
+    }
+
+  find_inferior (&all_threads, thread_search_callback, &search);
+
+  /* Prefer a stopped thread.  If none is found, try the current
+     thread.  Otherwise, take the first thread in the process.  If
+     none is found, undo the effects of
+     target->prepare_to_access_memory() and return error.  */
+  if (search.stopped != NULL)
+    thread = search.stopped;
+  else if (search.current != NULL)
+    thread = search.current;
+  else if (search.first != NULL)
+    thread = search.first;
+  else
+    {
+      done_accessing_memory ();
+      return 1;
+    }
+
+  current_thread = thread;
+  general_thread = ptid_of (thread);
+
+  return 0;
+}
+
+/* See target.h.  */
+
+void
+done_accessing_memory (void)
+{
+  if (the_target->done_accessing_memory != NULL)
+    the_target->done_accessing_memory ();
+
+  /* Restore the previous selected thread.  */
+  general_thread = prev_general_thread;
+  current_thread = find_thread_ptid (general_thread);
+}
+
 int
 read_inferior_memory (CORE_ADDR memaddr, unsigned char *myaddr, int len)
 {
diff --git a/gdb/gdbserver/target.h b/gdb/gdbserver/target.h
index 8903da5..a09ba2f 100644
--- a/gdb/gdbserver/target.h
+++ b/gdb/gdbserver/target.h
@@ -651,17 +651,11 @@ int start_non_stop (int nonstop);
 ptid_t mywait (ptid_t ptid, struct target_waitstatus *ourstatus, int options,
 	       int connected_wait);
 
-#define prepare_to_access_memory()		\
-  (the_target->prepare_to_access_memory		\
-   ? (*the_target->prepare_to_access_memory) () \
-   : 0)
+/* Prepare to read or write memory from the inferior process.  See the
+   corresponding target_ops methods for more details.  */
 
-#define done_accessing_memory()				\
-  do							\
-    {							\
-      if (the_target->done_accessing_memory)     	\
-	(*the_target->done_accessing_memory) ();  	\
-    } while (0)
+int prepare_to_access_memory (void);
+void done_accessing_memory (void);
 
 #define target_core_of_thread(ptid)		\
   (the_target->core_of_thread ? (*the_target->core_of_thread) (ptid) \