Re: [PATCH] Avoid get_ptrace_pid() usage on NetBSD in x86-bsd-nat.c

[Simon Marchi <simark@simark.ca>]

On 2020-03-18 12:13 p.m., Kamil Rytarowski wrote:
> Add gdb_ptrace() that wraps the ptrace(2) API and correctly passes
> the pid,lwp pair to the calls on NetBSD; and the result of
> get_ptrace_pid() on other BSD Operating Systems.
> 
> gdb/ChangeLog:
> 
> 	* x86-bsd-nat.c (gdb_ptrace): New.
> 	* (x86bsd_dr_get, x86bsd_dr_set): Use gdb_ptrace.
> ---
>  gdb/ChangeLog     |  5 +++++
>  gdb/x86-bsd-nat.c | 39 +++++++++++++++++++--------------------
>  2 files changed, 24 insertions(+), 20 deletions(-)
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 84964dc00ac..1beb835df78 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,8 @@
> +2020-03-18  Kamil Rytarowski  <n54@gmx.com>
> +
> +	* x86-bsd-nat.c (gdb_ptrace): New.
> +	* (x86bsd_dr_get, x86bsd_dr_set): Use gdb_ptrace.
> +
>  2020-03-17  Kamil Rytarowski  <n54@gmx.com>
> 
>  	* regformats/regdef.h: Put reg in gdb namespace.
> diff --git a/gdb/x86-bsd-nat.c b/gdb/x86-bsd-nat.c
> index 640a3c28110..37d0bfda37c 100644
> --- a/gdb/x86-bsd-nat.c
> +++ b/gdb/x86-bsd-nat.c
> @@ -33,6 +33,19 @@
>  #include "inf-ptrace.h"
>  \f
> 
> +static int
> +gdb_ptrace (PTRACE_TYPE_ARG1 request, ptid_t ptid, PTRACE_TYPE_ARG3 addr)
> +{
> +#ifdef __NetBSD__
> +  /* Support for NetBSD threads: unlike other ptrace implementations in this
> +     file, NetBSD requires that we pass both the pid and lwp.  */
> +  return ptrace (request, ptid.pid (), addr, ptid.lwp ());
> +#else
> +  pid_t pid = get_ptrace_pid (ptid);
> +  return ptrace (request, pid, addr, 0);
> +#endif
> +}
> +
>  #ifdef PT_GETXSTATE_INFO
>  size_t x86bsd_xsave_len;
>  #endif
> @@ -56,14 +69,9 @@ static unsigned long
>  x86bsd_dr_get (ptid_t ptid, int regnum)
>  {
>    struct dbreg dbregs;
> -#ifdef __NetBSD__
> -  int lwp = inferior_ptid.lwp ();
> -#else
> -  int lwp = 0;
> -#endif
> 
> -  if (ptrace (PT_GETDBREGS, get_ptrace_pid (inferior_ptid),
> -	      (PTRACE_TYPE_ARG3) &dbregs, lwp) == -1)
> +  if (gdb_ptrace (PT_GETDBREGS, inferior_ptid,
> +		  (PTRACE_TYPE_ARG3) &dbregs) == -1)
>      perror_with_name (_("Couldn't read debug registers"));

This function accepts a ptid parameter but does not use it.  Can you change
it so it uses the parameter, and not inferior_ptid?  All the callers pass
inferior_ptid, so it won't change the behavior.

> 
>    return DBREG_DRX ((&dbregs), regnum);
> @@ -73,14 +81,9 @@ static void
>  x86bsd_dr_set (int regnum, unsigned long value)
>  {
>    struct dbreg dbregs;
> -#ifdef __NetBSD__
> -  int lwp = inferior_ptid.lwp ();
> -#else
> -  int lwp = 0;
> -#endif
> 
> -  if (ptrace (PT_GETDBREGS, get_ptrace_pid (inferior_ptid),
> -              (PTRACE_TYPE_ARG3) &dbregs, lwp) == -1)
> +  if (gdb_ptrace (PT_GETDBREGS, inferior_ptid,
> +		  (PTRACE_TYPE_ARG3) &dbregs) == -1)
>      perror_with_name (_("Couldn't get debug registers"));

For symmetry, can you please change this function to accept a ptid, just like
x86bsd_dr_get, and update the callers to pass inferior_ptid?

Simon