[binutils-gdb] gdbserver: track current process as well as current thread

[binutils-gdb] gdbserver: track current process as well as current thread

[Pedro Alves]

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

commit 7f8acedeebe295fc8cc1d11ed971cbfc1942618c
Author: Pedro Alves <pedro@palves.net>
Date:   Tue Apr 5 13:57:11 2022 +0100

    gdbserver: track current process as well as current thread
    
    The recent commit 421490af33bf ("gdbserver/linux: Access memory even
    if threads are running") caused a regression in
    gdb.threads/access-mem-running-thread-exit.exp with gdbserver, which I
    somehow missed.  Like so:
    
     (gdb) print global_var
     Cannot access memory at address 0x555555558010
     (gdb) FAIL: gdb.threads/access-mem-running-thread-exit.exp: non-stop: access mem (print global_var after writing, inf=2, iter=1)
    
    The problem starts with GDB telling GDBserver to select a thread, via
    the Hg packet, which GDBserver complies with, then that thread exits,
    and GDB, without knowing the thread is gone, tries to write to memory,
    through the context of the previously selected Hg thread.
    
    GDBserver's GDB-facing memory access routines, gdb_read_memory and
    gdb_write_memory, call set_desired_thread to make GDBserver re-select
    the thread that GDB has selected with the Hg packet.  Since the thread
    is gone, set_desired_thread returns false, and the memory access
    fails.
    
    Now, to access memory, it doesn't really matter which thread is
    selected.  All we should need is the target process.  Even if the
    thread that GDB previously selected is gone, and GDB does not yet know
    about that exit, it shouldn't matter, GDBserver should still know
    which process that thread belonged to.
    
    Fix this by making GDBserver track the current process separately,
    like GDB also does.  Add a new set_desired_process routine that is
    similar to set_desired_thread, but just sets the current process,
    leaving the current thread as NULL.  Use it in the GDB-facing memory
    read and write routines, to avoid failing if the selected thread is
    gone, but the process is still around.
    
    Change-Id: I4ff97cb6f42558efbed224b30d5c71f6112d44cd

Diff:
---
 gdbserver/gdbthread.h  |  1 +
 gdbserver/inferiors.cc | 26 ++++++++++++++++++++------
 gdbserver/server.cc    |  4 ++--
 gdbserver/target.cc    | 44 ++++++++++++++++++++++++++++++++++++++++++--
 gdbserver/target.h     | 15 ++++++++++++++-
 5 files changed, 79 insertions(+), 11 deletions(-)

diff --git a/gdbserver/gdbthread.h b/gdbserver/gdbthread.h
index 10ae1cb7c87..8b897e73d33 100644
--- a/gdbserver/gdbthread.h
+++ b/gdbserver/gdbthread.h
@@ -252,6 +252,7 @@ public:
 
 private:
   bool m_dont_restore = false;
+  process_info *m_process;
   thread_info *m_thread;
 };
 
diff --git a/gdbserver/inferiors.cc b/gdbserver/inferiors.cc
index 678d93c77a1..3d0a8b0e716 100644
--- a/gdbserver/inferiors.cc
+++ b/gdbserver/inferiors.cc
@@ -26,6 +26,11 @@
 std::list<process_info *> all_processes;
 std::list<thread_info *> all_threads;
 
+/* The current process.  */
+static process_info *current_process_;
+
+/* The current thread.  This is either a thread of CURRENT_PROCESS, or
+   NULL.  */
 struct thread_info *current_thread;
 
 /* The current working directory used to start the inferior.
@@ -130,6 +135,7 @@ clear_inferiors (void)
   clear_dlls ();
 
   switch_to_thread (nullptr);
+  current_process_ = nullptr;
 }
 
 struct process_info *
@@ -153,6 +159,8 @@ remove_process (struct process_info *process)
   free_all_breakpoints (process);
   gdb_assert (find_thread_process (process) == NULL);
   all_processes.remove (process);
+  if (current_process () == process)
+    switch_to_process (nullptr);
   delete process;
 }
 
@@ -205,8 +213,7 @@ get_thread_process (const struct thread_info *thread)
 struct process_info *
 current_process (void)
 {
-  gdb_assert (current_thread != NULL);
-  return get_thread_process (current_thread);
+  return current_process_;
 }
 
 /* See gdbsupport/common-gdbthread.h.  */
@@ -223,6 +230,10 @@ switch_to_thread (process_stratum_target *ops, ptid_t ptid)
 void
 switch_to_thread (thread_info *thread)
 {
+  if (thread != nullptr)
+    current_process_ = get_thread_process (thread);
+  else
+    current_process_ = nullptr;
   current_thread = thread;
 }
 
