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]

teach gdbserver's regcache about unavailable registers, and reply 'x's to g/p packets, indicating unavailableness


This teaches gdbserver's regcache about unavailable (traceframe) registers,
and makes use of the long undocumented but recently documented (*)
remote protocol feature, where a reply to g/P with a register
value of 'x's indicates the register is not available.

(*) - http://sourceware.org/ml/gdb-patches/2010-12/msg00440.html

though on the gdb, the feature isn't completely wired up to "info reg" (or
most likely, regressed at some point).  Further down the road, "info
reg" will show:

 Found trace frame 0, tracepoint 2
 #0  main () at threads_partial.cc:58
 58        memset (&foo2.sfoo, 0xaa, sizeof foo2.sfoo);
 (gdb) info registers 
 rax            *value not available*
 ...
 r15            *value not available*
 rip            0x400851 0x400851 <main()+29>
 eflags         *value not available*
 ...

(The *value not available* is already in the tree, I
won't be adding it.  It's just not reachable in
this case yet.)

while currently, even after this patch, it still shows:

 Found trace frame 0, tracepoint 2
 #0  main () at threads_partial.cc:58
 58        memset (&foo2.sfoo, 0xaa, sizeof foo2.sfoo);
 (gdb) info registers 
 rax            0x0      0
 ...
 r15            0x0      0
 rip            0x400851 0x400851 <main()+29>
 eflags         0x0      [ ]
 ...


I've chosen to reuse regcache_supply(regcache, n, NULL)
                                                  ^^^^
as indication that the register is not available, rather than
keeping the current gdbserver's meaning that it's available but
with value 0, as it makes more sense this way IMO, and is what
the corresponding GDB implementation does too.

this makes this bit in gdbserver/tracepoint.c:

>  if (dataptr == NULL)
>    {
>      /* We don't like making up numbers, but GDB has all manner of
>        troubles when the target says there are no registers.  */
>      supply_regblock (regcache, NULL);
>
>      /* We can generally guess at a PC, although this will be
>        misleading for while-stepping frames and multi-location
>        tracepoints.  */
>      tpoint = find_next_tracepoint_by_number (NULL, tframe->tpnum);
>      if (tpoint != NULL)
>       regcache_write_pc (regcache, tpoint->address);
>    }

automatically supply registers as unavailable.

I considered making the current callers that pass NULL and want
0s pass in a memset'ed buffer (which would be effectivelly
the same), but ended up with a convenience
"supply_register_zeroed" variant.

Tested on x86_64-linux and applied.

-- 
Pedro Alves

