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

[Pedro Alves <palves@redhat.com>]

On 09/23/2014 06:26 AM, Yao Qi wrote:
> Jon Burgess <jburgess777@gmail.com> writes:
> 
>> +  /* The EXC_RETURN address indicates what type of transition
> 
> Nitpick: I'd like to say "EXC_RETURN (exception return address)" rather
> than "EXC_RETURN address", because its value is the magic address.
> 
>> +     the CPU makes when returning from the exception. A value
>> +     of 0xfffffffd causes the stack pointer to switch from
>> +     MSP to PSP. */
> 
> I'd like to replace MSP and PSP with SP_main and SP_process, which are
> used in the ARMv7-M ARM.
> 
> These details in your comments are described in ARMv7-M ARM.  IWBN to
> add something like:
> 
> See details in "B1.5.8 Exception return behavior" in "ARMv7-M
> Architecture Reference Manual".
> 
>> +  if (this_pc == 0xfffffffd) {
>> +    int pspreg;
>> +    struct regcache *regcache;
>> +    struct value *pspval;
>> +
>> +    pspreg = user_reg_map_name_to_regnum (gdbarch, "psp", 3);
> 
> The main sp and process sp aren't in the gdb's target description.
> Different gdb stubs (jtag probes) may name them differently.  OpenOCD
> names it as "psp", but our codesourcery sprite names it as "sp_main".
> We may need an array for the names of process sp, and iterate the array
> to find the number by different name variants.

Let's start by fixing this properly, please.  If GDB needs to know
about these registers, then they should be part of a known feature
in the target description.  According to B1.4 in the same document,
we see:

"The ARMv7-M profile has the following registers closely coupled to the core:

- general purpose registers R0-R12
- 2 Stack Pointer registers, SP_main and SP_process (banked versions of R13)
- the Link Register, LR (R14)
- the Program Counter, PC
- status registers for flags, exception/interrupt level, and execution state bits
- mask registers associated with managing the prioritization scheme for exceptions and interrupts
- a control register (CONTROL) to identify the current stack and thread mode privilege level.
"

Seems like even more core things other than SP_main/SP_process are
missing from org.gnu.gdb.arm.m-profile, when debugging at this level.

> 
>> +    gdb_assert (pspreg != -1);
> 
> This is too strong to me.  If the process sp isn't found, it means GDB
> stub doesn't provide process sp in the target description or the name of
> process sp isn't recognized by GDB.  It is not GDB's fault.  We can emit
> an error here, IMO.
> 
>> +
>> +    regcache = get_current_regcache ();
>> +    pspval = regcache_cooked_read_value (regcache, pspreg);
> 
> In general, getting the register value in unwinding from current
> regcache is wrong, because register value should be got from the
> previous frame.  However, in the case that getting process sp in
> exception on cortex-m, it is correct.  At this point, the program is
> still in exception, and main sp is used (process sp isn't used nor changed).
> When GDB is unwinding frames from the exception to the application, the
> process sp register is still valid.

I suppose this would still break if we have nested exceptions.

Would it not work to get the register from the frame instead of
the regcache directly?  If nothing saves/clobbers it up the frame
chain, the sentinel frame will end up unwinding/reading it from
the current regcache just the same.

Thanks,
Pedro Alves