diff mbox series

[v4,1/7] gdb: Allow replayed threads to read and write pseudo registers

Message ID 20240923151541.616723-3-guinevere@redhat.com
State New
Headers
Series Support for recording some AVX instructions |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Test passed

Commit Message

Guinevere Larsen Sept. 23, 2024, 3:15 p.m. UTC 
  In an effort to support AVX instructions when recording, we need to
allow replaying threads to access pseudo registers. Currently, if
we try to do that gdb will fail in a call to validate_registers_access,
because the thread is executing so GDB thinks it is unsafe to read
pseudo registers.

When replaying, the thread is really executing for all intents and
purposes, but the execution is just having GDB change values on
registers, so it will always be safe to read and write pseudo registers.
This commit changes validate_registers_access to allow access when we
are replaying. The check to whether we are replaying must not happen
when writing a core file, as record_full_list could be nullptr, so we
only check it if the thread is executing.

As of this commit, I don't know of a way to trigger this commit without
AVX support on record, so a test isn't provided. However, as soon as
record-full supports saving ymm registers, the AVX tests will test this
as well.
---
 gdb/record-full.c |  8 ++++++++
 gdb/record-full.h |  3 +++
 gdb/thread.c      | 15 +++++++++++++--
 3 files changed, 24 insertions(+), 2 deletions(-)

Comments

Tom Tromey Oct. 4, 2024, 7:54 p.m. UTC | #1 
>>>>> "Guinevere" == Guinevere Larsen <guinevere@redhat.com> writes:

Guinevere> +/* see record-full.h.  */
Guinevere> +bool
Guinevere> +record_full_is_replaying ()
Guinevere> +{
Guinevere> +  return (execution_direction == EXEC_REVERSE
Guinevere> +	  || record_full_list->next != nullptr);

I was inclined to approve this, but I was curious and I found the
existing RECORD_FULL_IS_REPLAY macro, and then this led me to
target_record_is_replaying -- the same thing, I think, but in a
future-proof way in case record-full is ever multi-target-capable.

So I wonder if the new function can be dispensed with and thread.c
changed to use target_record_is_replaying?

I guess that would affect non-record-full targets?  But ... would it
matter?  I can't really tell.

Tom
diff mbox series

Patch

diff --git a/gdb/record-full.c b/gdb/record-full.c
index a681ef9fe51..06b35fd45cb 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -370,6 +370,14 @@  record_full_is_used (void)
 	  || t == &record_full_core_ops);
 }
 
+/* see record-full.h.  */
+bool
+record_full_is_replaying ()
+{
+  return (execution_direction == EXEC_REVERSE
+	  || record_full_list->next != nullptr);
+}
+
 
 /* Command lists for "set/show record full".  */
 static struct cmd_list_element *set_record_full_cmdlist;
diff --git a/gdb/record-full.h b/gdb/record-full.h
index e8eb041311b..07f97d241a2 100644
--- a/gdb/record-full.h
+++ b/gdb/record-full.h
@@ -31,6 +31,9 @@  extern int record_full_arch_list_add_end (void);
 /* Returns true if the process record target is open.  */
 extern int record_full_is_used (void);
 
+/* Whether the inferior is being replayed, or is executing normally.  */
+extern bool record_full_is_replaying ();
+
 extern scoped_restore_tmpl<int> record_full_gdb_operation_disable_set ();
 
 #endif /* RECORD_FULL_H */
diff --git a/gdb/thread.c b/gdb/thread.c
index 4ee46936861..5892b158603 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -50,6 +50,7 @@ 
 #include "inline-frame.h"
 #include "stack.h"
 #include "interps.h"
+#include "record-full.h"
 
 /* See gdbthread.h.  */
 
@@ -998,7 +999,13 @@  validate_registers_access (void)
      reason, but is marked running from the user's perspective.  E.g.,
      the thread is waiting for its turn in the step-over queue.  */
   if (tp->executing ())
-    error (_("Selected thread is running."));
+    {
+      /* If we are replaying with the record-full subsystem, even though
+	 the thread is executing, it is always safe to read from it since
+	 replay execution is just GDB reading and writing to a regcache.  */
+      if (!record_full_is_replaying ())
+	error (_("Selected thread is running."));
+    }
 }
 
 /* See gdbthread.h.  */
@@ -1016,7 +1023,11 @@  can_access_registers_thread (thread_info *thread)
 
   /* ... or from a spinning thread.  FIXME: see validate_registers_access.  */
   if (thread->executing ())
-    return false;
+    {
+      /* See validate_registers_access.  */
+      if (!record_full_is_replaying ())
+	return false;
+    }
 
   return true;
 }