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

[Ronald Hecht <ronald.hecht@gmx.de>]

Hi,

Thank you for the review and the constructive feedback!

You are completely right about dropping the masking operation and 
checking for |sp > sp_mask| (or |addr_space_max|). I tested your 
suggested change and it indeed makes the logic much cleaner and works 
perfectly.

Regarding the limit on the stack slots (|loop_count|): I did a test run 
without my abort condition to see exactly what happens under the hood 
when the unwinder gets confused (e.g. due to a slightly corrupt stack or 
unpopped arguments) and fails to find the return address immediately.

Here is a snippet from my remote debug log:

(gdb) bt
[remote] Sending packet: $m1ae,2#c2
[remote] Received Ack
[remote] Packet received: D8E3
[remote] Sending packet: $me3d5,3#fd
[remote] Received Ack
[remote] Packet received: CDCAE3
[remote] Sending packet: $m1af,2#c3
[remote] Received Ack
...
[remote] Sending packet: $m1b0,2#8e
...
[remote] Sending packet: $m1b1,2#8f



As you can see, the Stack Pointer started at |0x01ae|. Because it 
couldn't find a matching |CALL| instruction, it started creeping up the 
memory byte-by-byte (|1ae|, |1af|, |1b0|, |1b1|...).

Without a hard limit, this loop will iterate all the way up to |0xffff|. 
That is over 65,000 iterations. Since each iteration triggers two 
|read_memory()| calls (one for the SP, one to check for the |CALL| 
instruction), GDB ends up sending over 130,000 remote packets over a 
slow serial or JTAG connection. This effectively hangs the debugger 
session for minutes.

As for why the limit is specifically set to |4| iterations: The Z80 
heavily relies on 16-bit register pairs (2 bytes). A limit of 4 bytes 
perfectly covers typical small stack offsets, such as two unpopped 
16-bit arguments or a temporary 32-bit push (which SDCC sometimes 
generates). Scanning further (e.g. 8+ bytes) drastically increases the 
risk of false positives. We would wander deeply into the caller's local 
variables, where hitting a random |0xCD| (CALL opcode) byte is quite 
likely. This would cause GDB to hallucinate a corrupted backtrace. A 
limit of 4 comfortably pulls the emergency brake before that happens.

I will prepare and send a |v2| patch shortly. It will include your clean 
|sp > addr_space_max| refactoring, keep the |loop_count| limit, and add 
a detailed code comment explaining this exact rationale for the magic 
number '4' as requested.

Thanks again for your guidance!

Best regards,
Ronald