Re: Re: [PATCH] Fix exception unwinding for ARM Cortex-M

["Fredrik Hederstierna" <fredrik.hederstierna@verisure.com>]

Hi James-Adam,

-----James-Adam Renquinha Henri <arenquinha@cimeq.qc.ca> wrote: -----
>Hi, I'm back from the dead.

Nice to see you back :)


>Seriously though, I was a bit too busy at work to pursue on this and
>I 
>didn't have a clear idea of where to start after the comments.
>Moreover, 
>as the patch did "just work" at my workplace, it was a bit difficult
>to 
>justify spending a lot more time digging information about target 
>descriptions, how to determine what features are present on target,
>etc.
>So thanks to push toward the progress of this issue, apparently with 
>your mail subject you successfully brought people that gives more 
>specific guidance to get this done.

I tried to check the status on your submission mid-July (https://sourceware.org/ml/gdb/2016-07/msg00011.html),
but since I got no directions from anyone in the community at that time, I decided to try continue myself to get this fixed.
We work alot with Cortex-M4F on a daily basis, and do really need this to be fixed.


>Indeed, the patch looks really close to what I've done, to the point 
>that I can see that, aside from a small refactoring, the changes
>between 
>our patches are mainly variable name changes and added comments. You 
>really wrote your patch from scratch? You've got rid of the various 
>calls to `user_reg_map_name_to_regnum', though, which is nice.

Yes I wrote it from scratch, but I agree my patch looks similar to yours, as its implementing according to ARM Cortex-M spec,
I guess it needs to be. But it absolutely helped me to have your ideas as starting point to look at, to see where modifications needed to be done,
so I do not mind if you would like to stand as co-author in the ChangeLog?
If you have FSF GDB Assignment, or would like to apply for assignment,
I can add your name to ChangeLog, its totally fine by me.

The important thing for me is to get this fixed, and who gets the actual and final credits I do not mind.
And I think it would be great if we could try to continue this work together, to get the remaining parts for Cortex-M support in GDB.
The more people who are working on this, the better.
I am not so experienced in GDB internals, and also seek guidance how to continue go get the last parts in place.
I think both to get GDB aware of MSP and PSP, and also FPCCR needs to be a bigger change and are more complicated.
My idea is to submit a bug entry in bugzilla on this, one issue for the MSP/PSP support, another one for FPCCR, so we can get track of this, and someone could be assigned.


> > Cortex-M has two stack pointers: MSP (Main Stack Pointer) and PSP 
>(Process Stack Pointer).
> > This is not handled when GDB tries to backtrace in the exception 
>stack unwinder.
> > Meaning for eg. Cortex-M4F its not possible to get any call-stack 
>backtrace if setting a breakpoint in ISR, and floating-point variable
>was used.
>
>Actually, the info here is inaccurate: FPU context stacking and the 
>MSP/PSP switch are two unrelated concepts. Meaning, you can fix the
>code 
>to deal with the PSP, but it still won't handle FPU register
>unstacking.

Correct, my plan now was to split the patch as Yao proposed.
The the MSP/PSP-fix and FPU-fix would be two different patches.


> > This patch was inspired by the great work done by James-Adam 
>Renquinha Henri, submitted April this year.
>Thanks :)
>
> > The next thing would then be to also add FPU context control reg 
>FPCCR, which is needed for retrieving info on the FPU lazy stacking.
> > Though its complicated I think and I will try to investigate an 
>'arm-m-fpu.xml' profile further, if this is solution perhaps.
>
>It indeed is just a bit more complicated. Let me summarize what needs
>to 
>be done. Have the ARMv7-M Architecture Reference Manual handy, see
>B1.5.7.
>
>- Check if lazy stacking is enabled (FPCCR.LSPEN == 1). If it's not,
>the 
>case is uncomplicated, registers are stacked as usual
>- If lazy stacking is active (FPCCR.LSACT == 1), the extended stack
>has 
>space reserved for the FPU registers (S0-S15, FPSCR), but there are
>not 
>stacked, they are still in FPU registers unmodified.
>
>So that adds more random fetches from "memory" of the target, because
>
>these are memory-mapped.
>
>ARM gives some example scenarios with chronograms of some important
>bits 
>with stacking information [1].

Yes, my idea was that GDB needs to be aware of FPCCR, so we need probably an new XML profile feature for this, just as for PSP/MSP.
But you are correct, its way more complicated. See also ARM Application Note I'm referring to in my patch comments.


> > ChangeLog entry should cover what do you change on the function
>level.
> > Please read https://sourceware.org/gdb/wiki/ContributionChecklist
>
>If only I knew that checklist! I searched for something like this,
>but 
>maybe it didn't use the correct search terms, I didn't find it.

If you want to stand as co-author in ChangeLog, its totally fine by me,
just give me a hint if you have the FSF assignment done,
and I also would be more than happy if you have time to continue your great work, and get the last parts of unwinding support for Cortex-M in place.


Thanks, and Kind Regards,
Fredrik