[RFC,2/5] gdb/arm: PR 29716 Fix FNC_RETURN stack selection in exception unwinder
Commit Message
Unwinding of FNC_RETURN selected the process stack whenever zero IPSR
indicated thread mode.
This does not comply
Arm v8-M Architecture Reference Manual
B3.8 Stack pointer
IDMLS "In Thread mode, CONTROL.SPSEL determines whether the PE uses
the main or process stack"
Check SPSEL bit of CONTROL_S register.
For simplicity the CONTROL_S is not tracked for changes
in the inner frames, the CONTROL_S value is passed unchanged from
the innermost frame.
Signed-off-by: Tomas Vanek <vanekt@fbl.cz>
---
gdb/arm-tdep.c | 28 +++++++++++++++++++++-------
1 file changed, 21 insertions(+), 7 deletions(-)
Comments
Hi Tomas,
I'd like to address the performance regression first before we get to these additional improvements.
I've been discussing PR 29738 with Torbjörn and have provided some feedback.
On 11/5/22 09:44, Tomas Vanek wrote:
> Unwinding of FNC_RETURN selected the process stack whenever zero IPSR
> indicated thread mode.
>
> This does not comply
> Arm v8-M Architecture Reference Manual
> B3.8 Stack pointer
> IDMLS "In Thread mode, CONTROL.SPSEL determines whether the PE uses
> the main or process stack"
>
> Check SPSEL bit of CONTROL_S register.
>
> For simplicity the CONTROL_S is not tracked for changes
> in the inner frames, the CONTROL_S value is passed unchanged from
> the innermost frame.
>
> Signed-off-by: Tomas Vanek <vanekt@fbl.cz>
> ---
> gdb/arm-tdep.c | 28 +++++++++++++++++++++-------
> 1 file changed, 21 insertions(+), 7 deletions(-)
>
> diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
> index 564ee43..4180277 100644
> --- a/gdb/arm-tdep.c
> +++ b/gdb/arm-tdep.c
> @@ -3485,13 +3485,27 @@ struct frame_unwind arm_stub_unwind = {
> return cache;
> }
>
> - ULONGEST xpsr = get_frame_register_unsigned (this_frame, ARM_PS_REGNUM);
> - if ((xpsr & 0x1ff) != 0)
> - /* Handler mode: This is the mode that exceptions are handled in. */
> - arm_cache_switch_prev_sp (cache, tdep, tdep->m_profile_msp_s_regnum);
> - else
> - /* Thread mode: This is the normal mode that programs run in. */
> - arm_cache_switch_prev_sp (cache, tdep, tdep->m_profile_psp_s_regnum);
> + bool spsel = true;
> + if (tdep->m_profile_control_s_regnum >= 0)
> + {
> + ULONGEST control_s
> + = get_frame_register_unsigned (this_frame,
> + tdep->m_profile_control_s_regnum);
> + spsel = (control_s & (1 << 1)) != 0;
> + }
> +
> + bool s_process_stack = false;
> + if (spsel)
> + {
> + ULONGEST xpsr = get_frame_register_unsigned (this_frame,
> + ARM_PS_REGNUM);
> + s_process_stack = (xpsr & 0x1ff) == 0;
> + }
> +
> + arm_cache_switch_prev_sp (cache, tdep,
> + s_process_stack ?
> + tdep->m_profile_psp_s_regnum :
> + tdep->m_profile_msp_s_regnum);
>
> CORE_ADDR unwound_sp = arm_cache_get_prev_sp_value (cache, tdep);
>
@@ -3485,13 +3485,27 @@ struct frame_unwind arm_stub_unwind = {
return cache;
}
- ULONGEST xpsr = get_frame_register_unsigned (this_frame, ARM_PS_REGNUM);
- if ((xpsr & 0x1ff) != 0)
- /* Handler mode: This is the mode that exceptions are handled in. */
- arm_cache_switch_prev_sp (cache, tdep, tdep->m_profile_msp_s_regnum);
- else
- /* Thread mode: This is the normal mode that programs run in. */
- arm_cache_switch_prev_sp (cache, tdep, tdep->m_profile_psp_s_regnum);
+ bool spsel = true;
+ if (tdep->m_profile_control_s_regnum >= 0)
+ {
+ ULONGEST control_s
+ = get_frame_register_unsigned (this_frame,
+ tdep->m_profile_control_s_regnum);
+ spsel = (control_s & (1 << 1)) != 0;
+ }
+
+ bool s_process_stack = false;
+ if (spsel)
+ {
+ ULONGEST xpsr = get_frame_register_unsigned (this_frame,
+ ARM_PS_REGNUM);
+ s_process_stack = (xpsr & 0x1ff) == 0;
+ }
+
+ arm_cache_switch_prev_sp (cache, tdep,
+ s_process_stack ?
+ tdep->m_profile_psp_s_regnum :
+ tdep->m_profile_msp_s_regnum);
CORE_ADDR unwound_sp = arm_cache_get_prev_sp_value (cache, tdep);