diff mbox

[1/8] gdb: Add write_guessed_tracepoint_pc hook to gdbarch.

Message ID 1453637529-26972-2-git-send-email-koriakin@0x04.net
State New, archived
Headers

Commit Message

Marcin Kościelnicki Jan. 24, 2016, 12:12 p.m. UTC 
  When we're looking at a tracefile trace frame where registers are not
available, and the tracepoint has only one location, we supply
the location's address as the PC register.  However, this only works
if PC is not a pseudo register.  Add a gdbarch hook that will handle
that for pseudo registers.

gdb/ChangeLog:

	* gdbarch.c: Regenerate.
	* gdbarch.h: Regenerate.
	* gdbarch.sh: Add write_guessed_tracepoint_pc hook.
	* tracefile.c (tracefile_fetch_registers): When PC is a pseudo,
	ask gdbarch to handle guessed PC via the new hook.
---
 gdb/ChangeLog   |  8 ++++++++
 gdb/gdbarch.c   | 32 ++++++++++++++++++++++++++++++++
 gdb/gdbarch.h   |  6 ++++++
 gdb/gdbarch.sh  |  1 +
 gdb/tracefile.c | 40 +++++++++++++++++++++++++++-------------
 5 files changed, 74 insertions(+), 13 deletions(-)

Comments

Andreas Arnez Jan. 26, 2016, 2:58 p.m. UTC | #1 
On Sun, Jan 24 2016, Marcin Kościelnicki wrote:

> When we're looking at a tracefile trace frame where registers are not
> available, and the tracepoint has only one location, we supply
> the location's address as the PC register.  However, this only works
> if PC is not a pseudo register.  Add a gdbarch hook that will handle
> that for pseudo registers.

> diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
> index 4ac6b90..b67ea72 100755
> --- a/gdb/gdbarch.sh
> +++ b/gdb/gdbarch.sh
> @@ -417,6 +417,7 @@ v:int:char_signed:::1:-1:1
>  #
>  F:CORE_ADDR:read_pc:struct regcache *regcache:regcache
>  F:void:write_pc:struct regcache *regcache, CORE_ADDR val:regcache, val
> +F:void:write_guessed_tracepoint_pc:struct regcache *regcache, CORE_ADDR val:regcache, val

Rather than write_guessed_tracepoint_pc, maybe the method could be named
more generically, like "supply_pc"?  Also, a comment describing the
method's purpose and interface would be nice, particularly how it
differs from the "write_pc" method or from regcache_write_pc().

>  # Function for getting target's idea of a frame pointer.  FIXME: GDB's
>  # whole scheme for dealing with "frames" and "frame pointers" needs a
>  # serious shakedown.
> diff --git a/gdb/tracefile.c b/gdb/tracefile.c
> index fef4ed9..7c2649d 100644
> --- a/gdb/tracefile.c
> +++ b/gdb/tracefile.c
> @@ -396,16 +396,18 @@ tracefile_fetch_registers (struct regcache *regcache, int regno)
>       as the address of the tracepoint.  */
>    pc_regno = gdbarch_pc_regnum (gdbarch);
>  
> -  /* XXX This guessing code below only works if the PC register isn't
> -     a pseudo-register.  The value of a pseudo-register isn't stored
> -     in the (non-readonly) regcache -- instead it's recomputed
> -     (probably from some other cached raw register) whenever the
> -     register is read.  This guesswork should probably move to some
> -     higher layer.  */

I wonder whether the last statement in this comment still applies?

