public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Luis Machado <luis.machado@arm.com>
To: "Torbjörn SVENSSON" <torbjorn.svensson@foss.st.com>,
	gdb-patches@sourceware.org
Subject: Re: [PATCH v2] gdb/arm: Cleanup of arm_m_exception_cache
Date: Tue, 9 Aug 2022 17:15:31 +0100	[thread overview]
Message-ID: <136b08eb-3680-b166-2ad8-1d8acdff6b34@arm.com> (raw)
In-Reply-To: <20220809153006.3249562-1-torbjorn.svensson@foss.st.com>

On 8/9/22 16:30, Torbjörn SVENSSON wrote:
> With this change, only valid content of LR is accepted for the current
> target. If the content for LR is anything but EXC_RETURN or FNC_RETURN
> will cause GDB to assert since it's an invalid state for the unwinder.
> FNC_RETURN pattern requires Security Extensions to be enabled or GDB
> will assert due to the bad state of the unwinder.
> 
> Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
> ---
>   gdb/arm-tdep.c | 380 ++++++++++++++++++++++++++-----------------------
>   1 file changed, 204 insertions(+), 176 deletions(-)
> 
> diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
> index cf8b610a381..299c416fe52 100644
> --- a/gdb/arm-tdep.c
> +++ b/gdb/arm-tdep.c
> @@ -3346,19 +3346,7 @@ arm_m_exception_cache (struct frame_info *this_frame)
>   {
>     struct gdbarch *gdbarch = get_frame_arch (this_frame);
>     arm_gdbarch_tdep *tdep = gdbarch_tdep<arm_gdbarch_tdep> (gdbarch);
> -  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
>     struct arm_prologue_cache *cache;
> -  CORE_ADDR lr;
> -  CORE_ADDR sp;
> -  CORE_ADDR unwound_sp;
> -  uint32_t sp_r0_offset = 0;
> -  LONGEST xpsr;
> -  uint32_t exc_return;
> -  bool fnc_return;
> -  uint32_t extended_frame_used;
> -  bool secure_stack_used = false;
> -  bool default_callee_register_stacking = false;
> -  bool exception_domain_is_secure = false;
>   
>     cache = FRAME_OBSTACK_ZALLOC (struct arm_prologue_cache);
>     arm_cache_init (cache, this_frame);
> @@ -3367,8 +3355,8 @@ arm_m_exception_cache (struct frame_info *this_frame)
>        describes which bits in LR that define which stack was used prior
>        to the exception and if FPU is used (causing extended stack frame).  */
>   
> -  lr = get_frame_register_unsigned (this_frame, ARM_LR_REGNUM);
> -  sp = get_frame_register_unsigned (this_frame, ARM_SP_REGNUM);
> +  CORE_ADDR lr = get_frame_register_unsigned (this_frame, ARM_LR_REGNUM);
> +  CORE_ADDR sp = get_frame_register_unsigned (this_frame, ARM_SP_REGNUM);
>   
>     /* ARMv7-M Architecture Reference "A2.3.1 Arm core registers"
>        states that LR is set to 0xffffffff on reset.  ARMv8-M Architecture
> @@ -3381,9 +3369,22 @@ arm_m_exception_cache (struct frame_info *this_frame)
>         return cache;
>       }
>   
> -  fnc_return = (((lr >> 24) & 0xff) == 0xfe);
> -  if (tdep->have_sec_ext && fnc_return)
> +  /* Check FNC_RETURN indicator bits (24-31).  */
> +  bool fnc_return = (((lr >> 24) & 0xff) == 0xfe);
> +  if (fnc_return)
>       {
> +      /* FNC_RETURN is only valid for targets with Security Extension.  */
> +      if (!tdep->have_sec_ext)
> +	{
> +	  error (_("While unwinding an exception frame, found unexpected Link "
> +		   "Register value 0x%lx.  This should not happen and may be "

You should use the %s format here and use phex to turn the 32-bit value to hex.

Also, since this is checking explicitly for a value and the Security Extension, we should
add that to the error message to make it more obvious what is failing.

"While unwinding an exception frame, found unexpected Link Register value %s that requires the security extension, but the extension was not found or is disabled.  This should not happen and may be caused by corrupt data or a bug in GDB."

> +		   "caused by corrupt data or a bug in GDB."), lr);
> +
> +	  /* Terminate any further stack unwinding by referring to self.  */
> +	  arm_cache_set_active_sp_value (cache, tdep, sp);
> +	  return cache;

Since you errored out, there's no use returning or executing any other statements after error.

> +	}
> +
>         if (!arm_unwind_secure_frames)
>   	{
>   	  warning (_("Non-secure to secure stack unwinding disabled."));
> @@ -3393,7 +3394,7 @@ arm_m_exception_cache (struct frame_info *this_frame)
>   	  return cache;
>   	}
>   
> -      xpsr = get_frame_register_unsigned (this_frame, ARM_PS_REGNUM);
> +      ULONGEST xpsr = get_frame_register_unsigned (this_frame, ARM_PS_REGNUM);
>         if ((xpsr & 0xff) != 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);
> @@ -3401,7 +3402,7 @@ arm_m_exception_cache (struct frame_info *this_frame)
>   	/* Thread mode: This is the normal mode that programs run in.  */
>   	arm_cache_switch_prev_sp (cache, tdep, tdep->m_profile_psp_s_regnum);
>   
> -      unwound_sp = arm_cache_get_prev_sp_value (cache, tdep);
> +      CORE_ADDR unwound_sp = arm_cache_get_prev_sp_value (cache, tdep);
>   
>         /* Stack layout for a function call from Secure to Non-Secure state
>   	 (ARMv8-M section B3.16):
> @@ -3426,17 +3427,23 @@ arm_m_exception_cache (struct frame_info *this_frame)
>       }
>   
>     /* Check EXC_RETURN indicator bits (24-31).  */
> -  exc_return = (((lr >> 24) & 0xff) == 0xff);
> +  bool exc_return = (((lr >> 24) & 0xff) == 0xff);
>     if (exc_return)
>       {
> +      int sp_regnum;
> +      bool secure_stack_used = false;
> +      bool default_callee_register_stacking = false;
> +      bool exception_domain_is_secure = false;
> +      enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> +
>         /* Check EXC_RETURN bit SPSEL if Main or Thread (process) stack used.  */
> -      bool process_stack_used = ((lr & (1 << 2)) != 0);
> +      bool process_stack_used = (bit (lr,2) != 0);
>   
>         if (tdep->have_sec_ext)
>   	{
> -	  secure_stack_used = ((lr & (1 << 6)) != 0);
> -	  default_callee_register_stacking = ((lr & (1 << 5)) != 0);
> -	  exception_domain_is_secure = ((lr & (1 << 0)) == 0);
> +	  secure_stack_used = (bit (lr,6) != 0);

Could you please address the formatting issues? space before `(`, space after `,`

> +	  default_callee_register_stacking = (bit (lr,5) != 0);> +	  exception_domain_is_secure = (bit (lr,0) == 0);
>   
>   	  /* Unwinding from non-secure to secure can trip security
>   	     measures.  In order to avoid the debugger being
> @@ -3456,187 +3463,208 @@ arm_m_exception_cache (struct frame_info *this_frame)
>   	    {
>   	      if (secure_stack_used)
>   		/* Secure thread (process) stack used, use PSP_S as SP.  */
> -		arm_cache_switch_prev_sp (cache, tdep, tdep->m_profile_psp_s_regnum);
> +		sp_regnum = tdep->m_profile_psp_s_regnum;
>   	      else
>   		/* Non-secure thread (process) stack used, use PSP_NS as SP.  */
> -		arm_cache_switch_prev_sp (cache, tdep, tdep->m_profile_psp_ns_regnum);
> +		sp_regnum = tdep->m_profile_psp_ns_regnum;
>   	    }
>   	  else
>   	    {
>   	      if (secure_stack_used)
>   		/* Secure main stack used, use MSP_S as SP.  */
> -		arm_cache_switch_prev_sp (cache, tdep, tdep->m_profile_msp_s_regnum);
> +		sp_regnum = tdep->m_profile_msp_s_regnum;
>   	      else
>   		/* Non-secure main stack used, use MSP_NS as SP.  */
> -		arm_cache_switch_prev_sp (cache, tdep, tdep->m_profile_msp_ns_regnum);
> +		sp_regnum = tdep->m_profile_msp_ns_regnum;
>   	    }
>   	}
>         else
>   	{
>   	  if (process_stack_used)
>   	    /* Thread (process) stack used, use PSP as SP.  */
> -	    arm_cache_switch_prev_sp (cache, tdep, tdep->m_profile_psp_regnum);
> +	    sp_regnum = tdep->m_profile_psp_regnum;
>   	  else
>   	    /* Main stack used, use MSP as SP.  */
> -	    arm_cache_switch_prev_sp (cache, tdep, tdep->m_profile_msp_regnum);
> -	}
> -    }
> -
> -  /* Fetch the SP to use for this frame.  */
> -  unwound_sp = arm_cache_get_prev_sp_value (cache, tdep);
> -
> -  /* Exception entry context stacking are described in ARMv8-M (section B3.19)
> -     and ARMv7-M (sections B1.5.6 and B1.5.7) Architecture Reference Manuals.
> -
> -     The following figure shows the structure of the stack frame when Security
> -     and Floating-point extensions are present.
> -
> -                          SP Offsets
> -            Without                         With
> -          Callee Regs                    Callee Regs
> -                                    (Secure -> Non-Secure)
> -                    +-------------------+
> -             0xA8   |                   |   0xD0
> -                    +===================+         --+  <-- Original SP
> -             0xA4   |        S31        |   0xCC    |
> -                    +-------------------+           |
> -                             ...                    |  Additional FP context
> -                    +-------------------+           |
> -             0x68   |        S16        |   0x90    |
> -                    +===================+         --+
> -             0x64   |      Reserved     |   0x8C    |
> -                    +-------------------+           |
> -             0x60   |       FPSCR       |   0x88    |
> -                    +-------------------+           |
> -             0x5C   |        S15        |   0x84    |  FP context
> -                    +-------------------+           |
> -                             ...                    |
> -                    +-------------------+           |
> -             0x20   |         S0        |   0x48    |
> -                    +===================+         --+
> -             0x1C   |       xPSR        |   0x44    |
> -                    +-------------------+           |
> -             0x18   |  Return address   |   0x40    |
> -                    +-------------------+           |
> -             0x14   |      LR(R14)      |   0x3C    |
> -                    +-------------------+           |
> -             0x10   |        R12        |   0x38    |  State context
> -                    +-------------------+           |
> -             0x0C   |         R3        |   0x34    |
> -                    +-------------------+           |
> -                             ...                    |
> -                    +-------------------+           |
> -             0x00   |         R0        |   0x28    |
> -                    +===================+         --+
> -                    |        R11        |   0x24    |
> -                    +-------------------+           |
> -                             ...                    |
> -                    +-------------------+           |  Additional state context
> -                    |         R4        |   0x08    |  when transitioning from
> -                    +-------------------+           |  Secure to Non-Secure
> -                    |      Reserved     |   0x04    |
> -                    +-------------------+           |
> -                    |  Magic signature  |   0x00    |
> -                    +===================+         --+  <-- New SP  */
> -
> -  /* With the Security extension, the hardware saves R4..R11 too.  */
> -  if (exc_return && tdep->have_sec_ext && secure_stack_used
> -      && (!default_callee_register_stacking || exception_domain_is_secure))
> -    {
> -      /* Read R4..R11 from the integer callee registers.  */
> -      cache->saved_regs[4].set_addr (unwound_sp + 0x08);
> -      cache->saved_regs[5].set_addr (unwound_sp + 0x0C);
> -      cache->saved_regs[6].set_addr (unwound_sp + 0x10);
> -      cache->saved_regs[7].set_addr (unwound_sp + 0x14);
> -      cache->saved_regs[8].set_addr (unwound_sp + 0x18);
> -      cache->saved_regs[9].set_addr (unwound_sp + 0x1C);
> -      cache->saved_regs[10].set_addr (unwound_sp + 0x20);
> -      cache->saved_regs[11].set_addr (unwound_sp + 0x24);
> -      sp_r0_offset = 0x28;
> -    }
> -
> -  /* 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
> -     "ARMv7-M Architecture Reference Manual".  */
> -  cache->saved_regs[0].set_addr (unwound_sp + sp_r0_offset);
> -  cache->saved_regs[1].set_addr (unwound_sp + sp_r0_offset + 0x04);
> -  cache->saved_regs[2].set_addr (unwound_sp + sp_r0_offset + 0x08);
> -  cache->saved_regs[3].set_addr (unwound_sp + sp_r0_offset + 0x0C);
> -  cache->saved_regs[ARM_IP_REGNUM].set_addr (unwound_sp + sp_r0_offset + 0x10);
> -  cache->saved_regs[ARM_LR_REGNUM].set_addr (unwound_sp + sp_r0_offset + 0x14);
> -  cache->saved_regs[ARM_PC_REGNUM].set_addr (unwound_sp + sp_r0_offset + 0x18);
> -  cache->saved_regs[ARM_PS_REGNUM].set_addr (unwound_sp + sp_r0_offset + 0x1C);
> -
> -  /* Check EXC_RETURN bit FTYPE if extended stack frame (FPU regs stored)
> -     type used.  */
> -  extended_frame_used = ((lr & (1 << 4)) == 0);
> -  if (exc_return && extended_frame_used)
> -    {
> -      int i;
> -      int fpu_regs_stack_offset;
> -      ULONGEST fpccr;
> -
> -      /* Read FPCCR register.  */
> -      gdb_assert (safe_read_memory_unsigned_integer (FPCCR,
> -                                                     ARM_INT_REGISTER_SIZE,
> -                                                     byte_order, &fpccr));
> -      bool fpccr_ts = bit (fpccr,26);
> -
> -      /* This code does not take into account the lazy stacking, see "Lazy
> -	 context save of FP state", in B1.5.7, also ARM AN298, supported
> -	 by Cortex-M4F architecture.
> -	 To fully handle this the FPCCR register (Floating-point Context
> -	 Control Register) needs to be read out and the bits ASPEN and LSPEN
> -	 could be checked to setup correct lazy stacked FP registers.
> -	 This register is located at address 0xE000EF34.  */
> -
> -      /* Extended stack frame type used.  */
> -      fpu_regs_stack_offset = unwound_sp + sp_r0_offset + 0x20;
> -      for (i = 0; i < 8; i++)
> -	{
> -	  cache->saved_regs[ARM_D0_REGNUM + i].set_addr (fpu_regs_stack_offset);
> -	  fpu_regs_stack_offset += 8;
> -	}
> -      cache->saved_regs[ARM_FPSCR_REGNUM].set_addr (unwound_sp + sp_r0_offset + 0x60);
> -      fpu_regs_stack_offset += 4;
> +	    sp_regnum = tdep->m_profile_msp_regnum;
> +	}
> +
> +      /* Set the active SP regnum.  */
> +      arm_cache_switch_prev_sp (cache, tdep, sp_regnum);
> +
> +      /* Fetch the SP to use for this frame.  */
> +      CORE_ADDR unwound_sp = arm_cache_get_prev_sp_value (cache, tdep);
> +
> +      /* Exception entry context stacking are described in ARMv8-M (section
> +	 B3.19) and ARMv7-M (sections B1.5.6 and B1.5.7) Architecture Reference
> +	 Manuals.
> +
> +	 The following figure shows the structure of the stack frame when
> +	 Security and Floating-point extensions are present.
> +
> +			      SP Offsets
> +		Without                         With
> +	      Callee Regs                    Callee Regs
> +					(Secure -> Non-Secure)
> +			+-------------------+
> +		 0xA8   |                   |   0xD0
> +			+===================+         --+  <-- Original SP
> +		 0xA4   |        S31        |   0xCC    |
> +			+-------------------+           |
> +				 ...                    |  Additional FP context
> +			+-------------------+           |
> +		 0x68   |        S16        |   0x90    |
> +			+===================+         --+
> +		 0x64   |      Reserved     |   0x8C    |
> +			+-------------------+           |
> +		 0x60   |       FPSCR       |   0x88    |
> +			+-------------------+           |
> +		 0x5C   |        S15        |   0x84    |  FP context
> +			+-------------------+           |
> +				 ...                    |
> +			+-------------------+           |
> +		 0x20   |         S0        |   0x48    |
> +			+===================+         --+
> +		 0x1C   |       xPSR        |   0x44    |
> +			+-------------------+           |
> +		 0x18   |  Return address   |   0x40    |
> +			+-------------------+           |
> +		 0x14   |      LR(R14)      |   0x3C    |
> +			+-------------------+           |
> +		 0x10   |        R12        |   0x38    |  State context
> +			+-------------------+           |
> +		 0x0C   |         R3        |   0x34    |
> +			+-------------------+           |
> +				 ...                    |
> +			+-------------------+           |
> +		 0x00   |         R0        |   0x28    |
> +			+===================+         --+
> +			|        R11        |   0x24    |
> +			+-------------------+           |
> +				 ...                    |
> +			+-------------------+           |  Additional state
> +			|         R4        |   0x08    |  context when
> +			+-------------------+           |  transitioning from
> +			|      Reserved     |   0x04    |  Secure to Non-Secure
> +			+-------------------+           |
> +			|  Magic signature  |   0x00    |
> +			+===================+         --+  <-- New SP  */
> +
> +      uint32_t sp_r0_offset = 0;
> +
> +      /* With the Security extension, the hardware saves R4..R11 too.  */
> +      if (tdep->have_sec_ext && secure_stack_used
> +	  && (!default_callee_register_stacking || exception_domain_is_secure))
> +	{
> +	  /* Read R4..R11 from the integer callee registers.  */
> +	  cache->saved_regs[4].set_addr (unwound_sp + 0x08);
> +	  cache->saved_regs[5].set_addr (unwound_sp + 0x0C);
> +	  cache->saved_regs[6].set_addr (unwound_sp + 0x10);
> +	  cache->saved_regs[7].set_addr (unwound_sp + 0x14);
> +	  cache->saved_regs[8].set_addr (unwound_sp + 0x18);
> +	  cache->saved_regs[9].set_addr (unwound_sp + 0x1C);
> +	  cache->saved_regs[10].set_addr (unwound_sp + 0x20);
> +	  cache->saved_regs[11].set_addr (unwound_sp + 0x24);
> +	  sp_r0_offset = 0x28;
> +	}
> +
> +      /* 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
> +	 "ARMv7-M Architecture Reference Manual".  */
> +      cache->saved_regs[0].set_addr (unwound_sp + sp_r0_offset);
> +      cache->saved_regs[1].set_addr (unwound_sp + sp_r0_offset + 0x04);
> +      cache->saved_regs[2].set_addr (unwound_sp + sp_r0_offset + 0x08);
> +      cache->saved_regs[3].set_addr (unwound_sp + sp_r0_offset + 0x0C);
> +      cache->saved_regs[ARM_IP_REGNUM].set_addr (unwound_sp + sp_r0_offset
> +						 + 0x10);
> +      cache->saved_regs[ARM_LR_REGNUM].set_addr (unwound_sp + sp_r0_offset
> +						 + 0x14);
> +      cache->saved_regs[ARM_PC_REGNUM].set_addr (unwound_sp + sp_r0_offset
> +						 + 0x18);
> +      cache->saved_regs[ARM_PS_REGNUM].set_addr (unwound_sp + sp_r0_offset
> +						 + 0x1C);
> +
> +      /* Check EXC_RETURN bit FTYPE if extended stack frame (FPU regs stored)
> +	 type used.  */
> +      bool extended_frame_used = (bit (lr,4) == 0);
> +      if (extended_frame_used)
> +	{
> +	  ULONGEST fpccr;
> +
> +	  /* Read FPCCR register.  */
> +	  gdb_assert (safe_read_memory_unsigned_integer (FPCCR,
> +							 ARM_INT_REGISTER_SIZE,
> +							 byte_order, &fpccr));
> +	  bool fpccr_ts = bit (fpccr,26);

Space after `,`

> +
> +	  /* This code does not take into account the lazy stacking, see "Lazy
> +	     context save of FP state", in B1.5.7, also ARM AN298, supported
> +	     by Cortex-M4F architecture.
> +	     To fully handle this the FPCCR register (Floating-point Context
> +	     Control Register) needs to be read out and the bits ASPEN and
> +	     LSPEN could be checked to setup correct lazy stacked FP registers.
> +	     This register is located at address 0xE000EF34.  */
> +
> +	  /* Extended stack frame type used.  */
> +	  CORE_ADDR addr = unwound_sp + sp_r0_offset + 0x20;
> +	  for (int i = 0; i < 8; i++)
> +	    {
> +	      cache->saved_regs[ARM_D0_REGNUM + i].set_addr (addr);
> +	      addr += 8;
> +	    }
> +	  cache->saved_regs[ARM_FPSCR_REGNUM].set_addr (unwound_sp
> +							+ sp_r0_offset + 0x60);
> +
> +	  if (tdep->have_sec_ext && !default_callee_register_stacking
> +	      && fpccr_ts)
> +	    {
> +	      /* Handle floating-point callee saved registers.  */
> +	      addr = unwound_sp + sp_r0_offset + 0x68;
> +	      for (int i = 8; i < 16; i++)
> +		{
> +		  cache->saved_regs[ARM_D0_REGNUM + i].set_addr (addr);
> +		  addr += 8;
> +		}
>   
> -      if (tdep->have_sec_ext && !default_callee_register_stacking && fpccr_ts)
> -	{
> -	  /* Handle floating-point callee saved registers.  */
> -	  fpu_regs_stack_offset = unwound_sp + sp_r0_offset + 0x68;
> -	  for (i = 8; i < 16; i++)
> +	      arm_cache_set_active_sp_value (cache, tdep,
> +					     unwound_sp + sp_r0_offset + 0xA8);
> +	    }
> +	  else
>   	    {
> -	      cache->saved_regs[ARM_D0_REGNUM + i].set_addr (fpu_regs_stack_offset);
> -	      fpu_regs_stack_offset += 8;
> +	      /* Offset 0x64 is reserved.  */
> +	      arm_cache_set_active_sp_value (cache, tdep,
> +					     unwound_sp + sp_r0_offset + 0x68);
>   	    }
> -
> -	  arm_cache_set_active_sp_value (cache, tdep,
> -					 unwound_sp + sp_r0_offset + 0xA8);
>   	}
>         else
>   	{
> -	  /* Offset 0x64 is reserved.  */
> +	  /* Standard stack frame type used.  */
>   	  arm_cache_set_active_sp_value (cache, tdep,
> -					 unwound_sp + sp_r0_offset + 0x68);
> +					 unwound_sp + sp_r0_offset + 0x20);
>   	}
> -    }
> -  else
> -    {
> -      /* Standard stack frame type used.  */
> -      arm_cache_set_active_sp_value (cache, tdep,
> -				     unwound_sp + sp_r0_offset + 0x20);
> +
> +      /* If bit 9 of the saved xPSR is set, then there is a four-byte
> +	 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 (bit (xpsr,9) != 0)
> +	{
> +	  CORE_ADDR new_sp = arm_cache_get_prev_sp_value (cache, tdep) + 4;
> +	  arm_cache_set_active_sp_value (cache, tdep, new_sp);
> +	}
> +
> +      return cache;
>       }
>   
> -  /* If bit 9 of the saved xPSR is set, then there is a four-byte
> -     aligner between the top of the 32-byte stack frame and the
> -     previous context's stack pointer.  */
> -  if (safe_read_memory_integer (unwound_sp + sp_r0_offset + 0x1C, 4,
> -				byte_order, &xpsr)
> -      && (xpsr & (1 << 9)) != 0)
> -    arm_cache_set_active_sp_value (cache, tdep,
> -				   arm_cache_get_prev_sp_value (cache, tdep) + 4);
> +  error (_("While unwinding an exception frame, found unexpected Link Register "
> +	   "value 0x%lx.  This should not happen and may be caused by corrupt "
> +	   "data or a bug in GDB."), lr);

Same comment about using %s and phex as opposed to %lx.

What does this case have that is different from the previous error? Does it contain an
unrecognized LR value? If so, we should mention that explicitly to make it as helpful to
the user as possible.


>   
> +  /* Terminate any further stack unwinding by referring to self.  */
> +  arm_cache_set_active_sp_value (cache, tdep, sp);
>     return cache;

This is dead code now. Nothing gets executed after error ().

>   }
>   


  reply	other threads:[~2022-08-09 16:15 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-09 15:30 Torbjörn SVENSSON
2022-08-09 16:15 ` Luis Machado [this message]
2022-08-09 17:13   ` Torbjorn SVENSSON
2022-08-10 10:53     ` Luis Machado

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=136b08eb-3680-b166-2ad8-1d8acdff6b34@arm.com \
    --to=luis.machado@arm.com \
    --cc=gdb-patches@sourceware.org \
    --cc=torbjorn.svensson@foss.st.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).