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

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

[Kamil Rytarowski]

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"));

   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 some mysterious reason, some of the reserved bits in the
@@ -92,12 +95,8 @@ x86bsd_dr_set (int regnum, unsigned long value)

   for (thread_info *thread : current_inferior ()->non_exited_threads ())
     {
-#ifdef __NetBSD__
-      lwp = thread->ptid.lwp ();
-#endif
-
-      if (ptrace (PT_SETDBREGS, get_ptrace_pid (thread->ptid),
-		  (PTRACE_TYPE_ARG3) &dbregs, lwp) == -1)
+      if (gdb_ptrace (PT_SETDBREGS, thread->ptid,
+		  (PTRACE_TYPE_ARG3) &dbregs) == -1)
 	perror_with_name (_("Couldn't write debug registers"));
     }
 }
--
2.25.0

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

[Simon Marchi]

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

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

[Kamil Rytarowski]

[-- Attachment #1.1: Type: text/plain, Size: 3464 bytes --]

On 19.03.2020 02:35, Simon Marchi wrote:
> 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.
> 

OK.

>>
>>    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?
> 

This is a good idea.. however it is intrusive now and requires patching
code shared with windows, linux, darwin, ...

I prefer to leave it for refactoring in future. I don't have environment
to test other Operating Systems than NetBSD.

> Simon
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Kamil Rytarowski]

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 | 38 ++++++++++++++++++--------------------
 2 files changed, 23 insertions(+), 20 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 7f87eceaf70..23a5932489f 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2020-03-19  Kamil Rytarowski  <n54@gmx.com>
+
+	* x86-bsd-nat.c (gdb_ptrace): New.
+	* (x86bsd_dr_get, x86bsd_dr_set): Use gdb_ptrace.
+
 2020-03-19  Andrew Burgess  <andrew.burgess@embecosm.com>

 	* remote.c (remote_target::process_stop_reply): Handle events for
diff --git a/gdb/x86-bsd-nat.c b/gdb/x86-bsd-nat.c
index 640a3c28110..c1da7d6ce30 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,8 @@ 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, ptid, (PTRACE_TYPE_ARG3) &dbregs, lwp) == -1)
     perror_with_name (_("Couldn't read debug registers"));

   return DBREG_DRX ((&dbregs), regnum);