2011-01-28  Pedro Alves  <pedro@codesourcery.com>

	gdb/gdbserver/
	* regcache.c (init_register_cache): Initialize
	regcache->register_status.
	(free_register_cache): Release regcache->register_status.
	(regcache_cpy): Copy register_status.
	(registers_to_string): Print 'x's for unavailable registers.
	(supply_register): Mark the register's status valid or
	unavailable, depending on whether a buffer was passed in or not.
	(supply_register_zeroed): New.
	(supply_regblock): Mark the registers' status valid or
	unavailable, depending on whether a buffer was passed in or not.
	* regcache.h (REG_UNAVAILABLE, REG_VALID): New defines.
	(struct regcache): New `register_status' field.
	(supply_register_zeroed): Declare.
	* i387-fp.c (i387_xsave_to_cache): Zero out registers using
	supply_register_zeroed, rather than passing a NULL buffer to
	supply_register.
	* tracepoint.c (fetch_traceframe_registers): Update comment.

---
 gdb/gdbserver/i387-fp.c    |    6 +--
 gdb/gdbserver/regcache.c   |   87 ++++++++++++++++++++++++++++++++++++++++++---
 gdb/gdbserver/regcache.h   |   18 +++++++++
 gdb/gdbserver/tracepoint.c |    3 -
 4 files changed, 104 insertions(+), 10 deletions(-)

Index: src/gdb/gdbserver/regcache.c
===================================================================
--- src.orig/gdb/gdbserver/regcache.c	2011-01-13 15:07:53.426074999 +0000
+++ src/gdb/gdbserver/regcache.c	2011-01-28 13:34:56.701414999 +0000
@@ -98,6 +98,8 @@ init_register_cache (struct regcache *re
 	 garbage.  */
       regcache->registers = xcalloc (1, register_bytes);
       regcache->registers_owned = 1;
+      regcache->register_status = xcalloc (1, num_registers);
+      gdb_assert (REG_UNAVAILABLE == 0);
     }
   else
 #else
@@ -108,6 +110,9 @@ init_register_cache (struct regcache *re
     {
       regcache->registers = regbuf;
       regcache->registers_owned = 0;
+#ifndef IN_PROCESS_AGENT
+      regcache->register_status = NULL;
+#endif
     }
 
   regcache->registers_valid = 0;
@@ -136,6 +141,7 @@ free_register_cache (struct regcache *re
     {
       if (regcache->registers_owned)
 	free (regcache->registers);
+      free (regcache->register_status);
       free (regcache);
     }
 }
@@ -146,6 +152,10 @@ void
 regcache_cpy (struct regcache *dst, struct regcache *src)
 {
   memcpy (dst->registers, src->registers, register_bytes);
+#ifndef IN_PROCESS_AGENT
+  if (dst->register_status != NULL && src->register_status != NULL)
+    memcpy (dst->register_status, src->register_status, num_registers);
+#endif
   dst->registers_valid = src->registers_valid;
 }
 
@@ -209,8 +219,23 @@ void
 registers_to_string (struct regcache *regcache, char *buf)
 {
   unsigned char *registers = regcache->registers;
+  int i;
 
-  convert_int_to_ascii (registers, buf, register_bytes);
+  for (i = 0; i < num_registers; i++)
+    {
+      if (regcache->register_status[i] == REG_VALID)
+	{
+	  convert_int_to_ascii (registers, buf, register_size (i));
+	  buf += register_size (i) * 2;
+	}
+      else
+	{
+	  memset (buf, 'x', register_size (i) * 2);
+	  buf += register_size (i) * 2;
+	}
+      registers += register_size (i);
+    }
+  *buf = '\0';
 }
 
 void
@@ -273,22 +298,74 @@ register_data (struct regcache *regcache
   return regcache->registers + (reg_defs[n].offset / 8);
 }
 
+/* Supply register N, whose contents are stored in BUF, to REGCACHE.
+   If BUF is NULL, the register's value is recorded as
+   unavailable.  */
+
 void
 supply_register (struct regcache *regcache, int n, const void *buf)
 {
   if (buf)
-    memcpy (register_data (regcache, n, 0), buf, register_size (n));
+    {
+      memcpy (register_data (regcache, n, 0), buf, register_size (n));
+#ifndef IN_PROCESS_AGENT
+      if (regcache->register_status != NULL)
+	regcache->register_status[n] = REG_VALID;
+#endif
+    }
   else
-    memset (register_data (regcache, n, 0), 0, register_size (n));
+    {
+      memset (register_data (regcache, n, 0), 0, register_size (n));
+#ifndef IN_PROCESS_AGENT
+      if (regcache->register_status != NULL)
+	regcache->register_status[n] = REG_UNAVAILABLE;
+#endif
+    }
 }
 
+/* Supply register N with value zero to REGCACHE.  */
+
+void
+supply_register_zeroed (struct regcache *regcache, int n)
+{
+  memset (register_data (regcache, n, 0), 0, register_size (n));
+#ifndef IN_PROCESS_AGENT
+  if (regcache->register_status != NULL)
+    regcache->register_status[n] = REG_VALID;
+#endif
+}
+
+/* Supply the whole register set whose contents are stored in BUF, to
+   REGCACHE.  If BUF is NULL, all the registers' values are recorded
+   as unavailable.  */
+
 void
 supply_regblock (struct regcache *regcache, const void *buf)
 {
   if (buf)
-    memcpy (regcache->registers, buf, register_bytes);
+    {
+      memcpy (regcache->registers, buf, register_bytes);
+#ifndef IN_PROCESS_AGENT
+      {
+	int i;
+
+	for (i = 0; i < num_registers; i++)
+	  regcache->register_status[i] = REG_VALID;
+      }
+#endif
+    }
   else
-    memset (regcache->registers, 0, register_bytes);
+    {
+      memset (regcache->registers, 0, register_bytes);
+#ifndef IN_PROCESS_AGENT
+      {
+	int i;
+
+	for (i = 0; i < num_registers; i++)
+	  regcache->register_status[i] = REG_UNAVAILABLE;
+      }
+#endif
+    }
 }
 
 #ifndef IN_PROCESS_AGENT
Index: src/gdb/gdbserver/regcache.h
===================================================================
--- src.orig/gdb/gdbserver/regcache.h	2011-01-13 15:07:53.436075000 +0000
+++ src/gdb/gdbserver/regcache.h	2011-01-28 13:09:48.141415001 +0000
@@ -23,15 +23,31 @@
 struct inferior_list_entry;
 struct thread_info;
 
+/* The register exists, it has a value, but we don't know what it is.
+   Used when inspecting traceframes.  */
+#define REG_UNAVAILABLE 0
+
+/* We know the register's value (and we have it cached).  */
+#define REG_VALID 1
+
 /* The data for the register cache.  Note that we have one per
    inferior; this is primarily for simplicity, as the performance
    benefit is minimal.  */
 
 struct regcache
 {
+  /* Whether the REGISTERS buffer's contents are valid.  If false, we
+     haven't fetched the registers from the target yet.  Not that this
+     register cache is _not_ pass-through, unlike GDB's.  Note that
+     "valid" here is unrelated to whether the registers are available
+     in a traceframe.  For that, check REGISTER_STATUS below.  */
   int registers_valid;
   int registers_owned;
   unsigned char *registers;
+#ifndef IN_PROCESS_AGENT
+  /* One of REG_UNAVAILBLE or REG_VALID.  */
+  unsigned char *register_status;
+#endif
 };
 
 struct regcache *init_register_cache (struct regcache *regcache,
@@ -84,6 +100,8 @@ extern const char *gdbserver_xmltarget;
 
 void supply_register (struct regcache *regcache, int n, const void *buf);
 
+void supply_register_zeroed (struct regcache *regcache, int n);
+
 void supply_register_by_name (struct regcache *regcache,
 			      const char *name, const void *buf);
 
Index: src/gdb/gdbserver/i387-fp.c
===================================================================
--- src.orig/gdb/gdbserver/i387-fp.c	2011-01-28 12:38:16.501414999 +0000
+++ src/gdb/gdbserver/i387-fp.c	2011-01-28 13:09:48.151415001 +0000
@@ -482,7 +482,7 @@ i387_xsave_to_cache (struct regcache *re
       if ((clear_bv & I386_XSTATE_X87) != 0)
 	{
 	  for (i = 0; i < 8; i++)
-	    supply_register (regcache, i + st0_regnum, NULL);
+	    supply_register_zeroed (regcache, i + st0_regnum);
 	}
       else
 	{
@@ -499,7 +499,7 @@ i387_xsave_to_cache (struct regcache *re
       if ((clear_bv & I386_XSTATE_SSE))
 	{
 	  for (i = 0; i < num_xmm_registers; i++)
-	    supply_register (regcache, i + xmm0_regnum, NULL);
+	    supply_register_zeroed (regcache, i + xmm0_regnum);
 	}
       else
 	{
@@ -516,7 +516,7 @@ i387_xsave_to_cache (struct regcache *re
       if ((clear_bv & I386_XSTATE_AVX) != 0)
 	{
 	  for (i = 0; i < num_xmm_registers; i++)
-	    supply_register (regcache, i + ymm0h_regnum, NULL);
+	    supply_register_zeroed (regcache, i + ymm0h_regnum);
 	}
       else
 	{
Index: src/gdb/gdbserver/tracepoint.c
===================================================================
--- src.orig/gdb/gdbserver/tracepoint.c	2011-01-13 15:11:01.000000000 +0000
+++ src/gdb/gdbserver/tracepoint.c	2011-01-28 13:24:17.391415002 +0000
@@ -4840,8 +4840,7 @@ fetch_traceframe_registers (int tfnum, s
   dataptr = traceframe_find_regblock (tframe, tfnum);
   if (dataptr == NULL)
     {
-      /* We don't like making up numbers, but GDB has all manner of
-	 troubles when the target says there are no registers.  */
+      /* Mark registers unavailable.  */
       supply_regblock (regcache, NULL);
 
       /* We can generally guess at a PC, although this will be


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