Re: [PATCH v2 2/2] gdb: retain thread-specific breakpoints in reverse execution targets

[Bruno Larsen <blarsen@redhat.com>]

On 07/07/2023 18:24, Magne Hov via Gdb-patches wrote:
> Thread-specific breakpoints are currently ignored and removed (since
> 49fa26b0411d990d36f3f6c095d167f3d12afdf4) if the corresponding thread
> has exited.  This makes sense for targets that only execute in the
> forward direction because those breakpoints can never be hit again, but
> for targets with reverse execution the same thread can be seen again.
> ---

Hi!

Thanks for working on this. Same as the other one, it looks good to go

Reviewed-By: Bruno Larsen <blarsen@redhat.com>

I hope a global maintainer or Markus look at it soon to approve it!

-- 
Cheers,
Bruno

>   gdb/breakpoint.c | 18 +++++++++++++-----
>   1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index da6c8de9d14..9a25c5f663d 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -3157,10 +3157,12 @@ insert_breakpoint_locations (void)
>   	continue;
>   
>         /* There is no point inserting thread-specific breakpoints if
> -	 the thread no longer exists.  ALL_BP_LOCATIONS bp_location
> -	 has BL->OWNER always non-NULL.  */
> +	 the thread no longer exists, unless the target supports
> +	 reverse execution.  ALL_BP_LOCATIONS bp_location has
> +	 BL->OWNER always non-NULL.  */
>         if (bl->owner->thread != -1
> -	  && !valid_global_thread_id (bl->owner->thread))
> +	  && !valid_global_thread_id (bl->owner->thread)
> +	  && !target_can_execute_reverse ())
>   	continue;
>   
>         switch_to_program_space_and_thread (bl->pspace);
> @@ -3245,12 +3247,18 @@ remove_breakpoints (void)
>     return val;
>   }
>   
> -/* When a thread exits, remove breakpoints that are related to
> -   that thread.  */
> +/* When a thread exits, remove breakpoints that are related to that
> +   thread and cannot be hit again.  */
>   
>   static void
>   remove_threaded_breakpoints (struct thread_info *tp, int silent)
>   {
> +  /* Targets that support reverse execution may navigate to a point in
> +     time where an exited thread reappears and where its breakpoints
> +     are still relevant.  */
> +  if (target_can_execute_reverse ())
> +    return;
> +
>     for (breakpoint &b : all_breakpoints_safe ())
>       {
>         if (b.thread == tp->global_num && user_breakpoint_p (&b))