Re: [RFC][PATCH] arm-tdep.c (arm_m_exception_cache): Handle stack switching to PSP during exception unwind.

[Sergio Durigan Junior <sergiodj@redhat.com>]

On Saturday, September 20 2014, Jon Burgess wrote:

> This my first GDB patch submission and I have not been through this FSF
> copyright assignment process.

Hi Jon,

Thanks for the patch.  I can submit the necessary form offlist so that
you can get started in the process of obtaining the copyright assignment
process.

Meanwhile, here goes a non-technical review, which is meant to fix
formatting issues.

> 2014-09-20  Jon Burgess  <jburgess777@gmail.com>
>
>         * arm-tdep.c (arm_m_exception_cache): Handle stack switching
>        to PSP during exception unwind.

This is almost perfect; the only thing missing is that the entry need to
be indented with TAB instead of spaces.  Take a look at the
gdb/ChangeLog file for examples.

> diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
> index 5cdfc5b..66a0ae8 100644
> --- a/gdb/arm-tdep.c
> +++ b/gdb/arm-tdep.c
> @@ -3010,6 +3010,7 @@ arm_m_exception_cache (struct frame_info
> *this_frame)
>    struct gdbarch *gdbarch = get_frame_arch (this_frame);
>    enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
>    struct arm_prologue_cache *cache;
> +  CORE_ADDR this_pc = get_frame_pc (this_frame);
>    CORE_ADDR unwound_sp;
>    LONGEST xpsr;
>  
> @@ -3019,6 +3020,24 @@ arm_m_exception_cache (struct frame_info
> *this_frame)
>    unwound_sp = get_frame_register_unsigned (this_frame,
>                                             ARM_SP_REGNUM);
>  
> +  /* The EXC_RETURN address indicates what type of transition
> +     the CPU makes when returning from the exception. A value
> +     of 0xfffffffd causes the stack pointer to switch from
> +     MSP to PSP. */

Two spaces after dot.

  /* The EXC_RETURN address indicates what type of transition
     the CPU makes when returning from the exception.  A value
     of 0xfffffffd causes the stack pointer to switch from
     MSP to PSP.  */

> +  if (this_pc == 0xfffffffd) {
> +    int pspreg;
> +    struct regcache *regcache;
> +    struct value *pspval;
> +
> +    pspreg = user_reg_map_name_to_regnum (gdbarch, "psp", 3);
> +    gdb_assert (pspreg != -1);
> +
> +    regcache = get_current_regcache ();
> +    pspval = regcache_cooked_read_value (regcache, pspreg);
> +    if (pspval && !value_lazy (pspval))
> +      unwound_sp = value_as_address (pspval);
> +  }

The GNU Coding Style says that the open/close brackets should be
indented one line below and two whitespaces after the "if" column, so it
would be:

  if (this_pc == 0xfffffffd)
    {
      int pspreg;
      struct regcache *regcache;
      struct value *pspval;

      pspreg = user_reg_map_name_to_regnum (gdbarch, "psp", 3);
      gdb_assert (pspreg != -1);

      regcache = get_current_regcache ();
      pspval = regcache_cooked_read_value (regcache, pspreg);
      if (pspval && !value_lazy (pspval))
        unwound_sp = value_as_address (pspval);
    }

Also, if a line has 8 whitespaces, they have to be converted to a TAB
char (but this is not the case here).

> +
>    /* The hardware saves eight 32-bit words, comprising xPSR,
>       ReturnAddress, LR (R14), R12, R3, R2, R1, R0.  See details in
>       "B1.5.6 Exception entry behavior" in

Cheers,

-- 
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/