Re: [PATCH] gdb: z80: Fix endless backtrace loop and assertion crashes

[Kevin Buettner <kevinb@redhat.com>]

Hi Ronald,

On Fri,  5 Jun 2026 20:56:47 +0200
Ronald Hecht <ronald.hecht@gmx.de> wrote:

> This patch fixes two distinct issues in the Z80 frame unwind cache:
> 
> 1. An infinite loop in the stack pointer wrapping logic. The `for (;;
> ++sp)` loop could run endlessly if the overflow condition `sp <
> this_base` was never met due to 16-bit address space wrapping. Introduced
> a `loop_count` to break out after 4 iterations as a sensible heuristic
> for Z80.
> 
> 2. A potential internal GDB assertion failure. The code previously called
>    `.addr()` directly on saved registers without verifying if they
> actually held an address, causing crashes when encountering unexpected
> register states (e.g. REG_UNKNOWN). Added a check for `.is_addr()`
> beforehand.
> 
> gdb/ChangeLog:
> 	* z80-tdep.c (z80_frame_unwind_cache): Prevent infinite loop by
> 	adding loop_count. Check is_addr() before calling addr().
> ---
>  gdb/z80-tdep.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/gdb/z80-tdep.c b/gdb/z80-tdep.c
> index 3a0d5f7e393..5e1c3a877b3 100644
> --- a/gdb/z80-tdep.c
> +++ b/gdb/z80-tdep.c
> @@ -597,6 +597,7 @@ z80_frame_unwind_cache (const frame_info_ptr
> &this_frame, CORE_ADDR sp;
>        CORE_ADDR sp_mask = (1 << gdbarch_ptr_bit(gdbarch)) - 1;
>        enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> +      int loop_count = 0;
>        /* Assume that the FP is this frame's SP but with that pushed
>  	 stack space added back.  */
>        this_base = get_frame_register_unsigned (this_frame,
> Z80_SP_REGNUM); @@ -604,7 +605,7 @@ z80_frame_unwind_cache (const
> frame_info_ptr &this_frame, for (;; ++sp)
>  	{
>  	  sp &= sp_mask;
> -	  if (sp < this_base)
> +	  if (++loop_count > 4 || sp < this_base)

I'm wondering if there's a better way to handle this?  It seems to me
that we should be able come up with a comparison against the unmasked
'sp' value which detects bottom of stack (or end of address space).  I
think that perhaps this might work:

      ...
      for (;; ++sp)
	{
	  if (sp > sp_mask)
	    { /* overflow, looks like end of stack */
	      sp = this_base + info->size;
	      break;
	    }
	  /* find return address */
	  read_memory (sp, buf, addr_len);
	  ...

Note that I got rid of the masking operation; It's no longer needed with
that 'sp > sp_mask' test.  But you might want to rename 'sp_mask' to
be 'addr_space_max' or some such. 

Can you test this version?

It may still be desirable to place a limit on the number of stack
slots that are checked in this code.  I have no objection to doing
this.  I only ask that you document the constant chosen ('4' perhaps)
and that you explain why it's likely enough for the z80 architecture. 
(Put the explanation in a code comment, instead of only the commit
log.)

Kevin