From 6b940e6a063ac13372b44a03a54b6be33d22a183 Mon Sep 17 00:00:00 2001 From: Pierre Langlois Date: Thu, 30 Jul 2015 18:05:00 +0100 Subject: [PATCH] Remove isize output argument from fast_tracepoint_valid_at 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. 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/ChangeLog | 16 ++++++++++++++++ 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 ++++----- 9 files changed, 31 insertions(+), 20 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 0e0f62c3103..4d604de556a 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,19 @@ +2015-07-30 Pierre Langlois + + * 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. + 2015-07-30 Yao Qi * arm-tdep.h (enum gdb_regnum): Move it to ... diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c index e9c622dd873..46a6db0896f 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 27f4787e842..18e22907c35 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 af0d16730c3..2a55a6f731c 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 c2893341d4b..07b38a3b633 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 7d6a0cfa79f..d714281b837 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 6c5d684b4d3..14a5f9c5ebe 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 371a2828df3..9d52d4ad5e1 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 69da5084add..4ac393b28f8 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 @@ -11106,12 +11107,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.30.2