Re: [PATCH v7 4/8] Refactor arm_software_single_step to use regcache.

Yao Qi
2015-12-11T11:37:17Z

Antoine Tremblay <antoine.tremblay@ericsson.com> writes:

Hi Antoine,
Patch is OK to me, two nits below.

> diff --git a/gdb/arm-linux-tdep.c b/gdb/arm-linux-tdep.c
> index acb5701..eb4341c 100644
> --- a/gdb/arm-linux-tdep.c
> +++ b/gdb/arm-linux-tdep.c
> @@ -808,6 +808,68 @@ arm_linux_sigreturn_return_addr (struct frame_info *frame,
> return 0;
> }
>
> +/* Calculate the offset from stack pointer of the pc register on the stack
> + in the case of a sigreturn or sigreturn_rt syscall. */
> +static int
> +arm_linux_sigreturn_next_pc_offset (unsigned long sp,
> + unsigned long sp_data,
> + unsigned long svc_number,
> + int is_sigreturn)
> +{
> + /* Offset of R0 register. */
> + int r0_offset = 0;
> + /* Offset of PC register. */
> + int pc_offset = 0;
> +
> + if (is_sigreturn)
> + {
> + if (sp_data == ARM_NEW_SIGFRAME_MAGIC)
> + r0_offset = ARM_UCONTEXT_SIGCONTEXT + ARM_SIGCONTEXT_R0;
> + else
> + r0_offset = ARM_SIGCONTEXT_R0;
> + }
> + else
> + {
> + if (sp_data == sp + ARM_OLD_RT_SIGFRAME_SIGINFO)
> + r0_offset = ARM_OLD_RT_SIGFRAME_UCONTEXT + ARM_UCONTEXT_SIGCONTEXT
> + + ARM_SIGCONTEXT_R0;
> + else
> + r0_offset = ARM_NEW_RT_SIGFRAME_UCONTEXT + ARM_UCONTEXT_SIGCONTEXT
> + + ARM_SIGCONTEXT_R0;

Nit: we can write these long lines like this,

if (sp_data == sp + ARM_OLD_RT_SIGFRAME_SIGINFO)
r0_offset = ARM_OLD_RT_SIGFRAME_UCONTEXT;
else
r0_offset = ARM_NEW_RT_SIGFRAME_UCONTEXT;

r0_offset += (ARM_UCONTEXT_SIGCONTEXT + ARM_SIGCONTEXT_R0);

>
> - arm_linux_sigreturn_return_addr (frame, svc_number, &return_addr, &is_thumb);
> + if (is_syscall (gdbarch, "sigreturn", svc_number)
> + || is_syscall (gdbarch, "rt_sigreturn", svc_number))
> + next_pc = arm_linux_sigreturn_next_pc (regcache, svc_number);
>
> /* Addresses for calling Thumb functions have the bit 0 set. */
> if (is_thumb)
> - return_addr |= 1;
> + next_pc |= 1;

Nit: use MAKE_THUMB_ADDR.

--
Yao (齐尧)