[PATCH] Remove isize output argument from fast_tracepoint_valid_at

[Pierre Langlois <pierre dot langlois at arm dot com>]

Hi all,

This patch removes the isize output argument from the
fast_tracepoint_valid_at gdbarch hook.  It was used to return the size
of the instruction that needs to be replaced when installing a fast
tracepoint.  Instead of getting this value from the
fast_tracepoint_valid_at hook, we can call the gdb_insn_length function.

If we do not do this, then architectures which do not have a restriction
on where to install the fast tracepoint will send uninitialized memory
off to GDBserver.  See remote_download_tracepoint:

~~~
int isize;

if (gdbarch_fast_tracepoint_valid_at (target_gdbarch (),
				      tpaddr, &isize, NULL))
  xsnprintf (buf + strlen (buf), BUF_SIZE - strlen (buf), ":F%x",
	     isize);
~~~

The default implementation of fast_tracepoint_valid_at will not set
isize resulting in uninitialized memory being sent.  Later on, GDBserver
could use this information to compute a jump offset.

Regression tested with native-gdbserver on x86_64.

Thanks,
Pierre

gdb/ChangeLog:

	* arch-utils.c (default_fast_tracepoint_valid_at): Remove unused
	isize argument.
	* arch-utils.h (default_fast_tracepoint_valid_at): Likewise.
	* breakpoint.c (check_fast_tracepoint_sals): Adjust call to
	gdbarch_fast_tracepoint_valid_at.
	* gdbarch.sh (fast_tracepoint_valid_at): Remove isize argument.
	* gdbarch.h: Regenerate.
	* gdbarch.c: Regenerate.
	* i386-tdep.c (i386_fast_tracepoint_valid_at): Remove isize
	argument.  Do not set it.
	* remote.c (remote_download_tracepoint): Adjust call to
	gdbarch_fast_tracepoint_valid_at.  Call gdb_insn_length to get
	the instruction length.
---
 gdb/arch-utils.c | 4 ++--
 gdb/arch-utils.h | 3 +--
 gdb/breakpoint.c | 3 +--
 gdb/gdbarch.c    | 4 ++--
 gdb/gdbarch.h    | 4 ++--
 gdb/gdbarch.sh   | 2 +-
 gdb/i386-tdep.c  | 6 ++----
 gdb/remote.c     | 9 ++++-----
 8 files changed, 15 insertions(+), 20 deletions(-)

diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c
index e9c622d..46a6db0 100644
--- a/gdb/arch-utils.c
+++ b/gdb/arch-utils.c
@@ -795,8 +795,8 @@ default_has_shared_address_space (struct gdbarch *gdbarch)
 }
 
 int
