* [PATCH v3] gdb/arm: Stop unwinding on error, but do not assert
@ 2022-10-13 10:53 Torbjörn SVENSSON
2022-10-13 11:22 ` Luis Machado
0 siblings, 1 reply; 2+ messages in thread
From: Torbjörn SVENSSON @ 2022-10-13 10:53 UTC (permalink / raw)
To: gdb-patches
When it's impossible to read the FPCCR and XPSR, the unwinding is
unpredictable as the it's not possible to determine the correct
frame size or padding.
The only sane thing to do in this condition is to stop the unwinding.
Example session wihtout this patch:
(gdb) bt
#0 SVC_Handler () at .../GPIO/GPIO_EXTI/Src/stm32f4xx_it.c:112
.../gdb/arm-tdep.c:3594: internal-error: arm_m_exception_cache: Assertion `safe_read_memory_unsigned_integer (FPCCR, ARM_INT_REGISTER_SIZE, byte_order, &fpccr)' failed.
A problem internal to GDB has been detected,
further debugging may prove unreliable.
----- Backtrace -----
0x5583bfb2a157 gdb_internal_backtrace_1
...
---------------------
This is a bug, please report it. For instructions, see:
<https://www.gnu.org/software/gdb/bugs/>.
Aborted (core dumped)
Example session with this patch:
(gdb) bt
#0 SVC_Handler () at .../GPIO/GPIO_EXTI/Src/stm32f4xx_it.c:112
warning: Could not fetch required FPCCR content. Further unwind is impossible.
#1 <signal handler called>
(gdb)
Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
---
gdb/arm-tdep.c | 35 +++++++++++++++++++++++++++++------
1 file changed, 29 insertions(+), 6 deletions(-)
diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
index 041e6afefed..bdbff247188 100644
--- a/gdb/arm-tdep.c
+++ b/gdb/arm-tdep.c
@@ -3591,9 +3591,13 @@ arm_m_exception_cache (frame_info_ptr this_frame)
ULONGEST fpcar;
/* Read FPCCR register. */
- gdb_assert (safe_read_memory_unsigned_integer (FPCCR,
- ARM_INT_REGISTER_SIZE,
- byte_order, &fpccr));
+ if (!safe_read_memory_unsigned_integer (FPCCR, ARM_INT_REGISTER_SIZE,
+ byte_order, &fpccr))
+ {
+ warning (_("Could not fetch required FPCCR content. Further "
+ "unwind is impossible."));
+ return nullptr;
+ }
/* Read FPCAR register. */
if (!safe_read_memory_unsigned_integer (FPCAR, ARM_INT_REGISTER_SIZE,
@@ -3669,9 +3673,15 @@ arm_m_exception_cache (frame_info_ptr this_frame)
aligner between the top of the 32-byte stack frame and the
previous context's stack pointer. */
ULONGEST xpsr;
- gdb_assert (safe_read_memory_unsigned_integer (cache->saved_regs[
- ARM_PS_REGNUM].addr (), 4,
- byte_order, &xpsr));
+ if (!safe_read_memory_unsigned_integer (cache->saved_regs[ARM_PS_REGNUM]
+ .addr (), ARM_INT_REGISTER_SIZE,
+ byte_order, &xpsr))
+ {
+ warning (_("Could not fetch required XPSR content. Further unwind "
+ "is impossible."));
+ return nullptr;
+ }
+
if (bit (xpsr, 9) != 0)
{
CORE_ADDR new_sp = arm_cache_get_prev_sp_value (cache, tdep) + 4;
@@ -3703,6 +3713,14 @@ arm_m_exception_this_id (frame_info_ptr this_frame,
*this_cache = arm_m_exception_cache (this_frame);
cache = (struct arm_prologue_cache *) *this_cache;
+ /* Unwind of this frame is not possible. Return outer_frame_id to stop the
+ unwinding. */
+ if (cache == nullptr)
+ {
+ *this_id = outer_frame_id;
+ return;
+ }
+
/* Our frame ID for a stub frame is the current SP and LR. */
arm_gdbarch_tdep *tdep
= gdbarch_tdep<arm_gdbarch_tdep> (get_frame_arch (this_frame));
@@ -3725,6 +3743,11 @@ arm_m_exception_prev_register (frame_info_ptr this_frame,
*this_cache = arm_m_exception_cache (this_frame);
cache = (struct arm_prologue_cache *) *this_cache;
+ /* It's not allowed to call prev_register when this_id has returned the
+ outer_frame_id. The arm_m_exception_cache function will return NULL when
+ the frame cannot be properly unwinded. */
+ gdb_assert (cache != nullptr);
+
/* The value was already reconstructed into PREV_SP. */
arm_gdbarch_tdep *tdep
= gdbarch_tdep<arm_gdbarch_tdep> (get_frame_arch (this_frame));
--
2.25.1
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH v3] gdb/arm: Stop unwinding on error, but do not assert
2022-10-13 10:53 [PATCH v3] gdb/arm: Stop unwinding on error, but do not assert Torbjörn SVENSSON
@ 2022-10-13 11:22 ` Luis Machado
0 siblings, 0 replies; 2+ messages in thread
From: Luis Machado @ 2022-10-13 11:22 UTC (permalink / raw)
To: Torbjörn SVENSSON, gdb-patches
Just one nit...
On 10/13/22 11:53, Torbjörn SVENSSON wrote:
> When it's impossible to read the FPCCR and XPSR, the unwinding is
> unpredictable as the it's not possible to determine the correct
> frame size or padding.
> The only sane thing to do in this condition is to stop the unwinding.
>
> Example session wihtout this patch:
>
> (gdb) bt
> #0 SVC_Handler () at .../GPIO/GPIO_EXTI/Src/stm32f4xx_it.c:112
> .../gdb/arm-tdep.c:3594: internal-error: arm_m_exception_cache: Assertion `safe_read_memory_unsigned_integer (FPCCR, ARM_INT_REGISTER_SIZE, byte_order, &fpccr)' failed.
> A problem internal to GDB has been detected,
> further debugging may prove unreliable.
> ----- Backtrace -----
> 0x5583bfb2a157 gdb_internal_backtrace_1
> ...
> ---------------------
>
> This is a bug, please report it. For instructions, see:
> <https://www.gnu.org/software/gdb/bugs/>.
>
> Aborted (core dumped)
>
> Example session with this patch:
>
> (gdb) bt
> #0 SVC_Handler () at .../GPIO/GPIO_EXTI/Src/stm32f4xx_it.c:112
> warning: Could not fetch required FPCCR content. Further unwind is impossible.
> #1 <signal handler called>
> (gdb)
>
> Signed-off-by: Torbjörn SVENSSON <torbjorn.svensson@foss.st.com>
> ---
> gdb/arm-tdep.c | 35 +++++++++++++++++++++++++++++------
> 1 file changed, 29 insertions(+), 6 deletions(-)
>
> diff --git a/gdb/arm-tdep.c b/gdb/arm-tdep.c
> index 041e6afefed..bdbff247188 100644
> --- a/gdb/arm-tdep.c
> +++ b/gdb/arm-tdep.c
> @@ -3591,9 +3591,13 @@ arm_m_exception_cache (frame_info_ptr this_frame)
> ULONGEST fpcar;
>
> /* Read FPCCR register. */
> - gdb_assert (safe_read_memory_unsigned_integer (FPCCR,
> - ARM_INT_REGISTER_SIZE,
> - byte_order, &fpccr));
> + if (!safe_read_memory_unsigned_integer (FPCCR, ARM_INT_REGISTER_SIZE,
> + byte_order, &fpccr))
> + {
> + warning (_("Could not fetch required FPCCR content. Further "
> + "unwind is impossible."));
unwind -> unwinding would read better.
> + return nullptr;
> + }
>
> /* Read FPCAR register. */
> if (!safe_read_memory_unsigned_integer (FPCAR, ARM_INT_REGISTER_SIZE,
> @@ -3669,9 +3673,15 @@ arm_m_exception_cache (frame_info_ptr this_frame)
> aligner between the top of the 32-byte stack frame and the
> previous context's stack pointer. */
> ULONGEST xpsr;
> - gdb_assert (safe_read_memory_unsigned_integer (cache->saved_regs[
> - ARM_PS_REGNUM].addr (), 4,
> - byte_order, &xpsr));
> + if (!safe_read_memory_unsigned_integer (cache->saved_regs[ARM_PS_REGNUM]
> + .addr (), ARM_INT_REGISTER_SIZE,
> + byte_order, &xpsr))
> + {
> + warning (_("Could not fetch required XPSR content. Further unwind "
> + "is impossible."));
Same here. unwind -> unwinding.
> + return nullptr;
> + }
> +
> if (bit (xpsr, 9) != 0)
> {
> CORE_ADDR new_sp = arm_cache_get_prev_sp_value (cache, tdep) + 4;
> @@ -3703,6 +3713,14 @@ arm_m_exception_this_id (frame_info_ptr this_frame,
> *this_cache = arm_m_exception_cache (this_frame);
> cache = (struct arm_prologue_cache *) *this_cache;
>
> + /* Unwind of this frame is not possible. Return outer_frame_id to stop the
Unwind -> Unwinding.
> + unwinding. */
> + if (cache == nullptr)
> + {
> + *this_id = outer_frame_id;
> + return;
> + }
> +
> /* Our frame ID for a stub frame is the current SP and LR. */
> arm_gdbarch_tdep *tdep
> = gdbarch_tdep<arm_gdbarch_tdep> (get_frame_arch (this_frame));
> @@ -3725,6 +3743,11 @@ arm_m_exception_prev_register (frame_info_ptr this_frame,
> *this_cache = arm_m_exception_cache (this_frame);
> cache = (struct arm_prologue_cache *) *this_cache;
>
> + /* It's not allowed to call prev_register when this_id has returned the
> + outer_frame_id. The arm_m_exception_cache function will return NULL when
> + the frame cannot be properly unwinded. */
> + gdb_assert (cache != nullptr);
> +
> /* The value was already reconstructed into PREV_SP. */
> arm_gdbarch_tdep *tdep
> = gdbarch_tdep<arm_gdbarch_tdep> (get_frame_arch (this_frame));
Otherwise LGTM.
No need to send a v4 with these changes. Make sure you fix the commit message to mention
the correct warnings.
Thanks!
Luis
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-10-13 11:22 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-13 10:53 [PATCH v3] gdb/arm: Stop unwinding on error, but do not assert Torbjörn SVENSSON
2022-10-13 11:22 ` Luis Machado
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).