@@ -73,14 +80,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, lwp) == -1)
     perror_with_name (_("Couldn't get debug registers"));

   /* For some mysterious reason, some of the reserved bits in the
@@ -92,12 +94,8 @@ x86bsd_dr_set (int regnum, unsigned long value)

   for (thread_info *thread : current_inferior ()->non_exited_threads ())
     {
-#ifdef __NetBSD__
-      lwp = thread->ptid.lwp ();
-#endif
-
-      if (ptrace (PT_SETDBREGS, get_ptrace_pid (thread->ptid),
-		  (PTRACE_TYPE_ARG3) &dbregs, lwp) == -1)
+      if (gdb_ptrace (PT_SETDBREGS, thread->ptid,
+		      (PTRACE_TYPE_ARG3) &dbregs, lwp) == -1)
 	perror_with_name (_("Couldn't write debug registers"));
     }
 }
--
2.25.0

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

[Kamil Rytarowski]

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 | 38 ++++++++++++++++++--------------------
 2 files changed, 23 insertions(+), 20 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 7f87eceaf70..23a5932489f 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2020-03-19  Kamil Rytarowski  <n54@gmx.com>
+
+	* x86-bsd-nat.c (gdb_ptrace): New.
+	* (x86bsd_dr_get, x86bsd_dr_set): Use gdb_ptrace.
+
 2020-03-19  Andrew Burgess  <andrew.burgess@embecosm.com>

 	* remote.c (remote_target::process_stop_reply): Handle events for
diff --git a/gdb/x86-bsd-nat.c b/gdb/x86-bsd-nat.c
index 640a3c28110..4332cb717bc 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,8 @@ 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, ptid, (PTRACE_TYPE_ARG3) &dbregs) == -1)
     perror_with_name (_("Couldn't read debug registers"));

   return DBREG_DRX ((&dbregs), regnum);
@@ -73,14 +80,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 some mysterious reason, some of the reserved bits in the
@@ -92,12 +94,8 @@ x86bsd_dr_set (int regnum, unsigned long value)

   for (thread_info *thread : current_inferior ()->non_exited_threads ())
     {
-#ifdef __NetBSD__
-      lwp = thread->ptid.lwp ();
-#endif
-
-      if (ptrace (PT_SETDBREGS, get_ptrace_pid (thread->ptid),
-		  (PTRACE_TYPE_ARG3) &dbregs, lwp) == -1)
+      if (gdb_ptrace (PT_SETDBREGS, thread->ptid,
+		      (PTRACE_TYPE_ARG3) &dbregs) == -1)
 	perror_with_name (_("Couldn't write debug registers"));
     }
 }
--
2.25.0

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

[Simon Marchi]

On 2020-03-19 8:47 a.m., Kamil Rytarowski wrote:
> This is a good idea.. however it is intrusive now and requires patching
> code shared with windows, linux, darwin, ...
> 
> I prefer to leave it for refactoring in future. I don't have environment
> to test other Operating Systems than NetBSD.

Hmm no, the callers of x86bsd_dr_set are only in this file (x86bsd_dr_set_control and
x86bsd_dr_set_addr).  They would now look like this:

static void
x86bsd_dr_set_control (unsigned long control)
{
  x86bsd_dr_set (inferior_ptid, 7, control);
}

static void
x86bsd_dr_set_addr (int regnum, CORE_ADDR addr)
{
  gdb_assert (regnum >= 0 && regnum <= 4);

  x86bsd_dr_set (inferior_ptid, regnum, addr);
}

Adding a ptid parameter to x86bsd_dr_set_control and x86bsd_dr_set_addr would be the
next logical step, but that indeed would be a more intrusive change, so I don't suggest
doing that right now.

Simon

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

[Kamil Rytarowski]

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_set): Add new argument `ptid'.
	* (x86bsd_dr_get, x86bsd_dr_set, x86bsd_dr_set_control,
	x86bsd_dr_set_addr): Update.
---
 gdb/ChangeLog     |  7 +++++++
 gdb/x86-bsd-nat.c | 43 ++++++++++++++++++++-----------------------
 2 files changed, 27 insertions(+), 23 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 7f87eceaf70..0955d648e79 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2020-03-19  Kamil Rytarowski  <n54@gmx.com>
+
+	* x86-bsd-nat.c (gdb_ptrace): New.
+	* (x86bsd_dr_set): Add new argument `ptid'.
+	* (x86bsd_dr_get, x86bsd_dr_set, x86bsd_dr_set_control,
+	x86bsd_dr_set_addr): Update.
+
 2020-03-19  Andrew Burgess  <andrew.burgess@embecosm.com>

 	* remote.c (remote_target::process_stop_reply): Handle events for
diff --git a/gdb/x86-bsd-nat.c b/gdb/x86-bsd-nat.c
index 640a3c28110..9e2bea1e020 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,31 +69,19 @@ 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, ptid, (PTRACE_TYPE_ARG3) &dbregs) == -1)
     perror_with_name (_("Couldn't read debug registers"));

   return DBREG_DRX ((&dbregs), regnum);
 }

 static void
-x86bsd_dr_set (int regnum, unsigned long value)
+x86bsd_dr_set (ptid_t ptid, 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, ptid, (PTRACE_TYPE_ARG3) &dbregs) == -1)
     perror_with_name (_("Couldn't get debug registers"));

   /* For some mysterious reason, some of the reserved bits in the
@@ -92,12 +93,8 @@ x86bsd_dr_set (int regnum, unsigned long value)

   for (thread_info *thread : current_inferior ()->non_exited_threads ())
     {
-#ifdef __NetBSD__
-      lwp = thread->ptid.lwp ();
-#endif
-
-      if (ptrace (PT_SETDBREGS, get_ptrace_pid (thread->ptid),
-		  (PTRACE_TYPE_ARG3) &dbregs, lwp) == -1)
+      if (gdb_ptrace (PT_SETDBREGS, thread->ptid,
+		      (PTRACE_TYPE_ARG3) &dbregs) == -1)
 	perror_with_name (_("Couldn't write debug registers"));
     }
 }
@@ -105,7 +102,7 @@ x86bsd_dr_set (int regnum, unsigned long value)
 static void
 x86bsd_dr_set_control (unsigned long control)
 {
-  x86bsd_dr_set (7, control);
+  x86bsd_dr_set (inferior_ptid, 7, control);
 }

 static void
@@ -113,7 +110,7 @@ x86bsd_dr_set_addr (int regnum, CORE_ADDR addr)
 {
   gdb_assert (regnum >= 0 && regnum <= 4);

-  x86bsd_dr_set (regnum, addr);
+  x86bsd_dr_set (inferior_ptid, regnum, addr);
 }

 static CORE_ADDR
--
2.25.0

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

[Kamil Rytarowski]

[-- Attachment #1.1: Type: text/plain, Size: 1050 bytes --]

On 19.03.2020 14:22, Simon Marchi wrote:
> On 2020-03-19 8:47 a.m., Kamil Rytarowski wrote:
>> This is a good idea.. however it is intrusive now and requires patching
>> code shared with windows, linux, darwin, ...
>>
>> I prefer to leave it for refactoring in future. I don't have environment
>> to test other Operating Systems than NetBSD.
> 
> Hmm no, the callers of x86bsd_dr_set are only in this file (x86bsd_dr_set_control and
> x86bsd_dr_set_addr).  They would now look like this:
> 
> static void
> x86bsd_dr_set_control (unsigned long control)
> {
>   x86bsd_dr_set (inferior_ptid, 7, control);
> }
> 
> static void
> x86bsd_dr_set_addr (int regnum, CORE_ADDR addr)
> {
>   gdb_assert (regnum >= 0 && regnum <= 4);
> 
>   x86bsd_dr_set (inferior_ptid, regnum, addr);
> }
> 
> Adding a ptid parameter to x86bsd_dr_set_control and x86bsd_dr_set_addr would be the
> next logical step, but that indeed would be a more intrusive change, so I don't suggest
> doing that right now.
> 

Done in v4.

> Simon
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Simon Marchi]

On 2020-03-19 9:36 a.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.

Thanks.  I can't test, but by inspection this patch LGTM.

Simon