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]

Re: [PATCH 1/2] [ARM] Fixup PC in software single step


Yao Qi writes:

> When I exercise GDBserver software single step, I see the following
> error, which has been already handled by GDB properly.
>
> In GDBserver log, we can see, GDBserver tries to single step instruction
> on 0xb6e0a6e4, and destination address is 0xffff0fe0,
>
>  stop pc is 0xb6e0a6e4
>  Writing f001f0e7 to 0xffff0fe0 in process 7132
>  Failed to insert breakpoint at 0xffff0fe0 (Input/output error).
>  Failed to insert breakpoint at 0xffff0fe0 (-1).
>
> (gdb) disassemble __aeabi_read_tp,+8
> Dump of assembler code from 0xb6e0a6e0 to 0xb6e0a6e8:
>    0xb6e0a6e0 <__aeabi_read_tp+0>:	mvn	r0, #61440	; 0xf000
>    0xb6e0a6e4 <__aeabi_read_tp+4>:	sub	pc, r0, #31
>
> however, it fails inserting breakpoint there.  This problem has already
> fixed by GDB, see comments in arm-linux-tdep.c:arm_linux_software_single_step
>
>       /* The Linux kernel offers some user-mode helpers in a high page.  We can
> 	 not read this page (as of 2.6.23), and even if we could then we
> 	 couldn't set breakpoints in it, and even if we could then the atomic
> 	 operations would fail when interrupted.  They are all called as
> 	 functions and return to the address in LR, so step to there
> 	 instead.  */
>
> so we need to do the same thing in GDB side as well.  This patch adds
> a new field fixup in arm_get_next_pcs_ops, so that we can fix up PC
> for arm-linux target.  In this way, both GDB and GDBserver can single
> step instructions going to kernel helpers.
>
> gdb:
>
> 2016-02-11  Yao Qi  <yao.qi@linaro.org>
>
> 	* arch/arm-get-next-pcs.c (arm_get_next_pcs): Call
> 	self->ops->fixup if it isn't NULL.
> 	* arch/arm-get-next-pcs.h: Include gdb_vecs.h.
> 	(struct arm_get_next_pcs_ops) <fixup>: New field.
> 	* arch/arm-linux.c: Include common-regcache.h and
> 	arch/arm-get-next-pcs.h.
> 	(arm_linux_get_next_pcs_fixup): New function.
> 	* arch/arm-linux.h (arm_linux_get_next_pcs_fixup): Declare.
> 	* arm-linux-tdep.c (arm_linux_get_next_pcs_ops): Initialize
> 	it with arm_linux_get_next_pcs_fixup.
> 	(arm_linux_software_single_step): Move code to
> 	arm_linux_get_next_pcs_fixup.
> 	* arm-tdep.c (arm_get_next_pcs_ops): Initialize it.
>
> gdb/gdbserver:
>
> 2016-02-11  Yao Qi  <yao.qi@linaro.org>
>
> 	* linux-arm-low.c (get_next_pcs_ops): Initialize it with
> 	arm_linux_get_next_pcs_fixup.
> ---
>  gdb/arch/arm-get-next-pcs.c   | 11 +++++++++++
>  gdb/arch/arm-get-next-pcs.h   |  4 ++++
>  gdb/arch/arm-linux.c          | 20 ++++++++++++++++++++
>  gdb/arch/arm-linux.h          |  4 ++++
>  gdb/arm-linux-tdep.c          | 16 +++-------------
>  gdb/arm-tdep.c                |  3 ++-
>  gdb/gdbserver/linux-arm-low.c |  3 ++-
>  7 files changed, 46 insertions(+), 15 deletions(-)
>
> diff --git a/gdb/arch/arm-get-next-pcs.c b/gdb/arch/arm-get-next-pcs.c
> index e840147..8404869 100644
> --- a/gdb/arch/arm-get-next-pcs.c
> +++ b/gdb/arch/arm-get-next-pcs.c
> @@ -923,5 +923,16 @@ arm_get_next_pcs (struct arm_get_next_pcs *self)
>  	next_pcs = arm_get_next_pcs_raw (self);
>      }
>  
> +  if (self->ops->fixup != NULL)
> +    {
> +      CORE_ADDR nextpc;
> +      int i;
> +
> +      for (i = 0; VEC_iterate (CORE_ADDR, next_pcs, i, nextpc); i++)
> +	{
> +	  nextpc = self->ops->fixup (self, nextpc);
> +	  VEC_replace (CORE_ADDR, next_pcs, i, nextpc);
> +	}
> +    }
>    return next_pcs;
>  }
> diff --git a/gdb/arch/arm-get-next-pcs.h b/gdb/arch/arm-get-next-pcs.h
> index 7cb0858..e038982 100644
> --- a/gdb/arch/arm-get-next-pcs.h
> +++ b/gdb/arch/arm-get-next-pcs.h
> @@ -19,6 +19,7 @@
>  
>  #ifndef ARM_GET_NEXT_PCS_H
>  #define ARM_GET_NEXT_PCS_H 1
> +#include "gdb_vecs.h"
>  
>  /* Forward declaration.  */
>  struct arm_get_next_pcs;
> @@ -30,6 +31,9 @@ struct arm_get_next_pcs_ops
>    CORE_ADDR (*syscall_next_pc) (struct arm_get_next_pcs *self, CORE_ADDR pc);
>    CORE_ADDR (*addr_bits_remove) (struct arm_get_next_pcs *self, CORE_ADDR val);
>    int (*is_thumb) (struct arm_get_next_pcs *self);
> +
> +  /* Fix up PC if needed.  */
> +  CORE_ADDR (*fixup) (struct arm_get_next_pcs *self, CORE_ADDR pc);
>  };
>  
>  /* Context for a get_next_pcs call on ARM.  */
> diff --git a/gdb/arch/arm-linux.c b/gdb/arch/arm-linux.c
> index b43e484..457080c 100644
> --- a/gdb/arch/arm-linux.c
> +++ b/gdb/arch/arm-linux.c
> @@ -18,8 +18,10 @@
>     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>  
>  #include "common-defs.h"
> +#include "common-regcache.h"
>  #include "arch/arm.h"
>  #include "arm-linux.h"
> +#include "arch/arm-get-next-pcs.h"
>  
>  /* Calculate the offset from stack pointer of the pc register on the stack
>     in the case of a sigreturn or sigreturn_rt syscall.  */
> @@ -55,3 +57,21 @@ arm_linux_sigreturn_next_pc_offset (unsigned long sp,
>  
>    return pc_offset;
>  }
> +
> +/* Implementation of "fixup" method of struct arm_get_next_pcs_ops
> +   for arm-linux.  */
> +
> +CORE_ADDR
> +arm_linux_get_next_pcs_fixup (struct arm_get_next_pcs *self,
> +			      CORE_ADDR nextpc)
> +{
> +  /* The Linux kernel offers some user-mode helpers in a high page.  We can
> +     not read this page (as of 2.6.23), and even if we could then we
> +     couldn't set breakpoints in it, and even if we could then the atomic
> +     operations would fail when interrupted.  They are all called as
> +     functions and return to the address in LR, so step to there
> +     instead.  */
> +  if (nextpc > 0xffff0000)
> +    nextpc = regcache_raw_get_unsigned (self->regcache, ARM_LR_REGNUM);
> +  return nextpc;
> +}
> diff --git a/gdb/arch/arm-linux.h b/gdb/arch/arm-linux.h
> index 31cb351..dfd634c 100644
> --- a/gdb/arch/arm-linux.h
> +++ b/gdb/arch/arm-linux.h
> @@ -71,4 +71,8 @@ arm_linux_sigreturn_next_pc_offset (unsigned long sp,
>  				    unsigned long svc_number,
>  				    int is_sigreturn);
>  
> +struct arm_get_next_pcs;
> +
> +CORE_ADDR arm_linux_get_next_pcs_fixup (struct arm_get_next_pcs *self,
> +					CORE_ADDR pc);
>  #endif /* ARM_LINUX_H */
> diff --git a/gdb/arm-linux-tdep.c b/gdb/arm-linux-tdep.c
> index 3421f3b..3dbf649 100644
> --- a/gdb/arm-linux-tdep.c
> +++ b/gdb/arm-linux-tdep.c
> @@ -274,7 +274,8 @@ static struct arm_get_next_pcs_ops arm_linux_get_next_pcs_ops = {
>    arm_get_next_pcs_read_memory_unsigned_integer,
>    arm_linux_get_next_pcs_syscall_next_pc,
>    arm_get_next_pcs_addr_bits_remove,
> -  arm_get_next_pcs_is_thumb
> +  arm_get_next_pcs_is_thumb,
> +  arm_linux_get_next_pcs_fixup,
>  };
>  
>  static void
> @@ -950,18 +951,7 @@ arm_linux_software_single_step (struct frame_info *frame)
>    next_pcs = arm_get_next_pcs (&next_pcs_ctx);
>  
>    for (i = 0; VEC_iterate (CORE_ADDR, next_pcs, i, pc); i++)
> -    {
> -      /* The Linux kernel offers some user-mode helpers in a high page.  We can
> -	 not read this page (as of 2.6.23), and even if we could then we
> -	 couldn't set breakpoints in it, and even if we could then the atomic
> -	 operations would fail when interrupted.  They are all called as
> -	 functions and return to the address in LR, so step to there
> -	 instead.  */
> -      if (pc > 0xffff0000)
> -	pc = get_frame_register_unsigned (frame, ARM_LR_REGNUM);
> -
> -      arm_insert_single_step_breakpoint (gdbarch, aspace, pc);
> -    }
> +    arm_insert_single_step_breakpoint (gdbarch, aspace, pc);
>  
>    do_cleanups (old_chain);
>  
> diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
> index 6ac05f0..bf59ea7 100644
> --- a/gdb/arm-tdep.c
> +++ b/gdb/arm-tdep.c
> @@ -247,7 +247,8 @@ static struct arm_get_next_pcs_ops arm_get_next_pcs_ops = {
>    arm_get_next_pcs_read_memory_unsigned_integer,
>    arm_get_next_pcs_syscall_next_pc,
>    arm_get_next_pcs_addr_bits_remove,
> -  arm_get_next_pcs_is_thumb
> +  arm_get_next_pcs_is_thumb,
> +  NULL,
>  };
>  
>  struct arm_prologue_cache
> diff --git a/gdb/gdbserver/linux-arm-low.c b/gdb/gdbserver/linux-arm-low.c
> index 0f62706..365f1c9 100644
> --- a/gdb/gdbserver/linux-arm-low.c
> +++ b/gdb/gdbserver/linux-arm-low.c
> @@ -157,7 +157,8 @@ static struct arm_get_next_pcs_ops get_next_pcs_ops = {
>    get_next_pcs_read_memory_unsigned_integer,
>    get_next_pcs_syscall_next_pc,
>    get_next_pcs_addr_bits_remove,
> -  get_next_pcs_is_thumb
> +  get_next_pcs_is_thumb,
> +  arm_linux_get_next_pcs_fixup,
>  };
>  
>  static int

Thanks for this, LGTM.


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