From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id bQYeMutn8mKPuCMAWB0awg (envelope-from ) for ; Tue, 09 Aug 2022 09:58:03 -0400 Received: by simark.ca (Postfix, from userid 112) id BB2501E5EA; Tue, 9 Aug 2022 09:58:03 -0400 (EDT) Authentication-Results: simark.ca; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.a=rsa-sha256 header.s=default header.b=V313ciiH; dkim-atps=neutral X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-3.0 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,NICE_REPLY_A,RDNS_DYNAMIC, URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 Received: from sourceware.org (ip-8-43-85-97.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 05E711E222 for ; Tue, 9 Aug 2022 09:58:02 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 8C6DB385AE41 for ; Tue, 9 Aug 2022 13:58:01 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 8C6DB385AE41 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1660053481; bh=R1qnq4aAAogsLDQu4mgsN8WRPnkkQ9XWigv+MFQ0dcI=; h=Date:Subject:To:References:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=V313ciiHylRtHm438H8VG9+yX+fGQB2n+xKAuZYS1Fgvy6wWaAlCZ69Z73XYX1REI SNzcxAt9c23q59rP2cITmv3Qz51r3xG+WYXgtmdwJVz+G3Wsgn6z0B4Z/KnV6Od1TX v7k4XlErru+CaY79LLlAdc26b5D0kMlPDH4nkRyk= Received: from mx07-00178001.pphosted.com (mx08-00178001.pphosted.com [91.207.212.93]) by sourceware.org (Postfix) with ESMTPS id 8BA25385AC22 for ; Tue, 9 Aug 2022 13:57:39 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 8BA25385AC22 Received: from pps.filterd (m0046660.ppops.net [127.0.0.1]) by mx07-00178001.pphosted.com (8.17.1.5/8.17.1.5) with ESMTP id 279DaXMX014147; Tue, 9 Aug 2022 15:57:37 +0200 Received: from beta.dmz-eu.st.com (beta.dmz-eu.st.com [164.129.1.35]) by mx07-00178001.pphosted.com (PPS) with ESMTPS id 3hurkx83nd-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 09 Aug 2022 15:57:37 +0200 Received: from euls16034.sgp.st.com (euls16034.sgp.st.com [10.75.44.20]) by beta.dmz-eu.st.com (STMicroelectronics) with ESMTP id 7A9AB100034; Tue, 9 Aug 2022 15:57:36 +0200 (CEST) Received: from Webmail-eu.st.com (shfdag1node3.st.com [10.75.129.71]) by euls16034.sgp.st.com (STMicroelectronics) with ESMTP id 558C9231527; Tue, 9 Aug 2022 15:57:36 +0200 (CEST) Received: from [10.210.54.218] (10.75.127.49) by SHFDAG1NODE3.st.com (10.75.129.71) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256) id 15.1.2308.20; Tue, 9 Aug 2022 15:43:17 +0200 Message-ID: <73982902-fa33-aa32-5bb4-14e92979623b@foss.st.com> Date: Tue, 9 Aug 2022 15:43:13 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0 Subject: Re: [PATCH] gdb/arm: Cleanup of arm_m_exception_cache Content-Language: en-US To: Luis Machado , References: <20220808075624.3126293-1-torbjorn.svensson@foss.st.com> <3e0260c4-8bfe-b398-2fd2-3af15a55b3d5@arm.com> In-Reply-To: <3e0260c4-8bfe-b398-2fd2-3af15a55b3d5@arm.com> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.75.127.49] X-ClientProxiedBy: SFHDAG2NODE3.st.com (10.75.127.6) To SHFDAG1NODE3.st.com (10.75.129.71) X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.883,Hydra:6.0.517,FMLib:17.11.122.1 definitions=2022-08-09_03,2022-08-09_02,2022-06-22_01 X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Torbjorn SVENSSON via Gdb-patches Reply-To: Torbjorn SVENSSON Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Sender: "Gdb-patches" Hello, Thanks for the review. Please see my comments below. On 8/9/22 15:02, Luis Machado wrote: > Hi, > > On 8/8/22 08:56, Torbjörn SVENSSON via Gdb-patches 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. > > > If we have corrupt data, do we risk running into this assertion? See my comments below on individual cases. > >> >> Signed-off-by: Torbjörn SVENSSON >> --- >>   gdb/arm-tdep.c | 343 ++++++++++++++++++++++++++----------------------- >>   1 file changed, 183 insertions(+), 160 deletions(-) >> >> diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c >> index 7d8d040f8f1..ad5ada39aea 100644 >> --- a/gdb/arm-tdep.c >> +++ b/gdb/arm-tdep.c >> @@ -3345,19 +3345,13 @@ arm_m_exception_cache (struct frame_info >> *this_frame) >>   { >>     struct gdbarch *gdbarch = get_frame_arch (this_frame); >>     arm_gdbarch_tdep *tdep = 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; >> +  ULONGEST xpsr; >> +  bool 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); >> @@ -3380,9 +3374,13 @@ arm_m_exception_cache (struct frame_info >> *this_frame) >>         return cache; >>       } >>   +  /* Check FNC_RETURN indicator bits (24-31).  */ >>     fnc_return = (((lr >> 24) & 0xff) == 0xfe); >> -  if (tdep->have_sec_ext && fnc_return) >> +  if (fnc_return) >>       { >> +      /* FNC_RETURN is only valid for targets with Security >> Extension.  */ >> +      gdb_assert (tdep->have_sec_ext); >> + > > An assertion is a bit of a strong hand here. It will crash GDB, > essentially. Should we go for an > error instead? That will stop the unwinder dead in its tracks and return. > > Unwinders may get corrupt data, so it is hard to rule out issues, even > if GDB is doing the right thing. Okay, lets do an error message instead and abort the unwind. What do you think the error message should say? "FNC_RETURN pattern is only valid for targets with Security Extension." - is that okay or do you have any other phase in mind? > >>         if (!arm_unwind_secure_frames) >>       { >>         warning (_("Non-secure to secure stack unwinding disabled.")); >> @@ -3428,6 +3426,14 @@ arm_m_exception_cache (struct frame_info >> *this_frame) >>     exc_return = (((lr >> 24) & 0xff) == 0xff); >>     if (exc_return) >>       { >> +      int sp_regnum; >> +      uint32_t sp_r0_offset = 0; >> +      bool extended_frame_used; > > extended_frame_used could be defined further down or removed. > > Since we're touching this code, it would be nice to get the variable > declarations closer to > where they're used. This makes things cleaner now that we can do it. > >> +      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); >>   @@ -3455,188 +3461,205 @@ 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); >> +        sp_regnum = tdep->m_profile_msp_regnum; >> +    } >>   -      /* 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.  */ >> +      /* Set the active SP regnum.  */ >> +      arm_cache_switch_prev_sp (cache, tdep, sp_regnum); >>   -      /* 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; >> +      /* Fetch the SP to use for this frame.  */ >> +      unwound_sp = arm_cache_get_prev_sp_value (cache, tdep); >>   -      if (tdep->have_sec_ext && !default_callee_register_stacking >> && fpccr_ts) >> +      /* 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 */ >> + >> +      /* 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.  */ >> +      extended_frame_used = ((lr & (1 << 4)) == 0); > > Declaring extended_frame_used here would be nice. That's the only > place where it is > used. > > Alternatively, you could check for bit 4 in the conditional expression > below and add a > comment explaining this is checking if the extended frame is being used. I'll move the declaration down here as I think the variable is helpful if every debugging this code. > >> +      if (extended_frame_used) >>       { >> -      /* Handle floating-point callee saved registers.  */ >> -      fpu_regs_stack_offset = unwound_sp + sp_r0_offset + 0x68; >> -      for (i = 8; i < 16; i++) >> -        { >> -          cache->saved_regs[ARM_D0_REGNUM + i].set_addr >> (fpu_regs_stack_offset); >> +      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; >>   -      arm_cache_set_active_sp_value (cache, tdep, >> -                     unwound_sp + sp_r0_offset + 0xA8); >> +      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++) >> +        { >> +          cache->saved_regs[ARM_D0_REGNUM + i] >> +            .set_addr (fpu_regs_stack_offset); >> +          fpu_regs_stack_offset += 8; >> +        } >> + >> +          arm_cache_set_active_sp_value (cache, tdep, >> +                         unwound_sp + sp_r0_offset + 0xA8); >> +        } >> +      else >> +        { >> +          /* Offset 0x64 is reserved.  */ >> +          arm_cache_set_active_sp_value (cache, tdep, >> +                         unwound_sp + sp_r0_offset + 0x68); >> +        } >>       } >>         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.  */ >> -  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); >> +      /* 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.  */ >> +      gdb_assert (safe_read_memory_unsigned_integer (cache->saved_regs[ >> +                             ARM_PS_REGNUM].addr (), 4, >> +                             byte_order, &xpsr)); >> +      if ((xpsr & (1 << 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; >> +      return cache; >> +    } >> + >> +  gdb_assert_not_reached ("Invalid LR contet"); > > "content". Again, this will crash GDB. Should we error out instead? This place is different from the security extension check above. When this line is reached, there is an internal error in the GDB source code as the sniffer has decided that $lr contains an EXC_PATTERN or FNC_PATTERN, but if you reach this line, it would mean that the sniffer and this function are out of sync. Do you think it's sane to assert or do you still want to have an error message instead? > >>   } >>     /* Implementation of function hook 'this_id' in >