From: Luis Machado <luis.machado@arm.com>
To: Torbjorn SVENSSON <torbjorn.svensson@foss.st.com>,
gdb-patches@sourceware.org
Subject: Re: [PATCH v2] gdb/arm: Cleanup of arm_m_exception_cache
Date: Wed, 10 Aug 2022 11:53:05 +0100 [thread overview]
Message-ID: <8f9e2323-0207-6d3e-43b8-7916523bda89@arm.com> (raw)
In-Reply-To: <e7a1874b-f2cd-6c81-3617-2e915848b02f@foss.st.com>
On 8/9/22 18:13, Torbjorn SVENSSON wrote:
> 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 <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."
> 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()>
From your description, it sounds like it.
>>
>>
>>> + /* 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 ().
>>
>>> }
>>
prev parent reply other threads:[~2022-08-10 10:53 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
2022-08-09 17:13 ` Torbjorn SVENSSON
2022-08-10 10:53 ` Luis Machado [this message]
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=8f9e2323-0207-6d3e-43b8-7916523bda89@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).