> -  if (pc_regno < 0 || pc_regno >= gdbarch_num_regs (gdbarch))
> +  if (pc_regno < 0)
>      return;
>  
> -  if (regno == -1 || regno == pc_regno)
> +  /* We try to guess PC if:
> +
> +     1) We want all registers, or
> +     2) PC is a real register, and we want exactly it, or
> +     3) PC is a pseudo register (we don't know which real register it
> +        corresponds to, so let's try to play safe).  */
> +
> +  if (regno == -1 || regno == pc_regno ||
> +      pc_regno >= gdbarch_num_regs (gdbarch))
>      {
>        struct tracepoint *tp = get_tracepoint (get_tracepoint_number ());
>        gdb_byte *regs;
> @@ -429,11 +431,23 @@ tracefile_fetch_registers (struct regcache *regcache, int regno)
>  	      return;
>  	    }
>  
> -	  regs = (gdb_byte *) alloca (register_size (gdbarch, pc_regno));
> -	  store_unsigned_integer (regs, register_size (gdbarch, pc_regno),
> -				  gdbarch_byte_order (gdbarch),
> -				  tp->base.loc->address);
> -	  regcache_raw_supply (regcache, pc_regno, regs);
> +	  if (pc_regno >= gdbarch_num_regs (gdbarch))
> +	    {
> +	      /* PC is a pseudo, let gdbarch deal with that.  If it doesn't
> +	         know how, just bail.  */

Nit: The indentation space of this line and the second one below
contains 8 consecutive spaces where a tab could be used instead.  This
occurs several times in the patch series.

> +	      if (gdbarch_write_guessed_tracepoint_pc_p (gdbarch))
> +	        gdbarch_write_guessed_tracepoint_pc (gdbarch, regcache,
> +						     tp->base.loc->address);
> +	    }
> +	  else
> +	    {
> +	      /* PC is a real register.  */
> +	      regs = (gdb_byte *) alloca (register_size (gdbarch, pc_regno));
> +	      store_unsigned_integer (regs, register_size (gdbarch, pc_regno),
> +				      gdbarch_byte_order (gdbarch),
> +				      tp->base.loc->address);
> +	      regcache_raw_supply (regcache, pc_regno, regs);
> +	    }
>  	}
>      }
>  }
diff mbox

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 057c14f..983a243 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,11 @@ 
+2016-01-24  Marcin Kościelnicki  <koriakin@0x04.net>
+
+	* gdbarch.c: Regenerate.
+	* gdbarch.h: Regenerate.
+	* gdbarch.sh: Add write_guessed_tracepoint_pc hook.
+	* tracefile.c (tracefile_fetch_registers): When PC is a pseudo,
+	ask gdbarch to handle guessed PC via the new hook.
+
 2016-01-22  John Baldwin  <jhb@FreeBSD.org>
 
 	* fbsd-nat.c (fbsd_pid_to_str): Adjust string format.
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index 4143744..21942b0 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -181,6 +181,7 @@  struct gdbarch
   int char_signed;
   gdbarch_read_pc_ftype *read_pc;
   gdbarch_write_pc_ftype *write_pc;
+  gdbarch_write_guessed_tracepoint_pc_ftype *write_guessed_tracepoint_pc;
   gdbarch_virtual_frame_pointer_ftype *virtual_frame_pointer;
   gdbarch_pseudo_register_read_ftype *pseudo_register_read;
   gdbarch_pseudo_register_read_value_ftype *pseudo_register_read_value;
@@ -523,6 +524,7 @@  verify_gdbarch (struct gdbarch *gdbarch)
     gdbarch->char_signed = 1;
   /* Skip verify of read_pc, has predicate.  */
   /* Skip verify of write_pc, has predicate.  */
+  /* Skip verify of write_guessed_tracepoint_pc, has predicate.  */
   /* Skip verify of virtual_frame_pointer, invalid_p == 0 */
   /* Skip verify of pseudo_register_read, has predicate.  */
   /* Skip verify of pseudo_register_read_value, has predicate.  */
