[v3] gdb/arm: Stop unwinding on error, but do not assert

  When it's impossible to read the FPCCR and XPSR, the unwinding is
unpredictable as the it's not possible to determine the correct
frame size or padding.
The only sane thing to do in this condition is to stop the unwinding.

Example session wihtout this patch:

  (gdb) bt
  #0  SVC_Handler () at .../GPIO/GPIO_EXTI/Src/stm32f4xx_it.c:112
  .../gdb/arm-tdep.c:3594: internal-error: arm_m_exception_cache: Assertion `safe_read_memory_unsigned_integer (FPCCR, ARM_INT_REGISTER_SIZE, byte_order, &fpccr)' failed.
  A problem internal to GDB has been detected,
  further debugging may prove unreliable.
  ----- Backtrace -----
  0x5583bfb2a157 gdb_internal_backtrace_1
  ...
  ---------------------

  This is a bug, please report it.  For instructions, see:
  <https://www.gnu.org/software/gdb/bugs/>.

  Aborted (core dumped)

Example session with this patch:

  (gdb) bt
  #0  SVC_Handler () at .../GPIO/GPIO_EXTI/Src/stm32f4xx_it.c:112
  warning: Could not fetch required FPCCR content.  Further unwind is impossible.
  #1  <signal handler called>
  (gdb)

Signed-off-by: Torbj?rn SVENSSON  <torbjorn.svensson@foss.st.com>
---
 gdb/arm-tdep.c | 35 +++++++++++++++++++++++++++++------
 1 file changed, 29 insertions(+), 6 deletions(-)
  

Just one nit...

On 10/13/22 11:53, Torbj?rn SVENSSON wrote:
> When it's impossible to read the FPCCR and XPSR, the unwinding is
> unpredictable as the it's not possible to determine the correct
> frame size or padding.
> The only sane thing to do in this condition is to stop the unwinding.
> 
> Example session wihtout this patch:
> 
>    (gdb) bt
>    #0  SVC_Handler () at .../GPIO/GPIO_EXTI/Src/stm32f4xx_it.c:112
>    .../gdb/arm-tdep.c:3594: internal-error: arm_m_exception_cache: Assertion `safe_read_memory_unsigned_integer (FPCCR, ARM_INT_REGISTER_SIZE, byte_order, &fpccr)' failed.
>    A problem internal to GDB has been detected,
>    further debugging may prove unreliable.
>    ----- Backtrace -----
>    0x5583bfb2a157 gdb_internal_backtrace_1
>    ...
>    ---------------------
> 
>    This is a bug, please report it.  For instructions, see:
>    <https://www.gnu.org/software/gdb/bugs/>.
> 
>    Aborted (core dumped)
> 
> Example session with this patch:
> 
>    (gdb) bt
>    #0  SVC_Handler () at .../GPIO/GPIO_EXTI/Src/stm32f4xx_it.c:112
>    warning: Could not fetch required FPCCR content.  Further unwind is impossible.
>    #1  <signal handler called>
>    (gdb)
> 
> Signed-off-by: Torbj?rn SVENSSON  <torbjorn.svensson@foss.st.com>
> ---
>   gdb/arm-tdep.c | 35 +++++++++++++++++++++++++++++------
>   1 file changed, 29 insertions(+), 6 deletions(-)
> 
> diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
> index 041e6afefed..bdbff247188 100644
> --- a/gdb/arm-tdep.c
> +++ b/gdb/arm-tdep.c
> @@ -3591,9 +3591,13 @@ arm_m_exception_cache (frame_info_ptr this_frame)
>   	  ULONGEST fpcar;
>   
>   	  /* Read FPCCR register.  */
> -	  gdb_assert (safe_read_memory_unsigned_integer (FPCCR,
> -							 ARM_INT_REGISTER_SIZE,
> -							 byte_order, &fpccr));
> +	 if (!safe_read_memory_unsigned_integer (FPCCR, ARM_INT_REGISTER_SIZE,
> +						 byte_order, &fpccr))
> +	   {
> +	     warning (_("Could not fetch required FPCCR content.  Further "
> +			"unwind is impossible."));

unwind -> unwinding would read better.

> +	     return nullptr;
> +	   }
>   
>   	  /* Read FPCAR register.  */
>   	  if (!safe_read_memory_unsigned_integer (FPCAR, ARM_INT_REGISTER_SIZE,
> @@ -3669,9 +3673,15 @@ arm_m_exception_cache (frame_info_ptr this_frame)
>   	 aligner between the top of the 32-byte stack frame and the
>   	 previous context's stack pointer.  */
>         ULONGEST xpsr;
> -      gdb_assert (safe_read_memory_unsigned_integer (cache->saved_regs[
> -						     ARM_PS_REGNUM].addr (), 4,
> -						     byte_order, &xpsr));
> +      if (!safe_read_memory_unsigned_integer (cache->saved_regs[ARM_PS_REGNUM]
> +					      .addr (), ARM_INT_REGISTER_SIZE,
> +					      byte_order, &xpsr))
> +	{
> +	  warning (_("Could not fetch required XPSR content.  Further unwind "
> +		     "is impossible."));

Same here. unwind -> unwinding.

> +	  return nullptr;
> +	}
> +
>         if (bit (xpsr, 9) != 0)
>   	{
>   	  CORE_ADDR new_sp = arm_cache_get_prev_sp_value (cache, tdep) + 4;
> @@ -3703,6 +3713,14 @@ arm_m_exception_this_id (frame_info_ptr this_frame,
>       *this_cache = arm_m_exception_cache (this_frame);
>     cache = (struct arm_prologue_cache *) *this_cache;
>   
> +  /* Unwind of this frame is not possible.  Return outer_frame_id to stop the

Unwind -> Unwinding.

> +     unwinding.  */
> +  if (cache == nullptr)
> +    {
> +      *this_id = outer_frame_id;
> +      return;
> +    }
> +
>     /* Our frame ID for a stub frame is the current SP and LR.  */
>     arm_gdbarch_tdep *tdep
>       = gdbarch_tdep<arm_gdbarch_tdep> (get_frame_arch (this_frame));
> @@ -3725,6 +3743,11 @@ arm_m_exception_prev_register (frame_info_ptr this_frame,
>       *this_cache = arm_m_exception_cache (this_frame);
>     cache = (struct arm_prologue_cache *) *this_cache;
>   
> +  /* It's not allowed to call prev_register when this_id has returned the
> +     outer_frame_id.  The arm_m_exception_cache function will return NULL when
> +     the frame cannot be properly unwinded.  */
> +  gdb_assert (cache != nullptr);
> +
>     /* The value was already reconstructed into PREV_SP.  */
>     arm_gdbarch_tdep *tdep
>       = gdbarch_tdep<arm_gdbarch_tdep> (get_frame_arch (this_frame));

Otherwise LGTM.

No need to send a v4 with these changes. Make sure you fix the commit message to mention
the correct warnings.

Thanks!
Luis