Re: [PATCH] gdb/linux-nat: Fix mem access ptrace fallback (PR threads/31579)

[Andrew Burgess <aburgess@redhat.com>]

Pedro Alves <pedro@palves.net> writes:

>
> diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
> index 2602e1f240d..d2b9aea724d 100644
> --- a/gdb/linux-nat.c
> +++ b/gdb/linux-nat.c

> @@ -2199,6 +2199,21 @@ mark_lwp_dead (lwp_info *lp, int status)
>    lp->stopped = 1;
>  }
>  
> +/* Return true if LP is dead, with a pending exit/signalled event.  */
> +
> +static bool
> +is_lwp_marked_dead (lwp_info *lp)
> +{
> +  switch (lp->waitstatus.kind ())
> +    {
> +    case TARGET_WAITKIND_EXITED:
> +    case TARGET_WAITKIND_THREAD_EXITED:
> +    case TARGET_WAITKIND_SIGNALLED:
> +      return true;
> +    }
> +  return false;
> +}

I wonder if this would be better as a method on waitstatus?  There's at
least one place in infrun.c (in handle_one) where we check for these
three statuses, and there's a few places where we check
TARGET_WAITKIND_EXITED and TARGET_WAITKIND_SIGNALLED ... and I suspect
we either _should_ be checking for TARGET_WAITKIND_THREAD_EXITED, or it
wouldn't matter if we did.

Not that I'm suggesting you should look at all those ... just I'm
wondering if it would be better to make is_lwp_marked_dead a waitstatus
method now, and we can then clean up other users later?

Consider this an optional suggestion, we can always make this a method
later if needed.

> +
>  /* Wait for LP to stop.  Returns the wait status, or 0 if the LWP has
>     exited.  */
>  
> @@ -3879,6 +3894,20 @@ linux_proc_xfer_memory_partial (int pid, gdb_byte *readbuf,
>  				const gdb_byte *writebuf, ULONGEST offset,
>  				LONGEST len, ULONGEST *xfered_len);
>  
> +/* Look for an LWP of PID that we know is ptrace-stopped.  Returns
> +   NULL if none is found.  */

s/NULL/nullptr/ ?

Approved-By: Andrew Burgess <aburgess@redhat.com>

Thanks,
Andrew