@@ -1402,6 +1404,12 @@  gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file)
                       "gdbarch_dump: vtable_function_descriptors = %s\n",
                       plongest (gdbarch->vtable_function_descriptors));
   fprintf_unfiltered (file,
+                      "gdbarch_dump: gdbarch_write_guessed_tracepoint_pc_p() = %d\n",
+                      gdbarch_write_guessed_tracepoint_pc_p (gdbarch));
+  fprintf_unfiltered (file,
+                      "gdbarch_dump: write_guessed_tracepoint_pc = <%s>\n",
+                      host_address_to_string (gdbarch->write_guessed_tracepoint_pc));
+  fprintf_unfiltered (file,
                       "gdbarch_dump: gdbarch_write_pc_p() = %d\n",
                       gdbarch_write_pc_p (gdbarch));
   fprintf_unfiltered (file,
@@ -1821,6 +1829,30 @@  set_gdbarch_write_pc (struct gdbarch *gdbarch,
   gdbarch->write_pc = write_pc;
 }
 
+int
+gdbarch_write_guessed_tracepoint_pc_p (struct gdbarch *gdbarch)
+{
+  gdb_assert (gdbarch != NULL);
+  return gdbarch->write_guessed_tracepoint_pc != NULL;
+}
+
+void
+gdbarch_write_guessed_tracepoint_pc (struct gdbarch *gdbarch, struct regcache *regcache, CORE_ADDR val)
+{
+  gdb_assert (gdbarch != NULL);
+  gdb_assert (gdbarch->write_guessed_tracepoint_pc != NULL);
+  if (gdbarch_debug >= 2)
+    fprintf_unfiltered (gdb_stdlog, "gdbarch_write_guessed_tracepoint_pc called\n");
+  gdbarch->write_guessed_tracepoint_pc (regcache, val);
+}
+
+void
+set_gdbarch_write_guessed_tracepoint_pc (struct gdbarch *gdbarch,
+                                         gdbarch_write_guessed_tracepoint_pc_ftype write_guessed_tracepoint_pc)
+{
+  gdbarch->write_guessed_tracepoint_pc = write_guessed_tracepoint_pc;
+}
+
 void
 gdbarch_virtual_frame_pointer (struct gdbarch *gdbarch, CORE_ADDR pc, int *frame_regnum, LONGEST *frame_offset)
 {
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index 3fadcd1..9e24d15 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -239,6 +239,12 @@  typedef void (gdbarch_write_pc_ftype) (struct regcache *regcache, CORE_ADDR val)
 extern void gdbarch_write_pc (struct gdbarch *gdbarch, struct regcache *regcache, CORE_ADDR val);
 extern void set_gdbarch_write_pc (struct gdbarch *gdbarch, gdbarch_write_pc_ftype *write_pc);
 
+extern int gdbarch_write_guessed_tracepoint_pc_p (struct gdbarch *gdbarch);
+
+typedef void (gdbarch_write_guessed_tracepoint_pc_ftype) (struct regcache *regcache, CORE_ADDR val);
+extern void gdbarch_write_guessed_tracepoint_pc (struct gdbarch *gdbarch, struct regcache *regcache, CORE_ADDR val);
+extern void set_gdbarch_write_guessed_tracepoint_pc (struct gdbarch *gdbarch, gdbarch_write_guessed_tracepoint_pc_ftype *write_guessed_tracepoint_pc);
+
 /* Function for getting target's idea of a frame pointer.  FIXME: GDB's
    whole scheme for dealing with "frames" and "frame pointers" needs a
    serious shakedown. */
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index 4ac6b90..b67ea72 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -417,6 +417,7 @@  v:int:char_signed:::1:-1:1
 #
 F:CORE_ADDR:read_pc:struct regcache *regcache:regcache
 F:void:write_pc:struct regcache *regcache, CORE_ADDR val:regcache, val
+F:void:write_guessed_tracepoint_pc:struct regcache *regcache, CORE_ADDR val:regcache, val
 # Function for getting target's idea of a frame pointer.  FIXME: GDB's
 # whole scheme for dealing with "frames" and "frame pointers" needs a
 # serious shakedown.
diff --git a/gdb/tracefile.c b/gdb/tracefile.c
index fef4ed9..7c2649d 100644
--- a/gdb/tracefile.c
+++ b/gdb/tracefile.c
@@ -396,16 +396,18 @@  tracefile_fetch_registers (struct regcache *regcache, int regno)
      as the address of the tracepoint.  */
   pc_regno = gdbarch_pc_regnum (gdbarch);
 
-  /* XXX This guessing code below only works if the PC register isn't
-     a pseudo-register.  The value of a pseudo-register isn't stored
-     in the (non-readonly) regcache -- instead it's recomputed
-     (probably from some other cached raw register) whenever the
-     register is read.  This guesswork should probably move to some
-     higher layer.  */
-  if (pc_regno < 0 || pc_regno >= gdbarch_num_regs (gdbarch))
+  if (pc_regno < 0)
     return;
 
-  if (regno == -1 || regno == pc_regno)
+  /* We try to guess PC if:
+
+     1) We want all registers, or
+     2) PC is a real register, and we want exactly it, or
+     3) PC is a pseudo register (we don't know which real register it
+        corresponds to, so let's try to play safe).  */
+
+  if (regno == -1 || regno == pc_regno ||
+      pc_regno >= gdbarch_num_regs (gdbarch))
     {
       struct tracepoint *tp = get_tracepoint (get_tracepoint_number ());
       gdb_byte *regs;
@@ -429,11 +431,23 @@  tracefile_fetch_registers (struct regcache *regcache, int regno)
 	      return;
 	    }
 
-	  regs = (gdb_byte *) alloca (register_size (gdbarch, pc_regno));
-	  store_unsigned_integer (regs, register_size (gdbarch, pc_regno),
-				  gdbarch_byte_order (gdbarch),
-				  tp->base.loc->address);
-	  regcache_raw_supply (regcache, pc_regno, regs);
+	  if (pc_regno >= gdbarch_num_regs (gdbarch))
+	    {
+	      /* PC is a pseudo, let gdbarch deal with that.  If it doesn't
+	         know how, just bail.  */
+	      if (gdbarch_write_guessed_tracepoint_pc_p (gdbarch))
+	        gdbarch_write_guessed_tracepoint_pc (gdbarch, regcache,
+						     tp->base.loc->address);
+	    }
+	  else
+	    {
+	      /* PC is a real register.  */
+	      regs = (gdb_byte *) alloca (register_size (gdbarch, pc_regno));
+	      store_unsigned_integer (regs, register_size (gdbarch, pc_regno),
+				      gdbarch_byte_order (gdbarch),
+				      tp->base.loc->address);
+	      regcache_raw_supply (regcache, pc_regno, regs);
+	    }
 	}
     }
 }