Re: [PATCH v2] Conditionally restore displaced stepping state after fork.

[Simon Marchi via Gdb-patches <gdb-patches@sourceware.org>]

On 2021-06-01 5:11 p.m., John Baldwin wrote:
> There is no default method for
> gdbarch_displaced_step_restore_all_in_ptid, so calling it
> unconditionally for fork events triggered an assertion failure on
> platforms that do not support displaced stepping.  To fix, only invoke
> the method if the gdbarch supports displaced stepping.
> 
> Note that not all gdbarches support both displaced stepping and fork
> events, so gdbarch validation does not require
> gdbarch_displaced_step_restore_all_in_ptid for any gdbarch supporting
> displaced stepping.  However, the internal assertion in
> gdbarch_displaced_step_restore_all_in_ptid should catch any gdbarches
> which do support both but fail to provide this method.
> 
> gdb/ChangeLog:
> 
> 	* infrun.c (handle_inferior_event): Only call
> 	gdbarch_displaced_step_restore_all_in_ptid if
> 	gdbarch_supports_displaced_stepping is true.
> ---
>  gdb/ChangeLog |  6 ++++++
>  gdb/infrun.c  | 16 ++++++++++++----
>  2 files changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 03910c0634..b0f448a35e 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,9 @@
> +2021-05-30  John Baldwin  <jhb@FreeBSD.org>
> +
> +	* infrun.c (handle_inferior_event): Only call
> +	gdbarch_displaced_step_restore_all_in_ptid if
> +	gdbarch_supports_displaced_stepping is true.
> +
>  2021-05-27  Simon Marchi  <simon.marchi@polymtl.ca>
>  
>  	* Fix tab after space indentation issues throughout.
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index e9624d2a9b..488bcc1e10 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -5493,10 +5493,18 @@ handle_inferior_event (struct execution_control_state *ecs)
>  	struct gdbarch *gdbarch = regcache->arch ();
>  	inferior *parent_inf = find_inferior_ptid (ecs->target, ecs->ptid);
>  
> -	/* If this is a fork (child gets its own address space copy) and some
> -	   displaced step buffers were in use at the time of the fork, restore
> -	   the displaced step buffer bytes in the child process.  */
> -	if (ecs->ws.kind == TARGET_WAITKIND_FORKED)
> +	/* If this is a fork (child gets its own address space copy)
> +	   and some displaced step buffers were in use at the time of
> +	   the fork, restore the displaced step buffer bytes in the
> +	   child process.
> +
> +	   Architectures which support displaced stepping and fork
> +	   events must supply an implementation of
> +	   gdbarch_displaced_step_restore_all_in_ptid.  This is not
> +	   enforced during gdbarch validation to support architectures
> +	   which support displaced stepping but not forks.  */
> +	if (ecs->ws.kind == TARGET_WAITKIND_FORKED
> +	    && gdbarch_supports_displaced_stepping (gdbarch))
>  	  gdbarch_displaced_step_restore_all_in_ptid
>  	    (gdbarch, parent_inf, ecs->ws.value.related_pid);
>  
> 

LGTM, thanks!

Simon