-default_fast_tracepoint_valid_at (struct gdbarch *gdbarch,
-				  CORE_ADDR addr, int *isize, char **msg)
+default_fast_tracepoint_valid_at (struct gdbarch *gdbarch, CORE_ADDR addr,
+				  char **msg)
 {
   /* We don't know if maybe the target has some way to do fast
      tracepoints that doesn't need gdbarch, so always say yes.  */
diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h
index 27f4787..18e2290 100644
--- a/gdb/arch-utils.h
+++ b/gdb/arch-utils.h
@@ -161,8 +161,7 @@ extern struct gdbarch *get_current_arch (void);
 extern int default_has_shared_address_space (struct gdbarch *);
 
 extern int default_fast_tracepoint_valid_at (struct gdbarch *gdbarch,
-					     CORE_ADDR addr,
-					     int *isize, char **msg);
+					     CORE_ADDR addr, char **msg);
 
 extern void default_remote_breakpoint_from_pc (struct gdbarch *,
 					       CORE_ADDR *pcptr, int *kindptr);
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index af0d167..2a55a6f 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -9406,8 +9406,7 @@ check_fast_tracepoint_sals (struct gdbarch *gdbarch,
 	 associated with SAL.  */
       if (sarch == NULL)
 	sarch = gdbarch;
-      rslt = gdbarch_fast_tracepoint_valid_at (sarch, sal->pc,
-					       NULL, &msg);
+      rslt = gdbarch_fast_tracepoint_valid_at (sarch, sal->pc, &msg);
       old_chain = make_cleanup (xfree, msg);
 
       if (!rslt)
diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c
index c289334..07b38a3 100644
--- a/gdb/gdbarch.c
+++ b/gdb/gdbarch.c
@@ -4394,13 +4394,13 @@ set_gdbarch_has_shared_address_space (struct gdbarch *gdbarch,
 }
 
 int
-gdbarch_fast_tracepoint_valid_at (struct gdbarch *gdbarch, CORE_ADDR addr, int *isize, char **msg)
+gdbarch_fast_tracepoint_valid_at (struct gdbarch *gdbarch, CORE_ADDR addr, char **msg)
 {
   gdb_assert (gdbarch != NULL);
   gdb_assert (gdbarch->fast_tracepoint_valid_at != NULL);
   if (gdbarch_debug >= 2)
     fprintf_unfiltered (gdb_stdlog, "gdbarch_fast_tracepoint_valid_at called\n");
-  return gdbarch->fast_tracepoint_valid_at (gdbarch, addr, isize, msg);
+  return gdbarch->fast_tracepoint_valid_at (gdbarch, addr, msg);
 }
 
 void
diff --git a/gdb/gdbarch.h b/gdb/gdbarch.h
index 7d6a0cf..d714281 100644
--- a/gdb/gdbarch.h
+++ b/gdb/gdbarch.h
@@ -1310,8 +1310,8 @@ extern void set_gdbarch_has_shared_address_space (struct gdbarch *gdbarch, gdbar
 
 /* True if a fast tracepoint can be set at an address. */
 
-typedef int (gdbarch_fast_tracepoint_valid_at_ftype) (struct gdbarch *gdbarch, CORE_ADDR addr, int *isize, char **msg);
-extern int gdbarch_fast_tracepoint_valid_at (struct gdbarch *gdbarch, CORE_ADDR addr, int *isize, char **msg);
+typedef int (gdbarch_fast_tracepoint_valid_at_ftype) (struct gdbarch *gdbarch, CORE_ADDR addr, char **msg);
+extern int gdbarch_fast_tracepoint_valid_at (struct gdbarch *gdbarch, CORE_ADDR addr, char **msg);
 extern void set_gdbarch_fast_tracepoint_valid_at (struct gdbarch *gdbarch, gdbarch_fast_tracepoint_valid_at_ftype *fast_tracepoint_valid_at);
 
 /* Return the "auto" target charset. */
diff --git a/gdb/gdbarch.sh b/gdb/gdbarch.sh
index 6c5d684..14a5f9c 100755
--- a/gdb/gdbarch.sh
+++ b/gdb/gdbarch.sh
@@ -1018,7 +1018,7 @@ v:int:has_global_breakpoints:::0:0::0
 m:int:has_shared_address_space:void:::default_has_shared_address_space::0
 
 # True if a fast tracepoint can be set at an address.
-m:int:fast_tracepoint_valid_at:CORE_ADDR addr, int *isize, char **msg:addr, isize, msg::default_fast_tracepoint_valid_at::0
+m:int:fast_tracepoint_valid_at:CORE_ADDR addr, char **msg:addr, msg::default_fast_tracepoint_valid_at::0
 
 # Return the "auto" target charset.
 f:const char *:auto_charset:void::default_auto_charset:default_auto_charset::0
diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
index 371a282..9d52d4a 100644
--- a/gdb/i386-tdep.c
+++ b/gdb/i386-tdep.c
@@ -8045,8 +8045,8 @@ static const int i386_record_regmap[] =
    string.  */
 
 static int
-i386_fast_tracepoint_valid_at (struct gdbarch *gdbarch,
-			       CORE_ADDR addr, int *isize, char **msg)
+i386_fast_tracepoint_valid_at (struct gdbarch *gdbarch, CORE_ADDR addr,
+			       char **msg)
 {
   int len, jumplen;
   static struct ui_file *gdb_null = NULL;
@@ -8078,8 +8078,6 @@ i386_fast_tracepoint_valid_at (struct gdbarch *gdbarch,
 
   /* Check for fit.  */
   len = gdb_print_insn (gdbarch, addr, gdb_null, NULL);
-  if (isize)
-    *isize = len;
 
   if (len < jumplen)
     {
diff --git a/gdb/remote.c b/gdb/remote.c
index 94899bd..3e685c4 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -44,6 +44,7 @@
 #include "gdb_bfd.h"
 #include "filestuff.h"
 #include "rsp-low.h"
+#include "disasm.h"
 
 #include <sys/time.h>
 
@@ -11082,12 +11083,10 @@ remote_download_tracepoint (struct target_ops *self, struct bp_location *loc)
 	 target capabilities at definition time.  */
       if (remote_supports_fast_tracepoints ())
 	{
-	  int isize;
-
-	  if (gdbarch_fast_tracepoint_valid_at (target_gdbarch (),
-						tpaddr, &isize, NULL))
+	  if (gdbarch_fast_tracepoint_valid_at (loc->gdbarch, tpaddr,
+						NULL))
 	    xsnprintf (buf + strlen (buf), BUF_SIZE - strlen (buf), ":F%x",
-		       isize);
+		       gdb_insn_length (loc->gdbarch, tpaddr));
 	  else
 	    /* If it passed validation at definition but fails now,
 	       something is very wrong.  */
-- 
2.4.6