@@ -231,9 +242,8 @@ switch_to_thread (thread_info *thread)
 void
 switch_to_process (process_info *proc)
 {
-  int pid = pid_of (proc);
-
-  switch_to_thread (find_any_thread_of_pid (pid));
+  current_process_ = proc;
+  current_thread = nullptr;
 }
 
 /* See gdbsupport/common-inferior.h.  */
@@ -254,6 +264,7 @@ set_inferior_cwd (std::string cwd)
 
 scoped_restore_current_thread::scoped_restore_current_thread ()
 {
+  m_process = current_process_;
   m_thread = current_thread;
 }
 
@@ -262,5 +273,8 @@ scoped_restore_current_thread::~scoped_restore_current_thread ()
   if (m_dont_restore)
     return;
 
-  switch_to_thread (m_thread);
+  if (m_thread != nullptr)
+    switch_to_thread (m_thread);
+  else
+    switch_to_process (m_process);
 }
diff --git a/gdbserver/server.cc b/gdbserver/server.cc
index 33c42714e72..f9c02a9c6da 100644
--- a/gdbserver/server.cc
+++ b/gdbserver/server.cc
@@ -1067,7 +1067,7 @@ gdb_read_memory (CORE_ADDR memaddr, unsigned char *myaddr, int len)
       /* (assume no half-trace half-real blocks for now) */
     }
 
-  if (set_desired_thread ())
+  if (set_desired_process ())
     res = read_inferior_memory (memaddr, myaddr, len);
   else
     res = 1;
@@ -1088,7 +1088,7 @@ gdb_write_memory (CORE_ADDR memaddr, const unsigned char *myaddr, int len)
     {
       int ret;
 
-      if (set_desired_thread ())
+      if (set_desired_process ())
 	ret = target_write_memory (memaddr, myaddr, len);
       else
 	ret = EIO;
diff --git a/gdbserver/target.cc b/gdbserver/target.cc
index 883242377c0..adcfe6e7bcc 100644
--- a/gdbserver/target.cc
+++ b/gdbserver/target.cc
@@ -29,16 +29,56 @@
 
 process_stratum_target *the_target;
 
-int
+/* See target.h.  */
+
+bool
 set_desired_thread ()
 {
   client_state &cs = get_client_state ();
   thread_info *found = find_thread_ptid (cs.general_thread);
 
-  switch_to_thread (found);
+  if (found == nullptr)
+    {
+      process_info *proc = find_process_pid (cs.general_thread.pid ());
+      if (proc == nullptr)
+	{
+	  threads_debug_printf
+	    ("did not find thread nor process for general_thread %s",
+	     cs.general_thread.to_string ().c_str ());
+	}
+      else
+	{
+	  threads_debug_printf
+	    ("did not find thread for general_thread %s, but found process",
+	     cs.general_thread.to_string ().c_str ());
+	}
+      switch_to_process (proc);
+    }
+  else
+    switch_to_thread (found);
+
   return (current_thread != NULL);
 }
 
+/* See target.h.  */
+
+bool
+set_desired_process ()
+{
+  client_state &cs = get_client_state ();
+
+  process_info *proc = find_process_pid (cs.general_thread.pid ());
+  if (proc == nullptr)
+    {
+      threads_debug_printf
+	("did not find process for general_thread %s",
+	 cs.general_thread.to_string ().c_str ());
+    }
+  switch_to_process (proc);
+
+  return proc != nullptr;
+}
+
 int
 read_inferior_memory (CORE_ADDR memaddr, unsigned char *myaddr, int len)
 {
diff --git a/gdbserver/target.h b/gdbserver/target.h
index f3172e2ed7e..6c536a30778 100644
--- a/gdbserver/target.h
+++ b/gdbserver/target.h
@@ -699,7 +699,20 @@ target_thread_pending_child (thread_info *thread)
 
 int read_inferior_memory (CORE_ADDR memaddr, unsigned char *myaddr, int len);
 
-int set_desired_thread ();
+/* Set GDBserver's current thread to the thread the client requested
+   via Hg.  Also switches the current process to the requested
+   process.  If the requested thread is not found in the thread list,
+   then the current thread is set to NULL.  Likewise, if the requested
+   process is not found in the process list, then the current process
+   is set to NULL.  Returns true if the requested thread was found,
+   false otherwise.  */
+
+bool set_desired_thread ();
+
+/* Set GDBserver's current process to the process the client requested
+   via Hg.  The current thread is set to NULL.  */
+
+bool set_desired_process ();
 
 std::string target_pid_to_str (ptid_t);