Re: [PATCH v2 1/4] Remove stale breakpoint step-over information

[Simon Marchi <simark@simark.ca>]

On 2019-11-06 3:51 p.m., Maciej W. Rozycki wrote:
> Fix an issue with the `step_over_info' structure introduced with commit 
> 31e77af205cf ("PR breakpoints/7143 - Watchpoint does not trigger when 
> first set"), where information stored in there is not invalidated in a 
> remote debug scenario where software stepping in the all-stop mode is 
> forcibly interrupted and the target connection then closed.  As a result 
> a subsequent target connection triggers an assertion failure on 
> `ecs->event_thread->control.trap_expected' and cannot be successfully 
> established, making the particular instance of GDB unusable.
> 
> This was observed with a `riscv64-linux-gnu' cross-GDB and RISC-V/Linux 
> `gdbserver' being developed and an attempt to step over the `ret' aka 
> `c.jr ra' instruction where the value held in the `ra' aka `x1' register 
> and therefore the address of a software-stepping breakpoint to insert is 
> 0, as follows:
> 
> (gdb) target remote 1.2.3.4:56789
> Remote debugging using 1.2.3.4:56789
> warning: while parsing target description (at line 4): Target description specified unknown architecture "riscv:rv64id"
> warning: Could not load XML target description; ignoring
> Reading symbols from .../sysroot/lib/ld-linux-riscv64-lp64d.so.1...
> 0x0000001555556ef0 in _start ()
>    from .../sysroot/lib/ld-linux-riscv64-lp64d.so.1
> (gdb) break main
> Breakpoint 1 at 0x1643c
> (gdb) continue
> Continuing.
> Cannot access memory at address 0x0
> (gdb) x /i $pc
> => 0x15555607b8 <__GI__dl_debug_state>: ret
> (gdb) print /x $ra
> $1 = 0x0
> (gdb) stepi
> ^C^CInterrupted while waiting for the program.
> Give up waiting? (y or n) y
> Quit
> (gdb) kill
> Kill the program being debugged? (y or n) y
> [Inferior 1 (process 8964) killed]
> (gdb) target remote 1.2.3.4:56789
> Remote debugging using 1.2.3.4:56789
> warning: while parsing target description (at line 4): Target description specified unknown architecture "riscv:rv64id"
> warning: Could not load XML target description; ignoring
> .../gdb/infrun.c:5299: internal-error: int finish_step_over(execution_control_state*): Assertion `ecs->event_thread->control.trap_expected' failed.
> A problem internal to GDB has been detected,
> further debugging may prove unreliable.
> Quit this debugging session? (y or n) y
> 
> This is a bug, please report it.  For instructions, see:
> <http://www.gnu.org/software/gdb/bugs/>.
> 
> .../gdb/infrun.c:5299: internal-error: int finish_step_over(execution_control_state*): Assertion `ecs->event_thread->control.trap_expected' failed.
> A problem internal to GDB has been detected,
> further debugging may prove unreliable.
> Create a core file of GDB? (y or n) n
> Command aborted.
> (gdb) 
> 
> Correct the issue by making a call to clear global breakpoint step-over 
> information from `clear_thread_inferior_resources'.  To do so add an 
> argument to `clear_step_over_info' giving the global thread number to 
> check against, complementing one given to `set_step_over_info' when the 
> information concerned is recorded, so that information is not removed 
> for a thread that has stayed alive in a multi-target scenario.
> 
> Adjust all the existing `clear_step_over_info' call sites accordingly 
> where a step over has completed successfully and the thread that has 
> hit the breakpoint is expected to be one for which the breakpoint has 
> been inserted.
> 
> 	gdb/
> 	* infrun.h (clear_step_over_info): New prototype.
> 	* infrun.c (thread_is_stepping_over_breakpoint): Move earlier 
> 	on.
> 	(clear_step_over_info): Use it.  Make external.
> 	(resume_1, finish_step_over, handle_signal_stop)
> 	(keep_going_stepped_thread, keep_going_pass_signal): Adjust 
> 	accordingly.
> 	* thread.c (clear_thread_inferior_resources): Call 
> 	`clear_step_over_info'.

This patch looks good to me, but I'd really prefer if Pedro could look at it,
since he's much more adept in this area.

Simon