From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id fmdEAPaV8mIUzSMAWB0awg (envelope-from ) for ; Tue, 09 Aug 2022 13:14:30 -0400 Received: by simark.ca (Postfix, from userid 112) id E7B051E5EA; Tue, 9 Aug 2022 13:14:29 -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=ZKZqmztb; 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 2D9561E13B for ; Tue, 9 Aug 2022 13:14:28 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id B864A3856243 for ; Tue, 9 Aug 2022 17:14:27 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org B864A3856243 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1660065267; bh=5AO+PlQR9my/chjLWu08xUhB+x5Q1T3VdjkV5KB1inw=; 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=ZKZqmztbhe5qOnfAKKGt0dlTUMF0NKRan/BZjc17T+GVUffd6AT802RoXEPYvTGcN WlUgc+ZwYcLFI9BQD5VnOpUq81+Nq0L9ZeljQezT1+dnzj0LhqE8puLn1IhcpPMi0B SwEwxT/qipUyVWfdxXlxI0pfIFuT1kFLSLC8nonA= Received: from mx07-00178001.pphosted.com (mx07-00178001.pphosted.com [185.132.182.106]) by sourceware.org (Postfix) with ESMTPS id 580EE3856DC6 for ; Tue, 9 Aug 2022 17:14:05 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 580EE3856DC6 Received: from pps.filterd (m0241204.ppops.net [127.0.0.1]) by mx07-00178001.pphosted.com (8.17.1.5/8.17.1.5) with ESMTP id 279F9vxe012992; Tue, 9 Aug 2022 19:14:03 +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 3hudt14a3u-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Tue, 09 Aug 2022 19:14:03 +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 33D2B10002A; Tue, 9 Aug 2022 19:14:02 +0200 (CEST) Received: from Webmail-eu.st.com (shfdag1node1.st.com [10.75.129.69]) by euls16034.sgp.st.com (STMicroelectronics) with ESMTP id 27F6024552A; Tue, 9 Aug 2022 19:14:02 +0200 (CEST) Received: from [10.210.54.218] (10.75.127.121) by SHFDAG1NODE1.st.com (10.75.129.69) 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 19:14:01 +0200 Message-ID: Date: Tue, 9 Aug 2022 19:13:39 +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 v2] gdb/arm: Cleanup of arm_m_exception_cache Content-Language: en-US To: Luis Machado , References: <20220809153006.3249562-1-torbjorn.svensson@foss.st.com> <136b08eb-3680-b166-2ad8-1d8acdff6b34@arm.com> In-Reply-To: <136b08eb-3680-b166-2ad8-1d8acdff6b34@arm.com> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.75.127.121] X-ClientProxiedBy: GPXDAG2NODE4.st.com (10.75.127.68) To SHFDAG1NODE1.st.com (10.75.129.69) 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_05,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" In addition to comments below, v3 will also contain a rephrased commit message to make it clear that GDB does not call assert, but instead errors out. On 8/9/22 18:15, Luis Machado wrote: > 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 >> --- >>   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 (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." Ok, string updated in v3. > >> +           "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. Was uncertain if error() would bail or if it would just print the message and continue. Thanks for letting me know! > >> +    } >> + >>         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 `,` I copied an existing usage of bit(), but sure. Will be corrected in v3. > >> +      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. The difference is that for the security extension check above, that is a wrongful state of the hardware. This case down here is when LR does not match an EXC_RETURN or FNC_RETURN pattern, thus is an inconsistency between the sniffer and the function that creates the cache object. The error case in the bottom should never happen as the sniffer has checked that LR matches either EXC_RETURN or FNC_RETURN, so it's more of a safe guard in case something is broken in GDB itself. The case with the security extension check should not happen neither, but I suppose it could happen if the data that is fetched from the target is corrupt in a certain way... Maybe the last one should be internal_error() instead of error()? > > >>   +  /* 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 (). > >>   } >