[v2,10/12] Fetch registers from correct thread in ravenscar-thread.c

Message ID 20200805190841.2506771-11-tromey@adacore.com
State New
Headers show
Series
  • Fix Ravenscar regressions
Related show

Commit Message

Tom Tromey Aug. 5, 2020, 7:08 p.m.
Fabien also noticed that gdb would not report a stop correctly when
using Ravenscar.  This patch fixes the bug by making a few changes:

* ravenscar_thread_target::wait now updates the inferior ptid before
  updating the thread list.  This ensures that a new thread is
  correctly associated with the underlying CPU.

* The fetch_registers, store_registers, and prepare_to_store methods
  now save and restore the regcache's ptid before doing the operation
  on the underlying live thread.  This ensures that gdb informs the
  remote of a thread it knows about, as opposed to using a Ravenscar
  thread, which probably will not be recognized.

2020-07-08  Tom Tromey  <tromey@adacore.com>

	* ravenscar-thread.c (ravenscar_thread_target::wait): Call
	update_inferior_ptid before update_thread_list.
	(temporarily_change_regcache_ptid): New class.
	(ravenscar_thread_target::fetch_registers)
	(ravenscar_thread_target::store_registers)
	(ravenscar_thread_target::prepare_to_store): Use base thread when
	forwarding operation.
---
 gdb/ChangeLog          | 10 +++++
 gdb/ravenscar-thread.c | 85 +++++++++++++++++++++++++++++++++---------
 2 files changed, 77 insertions(+), 18 deletions(-)

-- 
2.26.2

Patch

diff --git a/gdb/ravenscar-thread.c b/gdb/ravenscar-thread.c
index 91f09209bbc..459e5ea3ed3 100644
--- a/gdb/ravenscar-thread.c
+++ b/gdb/ravenscar-thread.c
@@ -457,20 +457,55 @@  ravenscar_thread_target::pid_to_str (ptid_t ptid)
   return string_printf ("Ravenscar Thread %#x", (int) ptid.tid ());
 }
 
+/* Temporarily set the ptid of a regcache to some other value.  When
+   this object is destroyed, the regcache's original ptid is
+   restored.  */
+
+class temporarily_change_regcache_ptid
+{
+public:
+
+  temporarily_change_regcache_ptid (struct regcache *regcache, ptid_t new_ptid)
+    : m_regcache (regcache),
+      m_save_ptid (regcache->ptid ())
+  {
+    m_regcache->set_ptid (new_ptid);
+  }
+
+  ~temporarily_change_regcache_ptid ()
+  {
+    m_regcache->set_ptid (m_save_ptid);
+  }
+
+private:
+
+  /* The regcache.  */
+  struct regcache *m_regcache;
+  /* The saved ptid.  */
+  ptid_t m_save_ptid;
+};
+
 void
 ravenscar_thread_target::fetch_registers (struct regcache *regcache, int regnum)
 {
   ptid_t ptid = regcache->ptid ();
 
-  if (runtime_initialized ()
-      && is_ravenscar_task (ptid)
-      && !task_is_currently_active (ptid))
+  if (runtime_initialized () && is_ravenscar_task (ptid))
     {
-      struct gdbarch *gdbarch = regcache->arch ();
-      struct ravenscar_arch_ops *arch_ops
-	= gdbarch_ravenscar_ops (gdbarch);
+      if (task_is_currently_active (ptid))
+	{
+	  ptid_t base = get_base_thread_from_ravenscar_task (ptid);
+	  temporarily_change_regcache_ptid changer (regcache, base);
+	  beneath ()->fetch_registers (regcache, regnum);
+	}
+      else
+	{
+	  struct gdbarch *gdbarch = regcache->arch ();
+	  struct ravenscar_arch_ops *arch_ops
+	    = gdbarch_ravenscar_ops (gdbarch);
 
-      arch_ops->fetch_registers (regcache, regnum);
+	  arch_ops->fetch_registers (regcache, regnum);
+	}
     }
   else
     beneath ()->fetch_registers (regcache, regnum);
@@ -482,15 +517,22 @@  ravenscar_thread_target::store_registers (struct regcache *regcache,
 {
   ptid_t ptid = regcache->ptid ();
 
-  if (runtime_initialized ()
-      && is_ravenscar_task (ptid)
-      && !task_is_currently_active (ptid))
+  if (runtime_initialized () && is_ravenscar_task (ptid))
     {
-      struct gdbarch *gdbarch = regcache->arch ();
-      struct ravenscar_arch_ops *arch_ops
-	= gdbarch_ravenscar_ops (gdbarch);
+      if (task_is_currently_active (ptid))
+	{
+	  ptid_t base = get_base_thread_from_ravenscar_task (ptid);
+	  temporarily_change_regcache_ptid changer (regcache, base);
+	  beneath ()->store_registers (regcache, regnum);
+	}
+      else
+	{
+	  struct gdbarch *gdbarch = regcache->arch ();
+	  struct ravenscar_arch_ops *arch_ops
+	    = gdbarch_ravenscar_ops (gdbarch);
 
-      arch_ops->store_registers (regcache, regnum);
+	  arch_ops->store_registers (regcache, regnum);
+	}
     }
   else
     beneath ()->store_registers (regcache, regnum);
@@ -501,11 +543,18 @@  ravenscar_thread_target::prepare_to_store (struct regcache *regcache)
 {
   ptid_t ptid = regcache->ptid ();
 
-  if (runtime_initialized ()
-      && is_ravenscar_task (ptid)
-      && !task_is_currently_active (ptid))
+  if (runtime_initialized () && is_ravenscar_task (ptid))
     {
-      /* Nothing.  */
+      if (task_is_currently_active (ptid))
+	{
+	  ptid_t base = get_base_thread_from_ravenscar_task (ptid);
+	  temporarily_change_regcache_ptid changer (regcache, base);
+	  beneath ()->prepare_to_store (regcache);
+	}
+      else
+	{
+	  /* Nothing.  */
+	}
     }
   else
     beneath ()->prepare_to_store (regcache);