[PATCH v2 00/10] Add minimal NetBSD/amd64 gdbserver support

[PATCH v2 00/10] Add minimal NetBSD/amd64 gdbserver support

[Kamil Rytarowski]

Extract reusable functionality from gdb/nbsd-nat.c into
gdb/nat/netbsd-nat.c and switch the gdb/nbsd-nat.c functions
(nbsd_nat_target::pid_to_exec_file, nbsd_nat_target::thread_alive,
nbsd_nat_target::thread_name, nbsd_nat_target::post_startup_inferior,
nbsd_nat_target::post_attach, nbsd_nat_target::xfer_partial,
nbsd_add_threads) to gdb/nat.

Add handle_eintr in gdbsupport to wrap syscalls and EINTR handling.

Add NetBSD/amd64 gdbserver support

Implement the following functionality: create_inferior,
post_create_inferior, attach, kill, detach, mourn, join, thread_alive,
resume, wait, fetch_registers, store_registers, read_memory, write_memory,
request_interrupt, supports_read_auxv, read_auxv,
supports_hardware_single_step, sw_breakpoint_from_kind,
supports_z_point_type, insert_point, remove_point,
stopped_by_sw_breakpoint, supports_qxfer_siginfo, qxfer_siginfo,
supports_stopped_by_sw_breakpoint, supports_non_stop,
supports_multi_process, supports_fork_events, supports_vfork_events,
supports_exec_events, supports_disable_randomization,
supports_qxfer_libraries_svr4, qxfer_libraries_svr4,
supports_pid_to_exec_file, pid_to_exec_file, thread_name,
supports_catch_syscall.

The only CPU architecture supported: x86_64.

Implement only support for hardware assisted single step and
software breakpoint.

Implement support only for regular X86 registers, thus no FPU.

Changes in v2:

 * handle_eintr downgraded from C++14 to C++11 and allow predefining the
   value of failure.
 * Enhance ChangeLog entries.
 * Remove srv_netbsd from /gdbserver/configure.srv.
 * Enhance wording and code style.
 * Remove function return type from netbsd_add_process().
 * Rename netbsd_nat::list_threads to netbsd_nat::for_each_thread.
 * Return error on failure in netbsd_process_target::kill.
 * Use core_addr_to_string() when printing CORE_ADDR.
 * Switch from (*target). to target->.
 * Simplify netbsd_process_target::sw_breakpoint_from_kind.
 * Remove netbsd_target_ops::process_qsupported() and
   x86_64_netbsd_update_xmltarget().

Kamil Rytarowski (10):
  Add handle_eintr to wrap EINTR handling in syscalls
  Register a placeholder for NetBSD shared functions in gdb/nat
  Build nat/netbsd-nat.o for the NetBSD native target
  Add netbsd_nat::pid_to_exec_file
  Add gdb/nat common functions for listing threads
  Add netbsd_nat::enable_proc_events in gdb/nat
  Add a common utility function to read and write siginfo_t in inferior
  Avoid double free in startup_inferior
  Switch local native code to gdb/nat shared functions
  Add minimal and functional NetBSD/amd64 gdbserver

 gdb/ChangeLog                  |   53 ++
 gdb/configure.nat              |    2 +-
 gdb/nat/fork-inferior.c        |    5 +-
 gdb/nat/netbsd-nat.c           |  213 +++++
 gdb/nat/netbsd-nat.h           |   46 ++
 gdb/nbsd-nat.c                 |  147 +---
 gdbserver/ChangeLog            |    9 +
 gdbserver/Makefile.in          |    3 +
 gdbserver/configure.srv        |    6 +
 gdbserver/netbsd-low.cc        | 1348 ++++++++++++++++++++++++++++++++
 gdbserver/netbsd-low.h         |  154 ++++
 gdbserver/netbsd-x86_64-low.cc |  187 +++++
 gdbsupport/ChangeLog           |    4 +
 gdbsupport/eintr.h             |   41 +
 14 files changed, 2082 insertions(+), 136 deletions(-)
 create mode 100644 gdb/nat/netbsd-nat.c
 create mode 100644 gdb/nat/netbsd-nat.h
 create mode 100644 gdbserver/netbsd-low.cc
 create mode 100644 gdbserver/netbsd-low.h
 create mode 100644 gdbserver/netbsd-x86_64-low.cc
 create mode 100644 gdbsupport/eintr.h

--
2.28.0

[PATCH v2 01/10] Add handle_eintr to wrap EINTR handling in syscalls

[Kamil Rytarowski]

gdbsupport/ChangeLog:

	* eintr.h: New file.
---
 gdbsupport/ChangeLog |  4 ++++
 gdbsupport/eintr.h   | 41 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+)
 create mode 100644 gdbsupport/eintr.h

diff --git a/gdbsupport/ChangeLog b/gdbsupport/ChangeLog
index a1960537384..d26000e36ac 100644
--- a/gdbsupport/ChangeLog
+++ b/gdbsupport/ChangeLog
@@ -1,3 +1,7 @@
+2020-09-04  Kamil Rytarowski  <n54@gmx.com>
+
+	* eintr.h: New file.
+
 2020-08-13  Simon Marchi  <simon.marchi@polymtl.ca>

 	* selftest.h (run_tests): Change parameter to array_view.
diff --git a/gdbsupport/eintr.h b/gdbsupport/eintr.h
new file mode 100644
index 00000000000..74b7038da60
--- /dev/null
+++ b/gdbsupport/eintr.h
@@ -0,0 +1,41 @@
+/* Utility for handling interrupted syscalls by signals.
+
+   Copyright (C) 2020 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef GDBSUPPORT_EINTR_H
+#define GDBSUPPORT_EINTR_H
+
+#include <cerrno>
+
+namespace gdb
+{
+template <typename Ret, typename Fun, typename... Args>
+inline Ret handle_eintr (const Ret &R, const Fun &F, const Args &... A)
+{
+  Ret ret;
+  do
+    {
+      errno = 0;
+      ret = F (A...);
+    }
+  while (ret == R && errno == EINTR);
+  return ret;
+}
+}
+
+#endif /* GDBSUPPORT_EINTR_H */
--
2.28.0

Re: [PATCH v2 01/10] Add handle_eintr to wrap EINTR handling in syscalls

[Simon Marchi]

On 2020-09-03 8:28 p.m., Kamil Rytarowski wrote:
> gdbsupport/ChangeLog:
> 
> 	* eintr.h: New file.
> ---
>  gdbsupport/ChangeLog |  4 ++++
>  gdbsupport/eintr.h   | 41 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 45 insertions(+)
>  create mode 100644 gdbsupport/eintr.h
> 
> diff --git a/gdbsupport/ChangeLog b/gdbsupport/ChangeLog
> index a1960537384..d26000e36ac 100644
> --- a/gdbsupport/ChangeLog
> +++ b/gdbsupport/ChangeLog
> @@ -1,3 +1,7 @@
> +2020-09-04  Kamil Rytarowski  <n54@gmx.com>
> +
> +	* eintr.h: New file.
> +
>  2020-08-13  Simon Marchi  <simon.marchi@polymtl.ca>
> 
>  	* selftest.h (run_tests): Change parameter to array_view.
> diff --git a/gdbsupport/eintr.h b/gdbsupport/eintr.h
> new file mode 100644
> index 00000000000..74b7038da60
> --- /dev/null
> +++ b/gdbsupport/eintr.h
> @@ -0,0 +1,41 @@
> +/* Utility for handling interrupted syscalls by signals.
> +
> +   Copyright (C) 2020 Free Software Foundation, Inc.
> +
> +   This file is part of GDB.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +#ifndef GDBSUPPORT_EINTR_H
> +#define GDBSUPPORT_EINTR_H
> +
> +#include <cerrno>
> +
> +namespace gdb
> +{
> +template <typename Ret, typename Fun, typename... Args>
> +inline Ret handle_eintr (const Ret &R, const Fun &F, const Args &... A)
> +{
> +  Ret ret;
> +  do
> +    {
> +      errno = 0;
> +      ret = F (A...);
> +    }
> +  while (ret == R && errno == EINTR);
> +  return ret;
> +}
> +}

Can you document this function a little bit, including a usage example?  I tried
to apply it in gdb/linux-nat.c, function async_file_mark, but I'm not sure I'm
doing it correctly.  In particular, what should I pass as the `R` parameter?  Does
that make sense?

  gdb::handle_eintr (-1, ::write, linux_nat_event_pipe[1], "+", 1);

Simon

Re: [PATCH v2 01/10] Add handle_eintr to wrap EINTR handling in syscalls

[Kamil Rytarowski]

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

On 07.09.2020 16:06, Simon Marchi wrote:
> On 2020-09-03 8:28 p.m., Kamil Rytarowski wrote:
>> gdbsupport/ChangeLog:
>>
>> 	* eintr.h: New file.
>> ---
>>  gdbsupport/ChangeLog |  4 ++++
>>  gdbsupport/eintr.h   | 41 +++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 45 insertions(+)
>>  create mode 100644 gdbsupport/eintr.h
>>
>> diff --git a/gdbsupport/ChangeLog b/gdbsupport/ChangeLog
>> index a1960537384..d26000e36ac 100644
>> --- a/gdbsupport/ChangeLog
>> +++ b/gdbsupport/ChangeLog
>> @@ -1,3 +1,7 @@
>> +2020-09-04  Kamil Rytarowski  <n54@gmx.com>
>> +
>> +	* eintr.h: New file.
>> +
>>  2020-08-13  Simon Marchi  <simon.marchi@polymtl.ca>
>>
>>  	* selftest.h (run_tests): Change parameter to array_view.
>> diff --git a/gdbsupport/eintr.h b/gdbsupport/eintr.h
>> new file mode 100644
>> index 00000000000..74b7038da60
>> --- /dev/null
>> +++ b/gdbsupport/eintr.h
>> @@ -0,0 +1,41 @@
>> +/* Utility for handling interrupted syscalls by signals.
>> +
>> +   Copyright (C) 2020 Free Software Foundation, Inc.
>> +
>> +   This file is part of GDB.
>> +
>> +   This program is free software; you can redistribute it and/or modify
>> +   it under the terms of the GNU General Public License as published by
>> +   the Free Software Foundation; either version 3 of the License, or
>> +   (at your option) any later version.
>> +
>> +   This program is distributed in the hope that it will be useful,
>> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +   GNU General Public License for more details.
>> +
>> +   You should have received a copy of the GNU General Public License
>> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>> +
>> +#ifndef GDBSUPPORT_EINTR_H
>> +#define GDBSUPPORT_EINTR_H
>> +
>> +#include <cerrno>
>> +
>> +namespace gdb
>> +{
>> +template <typename Ret, typename Fun, typename... Args>
>> +inline Ret handle_eintr (const Ret &R, const Fun &F, const Args &... A)
>> +{
>> +  Ret ret;
>> +  do
>> +    {
>> +      errno = 0;
>> +      ret = F (A...);
>> +    }
>> +  while (ret == R && errno == EINTR);
>> +  return ret;
>> +}
>> +}
> 
> Can you document this function a little bit, including a usage example?  I tried
> to apply it in gdb/linux-nat.c, function async_file_mark, but I'm not sure I'm
> doing it correctly.  In particular, what should I pass as the `R` parameter?  Does
> that make sense?
> 

I'm going to add an example and some documentation.

>   gdb::handle_eintr (-1, ::write, linux_nat_event_pipe[1], "+", 1);
> 

gdb::handle_eintr<ssize_t> (-1, ::write, linux_nat_event_pipe[1], "+", 1);

> Simon
> 



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

[PATCH] gdb::handle_eintr, remove need to specify return type (Re: [PATCH v2 01/10] Add handle_eintr to wrap EINTR handling in syscalls)

[Pedro Alves]

Hi!

On 9/7/20 3:59 PM, Kamil Rytarowski wrote:
> On 07.09.2020 16:06, Simon Marchi wrote:
>> On 2020-09-03 8:28 p.m., Kamil Rytarowski wrote:

>>> +#ifndef GDBSUPPORT_EINTR_H
>>> +#define GDBSUPPORT_EINTR_H
>>> +
>>> +#include <cerrno>
>>> +
>>> +namespace gdb
>>> +{
>>> +template <typename Ret, typename Fun, typename... Args>
>>> +inline Ret handle_eintr (const Ret &R, const Fun &F, const Args &... A)
>>> +{
>>> +  Ret ret;
>>> +  do
>>> +    {
>>> +      errno = 0;
>>> +      ret = F (A...);
>>> +    }
>>> +  while (ret == R && errno == EINTR);
>>> +  return ret;
>>> +}
>>> +}
>>
>> Can you document this function a little bit, including a usage example?  I tried
>> to apply it in gdb/linux-nat.c, function async_file_mark, but I'm not sure I'm
>> doing it correctly.  In particular, what should I pass as the `R` parameter?  Does
>> that make sense?
>>
> 
> I'm going to add an example and some documentation.
> 
>>   gdb::handle_eintr (-1, ::write, linux_nat_event_pipe[1], "+", 1);
>>
> 
> gdb::handle_eintr<ssize_t> (-1, ::write, linux_nat_event_pipe[1], "+", 1);

Going through my email backlog I noticed this.

This requirement to explicitly specify the return type at each caller
seems unnecessary.  We can make the compiler deduce it for us based on
the return type of the wrapped function.  I gave it a shot -- do you see
an issue with the change below?

I adjusted the NetBSD files, but I don't have an easy way to confirm
they still build OK, though I expect they do.

I converted Linux's my_waitpid to use the adjusted handle_eintr, and
it worked OK.

From 0c2cf066f3f34a9aa3de7aab7bc2bca80b557b45 Mon Sep 17 00:00:00 2001
From: Pedro Alves <pedro@palves.net>
Date: Tue, 8 Sep 2020 17:34:41 +0100
Subject: [PATCH] gdb::handle_eintr, remove need to specify return type

This eliminates the need to specify the return type when using
handle_eintr.  We let the compiler deduce it for us.

Also, use lowercase for function parameter names.  Uppercase should
only be used on template parameters.

gdb/ChangeLog:

	* nat/linux-waitpid.c: Include "gdbsupport/eintr.h".
	(my_waitpid): Use gdb::handle_eintr.

gdbserver/ChangeLog:

	* netbsd-low.cc (netbsd_waitpid, netbsd_process_target::kill)
	(netbsd_qxfer_libraries_svr4): Use gdb::handle_eintr without
	explicit type.

gdbsupport/ChangeLog:

	* eintr.h: Include "gdbsupport/traits.h".
	(handle_eintr): Replace Ret template parameter with ErrorValType.
	Use it as type of the failure value.  Deduce the function's return
	type using gdb::return_type.  Use lowercase for function parameter
	names.
	* traits.h (struct return_type): New.
---
 gdb/nat/linux-waitpid.c | 11 ++---------
 gdbserver/netbsd-low.cc | 10 +++++-----
 gdbsupport/eintr.h      | 29 ++++++++++++++++++-----------
 gdbsupport/traits.h     | 11 +++++++++++
 4 files changed, 36 insertions(+), 25 deletions(-)

diff --git a/gdb/nat/linux-waitpid.c b/gdb/nat/linux-waitpid.c
index f50e0c7bcff..d066239220a 100644
--- a/gdb/nat/linux-waitpid.c
+++ b/gdb/nat/linux-waitpid.c
@@ -22,6 +22,7 @@
 #include "linux-nat.h"
 #include "linux-waitpid.h"
 #include "gdbsupport/gdb_wait.h"
+#include "gdbsupport/eintr.h"
 
 /* Convert wait status STATUS to a string.  Used for printing debug
    messages only.  */
@@ -54,13 +55,5 @@ status_to_str (int status)
 int
 my_waitpid (int pid, int *status, int flags)
 {
-  int ret;
-
-  do
-    {
-      ret = waitpid (pid, status, flags);
-    }
-  while (ret == -1 && errno == EINTR);
-
-  return ret;
+  return gdb::handle_eintr (-1, ::waitpid, pid, status, flags);
 }
diff --git a/gdbserver/netbsd-low.cc b/gdbserver/netbsd-low.cc
index 30028d3a384..519614d0473 100644
--- a/gdbserver/netbsd-low.cc
+++ b/gdbserver/netbsd-low.cc
@@ -245,7 +245,7 @@ netbsd_waitpid (ptid_t ptid, struct target_waitstatus *ourstatus,
   int options = (target_options & TARGET_WNOHANG) ? WNOHANG : 0;
 
   pid_t pid
-    = gdb::handle_eintr<int> (-1, ::waitpid, ptid.pid (), &status, options);
+    = gdb::handle_eintr (-1, ::waitpid, ptid.pid (), &status, options);
 
   if (pid == -1)
     perror_with_name (_("Child process unexpectedly missing"));
@@ -456,7 +456,7 @@ netbsd_process_target::kill (process_info *process)
     return -1;
 
   int status;
-  if (gdb::handle_eintr<int> (-1, ::waitpid, pid, &status, 0) == -1)
+  if (gdb::handle_eintr (-1, ::waitpid, pid, &status, 0) == -1)
     return -1;
   mourn (process);
   return 0;
@@ -1149,15 +1149,15 @@ netbsd_qxfer_libraries_svr4 (const pid_t pid, const char *annex,
 static bool
 elf_64_file_p (const char *file)
 {
-  int fd = gdb::handle_eintr<int> (-1, ::open, file, O_RDONLY);
+  int fd = gdb::handle_eintr (-1, ::open, file, O_RDONLY);
   if (fd < 0)
     perror_with_name (("open"));
 
   Elf64_Ehdr header;
-  ssize_t ret = gdb::handle_eintr<ssize_t> (-1, ::read, fd, &header, sizeof (header));
+  ssize_t ret = gdb::handle_eintr (-1, ::read, fd, &header, sizeof (header));
   if (ret == -1)
     perror_with_name (("read"));
-  gdb::handle_eintr<int> (-1, ::close, fd);
+  gdb::handle_eintr (-1, ::close, fd);
   if (ret != sizeof (header))
     error ("Cannot read ELF file header: %s", file);
 
diff --git a/gdbsupport/eintr.h b/gdbsupport/eintr.h
index 64ff5940b75..ab8310e561c 100644
--- a/gdbsupport/eintr.h
+++ b/gdbsupport/eintr.h
@@ -22,6 +22,8 @@
 
 #include <cerrno>
 
+#include "gdbsupport/traits.h"
+
 namespace gdb
 {
 /* Repeat a system call interrupted with a signal.
@@ -43,25 +45,30 @@ namespace gdb
 
    You could wrap it by writing the wrapped form:
 
-   ssize_t ret = gdb::handle_eintr<ssize_t> (-1, ::write, pipe[1], "+", 1);
+   ssize_t ret = gdb::handle_eintr (-1, ::write, pipe[1], "+", 1);
 
-   The RET typename specifies the return type of the wrapped system call, which
-   is typically int or ssize_t.  The R argument specifies the failure value
-   indicating the interrupted syscall when calling the F function with
-   the A... arguments.  */
+   ERRVAL specifies the failure value indicating that the call to the
+   F function with ARGS... arguments was possibly interrupted with a
+   signal.  */
 
-template <typename Ret, typename Fun, typename... Args>
-inline Ret handle_eintr (const Ret &R, const Fun &F, const Args &... A)
+template <typename ErrorValType,
+	  typename Fun,
+	  typename... Args>
+inline typename return_type<Fun>::type
+handle_eintr (ErrorValType errval, const Fun &f, const Args &... args)
 {
-  Ret ret;
+  typename return_type<Fun>::type ret;
+
   do
     {
       errno = 0;
-      ret = F (A...);
+      ret = f (args...);
     }
-  while (ret == R && errno == EINTR);
+  while (ret == errval && errno == EINTR);
+
   return ret;
 }
-}
+
+} /* namespace gdb */
 
 #endif /* GDBSUPPORT_EINTR_H */
diff --git a/gdbsupport/traits.h b/gdbsupport/traits.h
index 93b609ac109..f161ca00fa6 100644
--- a/gdbsupport/traits.h
+++ b/gdbsupport/traits.h
@@ -43,6 +43,17 @@
 
 namespace gdb {
 
+/* Use partial specialization to get access a function's return
+   type.  */
+template<class Signature>
+struct return_type;
+
+template<typename Res, typename... Args>
+struct return_type<Res (Args...)>
+{
+  typedef Res type;
+};
+
 /* Pre C++14-safe (CWG 1558) version of C++17's std::void_t.  See
    <http://en.cppreference.com/w/cpp/types/void_t>.  */
 

base-commit: 87a37e5e078f506fa9905b74e9238593c537fcd5
-- 
2.14.5

Re: [PATCH] gdb::handle_eintr, remove need to specify return type (Re: [PATCH v2 01/10] Add handle_eintr to wrap EINTR handling in syscalls)

[Kamil Rytarowski]

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

On 12.10.2020 19:56, Pedro Alves wrote:
> Hi!
> 

Hi Pedro,

> On 9/7/20 3:59 PM, Kamil Rytarowski wrote:
>> On 07.09.2020 16:06, Simon Marchi wrote:
>>> On 2020-09-03 8:28 p.m., Kamil Rytarowski wrote:
> 
>>>> +#ifndef GDBSUPPORT_EINTR_H
>>>> +#define GDBSUPPORT_EINTR_H
>>>> +
>>>> +#include <cerrno>
>>>> +
>>>> +namespace gdb
>>>> +{
>>>> +template <typename Ret, typename Fun, typename... Args>
>>>> +inline Ret handle_eintr (const Ret &R, const Fun &F, const Args &... A)
>>>> +{
>>>> +  Ret ret;
>>>> +  do
>>>> +    {
>>>> +      errno = 0;
>>>> +      ret = F (A...);
>>>> +    }
>>>> +  while (ret == R && errno == EINTR);
>>>> +  return ret;
>>>> +}
>>>> +}
>>>
>>> Can you document this function a little bit, including a usage example?  I tried
>>> to apply it in gdb/linux-nat.c, function async_file_mark, but I'm not sure I'm
>>> doing it correctly.  In particular, what should I pass as the `R` parameter?  Does
>>> that make sense?
>>>
>>
>> I'm going to add an example and some documentation.
>>
>>>   gdb::handle_eintr (-1, ::write, linux_nat_event_pipe[1], "+", 1);
>>>
>>
>> gdb::handle_eintr<ssize_t> (-1, ::write, linux_nat_event_pipe[1], "+", 1);
> 
> Going through my email backlog I noticed this.
> 
> This requirement to explicitly specify the return type at each caller
> seems unnecessary.  We can make the compiler deduce it for us based on
> the return type of the wrapped function.  I gave it a shot -- do you see
> an issue with the change below?
> 

This patch applied on trunk gives me:

gmake[2]: Wejście do katalogu '/public/binutils-gdb-netbsd/build/gdbserver'
  CXX    netbsd-low.o
../../gdbserver/netbsd-low.cc: In function ‘bool elf_64_file_p(const
char*)’:
../../gdbserver/netbsd-low.cc:1152:57: error: no matching function for
call to ‘handle_eintr(int, int (&)(const char*, int, ...), const char*&,
int)’
 1152 |   int fd = gdb::handle_eintr (-1, ::open, file, O_RDONLY);
      |                                                         ^
In file included from ../../gdbserver/netbsd-low.cc:37:
../../gdbserver/../gdbsupport/eintr.h:58:1: note: candidate:
‘template<class ErrorValType, class Fun, class ... Args> typename
gdb::return_type<Fun>::type gdb::handle_eintr(ErrorValType, const Fun&,
const Args& ...)’
   58 | handle_eintr (ErrorValType errval, const Fun &f, const Args &...
args)
      | ^~~~~~~~~~~~
../../gdbserver/../gdbsupport/eintr.h:58:1: note:   template argument
deduction/substitution failed:
../../gdbserver/../gdbsupport/eintr.h: In substitution of
‘template<class ErrorValType, class Fun, class ... Args> typename
gdb::return_type<Fun>::type gdb::handle_eintr(ErrorValType, const Fun&,
const Args& ...) [with ErrorValType = int; Fun = int(const char*, int,
...); Args = {const char*, int}]’:
../../gdbserver/netbsd-low.cc:1152:57:   required from here
../../gdbserver/../gdbsupport/eintr.h:58:1: error: invalid use of
incomplete type ‘struct gdb::return_type<int(const char*, int, ...)>’
In file included from ../../gdbserver/../gdbsupport/poison.h:23,
                 from ../../gdbserver/../gdbsupport/common-utils.h:26,
                 from ../../gdbserver/../gdbsupport/common-defs.h:125,
                 from ../../gdbserver/server.h:22,
                 from ../../gdbserver/netbsd-low.cc:18:
../../gdbserver/../gdbsupport/traits.h:49:8: note: declaration of
‘struct gdb::return_type<int(const char*, int, ...)>’
   49 | struct return_type;
      |        ^~~~~~~~~~~
gmake[2]: *** [Makefile:551: netbsd-low.o] Błąd 1


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

[PATCH v2] gdb::handle_eintr, remove need to specify return type

[Pedro Alves]

On 10/13/20 2:43 PM, Kamil Rytarowski wrote:

> This patch applied on trunk gives me:
> 
> gmake[2]: Wejście do katalogu '/public/binutils-gdb-netbsd/build/gdbserver'
>   CXX    netbsd-low.o
> ../../gdbserver/netbsd-low.cc: In function ‘bool elf_64_file_p(const
> char*)’:
> ../../gdbserver/netbsd-low.cc:1152:57: error: no matching function for
> call to ‘handle_eintr(int, int (&)(const char*, int, ...), const char*&,
> int)’
>  1152 |   int fd = gdb::handle_eintr (-1, ::open, file, O_RDONLY);
>       |                                                         ^
> In file included from ../../gdbserver/netbsd-low.cc:37:
> ../../gdbserver/../gdbsupport/eintr.h:58:1: note: candidate:
> ‘template<class ErrorValType, class Fun, class ... Args> typename
> gdb::return_type<Fun>::type gdb::handle_eintr(ErrorValType, const Fun&,
> const Args& ...)’
>    58 | handle_eintr (ErrorValType errval, const Fun &f, const Args &...
> args)
>       | ^~~~~~~~~~~~
> ../../gdbserver/../gdbsupport/eintr.h:58:1: note:   template argument
> deduction/substitution failed:
> ../../gdbserver/../gdbsupport/eintr.h: In substitution of
> ‘template<class ErrorValType, class Fun, class ... Args> typename
> gdb::return_type<Fun>::type gdb::handle_eintr(ErrorValType, const Fun&,
> const Args& ...) [with ErrorValType = int; Fun = int(const char*, int,
> ...); Args = {const char*, int}]’:
> ../../gdbserver/netbsd-low.cc:1152:57:   required from here
> ../../gdbserver/../gdbsupport/eintr.h:58:1: error: invalid use of
> incomplete type ‘struct gdb::return_type<int(const char*, int, ...)>’
> In file included from ../../gdbserver/../gdbsupport/poison.h:23,
>                  from ../../gdbserver/../gdbsupport/common-utils.h:26,
>                  from ../../gdbserver/../gdbsupport/common-defs.h:125,
>                  from ../../gdbserver/server.h:22,
>                  from ../../gdbserver/netbsd-low.cc:18:
> ../../gdbserver/../gdbsupport/traits.h:49:8: note: declaration of
> ‘struct gdb::return_type<int(const char*, int, ...)>’
>    49 | struct return_type;
>       |        ^~~~~~~~~~~

Huh, Fun is clearly a function, yet the compiler doesn't pick
the partial specialization.

I could reproduce it here, seems to be related to open being varargs.

Anyhow, we don't really need the new trait.  We can just 
use decltype and a trailing return.  I don't know why I didn't
think of it in the first place.

Here's an updated patch.  I believe this one should work for you too.


From 37d06f47a379e44a895f68a22cbb0f745da33ec5 Mon Sep 17 00:00:00 2001
From: Pedro Alves <pedro@palves.net>
Date: Tue, 8 Sep 2020 17:34:41 +0100
Subject: [PATCH v2] gdb::handle_eintr, remove need to specify return type

This eliminates the need to specify the return type when using
handle_eintr.  We let the compiler deduce it for us.

Also, use lowercase for function parameter names.  Uppercase should
only be used on template parameters.

gdb/ChangeLog:

	* nat/linux-waitpid.c: Include "gdbsupport/eintr.h".
	(my_waitpid): Use gdb::handle_eintr.

gdbserver/ChangeLog:

	* netbsd-low.cc (netbsd_waitpid, netbsd_process_target::kill)
	(netbsd_qxfer_libraries_svr4): Use gdb::handle_eintr without
	explicit type.

gdbsupport/ChangeLog:

	* eintr.h (handle_eintr): Replace Ret template parameter with
	ErrorValType.  Use it as type of the failure value.  Deduce the
	function's return type using decltype.  Use lowercase for function
	parameter names.
---
 gdb/nat/linux-waitpid.c | 11 ++---------
 gdbserver/netbsd-low.cc | 10 +++++-----
 gdbsupport/eintr.h      | 32 +++++++++++++++++++-------------
 3 files changed, 26 insertions(+), 27 deletions(-)

diff --git a/gdb/nat/linux-waitpid.c b/gdb/nat/linux-waitpid.c
index f50e0c7bcff..d066239220a 100644
--- a/gdb/nat/linux-waitpid.c
+++ b/gdb/nat/linux-waitpid.c
@@ -22,6 +22,7 @@
 #include "linux-nat.h"
 #include "linux-waitpid.h"
 #include "gdbsupport/gdb_wait.h"
+#include "gdbsupport/eintr.h"
 
 /* Convert wait status STATUS to a string.  Used for printing debug
    messages only.  */
@@ -54,13 +55,5 @@ status_to_str (int status)
 int
 my_waitpid (int pid, int *status, int flags)
 {
-  int ret;
-
-  do
-    {
-      ret = waitpid (pid, status, flags);
-    }
-  while (ret == -1 && errno == EINTR);
-
-  return ret;
+  return gdb::handle_eintr (-1, ::waitpid, pid, status, flags);
 }
diff --git a/gdbserver/netbsd-low.cc b/gdbserver/netbsd-low.cc
index 30028d3a384..519614d0473 100644
--- a/gdbserver/netbsd-low.cc
+++ b/gdbserver/netbsd-low.cc
@@ -245,7 +245,7 @@ netbsd_waitpid (ptid_t ptid, struct target_waitstatus *ourstatus,
   int options = (target_options & TARGET_WNOHANG) ? WNOHANG : 0;
 
   pid_t pid
-    = gdb::handle_eintr<int> (-1, ::waitpid, ptid.pid (), &status, options);
+    = gdb::handle_eintr (-1, ::waitpid, ptid.pid (), &status, options);
 
   if (pid == -1)
     perror_with_name (_("Child process unexpectedly missing"));
@@ -456,7 +456,7 @@ netbsd_process_target::kill (process_info *process)
     return -1;
 
   int status;
-  if (gdb::handle_eintr<int> (-1, ::waitpid, pid, &status, 0) == -1)
+  if (gdb::handle_eintr (-1, ::waitpid, pid, &status, 0) == -1)
     return -1;
   mourn (process);
   return 0;
@@ -1149,15 +1149,15 @@ netbsd_qxfer_libraries_svr4 (const pid_t pid, const char *annex,
 static bool
 elf_64_file_p (const char *file)
 {
-  int fd = gdb::handle_eintr<int> (-1, ::open, file, O_RDONLY);
+  int fd = gdb::handle_eintr (-1, ::open, file, O_RDONLY);
   if (fd < 0)
     perror_with_name (("open"));
 
   Elf64_Ehdr header;
-  ssize_t ret = gdb::handle_eintr<ssize_t> (-1, ::read, fd, &header, sizeof (header));
+  ssize_t ret = gdb::handle_eintr (-1, ::read, fd, &header, sizeof (header));
   if (ret == -1)
     perror_with_name (("read"));
-  gdb::handle_eintr<int> (-1, ::close, fd);
+  gdb::handle_eintr (-1, ::close, fd);
   if (ret != sizeof (header))
     error ("Cannot read ELF file header: %s", file);
 
diff --git a/gdbsupport/eintr.h b/gdbsupport/eintr.h
index 64ff5940b75..02c26e4def1 100644
--- a/gdbsupport/eintr.h
+++ b/gdbsupport/eintr.h
@@ -43,25 +43,31 @@ namespace gdb
 
    You could wrap it by writing the wrapped form:
 
-   ssize_t ret = gdb::handle_eintr<ssize_t> (-1, ::write, pipe[1], "+", 1);
-
-   The RET typename specifies the return type of the wrapped system call, which
-   is typically int or ssize_t.  The R argument specifies the failure value
-   indicating the interrupted syscall when calling the F function with
-   the A... arguments.  */
-
-template <typename Ret, typename Fun, typename... Args>
-inline Ret handle_eintr (const Ret &R, const Fun &F, const Args &... A)
+   ssize_t ret = gdb::handle_eintr (-1, ::write, pipe[1], "+", 1);
+
+   ERRVAL specifies the failure value indicating that the call to the
+   F function with ARGS... arguments was possibly interrupted with a
+   signal.  */
+
+template <typename ErrorValType,
+	  typename Fun,
+	  typename... Args>
+inline auto
+handle_eintr (ErrorValType errval, const Fun &f, const Args &... args)
+  -> decltype (f(args...))
 {
-  Ret ret;
+  decltype (f(args...)) ret;
+
   do
     {
       errno = 0;
-      ret = F (A...);
+      ret = f (args...);
     }
-  while (ret == R && errno == EINTR);
+  while (ret == errval && errno == EINTR);
+
   return ret;
 }
-}
+
+} /* namespace gdb */
 
 #endif /* GDBSUPPORT_EINTR_H */

base-commit: 87a37e5e078f506fa9905b74e9238593c537fcd5
-- 
2.14.5

Re: [PATCH v2] gdb::handle_eintr, remove need to specify return type

[Kamil Rytarowski]

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

On 13.10.2020 16:17, Pedro Alves wrote:
> On 10/13/20 2:43 PM, Kamil Rytarowski wrote:
> 
>> This patch applied on trunk gives me:
>>
>> gmake[2]: Wejście do katalogu '/public/binutils-gdb-netbsd/build/gdbserver'
>>   CXX    netbsd-low.o
>> ../../gdbserver/netbsd-low.cc: In function ‘bool elf_64_file_p(const
>> char*)’:
>> ../../gdbserver/netbsd-low.cc:1152:57: error: no matching function for
>> call to ‘handle_eintr(int, int (&)(const char*, int, ...), const char*&,
>> int)’
>>  1152 |   int fd = gdb::handle_eintr (-1, ::open, file, O_RDONLY);
>>       |                                                         ^
>> In file included from ../../gdbserver/netbsd-low.cc:37:
>> ../../gdbserver/../gdbsupport/eintr.h:58:1: note: candidate:
>> ‘template<class ErrorValType, class Fun, class ... Args> typename
>> gdb::return_type<Fun>::type gdb::handle_eintr(ErrorValType, const Fun&,
>> const Args& ...)’
>>    58 | handle_eintr (ErrorValType errval, const Fun &f, const Args &...
>> args)
>>       | ^~~~~~~~~~~~
>> ../../gdbserver/../gdbsupport/eintr.h:58:1: note:   template argument
>> deduction/substitution failed:
>> ../../gdbserver/../gdbsupport/eintr.h: In substitution of
>> ‘template<class ErrorValType, class Fun, class ... Args> typename
>> gdb::return_type<Fun>::type gdb::handle_eintr(ErrorValType, const Fun&,
>> const Args& ...) [with ErrorValType = int; Fun = int(const char*, int,
>> ...); Args = {const char*, int}]’:
>> ../../gdbserver/netbsd-low.cc:1152:57:   required from here
>> ../../gdbserver/../gdbsupport/eintr.h:58:1: error: invalid use of
>> incomplete type ‘struct gdb::return_type<int(const char*, int, ...)>’
>> In file included from ../../gdbserver/../gdbsupport/poison.h:23,
>>                  from ../../gdbserver/../gdbsupport/common-utils.h:26,
>>                  from ../../gdbserver/../gdbsupport/common-defs.h:125,
>>                  from ../../gdbserver/server.h:22,
>>                  from ../../gdbserver/netbsd-low.cc:18:
>> ../../gdbserver/../gdbsupport/traits.h:49:8: note: declaration of
>> ‘struct gdb::return_type<int(const char*, int, ...)>’
>>    49 | struct return_type;
>>       |        ^~~~~~~~~~~
> 
> Huh, Fun is clearly a function, yet the compiler doesn't pick
> the partial specialization.
> 
> I could reproduce it here, seems to be related to open being varargs.
> 
> Anyhow, we don't really need the new trait.  We can just 
> use decltype and a trailing return.  I don't know why I didn't
> think of it in the first place.
> 
> Here's an updated patch.  I believe this one should work for you too.
> 

This version works for me.

> 
> From 37d06f47a379e44a895f68a22cbb0f745da33ec5 Mon Sep 17 00:00:00 2001
> From: Pedro Alves <pedro@palves.net>
> Date: Tue, 8 Sep 2020 17:34:41 +0100
> Subject: [PATCH v2] gdb::handle_eintr, remove need to specify return type
> 
> This eliminates the need to specify the return type when using
> handle_eintr.  We let the compiler deduce it for us.
> 
> Also, use lowercase for function parameter names.  Uppercase should
> only be used on template parameters.
> 
> gdb/ChangeLog:
> 
> 	* nat/linux-waitpid.c: Include "gdbsupport/eintr.h".
> 	(my_waitpid): Use gdb::handle_eintr.
> 
> gdbserver/ChangeLog:
> 
> 	* netbsd-low.cc (netbsd_waitpid, netbsd_process_target::kill)
> 	(netbsd_qxfer_libraries_svr4): Use gdb::handle_eintr without
> 	explicit type.
> 
> gdbsupport/ChangeLog:
> 
> 	* eintr.h (handle_eintr): Replace Ret template parameter with
> 	ErrorValType.  Use it as type of the failure value.  Deduce the
> 	function's return type using decltype.  Use lowercase for function
> 	parameter names.
> ---
>  gdb/nat/linux-waitpid.c | 11 ++---------
>  gdbserver/netbsd-low.cc | 10 +++++-----
>  gdbsupport/eintr.h      | 32 +++++++++++++++++++-------------
>  3 files changed, 26 insertions(+), 27 deletions(-)
> 
> diff --git a/gdb/nat/linux-waitpid.c b/gdb/nat/linux-waitpid.c
> index f50e0c7bcff..d066239220a 100644
> --- a/gdb/nat/linux-waitpid.c
> +++ b/gdb/nat/linux-waitpid.c
> @@ -22,6 +22,7 @@
>  #include "linux-nat.h"
>  #include "linux-waitpid.h"
>  #include "gdbsupport/gdb_wait.h"
> +#include "gdbsupport/eintr.h"
>  
>  /* Convert wait status STATUS to a string.  Used for printing debug
>     messages only.  */
> @@ -54,13 +55,5 @@ status_to_str (int status)
>  int
>  my_waitpid (int pid, int *status, int flags)
>  {
> -  int ret;
> -
> -  do
> -    {
> -      ret = waitpid (pid, status, flags);
> -    }
> -  while (ret == -1 && errno == EINTR);
> -
> -  return ret;
> +  return gdb::handle_eintr (-1, ::waitpid, pid, status, flags);
>  }
> diff --git a/gdbserver/netbsd-low.cc b/gdbserver/netbsd-low.cc
> index 30028d3a384..519614d0473 100644
> --- a/gdbserver/netbsd-low.cc
> +++ b/gdbserver/netbsd-low.cc
> @@ -245,7 +245,7 @@ netbsd_waitpid (ptid_t ptid, struct target_waitstatus *ourstatus,
>    int options = (target_options & TARGET_WNOHANG) ? WNOHANG : 0;
>  
>    pid_t pid
> -    = gdb::handle_eintr<int> (-1, ::waitpid, ptid.pid (), &status, options);
> +    = gdb::handle_eintr (-1, ::waitpid, ptid.pid (), &status, options);
>  
>    if (pid == -1)
>      perror_with_name (_("Child process unexpectedly missing"));
> @@ -456,7 +456,7 @@ netbsd_process_target::kill (process_info *process)
>      return -1;
>  
>    int status;
> -  if (gdb::handle_eintr<int> (-1, ::waitpid, pid, &status, 0) == -1)
> +  if (gdb::handle_eintr (-1, ::waitpid, pid, &status, 0) == -1)
>      return -1;
>    mourn (process);
>    return 0;
> @@ -1149,15 +1149,15 @@ netbsd_qxfer_libraries_svr4 (const pid_t pid, const char *annex,
>  static bool
>  elf_64_file_p (const char *file)
>  {
> -  int fd = gdb::handle_eintr<int> (-1, ::open, file, O_RDONLY);
> +  int fd = gdb::handle_eintr (-1, ::open, file, O_RDONLY);
>    if (fd < 0)
>      perror_with_name (("open"));
>  
>    Elf64_Ehdr header;
> -  ssize_t ret = gdb::handle_eintr<ssize_t> (-1, ::read, fd, &header, sizeof (header));
> +  ssize_t ret = gdb::handle_eintr (-1, ::read, fd, &header, sizeof (header));
>    if (ret == -1)
>      perror_with_name (("read"));
> -  gdb::handle_eintr<int> (-1, ::close, fd);
> +  gdb::handle_eintr (-1, ::close, fd);
>    if (ret != sizeof (header))
>      error ("Cannot read ELF file header: %s", file);
>  
> diff --git a/gdbsupport/eintr.h b/gdbsupport/eintr.h
> index 64ff5940b75..02c26e4def1 100644
> --- a/gdbsupport/eintr.h
> +++ b/gdbsupport/eintr.h
> @@ -43,25 +43,31 @@ namespace gdb
>  
>     You could wrap it by writing the wrapped form:
>  
> -   ssize_t ret = gdb::handle_eintr<ssize_t> (-1, ::write, pipe[1], "+", 1);
> -
> -   The RET typename specifies the return type of the wrapped system call, which
> -   is typically int or ssize_t.  The R argument specifies the failure value
> -   indicating the interrupted syscall when calling the F function with
> -   the A... arguments.  */
> -
> -template <typename Ret, typename Fun, typename... Args>
> -inline Ret handle_eintr (const Ret &R, const Fun &F, const Args &... A)
> +   ssize_t ret = gdb::handle_eintr (-1, ::write, pipe[1], "+", 1);
> +
> +   ERRVAL specifies the failure value indicating that the call to the
> +   F function with ARGS... arguments was possibly interrupted with a
> +   signal.  */
> +
> +template <typename ErrorValType,
> +	  typename Fun,
> +	  typename... Args>
> +inline auto
> +handle_eintr (ErrorValType errval, const Fun &f, const Args &... args)
> +  -> decltype (f(args...))
>  {
> -  Ret ret;
> +  decltype (f(args...)) ret;
> +
>    do
>      {
>        errno = 0;
> -      ret = F (A...);
> +      ret = f (args...);
>      }
> -  while (ret == R && errno == EINTR);
> +  while (ret == errval && errno == EINTR);
> +
>    return ret;
>  }
> -}
> +
> +} /* namespace gdb */
>  
>  #endif /* GDBSUPPORT_EINTR_H */
> 
> base-commit: 87a37e5e078f506fa9905b74e9238593c537fcd5
> 



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

Re: [PATCH v2] gdb::handle_eintr, remove need to specify return type

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:

Pedro> This eliminates the need to specify the return type when using
Pedro> handle_eintr.  We let the compiler deduce it for us.

Thanks for doing this.  This is closer to how I imagined this wrapper
being written.

Pedro> +template <typename ErrorValType,
Pedro> +	  typename Fun,
Pedro> +	  typename... Args>
Pedro> +inline auto
Pedro> +handle_eintr (ErrorValType errval, const Fun &f, const Args &... args)
Pedro> +  -> decltype (f(args...))

It seems to me that errval and ErrorValType are unchanging properties of
the function being wrapped.  And, normally they are -1 / int.

Also is there ever a case where the return type isn't the same as
ErrorValType?

So maybe instead of requiring these to all be redundantly specified, the
template could use a helper template class that specifies these things
(defaulting to the usual), and then one would write:

pid_t pid = gdb::handle_eintr<::waitpid> (...normal waitpid args);

I'm not sure it's really worth implementing this, but it's closer to
what I was picturing initially.

Tom

Re: [PATCH v2] gdb::handle_eintr, remove need to specify return type

[Pedro Alves]

On 10/16/20 9:51 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:
> 
> Pedro> This eliminates the need to specify the return type when using
> Pedro> handle_eintr.  We let the compiler deduce it for us.
> 
> Thanks for doing this.  This is closer to how I imagined this wrapper
> being written.
> 
> Pedro> +template <typename ErrorValType,
> Pedro> +	  typename Fun,
> Pedro> +	  typename... Args>
> Pedro> +inline auto
> Pedro> +handle_eintr (ErrorValType errval, const Fun &f, const Args &... args)
> Pedro> +  -> decltype (f(args...))
> 
> It seems to me that errval and ErrorValType are unchanging properties of
> the function being wrapped.  And, normally they are -1 / int.
> 
> Also is there ever a case where the return type isn't the same as
> ErrorValType?

I don't think so.  But I don't think it really matters -- I look at
it as ErrorValType being the type of the value used to compare with
the wrapped function's return type.

Like, with:

   ssize_t read(int fildes, void *buf, size_t nbyte);

we normally write:

   ssize_t res = read (fildes, buf, nbyte);
   if (res == -1)

instead of 

   ssize_t res = read (fildes, buf, nbyte);
   if (res == (ssize_t) -1)

So ErrorValType is the type of the "-1" expression.  It just
has to be convertible to the return type.

If we get rid of the ErrorValType template parameter, then we
could do instead:

 template<typename Fun, typename... Args>
 using return_type = decltype (std::declval<Fun> () (std::declval<Args> ()...));

 template <typename Fun, typename... Args>
 inline auto
 handle_eintr (return_type<Fun, Args...> errval,
 	      const Fun &f, const Args &... args)
   -> decltype (f (args...))
 {
   decltype (f (args...)) ret;

   do
     {
       errno = 0;
       ret = f (args...);
     }
   while (ret == errval && errno == EINTR);
 
   return ret;
 }

That works for me too, though it looks a little more complicated,
I think.  

Let me know which version you prefer.

> So maybe instead of requiring these to all be redundantly specified, the
> template could use a helper template class that specifies these things
> (defaulting to the usual), and then one would write:
> 
> pid_t pid = gdb::handle_eintr<::waitpid> (...normal waitpid args);
> 
> I'm not sure it's really worth implementing this, but it's closer to
> what I was picturing initially.

That thought initially crossed my mind too, but IMHO it's not worth
it.  You have to write the helper template class, so it
doesn't look like a net win over writing a plain wrapper like:

 int
 my_waitpid (int pid, int *status, int flags)
 {
   return gdb::handle_eintr (-1, ::waitpid, pid, status, flags);
 }

and using my_waitpid throughout.  I.e., this way we also only
write the -1 once.

Re: [PATCH v2] gdb::handle_eintr, remove need to specify return type

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:

Pedro> That works for me too, though it looks a little more complicated,
Pedro> I think.  

Pedro> Let me know which version you prefer.

I don't have much preference but I suppose simpler is better.

Tom

Re: [PATCH v2] gdb::handle_eintr, remove need to specify return type

[Pedro Alves]

On 10/26/20 2:20 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <pedro@palves.net> writes:
> 
> Pedro> That works for me too, though it looks a little more complicated,
> Pedro> I think.  
> 
> Pedro> Let me know which version you prefer.
> 
> I don't have much preference but I suppose simpler is better.

Alright, I pushed the simpler version.

Thanks,
Pedro Alves

[PATCH v2 02/10] Register a placeholder for NetBSD shared functions in gdb/nat

[Kamil Rytarowski]

gdb/ChangeLog:

	* netbsd-nat.h: New file.
	* netbsd-nat.c: Likewise.
---
 gdb/ChangeLog        |  5 +++++
 gdb/nat/netbsd-nat.c | 24 ++++++++++++++++++++++++
 gdb/nat/netbsd-nat.h | 27 +++++++++++++++++++++++++++
 3 files changed, 56 insertions(+)
 create mode 100644 gdb/nat/netbsd-nat.c
 create mode 100644 gdb/nat/netbsd-nat.h

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index faa3a612547..430f9b58a78 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2020-09-04  Kamil Rytarowski  <n54@gmx.com>
+
+	* netbsd-nat.h: New file.
+	* netbsd-nat.c: Likewise.
+
 2020-09-03  Alok Kumar Sharma  <AlokKumar.Sharma@amd.com>

 	* gdb/i386-tdep.c (i386_floatformat_for_type): Added conditions
diff --git a/gdb/nat/netbsd-nat.c b/gdb/nat/netbsd-nat.c
new file mode 100644
index 00000000000..2b5a4183e30
--- /dev/null
+++ b/gdb/nat/netbsd-nat.c
@@ -0,0 +1,24 @@
+/* Internal interfaces for the NetBSD code.
+
+   Copyright (C) 2006-2020 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "nat/netbsd-nat.h"
+
+namespace netbsd_nat
+{
+}
diff --git a/gdb/nat/netbsd-nat.h b/gdb/nat/netbsd-nat.h
new file mode 100644
index 00000000000..5fa08746610
--- /dev/null
+++ b/gdb/nat/netbsd-nat.h
@@ -0,0 +1,27 @@
+/* Internal interfaces for the NetBSD code.
+
+   Copyright (C) 2006-2020 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef NAT_NETBSD_NAT_H
+#define NAT_NETBSD_NAT_H
+
+namespace netbsd_nat
+{
+}
+
+#endif
--
2.28.0

Re: [PATCH v2 02/10] Register a placeholder for NetBSD shared functions in gdb/nat

[Simon Marchi]

On 2020-09-03 8:28 p.m., Kamil Rytarowski wrote:
> gdb/ChangeLog:
> 
> 	* netbsd-nat.h: New file.
> 	* netbsd-nat.c: Likewise.
> ---
>  gdb/ChangeLog        |  5 +++++
>  gdb/nat/netbsd-nat.c | 24 ++++++++++++++++++++++++
>  gdb/nat/netbsd-nat.h | 27 +++++++++++++++++++++++++++
>  3 files changed, 56 insertions(+)
>  create mode 100644 gdb/nat/netbsd-nat.c
>  create mode 100644 gdb/nat/netbsd-nat.h
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index faa3a612547..430f9b58a78 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,8 @@
> +2020-09-04  Kamil Rytarowski  <n54@gmx.com>
> +
> +	* netbsd-nat.h: New file.
> +	* netbsd-nat.c: Likewise.
> +
>  2020-09-03  Alok Kumar Sharma  <AlokKumar.Sharma@amd.com>
> 
>  	* gdb/i386-tdep.c (i386_floatformat_for_type): Added conditions
> diff --git a/gdb/nat/netbsd-nat.c b/gdb/nat/netbsd-nat.c
> new file mode 100644
> index 00000000000..2b5a4183e30
> --- /dev/null
> +++ b/gdb/nat/netbsd-nat.c
> @@ -0,0 +1,24 @@
> +/* Internal interfaces for the NetBSD code.
> +
> +   Copyright (C) 2006-2020 Free Software Foundation, Inc.
> +
> +   This file is part of GDB.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +#include "nat/netbsd-nat.h"
> +
> +namespace netbsd_nat
> +{
> +}
> diff --git a/gdb/nat/netbsd-nat.h b/gdb/nat/netbsd-nat.h
> new file mode 100644
> index 00000000000..5fa08746610
> --- /dev/null
> +++ b/gdb/nat/netbsd-nat.h
> @@ -0,0 +1,27 @@
> +/* Internal interfaces for the NetBSD code.
> +
> +   Copyright (C) 2006-2020 Free Software Foundation, Inc.

I presume that the copyright should say 2020 only?  It depends on the code that
ends up here.  If you copy or move some code from some other file, in theory the
copyright date should follow.

Simon

Re: [PATCH v2 02/10] Register a placeholder for NetBSD shared functions in gdb/nat

[Kamil Rytarowski]

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

On 07.09.2020 20:44, Simon Marchi wrote:
> On 2020-09-03 8:28 p.m., Kamil Rytarowski wrote:
>> gdb/ChangeLog:
>>
>> 	* netbsd-nat.h: New file.
>> 	* netbsd-nat.c: Likewise.
>> ---
>>  gdb/ChangeLog        |  5 +++++
>>  gdb/nat/netbsd-nat.c | 24 ++++++++++++++++++++++++
>>  gdb/nat/netbsd-nat.h | 27 +++++++++++++++++++++++++++
>>  3 files changed, 56 insertions(+)
>>  create mode 100644 gdb/nat/netbsd-nat.c
>>  create mode 100644 gdb/nat/netbsd-nat.h
>>
>> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
>> index faa3a612547..430f9b58a78 100644
>> --- a/gdb/ChangeLog
>> +++ b/gdb/ChangeLog
>> @@ -1,3 +1,8 @@
>> +2020-09-04  Kamil Rytarowski  <n54@gmx.com>
>> +
>> +	* netbsd-nat.h: New file.
>> +	* netbsd-nat.c: Likewise.
>> +
>>  2020-09-03  Alok Kumar Sharma  <AlokKumar.Sharma@amd.com>
>>
>>  	* gdb/i386-tdep.c (i386_floatformat_for_type): Added conditions
>> diff --git a/gdb/nat/netbsd-nat.c b/gdb/nat/netbsd-nat.c
>> new file mode 100644
>> index 00000000000..2b5a4183e30
>> --- /dev/null
>> +++ b/gdb/nat/netbsd-nat.c
>> @@ -0,0 +1,24 @@
>> +/* Internal interfaces for the NetBSD code.
>> +
>> +   Copyright (C) 2006-2020 Free Software Foundation, Inc.
>> +
>> +   This file is part of GDB.
>> +
>> +   This program is free software; you can redistribute it and/or modify
>> +   it under the terms of the GNU General Public License as published by
>> +   the Free Software Foundation; either version 3 of the License, or
>> +   (at your option) any later version.
>> +
>> +   This program is distributed in the hope that it will be useful,
>> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +   GNU General Public License for more details.
>> +
>> +   You should have received a copy of the GNU General Public License
>> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>> +
>> +#include "nat/netbsd-nat.h"
>> +
>> +namespace netbsd_nat
>> +{
>> +}
>> diff --git a/gdb/nat/netbsd-nat.h b/gdb/nat/netbsd-nat.h
>> new file mode 100644
>> index 00000000000..5fa08746610
>> --- /dev/null
>> +++ b/gdb/nat/netbsd-nat.h
>> @@ -0,0 +1,27 @@
>> +/* Internal interfaces for the NetBSD code.
>> +
>> +   Copyright (C) 2006-2020 Free Software Foundation, Inc.
> 
> I presume that the copyright should say 2020 only?  It depends on the code that
> ends up here.  If you copy or move some code from some other file, in theory the
> copyright date should follow.
> 

This copyright years are picked from nbsd-nat.c. They could be narrowed
to 2020, but the original file containing these functions uses this range.

The functions from nbsd-nat.c are copied almost as is.

> Simon
> 



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

[PATCH v2 03/10] Build nat/netbsd-nat.o for the NetBSD native target

[Kamil Rytarowski]

gdb/ChangeLog:

	* configure.nat (NATDEPFILES): Add nat/netbsd-nat.o when needed.
---
 gdb/ChangeLog     | 4 ++++
 gdb/configure.nat | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 430f9b58a78..9e9d4e16e5d 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,7 @@
+2020-09-04  Kamil Rytarowski  <n54@gmx.com>
+
+	* configure.nat (NATDEPFILES): Add nat/netbsd-nat.o when needed.
+
 2020-09-04  Kamil Rytarowski  <n54@gmx.com>

 	* netbsd-nat.h: New file.
diff --git a/gdb/configure.nat b/gdb/configure.nat
index 6ea25834954..3e94a064aeb 100644
--- a/gdb/configure.nat
+++ b/gdb/configure.nat
@@ -68,7 +68,7 @@ case ${gdb_host} in
 	LOADLIBES='-lkvm'
 	;;
     nbsd*)
-	NATDEPFILES='fork-child.o nat/fork-inferior.o inf-ptrace.o'
+	NATDEPFILES='fork-child.o nat/fork-inferior.o nat/netbsd-nat.o inf-ptrace.o'
 	HAVE_NATIVE_GCORE_HOST=1
 	;;
     obsd*)
--
2.28.0

[PATCH v2 04/10] Add netbsd_nat::pid_to_exec_file

[Kamil Rytarowski]

gdb/ChangeLog:

        * netbsd-nat.h: Include <unistd.h>.
        * (netbsd_nat::pid_to_exec_file): Add.
        * netbsd-nat.c: Include <sys/types.h> and <sys/sysctl.h>.
        * (netbsd_nat::pid_to_exec_file) Add.
---
 gdb/ChangeLog        |  7 +++++++
 gdb/nat/netbsd-nat.c | 18 ++++++++++++++++++
 gdb/nat/netbsd-nat.h |  5 +++++
 3 files changed, 30 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 9e9d4e16e5d..335d6b7271f 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2020-09-04  Kamil Rytarowski  <n54@gmx.com>
+
+	* netbsd-nat.h: Include <unistd.h>.
+	* (netbsd_nat::pid_to_exec_file): Add.
+	* netbsd-nat.c: Include <sys/types.h> and <sys/sysctl.h>.
+	* (netbsd_nat::pid_to_exec_file) Add.
+
 2020-09-04  Kamil Rytarowski  <n54@gmx.com>

 	* configure.nat (NATDEPFILES): Add nat/netbsd-nat.o when needed.
diff --git a/gdb/nat/netbsd-nat.c b/gdb/nat/netbsd-nat.c
index 2b5a4183e30..297188bb8b4 100644
--- a/gdb/nat/netbsd-nat.c
+++ b/gdb/nat/netbsd-nat.c
@@ -19,6 +19,24 @@

 #include "nat/netbsd-nat.h"

+#include <sys/types.h>
+#include <sys/sysctl.h>
+
 namespace netbsd_nat
 {
+
+/* Return the executable file name of a process specified by PID.  Returns the
+   string in a static buffer.  */
+
+char *
+pid_to_exec_file (pid_t pid)
+{
+  static char buf[PATH_MAX];
+  int mib[4] = {CTL_KERN, KERN_PROC_ARGS, pid, KERN_PROC_PATHNAME};
+  size_t buflen = sizeof (buf);
+  if (::sysctl (mib, ARRAY_SIZE (mib), buf, &buflen, NULL, 0))
+    return NULL;
+  return buf;
+}
+
 }
diff --git a/gdb/nat/netbsd-nat.h b/gdb/nat/netbsd-nat.h
index 5fa08746610..a5f664d95f4 100644
--- a/gdb/nat/netbsd-nat.h
+++ b/gdb/nat/netbsd-nat.h
@@ -20,8 +20,13 @@
 #ifndef NAT_NETBSD_NAT_H
 #define NAT_NETBSD_NAT_H

+#include <unistd.h>
+
 namespace netbsd_nat
 {
+
+extern char *pid_to_exec_file (pid_t pid);
+
 }

 #endif
--
2.28.0

Re: [PATCH v2 04/10] Add netbsd_nat::pid_to_exec_file

[Andrew Burgess]

* Kamil Rytarowski <n54@gmx.com> [2020-09-04 02:28:59 +0200]:

> gdb/ChangeLog:
> 
>         * netbsd-nat.h: Include <unistd.h>.
>         * (netbsd_nat::pid_to_exec_file): Add.
>         * netbsd-nat.c: Include <sys/types.h> and <sys/sysctl.h>.
>         * (netbsd_nat::pid_to_exec_file) Add.
> ---
>  gdb/ChangeLog        |  7 +++++++
>  gdb/nat/netbsd-nat.c | 18 ++++++++++++++++++
>  gdb/nat/netbsd-nat.h |  5 +++++
>  3 files changed, 30 insertions(+)
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 9e9d4e16e5d..335d6b7271f 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,10 @@
> +2020-09-04  Kamil Rytarowski  <n54@gmx.com>
> +
> +	* netbsd-nat.h: Include <unistd.h>.
> +	* (netbsd_nat::pid_to_exec_file): Add.
> +	* netbsd-nat.c: Include <sys/types.h> and <sys/sysctl.h>.
> +	* (netbsd_nat::pid_to_exec_file) Add.
> +
>  2020-09-04  Kamil Rytarowski  <n54@gmx.com>
> 
>  	* configure.nat (NATDEPFILES): Add nat/netbsd-nat.o when needed.
> diff --git a/gdb/nat/netbsd-nat.c b/gdb/nat/netbsd-nat.c
> index 2b5a4183e30..297188bb8b4 100644
> --- a/gdb/nat/netbsd-nat.c
> +++ b/gdb/nat/netbsd-nat.c
> @@ -19,6 +19,24 @@
> 
>  #include "nat/netbsd-nat.h"
> 
> +#include <sys/types.h>
> +#include <sys/sysctl.h>
> +
>  namespace netbsd_nat
>  {
> +
> +/* Return the executable file name of a process specified by PID.  Returns the
> +   string in a static buffer.  */
> +
> +char *
> +pid_to_exec_file (pid_t pid)

The convention in GDB is to document global functions in the header
file, and leave a comment '/* See xxxx.h.  */' in the source file.

Thanks,
Andrew


> +{
> +  static char buf[PATH_MAX];
> +  int mib[4] = {CTL_KERN, KERN_PROC_ARGS, pid, KERN_PROC_PATHNAME};
> +  size_t buflen = sizeof (buf);
> +  if (::sysctl (mib, ARRAY_SIZE (mib), buf, &buflen, NULL, 0))
> +    return NULL;
> +  return buf;
> +}
> +
>  }
> diff --git a/gdb/nat/netbsd-nat.h b/gdb/nat/netbsd-nat.h
> index 5fa08746610..a5f664d95f4 100644
> --- a/gdb/nat/netbsd-nat.h
> +++ b/gdb/nat/netbsd-nat.h
> @@ -20,8 +20,13 @@
>  #ifndef NAT_NETBSD_NAT_H
>  #define NAT_NETBSD_NAT_H
> 
> +#include <unistd.h>
> +
>  namespace netbsd_nat
>  {
> +
> +extern char *pid_to_exec_file (pid_t pid);
> +
>  }
> 
>  #endif
> --
> 2.28.0
> 

Re: [PATCH v2 04/10] Add netbsd_nat::pid_to_exec_file

[Kamil Rytarowski]

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

On 07.09.2020 09:57, Andrew Burgess wrote:
> * Kamil Rytarowski <n54@gmx.com> [2020-09-04 02:28:59 +0200]:
> 
>> gdb/ChangeLog:
>>
>>         * netbsd-nat.h: Include <unistd.h>.
>>         * (netbsd_nat::pid_to_exec_file): Add.
>>         * netbsd-nat.c: Include <sys/types.h> and <sys/sysctl.h>.
>>         * (netbsd_nat::pid_to_exec_file) Add.
>> ---
>>  gdb/ChangeLog        |  7 +++++++
>>  gdb/nat/netbsd-nat.c | 18 ++++++++++++++++++
>>  gdb/nat/netbsd-nat.h |  5 +++++
>>  3 files changed, 30 insertions(+)
>>
>> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
>> index 9e9d4e16e5d..335d6b7271f 100644
>> --- a/gdb/ChangeLog
>> +++ b/gdb/ChangeLog
>> @@ -1,3 +1,10 @@
>> +2020-09-04  Kamil Rytarowski  <n54@gmx.com>
>> +
>> +	* netbsd-nat.h: Include <unistd.h>.
>> +	* (netbsd_nat::pid_to_exec_file): Add.
>> +	* netbsd-nat.c: Include <sys/types.h> and <sys/sysctl.h>.
>> +	* (netbsd_nat::pid_to_exec_file) Add.
>> +
>>  2020-09-04  Kamil Rytarowski  <n54@gmx.com>
>>
>>  	* configure.nat (NATDEPFILES): Add nat/netbsd-nat.o when needed.
>> diff --git a/gdb/nat/netbsd-nat.c b/gdb/nat/netbsd-nat.c
>> index 2b5a4183e30..297188bb8b4 100644
>> --- a/gdb/nat/netbsd-nat.c
>> +++ b/gdb/nat/netbsd-nat.c
>> @@ -19,6 +19,24 @@
>>
>>  #include "nat/netbsd-nat.h"
>>
>> +#include <sys/types.h>
>> +#include <sys/sysctl.h>
>> +
>>  namespace netbsd_nat
>>  {
>> +
>> +/* Return the executable file name of a process specified by PID.  Returns the
>> +   string in a static buffer.  */
>> +
>> +char *
>> +pid_to_exec_file (pid_t pid)
> 
> The convention in GDB is to document global functions in the header
> file, and leave a comment '/* See xxxx.h.  */' in the source file.
> 
> Thanks,
> Andrew
> 
> 

OK. I will do it in v3.

>> +{
>> +  static char buf[PATH_MAX];
>> +  int mib[4] = {CTL_KERN, KERN_PROC_ARGS, pid, KERN_PROC_PATHNAME};
>> +  size_t buflen = sizeof (buf);
>> +  if (::sysctl (mib, ARRAY_SIZE (mib), buf, &buflen, NULL, 0))
>> +    return NULL;
>> +  return buf;
>> +}
>> +
>>  }
>> diff --git a/gdb/nat/netbsd-nat.h b/gdb/nat/netbsd-nat.h
>> index 5fa08746610..a5f664d95f4 100644
>> --- a/gdb/nat/netbsd-nat.h
>> +++ b/gdb/nat/netbsd-nat.h
>> @@ -20,8 +20,13 @@
>>  #ifndef NAT_NETBSD_NAT_H
>>  #define NAT_NETBSD_NAT_H
>>
>> +#include <unistd.h>
>> +
>>  namespace netbsd_nat
>>  {
>> +
>> +extern char *pid_to_exec_file (pid_t pid);
>> +
>>  }
>>
>>  #endif
>> --
>> 2.28.0
>>



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

Re: [PATCH v2 04/10] Add netbsd_nat::pid_to_exec_file

[Simon Marchi]

On 2020-09-07 9:36 a.m., Kamil Rytarowski wrote:
>> The convention in GDB is to document global functions in the header
>> file, and leave a comment '/* See xxxx.h.  */' in the source file.
>>
>> Thanks,
>> Andrew
>>
>>
> OK. I will do it in v3.
> 

Thanks, don't forget to apply it in other other patches too (such as 05/10).

Simon

Re: [PATCH v2 04/10] Add netbsd_nat::pid_to_exec_file

[Simon Marchi]

On 2020-09-03 8:28 p.m., Kamil Rytarowski wrote:
> gdb/ChangeLog:
> 
>         * netbsd-nat.h: Include <unistd.h>.
>         * (netbsd_nat::pid_to_exec_file): Add.
>         * netbsd-nat.c: Include <sys/types.h> and <sys/sysctl.h>.
>         * (netbsd_nat::pid_to_exec_file) Add.
> ---
>  gdb/ChangeLog        |  7 +++++++
>  gdb/nat/netbsd-nat.c | 18 ++++++++++++++++++
>  gdb/nat/netbsd-nat.h |  5 +++++
>  3 files changed, 30 insertions(+)
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 9e9d4e16e5d..335d6b7271f 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,10 @@
> +2020-09-04  Kamil Rytarowski  <n54@gmx.com>
> +
> +	* netbsd-nat.h: Include <unistd.h>.
> +	* (netbsd_nat::pid_to_exec_file): Add.
> +	* netbsd-nat.c: Include <sys/types.h> and <sys/sysctl.h>.
> +	* (netbsd_nat::pid_to_exec_file) Add.
> +
>  2020-09-04  Kamil Rytarowski  <n54@gmx.com>
> 
>  	* configure.nat (NATDEPFILES): Add nat/netbsd-nat.o when needed.
> diff --git a/gdb/nat/netbsd-nat.c b/gdb/nat/netbsd-nat.c
> index 2b5a4183e30..297188bb8b4 100644
> --- a/gdb/nat/netbsd-nat.c
> +++ b/gdb/nat/netbsd-nat.c
> @@ -19,6 +19,24 @@
> 
>  #include "nat/netbsd-nat.h"
> 
> +#include <sys/types.h>
> +#include <sys/sysctl.h>
> +
>  namespace netbsd_nat
>  {
> +
> +/* Return the executable file name of a process specified by PID.  Returns the
> +   string in a static buffer.  */
> +
> +char *

Since there's no reason for the caller to modify this string, I'd suggest returning `const char *`.

> +pid_to_exec_file (pid_t pid)
> +{
> +  static char buf[PATH_MAX];
> +  int mib[4] = {CTL_KERN, KERN_PROC_ARGS, pid, KERN_PROC_PATHNAME};
> +  size_t buflen = sizeof (buf);
> +  if (::sysctl (mib, ARRAY_SIZE (mib), buf, &buflen, NULL, 0))
> +    return NULL;

!= 0

Simon

Re: [PATCH v2 04/10] Add netbsd_nat::pid_to_exec_file

[Kamil Rytarowski]

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

On 07.09.2020 20:47, Simon Marchi wrote:
> On 2020-09-03 8:28 p.m., Kamil Rytarowski wrote:
>> gdb/ChangeLog:
>>
>>         * netbsd-nat.h: Include <unistd.h>.
>>         * (netbsd_nat::pid_to_exec_file): Add.
>>         * netbsd-nat.c: Include <sys/types.h> and <sys/sysctl.h>.
>>         * (netbsd_nat::pid_to_exec_file) Add.
>> ---
>>  gdb/ChangeLog        |  7 +++++++
>>  gdb/nat/netbsd-nat.c | 18 ++++++++++++++++++
>>  gdb/nat/netbsd-nat.h |  5 +++++
>>  3 files changed, 30 insertions(+)
>>
>> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
>> index 9e9d4e16e5d..335d6b7271f 100644
>> --- a/gdb/ChangeLog
>> +++ b/gdb/ChangeLog
>> @@ -1,3 +1,10 @@
>> +2020-09-04  Kamil Rytarowski  <n54@gmx.com>
>> +
>> +	* netbsd-nat.h: Include <unistd.h>.
>> +	* (netbsd_nat::pid_to_exec_file): Add.
>> +	* netbsd-nat.c: Include <sys/types.h> and <sys/sysctl.h>.
>> +	* (netbsd_nat::pid_to_exec_file) Add.
>> +
>>  2020-09-04  Kamil Rytarowski  <n54@gmx.com>
>>
>>  	* configure.nat (NATDEPFILES): Add nat/netbsd-nat.o when needed.
>> diff --git a/gdb/nat/netbsd-nat.c b/gdb/nat/netbsd-nat.c
>> index 2b5a4183e30..297188bb8b4 100644
>> --- a/gdb/nat/netbsd-nat.c
>> +++ b/gdb/nat/netbsd-nat.c
>> @@ -19,6 +19,24 @@
>>
>>  #include "nat/netbsd-nat.h"
>>
>> +#include <sys/types.h>
>> +#include <sys/sysctl.h>
>> +
>>  namespace netbsd_nat
>>  {
>> +
>> +/* Return the executable file name of a process specified by PID.  Returns the
>> +   string in a static buffer.  */
>> +
>> +char *
> 
> Since there's no reason for the caller to modify this string, I'd suggest returning `const char *`.
> 

OK

>> +pid_to_exec_file (pid_t pid)
>> +{
>> +  static char buf[PATH_MAX];
>> +  int mib[4] = {CTL_KERN, KERN_PROC_ARGS, pid, KERN_PROC_PATHNAME};
>> +  size_t buflen = sizeof (buf);
>> +  if (::sysctl (mib, ARRAY_SIZE (mib), buf, &buflen, NULL, 0))
>> +    return NULL;
> 
> != 0
> 

OK.

> Simon
> 



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

[PATCH v2 05/10] Add gdb/nat common functions for listing threads

[Kamil Rytarowski]

Add netbsd_nat::netbsd_thread_lister a generic thread lister, used
internally in netbsd-nat.c, copied from gdb/nbsd-nat.c.

Add public extern functions for listing threads:
 * netbsd_nat::thread_alive
 * netbsd_nat::thread_name
 * netbsd_nat::for_each_thread

gdb/ChangeLog:

	* netbsd-nat.h: Include "gdbsupport/function-view.h".
	* (netbsd_nat::thread_alive, netbsd_nat::thread_name)
	(netbsd_nat::for_each_thread): Add.
	* netbsd-nat.c: Include "gdbsupport/common-defs.h" and
	"gdbsupport/common-debug.h".
	* (netbsd_nat::netbsd_thread_lister)
	(netbsd_nat::thread_alive, netbsd_nat::thread_name)
	(netbsd_nat::for_each_thread): Add.
---
 gdb/ChangeLog        |  11 ++++
 gdb/nat/netbsd-nat.c | 123 +++++++++++++++++++++++++++++++++++++++++++
 gdb/nat/netbsd-nat.h |   8 +++
 3 files changed, 142 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 335d6b7271f..b9b028ebe68 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,14 @@
+2020-09-04  Kamil Rytarowski  <n54@gmx.com>
+
+	* netbsd-nat.h: Include "gdbsupport/function-view.h".
+	* (netbsd_nat::thread_alive, netbsd_nat::thread_name)
+	(netbsd_nat::for_each_thread): Add.
+	* netbsd-nat.c: Include "gdbsupport/common-defs.h" and
+	"gdbsupport/common-debug.h".
+	* (netbsd_nat::netbsd_thread_lister)
+	(netbsd_nat::thread_alive, netbsd_nat::thread_name)
+	(netbsd_nat::for_each_thread): Add.
+
 2020-09-04  Kamil Rytarowski  <n54@gmx.com>

 	* netbsd-nat.h: Include <unistd.h>.
diff --git a/gdb/nat/netbsd-nat.c b/gdb/nat/netbsd-nat.c
index 297188bb8b4..bd28e116b56 100644
--- a/gdb/nat/netbsd-nat.c
+++ b/gdb/nat/netbsd-nat.c
@@ -17,11 +17,17 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */

+#include "gdbsupport/common-defs.h"
 #include "nat/netbsd-nat.h"
+#include "gdbsupport/common-debug.h"

 #include <sys/types.h>
 #include <sys/sysctl.h>

+#include <cstring>
+
+#include "gdbsupport/function-view.h"
+
 namespace netbsd_nat
 {

@@ -39,4 +45,121 @@ pid_to_exec_file (pid_t pid)
   return buf;
 }

+/* Generic thread (LWP) lister within a specified process.  The callback
+   parameters is a C++ function that is called for each detected thread.  */
+
+static bool
+netbsd_thread_lister (const pid_t pid,
+		      gdb::function_view<bool (const struct kinfo_lwp *)>
+		      callback)
+{
+  int mib[5] = {CTL_KERN, KERN_LWP, pid, sizeof (struct kinfo_lwp), 0};
+  size_t size;
+
+  if (sysctl (mib, ARRAY_SIZE (mib), NULL, &size, NULL, 0) == -1 || size == 0)
+    perror_with_name (("sysctl"));
+
+  mib[4] = size / sizeof (size_t);
+
+  gdb::unique_xmalloc_ptr<struct kinfo_lwp[]> kl
+    ((struct kinfo_lwp *) xcalloc (size, 1));
+
+  if (sysctl (mib, ARRAY_SIZE (mib), kl.get (), &size, NULL, 0) == -1
+      || size == 0)
+    perror_with_name (("sysctl"));
+
+  for (size_t i = 0; i < size / sizeof (struct kinfo_lwp); i++)
+    {
+      struct kinfo_lwp *l = &kl[i];
+
+      /* Return true if the specified thread is alive.  */
+      auto lwp_alive
+	= [] (struct kinfo_lwp *lwp)
+	  {
+	    switch (lwp->l_stat)
+	      {
+	      case LSSLEEP:
+	      case LSRUN:
+	      case LSONPROC:
+	      case LSSTOP:
+	      case LSSUSPENDED:
+		return true;
+	      default:
+		return false;
+	      }
+	  };
+
+      /* Ignore embryonic or demised threads.  */
+      if (!lwp_alive (l))
+	continue;
+
+      if (callback (l))
+	return true;
+    }
+
+  return false;
+}
+
+/* Return true if PTID is still active in the inferior.  */
+
+bool
+thread_alive (ptid_t ptid)
+{
+  pid_t pid = ptid.pid ();
+  lwpid_t lwp = ptid.lwp ();
+
+  auto fn
+    = [&lwp] (const struct kinfo_lwp *kl)
+      {
+        return kl->l_lid == lwp;
+      };
+
+  return netbsd_thread_lister (pid, fn);
+}
+
+/* Return the name assigned to a thread by an application.  Returns
+   the string in a static buffer.  */
+
+const char *
+thread_name (ptid_t ptid)
+{
+  pid_t pid = ptid.pid ();
+  lwpid_t lwp = ptid.lwp ();
+
+  static char buf[KI_LNAMELEN] = {};
+
+  auto fn
+    = [&lwp] (const struct kinfo_lwp *kl)
+      {
+	if (kl->l_lid == lwp)
+	  {
+	    xsnprintf (buf, sizeof buf, "%s", kl->l_name);
+	    return true;
+	  }
+	return false;
+      };
+
+  if (netbsd_thread_lister (pid, fn))
+    return buf;
+  else
+    return NULL;
+}
+
+/* A generic thread lister within a specific PID.  The CALLBACK parameter
+   is a C++ function that is called for each detected thread.  */
+
+void
+for_each_thread (pid_t pid, gdb::function_view<void (ptid_t)> callback)
+{
+  auto fn
+    = [&callback, &pid] (const struct kinfo_lwp *kl)
+      {
+	ptid_t ptid = ptid_t (pid, kl->l_lid, 0);
+	callback (ptid);
+	return false;
+      };
+
+  netbsd_thread_lister (pid, fn);
+}
+
 }
diff --git a/gdb/nat/netbsd-nat.h b/gdb/nat/netbsd-nat.h
index a5f664d95f4..c3a58ae7f49 100644
--- a/gdb/nat/netbsd-nat.h
+++ b/gdb/nat/netbsd-nat.h
@@ -20,6 +20,8 @@
 #ifndef NAT_NETBSD_NAT_H
 #define NAT_NETBSD_NAT_H

+#include "gdbsupport/function-view.h"
+
 #include <unistd.h>

 namespace netbsd_nat
@@ -27,6 +29,12 @@ namespace netbsd_nat

 extern char *pid_to_exec_file (pid_t pid);

+extern bool thread_alive (ptid_t ptid);
+
+extern const char *thread_name (ptid_t ptid);
+
+extern void for_each_thread (pid_t pid,
+			     gdb::function_view<void (ptid_t)> callback);
 }

 #endif
--
2.28.0

Re: [PATCH v2 05/10] Add gdb/nat common functions for listing threads

[Simon Marchi]

On 2020-09-03 8:29 p.m., Kamil Rytarowski wrote:
> @@ -39,4 +45,121 @@ pid_to_exec_file (pid_t pid)
>    return buf;
>  }
> 
> +/* Generic thread (LWP) lister within a specified process.  The callback
> +   parameters is a C++ function that is called for each detected thread.  */

The behavior seems to be that if the callback returns true, the iteration is
stopped.  If so, it would be good to document it.  As well as the return value
of this function.

> +
> +static bool
> +netbsd_thread_lister (const pid_t pid,
> +		      gdb::function_view<bool (const struct kinfo_lwp *)>
> +		      callback)
> +{
> +  int mib[5] = {CTL_KERN, KERN_LWP, pid, sizeof (struct kinfo_lwp), 0};
> +  size_t size;
> +
> +  if (sysctl (mib, ARRAY_SIZE (mib), NULL, &size, NULL, 0) == -1 || size == 0)
> +    perror_with_name (("sysctl"));
> +
> +  mib[4] = size / sizeof (size_t);
> +
> +  gdb::unique_xmalloc_ptr<struct kinfo_lwp[]> kl
> +    ((struct kinfo_lwp *) xcalloc (size, 1));
> +
> +  if (sysctl (mib, ARRAY_SIZE (mib), kl.get (), &size, NULL, 0) == -1
> +      || size == 0)
> +    perror_with_name (("sysctl"));

Is there a chance that the number of threads changes between the two sysctl
calls?  Or does that assume that the process is not executing?  It would be
good to spell out any such assumption in a comment.

> +
> +  for (size_t i = 0; i < size / sizeof (struct kinfo_lwp); i++)
> +    {
> +      struct kinfo_lwp *l = &kl[i];
> +
> +      /* Return true if the specified thread is alive.  */
> +      auto lwp_alive
> +	= [] (struct kinfo_lwp *lwp)
> +	  {
> +	    switch (lwp->l_stat)
> +	      {
> +	      case LSSLEEP:
> +	      case LSRUN:
> +	      case LSONPROC:
> +	      case LSSTOP:
> +	      case LSSUSPENDED:
> +		return true;
> +	      default:
> +		return false;
> +	      }
> +	  };
> +
> +      /* Ignore embryonic or demised threads.  */
> +      if (!lwp_alive (l))
> +	continue;
> +
> +      if (callback (l))
> +	return true;
> +    }
> +
> +  return false;
> +}
> +
> +/* Return true if PTID is still active in the inferior.  */
> +
> +bool
> +thread_alive (ptid_t ptid)
> +{
> +  pid_t pid = ptid.pid ();
> +  lwpid_t lwp = ptid.lwp ();
> +
> +  auto fn
> +    = [&lwp] (const struct kinfo_lwp *kl)
> +      {
> +        return kl->l_lid == lwp;
> +      };

Just a nit, but would it be better to capture variables known to be
scalars as value instead of as reference?

Simon

Re: [PATCH v2 05/10] Add gdb/nat common functions for listing threads

[Kamil Rytarowski]

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

On 07.09.2020 20:59, Simon Marchi wrote:
> On 2020-09-03 8:29 p.m., Kamil Rytarowski wrote:
>> @@ -39,4 +45,121 @@ pid_to_exec_file (pid_t pid)
>>    return buf;
>>  }
>>
>> +/* Generic thread (LWP) lister within a specified process.  The callback
>> +   parameters is a C++ function that is called for each detected thread.  */
> 
> The behavior seems to be that if the callback returns true, the iteration is
> stopped.  If so, it would be good to document it.  As well as the return value
> of this function.
> 

OK.

>> +
>> +static bool
>> +netbsd_thread_lister (const pid_t pid,
>> +		      gdb::function_view<bool (const struct kinfo_lwp *)>
>> +		      callback)
>> +{
>> +  int mib[5] = {CTL_KERN, KERN_LWP, pid, sizeof (struct kinfo_lwp), 0};
>> +  size_t size;
>> +
>> +  if (sysctl (mib, ARRAY_SIZE (mib), NULL, &size, NULL, 0) == -1 || size == 0)
>> +    perror_with_name (("sysctl"));
>> +
>> +  mib[4] = size / sizeof (size_t);
>> +
>> +  gdb::unique_xmalloc_ptr<struct kinfo_lwp[]> kl
>> +    ((struct kinfo_lwp *) xcalloc (size, 1));
>> +
>> +  if (sysctl (mib, ARRAY_SIZE (mib), kl.get (), &size, NULL, 0) == -1
>> +      || size == 0)
>> +    perror_with_name (("sysctl"));
> 
> Is there a chance that the number of threads changes between the two sysctl
> calls?  Or does that assume that the process is not executing?  It would be
> good to spell out any such assumption in a comment.
> 

This function could be used for alive processes, but currently it is
used only for the stopped ones, thus the number of threads does not change.

>> +
>> +  for (size_t i = 0; i < size / sizeof (struct kinfo_lwp); i++)
>> +    {
>> +      struct kinfo_lwp *l = &kl[i];
>> +
>> +      /* Return true if the specified thread is alive.  */
>> +      auto lwp_alive
>> +	= [] (struct kinfo_lwp *lwp)
>> +	  {
>> +	    switch (lwp->l_stat)
>> +	      {
>> +	      case LSSLEEP:
>> +	      case LSRUN:
>> +	      case LSONPROC:
>> +	      case LSSTOP:
>> +	      case LSSUSPENDED:
>> +		return true;
>> +	      default:
>> +		return false;
>> +	      }
>> +	  };
>> +
>> +      /* Ignore embryonic or demised threads.  */
>> +      if (!lwp_alive (l))
>> +	continue;
>> +
>> +      if (callback (l))
>> +	return true;
>> +    }
>> +
>> +  return false;
>> +}
>> +
>> +/* Return true if PTID is still active in the inferior.  */
>> +
>> +bool
>> +thread_alive (ptid_t ptid)
>> +{
>> +  pid_t pid = ptid.pid ();
>> +  lwpid_t lwp = ptid.lwp ();
>> +
>> +  auto fn
>> +    = [&lwp] (const struct kinfo_lwp *kl)
>> +      {
>> +        return kl->l_lid == lwp;
>> +      };
> 
> Just a nit, but would it be better to capture variables known to be
> scalars as value instead of as reference?
> 

It's not critical here, but I will change it to capture by value.

> Simon
> 



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

[PATCH v2 06/10] Add netbsd_nat::enable_proc_events in gdb/nat

[Kamil Rytarowski]

Add generic function to enable debugger events in a process.

gdb/ChangeLog:

        * netbsd-nat.h (netbsd_nat::enable_proc_events): Add.
	* netbsd-nat.c: Include <sys/ptrace.h>.
	* (netbsd_nat::enable_proc_events): Add.
---
 gdb/ChangeLog        |  6 ++++++
 gdb/nat/netbsd-nat.c | 18 ++++++++++++++++++
 gdb/nat/netbsd-nat.h |  2 ++
 3 files changed, 26 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index b9b028ebe68..0db297a6cdd 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2020-09-04  Kamil Rytarowski  <n54@gmx.com>
+
+	* netbsd-nat.h (netbsd_nat::enable_proc_events): Add.
+	* netbsd-nat.c: Include <sys/ptrace.h>.
+	* (netbsd_nat::enable_proc_events): Add.
+
 2020-09-04  Kamil Rytarowski  <n54@gmx.com>

 	* netbsd-nat.h: Include "gdbsupport/function-view.h".
diff --git a/gdb/nat/netbsd-nat.c b/gdb/nat/netbsd-nat.c
index bd28e116b56..1220b272c8c 100644
--- a/gdb/nat/netbsd-nat.c
+++ b/gdb/nat/netbsd-nat.c
@@ -22,6 +22,7 @@
 #include "gdbsupport/common-debug.h"

 #include <sys/types.h>
+#include <sys/ptrace.h>
 #include <sys/sysctl.h>

 #include <cstring>
@@ -162,4 +163,21 @@ for_each_thread (pid_t pid, gdb::function_view<void (ptid_t)> callback)
   netbsd_thread_lister (pid, fn);
 }

+/* Enable additional event reporting in a new process specified by PID.  */
+
+void
+enable_proc_events (pid_t pid)
+{
+  int events;
+
+  if (ptrace (PT_GET_EVENT_MASK, pid, &events, sizeof (events)) == -1)
+    perror_with_name (("ptrace"));
+
+  events |= PTRACE_LWP_CREATE;
+  events |= PTRACE_LWP_EXIT;
+
+  if (ptrace (PT_SET_EVENT_MASK, pid, &events, sizeof (events)) == -1)
+    perror_with_name (("ptrace"));
+}
+
 }
diff --git a/gdb/nat/netbsd-nat.h b/gdb/nat/netbsd-nat.h
index c3a58ae7f49..58021b2398e 100644
--- a/gdb/nat/netbsd-nat.h
+++ b/gdb/nat/netbsd-nat.h
@@ -35,6 +35,8 @@ extern const char *thread_name (ptid_t ptid);

 extern void for_each_thread (pid_t pid,
 			     gdb::function_view<void (ptid_t)> callback);
+
+extern void enable_proc_events (pid_t pid);
 }

 #endif
--
2.28.0

[PATCH v2 07/10] Add a common utility function to read and write siginfo_t in inferior

[Kamil Rytarowski]

gdb/ChangeLog:

        * netbsd-nat.h (netbsd_nat::qxfer_siginfo): Add.
        * netbsd-nat.c (netbsd_nat::qxfer_siginfo): Likewise.
---
 gdb/ChangeLog        |  5 +++++
 gdb/nat/netbsd-nat.c | 30 ++++++++++++++++++++++++++++++
 gdb/nat/netbsd-nat.h |  4 ++++
 3 files changed, 39 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 0db297a6cdd..b96e7bf08e8 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2020-09-04  Kamil Rytarowski  <n54@gmx.com>
+
+	* netbsd-nat.h (netbsd_nat::qxfer_siginfo): Add.
+	* netbsd-nat.c (netbsd_nat::qxfer_siginfo): Likewise.
+
 2020-09-04  Kamil Rytarowski  <n54@gmx.com>

 	* netbsd-nat.h (netbsd_nat::enable_proc_events): Add.
diff --git a/gdb/nat/netbsd-nat.c b/gdb/nat/netbsd-nat.c
index 1220b272c8c..a13b1509d5a 100644
--- a/gdb/nat/netbsd-nat.c
+++ b/gdb/nat/netbsd-nat.c
@@ -180,4 +180,34 @@ enable_proc_events (pid_t pid)
     perror_with_name (("ptrace"));
 }

+/* Implement reading and writing of inferior's siginfo_t specified by PID.
+   Returns -1 on failure and the number of bytes on a successful transfer.  */
+
+int
+qxfer_siginfo (pid_t pid, const char *annex, unsigned char *readbuf,
+	       unsigned const char *writebuf, CORE_ADDR offset, int len)
+{
+  ptrace_siginfo_t psi;
+
+  if (offset > sizeof (siginfo_t))
+    return -1;
+
+  if (ptrace (PT_GET_SIGINFO, pid, &psi, sizeof (psi)) == -1)
+    return -1;
+
+  if (offset + len > sizeof (siginfo_t))
+    len = sizeof (siginfo_t) - offset;
+
+  if (readbuf != NULL)
+    memcpy (readbuf, ((gdb_byte *) &psi.psi_siginfo) + offset, len);
+  else
+    {
+      memcpy (((gdb_byte *) &psi.psi_siginfo) + offset, writebuf, len);
+
+      if (ptrace (PT_SET_SIGINFO, pid, &psi, sizeof (psi)) == -1)
+	return -1;
+    }
+  return len;
+}
+
 }
diff --git a/gdb/nat/netbsd-nat.h b/gdb/nat/netbsd-nat.h
index 58021b2398e..74b4b3ecca5 100644
--- a/gdb/nat/netbsd-nat.h
+++ b/gdb/nat/netbsd-nat.h
@@ -37,6 +37,10 @@ extern void for_each_thread (pid_t pid,
 			     gdb::function_view<void (ptid_t)> callback);

 extern void enable_proc_events (pid_t pid);
+
+extern int qxfer_siginfo (pid_t pid, const char *annex, unsigned char *readbuf,
+			  unsigned const char *writebuf, CORE_ADDR offset,
+			  int len);
 }

 #endif
--
2.28.0

[PATCH v2 08/10] Avoid double free in startup_inferior

[Kamil Rytarowski]

Do not free the last execd pathname as it will be used in
prepare_resume_reply(), after attaching a client side.

gdb/ChangeLog:

	* fork-inferior.c (startup_inferior): Avoid double free.
---
 gdb/ChangeLog           | 4 ++++
 gdb/nat/fork-inferior.c | 5 ++++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index b96e7bf08e8..1013f6a0b3c 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,7 @@
+2020-09-04  Kamil Rytarowski  <n54@gmx.com>
+
+	* fork-inferior.c (startup_inferior): Avoid double free.
+
 2020-09-04  Kamil Rytarowski  <n54@gmx.com>

 	* netbsd-nat.h (netbsd_nat::qxfer_siginfo): Add.
diff --git a/gdb/nat/fork-inferior.c b/gdb/nat/fork-inferior.c
index 1185ef8998b..94ab0b9cbc2 100644
--- a/gdb/nat/fork-inferior.c
+++ b/gdb/nat/fork-inferior.c
@@ -526,7 +526,10 @@ startup_inferior (process_stratum_target *proc_target, pid_t pid, int ntraps,

 	  case TARGET_WAITKIND_EXECD:
 	    /* Handle EXEC signals as if they were SIGTRAP signals.  */
-	    xfree (ws.value.execd_pathname);
+	    /* Do not free the last execd pathname as it will be used in
+	       prepare_resume_reply(), after attaching a client side.  */
+	    if (pending_execs != 1)
+	      xfree (ws.value.execd_pathname);
 	    resume_signal = GDB_SIGNAL_TRAP;
 	    switch_to_thread (proc_target, event_ptid);
 	    break;
--
2.28.0

Re: [PATCH v2 08/10] Avoid double free in startup_inferior

[Simon Marchi]

On 2020-09-03 8:29 p.m., Kamil Rytarowski wrote:
> Do not free the last execd pathname as it will be used in
> prepare_resume_reply(), after attaching a client side.

Ok, so this function returns to its caller the last waitstatus.  So indeed we
want to clean up all the watstatus objects except the last one, which we hand
over to the caller.

> gdb/ChangeLog:
> 
> 	* fork-inferior.c (startup_inferior): Avoid double free.
> ---
>  gdb/ChangeLog           | 4 ++++
>  gdb/nat/fork-inferior.c | 5 ++++-
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index b96e7bf08e8..1013f6a0b3c 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,7 @@
> +2020-09-04  Kamil Rytarowski  <n54@gmx.com>
> +
> +	* fork-inferior.c (startup_inferior): Avoid double free.
> +
>  2020-09-04  Kamil Rytarowski  <n54@gmx.com>
> 
>  	* netbsd-nat.h (netbsd_nat::qxfer_siginfo): Add.
> diff --git a/gdb/nat/fork-inferior.c b/gdb/nat/fork-inferior.c
> index 1185ef8998b..94ab0b9cbc2 100644
> --- a/gdb/nat/fork-inferior.c
> +++ b/gdb/nat/fork-inferior.c
> @@ -526,7 +526,10 @@ startup_inferior (process_stratum_target *proc_target, pid_t pid, int ntraps,
> 
>  	  case TARGET_WAITKIND_EXECD:
>  	    /* Handle EXEC signals as if they were SIGTRAP signals.  */
> -	    xfree (ws.value.execd_pathname);
> +	    /* Do not free the last execd pathname as it will be used in
> +	       prepare_resume_reply(), after attaching a client side.  */

Since this is common code, let's just write it in a gdbserver or gdb agnostic way.

I'd suggest something like: Free the exec'ed pathname, but only if this isn't the
waitstatus we are returning to the caller.

Simon

Re: [PATCH v2 08/10] Avoid double free in startup_inferior

[Kamil Rytarowski]

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

On 07.09.2020 21:19, Simon Marchi wrote:
> On 2020-09-03 8:29 p.m., Kamil Rytarowski wrote:
>> Do not free the last execd pathname as it will be used in
>> prepare_resume_reply(), after attaching a client side.
> 
> Ok, so this function returns to its caller the last waitstatus.  So indeed we
> want to clean up all the watstatus objects except the last one, which we hand
> over to the caller.
> 
>> gdb/ChangeLog:
>>
>> 	* fork-inferior.c (startup_inferior): Avoid double free.
>> ---
>>  gdb/ChangeLog           | 4 ++++
>>  gdb/nat/fork-inferior.c | 5 ++++-
>>  2 files changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
>> index b96e7bf08e8..1013f6a0b3c 100644
>> --- a/gdb/ChangeLog
>> +++ b/gdb/ChangeLog
>> @@ -1,3 +1,7 @@
>> +2020-09-04  Kamil Rytarowski  <n54@gmx.com>
>> +
>> +	* fork-inferior.c (startup_inferior): Avoid double free.
>> +
>>  2020-09-04  Kamil Rytarowski  <n54@gmx.com>
>>
>>  	* netbsd-nat.h (netbsd_nat::qxfer_siginfo): Add.
>> diff --git a/gdb/nat/fork-inferior.c b/gdb/nat/fork-inferior.c
>> index 1185ef8998b..94ab0b9cbc2 100644
>> --- a/gdb/nat/fork-inferior.c
>> +++ b/gdb/nat/fork-inferior.c
>> @@ -526,7 +526,10 @@ startup_inferior (process_stratum_target *proc_target, pid_t pid, int ntraps,
>>
>>  	  case TARGET_WAITKIND_EXECD:
>>  	    /* Handle EXEC signals as if they were SIGTRAP signals.  */
>> -	    xfree (ws.value.execd_pathname);
>> +	    /* Do not free the last execd pathname as it will be used in
>> +	       prepare_resume_reply(), after attaching a client side.  */
> 
> Since this is common code, let's just write it in a gdbserver or gdb agnostic way.
> 
> I'd suggest something like: Free the exec'ed pathname, but only if this isn't the
> waitstatus we are returning to the caller.
> 

Please be more specific how to fix.

> Simon
> 



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

Re: [PATCH v2 08/10] Avoid double free in startup_inferior

[Simon Marchi]

On 2020-09-07 8:54 p.m., Kamil Rytarowski wrote:
> On 07.09.2020 21:19, Simon Marchi wrote:
>> On 2020-09-03 8:29 p.m., Kamil Rytarowski wrote:
>>> Do not free the last execd pathname as it will be used in
>>> prepare_resume_reply(), after attaching a client side.
>>
>> Ok, so this function returns to its caller the last waitstatus.  So indeed we
>> want to clean up all the watstatus objects except the last one, which we hand
>> over to the caller.
>>
>>> gdb/ChangeLog:
>>>
>>> 	* fork-inferior.c (startup_inferior): Avoid double free.
>>> ---
>>>  gdb/ChangeLog           | 4 ++++
>>>  gdb/nat/fork-inferior.c | 5 ++++-
>>>  2 files changed, 8 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
>>> index b96e7bf08e8..1013f6a0b3c 100644
>>> --- a/gdb/ChangeLog
>>> +++ b/gdb/ChangeLog
>>> @@ -1,3 +1,7 @@
>>> +2020-09-04  Kamil Rytarowski  <n54@gmx.com>
>>> +
>>> +	* fork-inferior.c (startup_inferior): Avoid double free.
>>> +
>>>  2020-09-04  Kamil Rytarowski  <n54@gmx.com>
>>>
>>>  	* netbsd-nat.h (netbsd_nat::qxfer_siginfo): Add.
>>> diff --git a/gdb/nat/fork-inferior.c b/gdb/nat/fork-inferior.c
>>> index 1185ef8998b..94ab0b9cbc2 100644
>>> --- a/gdb/nat/fork-inferior.c
>>> +++ b/gdb/nat/fork-inferior.c
>>> @@ -526,7 +526,10 @@ startup_inferior (process_stratum_target *proc_target, pid_t pid, int ntraps,
>>>
>>>  	  case TARGET_WAITKIND_EXECD:
>>>  	    /* Handle EXEC signals as if they were SIGTRAP signals.  */
>>> -	    xfree (ws.value.execd_pathname);
>>> +	    /* Do not free the last execd pathname as it will be used in
>>> +	       prepare_resume_reply(), after attaching a client side.  */
>>
>> Since this is common code, let's just write it in a gdbserver or gdb agnostic way.
>>
>> I'd suggest something like: Free the exec'ed pathname, but only if this isn't the
>> waitstatus we are returning to the caller.
>>
> 
> Please be more specific how to fix.

Sorry, I just meant "write the comment in a gdbserver of gdb agnostic way".  In particular,
don't mention prepare_resume_reply, which is gdbserver-specific (whereas the code is shared.

I suggest using the phrasing above for the comment.

Simon

[PATCH v2 09/10] Switch local native code to gdb/nat shared functions

[Kamil Rytarowski]

No functional change as the same functionality inlined in nbsd-nat.c
is offered in gdb/nat/netbsd-nat.c.

gdb/ChangeLog:

	* nbsd-nat.c: Include "nat/netbsd-nat.h".
	* (nbsd_nat_target::pid_to_exec_file)
	(nbsd_nat_target::thread_alive, nbsd_nat_target::thread_name)
	(nbsd_nat_target::post_startup_inferior)
	(nbsd_nat_target::post_attach, nbsd_nat_target::xfer_partial)
	(nbsd_add_threads): Switch local code to common gdb/nat functions.
	* (nbsd_pid_to_cmdline): Call sysctl from the global namespace.
	* (nbsd_thread_lister): Remove.
---
 gdb/ChangeLog  |  11 ++++
 gdb/nbsd-nat.c | 147 +++++--------------------------------------------
 2 files changed, 24 insertions(+), 134 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 1013f6a0b3c..2717101ebdd 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,14 @@
+2020-09-04  Kamil Rytarowski  <n54@gmx.com>
+
+	* nbsd-nat.c: Include "nat/netbsd-nat.h".
+	* (nbsd_nat_target::pid_to_exec_file)
+	(nbsd_nat_target::thread_alive, nbsd_nat_target::thread_name)
+	(nbsd_nat_target::post_startup_inferior)
+	(nbsd_nat_target::post_attach, nbsd_nat_target::xfer_partial)
+	(nbsd_add_threads): Switch local code to common gdb/nat functions.
+	* (nbsd_pid_to_cmdline): Call sysctl from the global namespace.
+	* (nbsd_thread_lister): Remove.
+
 2020-09-04  Kamil Rytarowski  <n54@gmx.com>

 	* fork-inferior.c (startup_inferior): Avoid double free.
diff --git a/gdb/nbsd-nat.c b/gdb/nbsd-nat.c
index 52c4d185695..8326fdc6552 100644
--- a/gdb/nbsd-nat.c
+++ b/gdb/nbsd-nat.c
@@ -20,6 +20,7 @@
 #include "defs.h"

 #include "nbsd-nat.h"
+#include "nat/netbsd-nat.h"
 #include "gdbthread.h"
 #include "nbsd-tdep.h"
 #include "inferior.h"
@@ -36,13 +37,7 @@
 char *
 nbsd_nat_target::pid_to_exec_file (int pid)
 {
-  static char buf[PATH_MAX];
-  size_t buflen;
-  int mib[4] = {CTL_KERN, KERN_PROC_ARGS, pid, KERN_PROC_PATHNAME};
-  buflen = sizeof (buf);
-  if (sysctl (mib, ARRAY_SIZE (mib), buf, &buflen, NULL, 0))
-    return NULL;
-  return buf;
+  return netbsd_nat::pid_to_exec_file (pid);
 }

 /* Return the current directory for the process identified by PID.  */
@@ -80,12 +75,12 @@ nbsd_pid_to_cmdline (int pid)
   int mib[4] = {CTL_KERN, KERN_PROC_ARGS, pid, KERN_PROC_ARGV};

   size_t size = 0;
-  if (sysctl (mib, ARRAY_SIZE (mib), NULL, &size, NULL, 0) == -1 || size == 0)
+  if (::sysctl (mib, ARRAY_SIZE (mib), NULL, &size, NULL, 0) == -1 || size == 0)
     return nullptr;

   gdb::unique_xmalloc_ptr<char[]> args (XNEWVAR (char, size));

-  if (sysctl (mib, ARRAY_SIZE (mib), args.get (), &size, NULL, 0) == -1
+  if (::sysctl (mib, ARRAY_SIZE (mib), args.get (), &size, NULL, 0) == -1
       || size == 0)
     return nullptr;

@@ -99,76 +94,12 @@ nbsd_pid_to_cmdline (int pid)
   return args;
 }

-/* Generic thread (LWP) lister within a specified process.  The callback
-   parameters is a C++ function that is called for each detected thread.  */
-
-static bool
-nbsd_thread_lister (const pid_t pid,
-		    gdb::function_view<bool (const struct kinfo_lwp *)>
-		    callback)
-{
-  int mib[5] = {CTL_KERN, KERN_LWP, pid, sizeof (struct kinfo_lwp), 0};
-  size_t size;
-
-  if (sysctl (mib, ARRAY_SIZE (mib), NULL, &size, NULL, 0) == -1 || size == 0)
-    perror_with_name (("sysctl"));
-
-  mib[4] = size / sizeof (size_t);
-
-  gdb::unique_xmalloc_ptr<struct kinfo_lwp[]> kl
-    ((struct kinfo_lwp *) xcalloc (size, 1));
-
-  if (sysctl (mib, ARRAY_SIZE (mib), kl.get (), &size, NULL, 0) == -1
-      || size == 0)
-    perror_with_name (("sysctl"));
-
-  for (size_t i = 0; i < size / sizeof (struct kinfo_lwp); i++)
-    {
-      struct kinfo_lwp *l = &kl[i];
-
-      /* Return true if the specified thread is alive.  */
-      auto lwp_alive
-	= [] (struct kinfo_lwp *lwp)
-	  {
-	    switch (lwp->l_stat)
-	      {
-	      case LSSLEEP:
-	      case LSRUN:
-	      case LSONPROC:
-	      case LSSTOP:
-	      case LSSUSPENDED:
-		return true;
-	      default:
-		return false;
-	      }
-	  };
-
-      /* Ignore embryonic or demised threads.  */
-      if (!lwp_alive (l))
-	continue;
-
-      if (callback (l))
-	return true;
-    }
-
-  return false;
-}
-
 /* Return true if PTID is still active in the inferior.  */

 bool
 nbsd_nat_target::thread_alive (ptid_t ptid)
 {
-  pid_t pid = ptid.pid ();
-  int lwp = ptid.lwp ();
-
-  auto fn
-    = [&lwp] (const struct kinfo_lwp *kl)
-      {
-        return kl->l_lid == lwp;
-      };
-
-  return nbsd_thread_lister (pid, fn);
+  return netbsd_nat::thread_alive (ptid);
 }

 /* Return the name assigned to a thread by an application.  Returns
@@ -178,26 +109,7 @@ const char *
 nbsd_nat_target::thread_name (struct thread_info *thr)
 {
   ptid_t ptid = thr->ptid;
-  pid_t pid = ptid.pid ();
-  int lwp = ptid.lwp ();
-
-  static char buf[KI_LNAMELEN] = {};
-
-  auto fn
-    = [&lwp] (const struct kinfo_lwp *kl)
-      {
-	if (kl->l_lid == lwp)
-	  {
-	    xsnprintf (buf, sizeof buf, "%s", kl->l_name);
-	    return true;
-	  }
-	return false;
-      };
-
-  if (nbsd_thread_lister (pid, fn))
-    return buf;
-  else
-    return NULL;
+  return netbsd_nat::thread_name (ptid);
 }

 /* Implement the "post_attach" target_ops method.  */
@@ -206,9 +118,8 @@ static void
 nbsd_add_threads (nbsd_nat_target *target, pid_t pid)
 {
   auto fn
-    = [&target, &pid] (const struct kinfo_lwp *kl)
+    = [&target] (ptid_t ptid)
       {
-	ptid_t ptid = ptid_t (pid, kl->l_lid, 0);
 	if (!in_thread_list (target, ptid))
 	  {
 	    if (inferior_ptid.lwp () == 0)
@@ -216,27 +127,9 @@ nbsd_add_threads (nbsd_nat_target *target, pid_t pid)
 	    else
 	      add_thread (target, ptid);
 	  }
-	return false;
       };

-  nbsd_thread_lister (pid, fn);
-}
-
-/* Enable additional event reporting on new processes.  */
-
-static void
-nbsd_enable_proc_events (pid_t pid)
-{
-  int events;
-
-  if (ptrace (PT_GET_EVENT_MASK, pid, &events, sizeof (events)) == -1)
-    perror_with_name (("ptrace"));
-
-  events |= PTRACE_LWP_CREATE;
-  events |= PTRACE_LWP_EXIT;
-
-  if (ptrace (PT_SET_EVENT_MASK, pid, &events, sizeof (events)) == -1)
-    perror_with_name (("ptrace"));
+  netbsd_nat::for_each_thread (pid, fn);
 }

 /* Implement the "post_startup_inferior" target_ops method.  */
@@ -244,7 +137,7 @@ nbsd_enable_proc_events (pid_t pid)
 void
 nbsd_nat_target::post_startup_inferior (ptid_t ptid)
 {
-  nbsd_enable_proc_events (ptid.pid ());
+  netbsd_nat::enable_proc_events (ptid.pid ());
 }

 /* Implement the "post_attach" target_ops method.  */
@@ -252,7 +145,7 @@ nbsd_nat_target::post_startup_inferior (ptid_t ptid)
 void
 nbsd_nat_target::post_attach (int pid)
 {
-  nbsd_enable_proc_events (pid);
+  netbsd_nat::enable_proc_events (pid);
   nbsd_add_threads (this, pid);
 }

@@ -861,26 +754,12 @@ nbsd_nat_target::xfer_partial (enum target_object object,
     {
     case TARGET_OBJECT_SIGNAL_INFO:
       {
-	ptrace_siginfo_t psi;
+	len = netbsd_nat::qxfer_siginfo(pid, annex, readbuf, writebuf, offset,
+					len);

-	if (offset > sizeof (siginfo_t))
+	if (len == -1)
 	  return TARGET_XFER_E_IO;

-	if (ptrace (PT_GET_SIGINFO, pid, &psi, sizeof (psi)) == -1)
-	  return TARGET_XFER_E_IO;
-
-	if (offset + len > sizeof (siginfo_t))
-	  len = sizeof (siginfo_t) - offset;
-
-	if (readbuf != NULL)
-	  memcpy (readbuf, ((gdb_byte *) &psi.psi_siginfo) + offset, len);
-	else
-	  {
-	    memcpy (((gdb_byte *) &psi.psi_siginfo) + offset, writebuf, len);
-
-	    if (ptrace (PT_SET_SIGINFO, pid, &psi, sizeof (psi)) == -1)
-	      return TARGET_XFER_E_IO;
-	  }
 	*xfered_len = len;
 	return TARGET_XFER_OK;
       }
--
2.28.0

Re: [PATCH v2 09/10] Switch local native code to gdb/nat shared functions

[Simon Marchi]

On 2020-09-03 8:29 p.m., Kamil Rytarowski wrote:
> No functional change as the same functionality inlined in nbsd-nat.c
> is offered in gdb/nat/netbsd-nat.c.
> 
> gdb/ChangeLog:
> 
> 	* nbsd-nat.c: Include "nat/netbsd-nat.h".
> 	* (nbsd_nat_target::pid_to_exec_file)
> 	(nbsd_nat_target::thread_alive, nbsd_nat_target::thread_name)
> 	(nbsd_nat_target::post_startup_inferior)
> 	(nbsd_nat_target::post_attach, nbsd_nat_target::xfer_partial)
> 	(nbsd_add_threads): Switch local code to common gdb/nat functions.
> 	* (nbsd_pid_to_cmdline): Call sysctl from the global namespace.
> 	* (nbsd_thread_lister): Remove.

Just a though for next time: I think you could have done the changes here
progressively.  For example, move netbsd_thread_lister to nat, adjust the
code here at the same time.  Move the thread_alive function, adjust the
code here, etc.  It would have been easier to see that it was mostly just
functionality moved from one place to the other, up to this patch I thought
it was all new code.

Simon

Re: [PATCH v2 09/10] Switch local native code to gdb/nat shared functions

[Kamil Rytarowski]

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

On 07.09.2020 21:24, Simon Marchi wrote:
> On 2020-09-03 8:29 p.m., Kamil Rytarowski wrote:
>> No functional change as the same functionality inlined in nbsd-nat.c
>> is offered in gdb/nat/netbsd-nat.c.
>>
>> gdb/ChangeLog:
>>
>> 	* nbsd-nat.c: Include "nat/netbsd-nat.h".
>> 	* (nbsd_nat_target::pid_to_exec_file)
>> 	(nbsd_nat_target::thread_alive, nbsd_nat_target::thread_name)
>> 	(nbsd_nat_target::post_startup_inferior)
>> 	(nbsd_nat_target::post_attach, nbsd_nat_target::xfer_partial)
>> 	(nbsd_add_threads): Switch local code to common gdb/nat functions.
>> 	* (nbsd_pid_to_cmdline): Call sysctl from the global namespace.
>> 	* (nbsd_thread_lister): Remove.
> 
> Just a though for next time: I think you could have done the changes here
> progressively.  For example, move netbsd_thread_lister to nat, adjust the
> code here at the same time.  Move the thread_alive function, adjust the
> code here, etc.  It would have been easier to see that it was mostly just
> functionality moved from one place to the other, up to this patch I thought
> it was all new code.
> 

OK.

> Simon
> 



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

[PATCH v2 10/10] Add minimal and functional NetBSD/amd64 gdbserver

[Kamil Rytarowski]

Implement the following functionality: create_inferior,
post_create_inferior, attach, kill, detach, mourn, join, thread_alive,
resume, wait, fetch_registers, store_registers, read_memory, write_memory,
request_interrupt, supports_read_auxv, read_auxv,
supports_hardware_single_step, sw_breakpoint_from_kind,
supports_z_point_type, insert_point, remove_point,
stopped_by_sw_breakpoint, supports_qxfer_siginfo, qxfer_siginfo,
supports_stopped_by_sw_breakpoint, supports_non_stop,
supports_multi_process, supports_fork_events, supports_vfork_events,
supports_exec_events, supports_disable_randomization,
supports_qxfer_libraries_svr4, qxfer_libraries_svr4,
supports_pid_to_exec_file, pid_to_exec_file, thread_name,
supports_catch_syscall.

The only CPU architecture supported: x86_64.

Implement only support for hardware assisted single step and
software breakpoint.

Implement support only for regular X86 registers, thus no FPU.

gdbserver/ChangeLog:

       * netbsd-low.cc: Add.
       * netbsd-low.h: Likewise.
       * netbsd-x86_64-low.cc: Likewise.
       * Makefile.in (SFILES): Register "netbsd-low.cc", "netbsd-low.h",
       "netbsd-x86_64-low.cc".
       * configure.srv: Add x86_64-*-netbsd*.
---
 gdbserver/ChangeLog            |    9 +
 gdbserver/Makefile.in          |    3 +
 gdbserver/configure.srv        |    6 +
 gdbserver/netbsd-low.cc        | 1348 ++++++++++++++++++++++++++++++++
 gdbserver/netbsd-low.h         |  154 ++++
 gdbserver/netbsd-x86_64-low.cc |  187 +++++
 6 files changed, 1707 insertions(+)
 create mode 100644 gdbserver/netbsd-low.cc
 create mode 100644 gdbserver/netbsd-low.h
 create mode 100644 gdbserver/netbsd-x86_64-low.cc

diff --git a/gdbserver/ChangeLog b/gdbserver/ChangeLog
index e437493b56d..022d0bbfee6 100644
--- a/gdbserver/ChangeLog
+++ b/gdbserver/ChangeLog
@@ -1,3 +1,12 @@
+2020-09-04  Kamil Rytarowski  <n54@gmx.com>
+
+	* netbsd-low.cc: Add.
+	* netbsd-low.h: Likewise.
+	* netbsd-x86_64-low.cc: Likewise.
+	* Makefile.in (SFILES): Register "netbsd-low.cc", "netbsd-low.h",
+	"netbsd-x86_64-low.cc".
+	* configure.srv: Add x86_64-*-netbsd*.
+
 2020-08-13  Simon Marchi  <simon.marchi@polymtl.ca>

 	* server.cc (captured_main): Accept multiple `--selftest=`
diff --git a/gdbserver/Makefile.in b/gdbserver/Makefile.in
index 9d7687be534..1d597b14523 100644
--- a/gdbserver/Makefile.in
+++ b/gdbserver/Makefile.in
@@ -193,6 +193,9 @@ SFILES = \
 	$(srcdir)/linux-x86-low.cc \
 	$(srcdir)/linux-xtensa-low.cc \
 	$(srcdir)/mem-break.cc \
+	$(srcdir)/netbsd-low.cc \
+	$(srcdir)/netbsd-low.h \
+	$(srcdir)/netbsd-x86_64-low.cc \
 	$(srcdir)/proc-service.cc \
 	$(srcdir)/proc-service.list \
 	$(srcdir)/regcache.cc \
diff --git a/gdbserver/configure.srv b/gdbserver/configure.srv
index 5e33bd9c54d..5ac4b8f809e 100644
--- a/gdbserver/configure.srv
+++ b/gdbserver/configure.srv
@@ -349,6 +349,12 @@ case "${gdbserver_host}" in
 			srv_tgtobj="${srv_tgtobj} nat/windows-nat.o"
 			srv_tgtobj="${srv_tgtobj} arch/amd64.o arch/i386.o"
 			;;
+  x86_64-*-netbsd*)	srv_regobj=""
+			srv_tgtobj="netbsd-low.o netbsd-x86_64-low.o fork-child.o"
+			srv_tgtobj="${srv_tgtobj} nat/fork-inferior.o"
+			srv_tgtobj="${srv_tgtobj} nat/netbsd-nat.o"
+			srv_tgtobj="${srv_tgtobj} arch/amd64.o"
+			;;

   xtensa*-*-linux*)	srv_regobj=reg-xtensa.o
 			srv_tgtobj="$srv_linux_obj linux-xtensa-low.o"
diff --git a/gdbserver/netbsd-low.cc b/gdbserver/netbsd-low.cc
new file mode 100644
index 00000000000..27006615e7b
--- /dev/null
+++ b/gdbserver/netbsd-low.cc
@@ -0,0 +1,1348 @@
+/* Copyright (C) 2020 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include "server.h"
+#include "target.h"
+#include "netbsd-low.h"
+#include "nat/netbsd-nat.h"
+
+#include <sys/param.h>
+#include <sys/types.h>
+
+#include <sys/ptrace.h>
+#include <sys/sysctl.h>
+
+#include <limits.h>
+#include <unistd.h>
+#include <signal.h>
+
+#include <elf.h>
+
+#include <type_traits>
+
+#include "gdbsupport/eintr.h"
+#include "gdbsupport/gdb_wait.h"
+#include "gdbsupport/filestuff.h"
+#include "gdbsupport/common-inferior.h"
+#include "nat/fork-inferior.h"
+#include "hostio.h"
+
+int using_threads = 1;
+
+const struct target_desc *netbsd_tdesc;
+
+/* Call add_process with the given parameters, and initialize
+   the process' private data.  */
+
+static void
+netbsd_add_process (int pid, int attached)
+{
+  struct process_info *proc = add_process (pid, attached);
+  proc->tdesc = netbsd_tdesc;
+  proc->priv = nullptr;
+}
+
+/* Callback used by fork_inferior to start tracing the inferior.  */
+
+static void
+netbsd_ptrace_fun ()
+{
+  /* Switch child to its own process group so that signals won't
+     directly affect GDBserver. */
+  if (setpgid (0, 0) < 0)
+    trace_start_error_with_name (("setpgid"));
+
+  if (ptrace (PT_TRACE_ME, 0, nullptr, 0) < 0)
+    trace_start_error_with_name (("ptrace"));
+
+  /* If GDBserver is connected to gdb via stdio, redirect the inferior's
+     stdout to stderr so that inferior i/o doesn't corrupt the connection.
+     Also, redirect stdin to /dev/null.  */
+  if (remote_connection_is_stdio ())
+    {
+      if (close (0) < 0)
+	trace_start_error_with_name (("close"));
+      if (open ("/dev/null", O_RDONLY) < 0)
+	trace_start_error_with_name (("open"));
+      if (dup2 (2, 1) < 0)
+	trace_start_error_with_name (("dup2"));
+      if (write (2, "stdin/stdout redirected\n",
+		 sizeof ("stdin/stdout redirected\n") - 1) < 0)
+	{
+	  /* Errors ignored.  */
+	}
+    }
+}
+
+/* Register threads in the process specified by PID.  */
+
+static void
+netbsd_add_threads (pid_t pid)
+{
+  auto fn
+    = [] (ptid_t ptid)
+      {
+	add_thread (ptid, nullptr);
+      };
+
+  netbsd_nat::for_each_thread (pid, fn);
+}
+
+/* Implement the create_inferior method of the target_ops vector.  */
+
+int
+netbsd_process_target::create_inferior (const char *program,
+					const std::vector<char *> &program_args)
+{
+  std::string str_program_args = construct_inferior_arguments (program_args);
+
+  pid_t pid = fork_inferior (program, str_program_args.c_str (),
+			     get_environ ()->envp (), netbsd_ptrace_fun,
+			     nullptr, nullptr, nullptr, nullptr);
+
+  netbsd_add_process (pid, 0);
+
+  post_fork_inferior (pid, program);
+
+  return pid;
+}
+
+/* Implement the post_create_inferior target_ops method.  */
+
+void
+netbsd_process_target::post_create_inferior ()
+{
+  pid_t pid = current_process ()->pid;
+  netbsd_nat::enable_proc_events (pid);
+}
+
+/* Implement the attach target_ops method.  */
+
+int
+netbsd_process_target::attach (unsigned long pid)
+{
+
+  if (ptrace (PT_ATTACH, pid, nullptr, 0) != 0)
+    error ("Cannot attach to process %lu: %s (%d)\n", pid,
+	    safe_strerror (errno), errno);
+
+  netbsd_add_process (pid, 1);
+  netbsd_nat::enable_proc_events (pid);
+  netbsd_add_threads (pid);
+
+  return 0;
+}
+
+/* Returns true if GDB is interested in any child syscalls.  */
+
+static bool
+gdb_catching_syscalls_p (pid_t pid)
+{
+  struct process_info *proc = find_process_pid (pid);
+  return !proc->syscalls_to_catch.empty ();
+}
+
+/* Implement the resume target_ops method.  */
+
+void
+netbsd_process_target::resume (struct thread_resume *resume_info, size_t n)
+{
+  ptid_t resume_ptid = resume_info[0].thread;
+  const int signal = resume_info[0].sig;
+  const bool step = resume_info[0].kind == resume_step;
+
+  if (resume_ptid == minus_one_ptid)
+    resume_ptid = ptid_of (current_thread);
+
+  const pid_t pid = resume_ptid.pid ();
+  const lwpid_t lwp = resume_ptid.lwp ();
+  regcache_invalidate_pid (pid);
+
+  auto fn
+    = [&] (ptid_t ptid)
+      {
+	if (step)
+	  {
+	    if (ptid.lwp () == lwp || n != 1)
+	      {
+		if (ptrace (PT_SETSTEP, pid, NULL, ptid.lwp ()) == -1)
+		  perror_with_name (("ptrace"));
+		if (ptrace (PT_RESUME, pid, NULL, ptid.lwp ()) == -1)
+		  perror_with_name (("ptrace"));
+	      }
+	    else
+	      {
+		if (ptrace (PT_CLEARSTEP, pid, NULL, ptid.lwp ()) == -1)
+		  perror_with_name (("ptrace"));
+		if (ptrace (PT_SUSPEND, pid, NULL, ptid.lwp ()) == -1)
+		  perror_with_name (("ptrace"));
+	      }
+	  }
+	else
+	  {
+	    if (ptrace (PT_CLEARSTEP, pid, NULL, ptid.lwp ()) == -1)
+	      perror_with_name (("ptrace"));
+	    if (ptrace (PT_RESUME, pid, NULL, ptid.lwp ()) == -1)
+	      perror_with_name (("ptrace"));
+	  }
+      };
+
+  netbsd_nat::for_each_thread (pid, fn);
+
+  int request = gdb_catching_syscalls_p (pid) ? PT_CONTINUE : PT_SYSCALL;
+
+  errno = 0;
+  ptrace (request, pid, (void *)1, signal);
+  if (errno)
+    perror_with_name (("ptrace"));
+}
+
+/* Returns true if GDB is interested in the reported SYSNO syscall.  */
+
+static bool
+netbsd_catch_this_syscall (int sysno)
+{
+  struct process_info *proc = current_process ();
+
+  if (proc->syscalls_to_catch.empty ())
+    return false;
+
+  if (proc->syscalls_to_catch[0] == ANY_SYSCALL)
+    return true;
+
+  for (int iter : proc->syscalls_to_catch)
+    if (iter == sysno)
+      return true;
+
+  return false;
+}
+
+/* Helper function for child_wait and the derivatives of child_wait.
+   HOSTSTATUS is the waitstatus from wait() or the equivalent; store our
+   translation of that in OURSTATUS.  */
+
+static void
+netbsd_store_waitstatus (struct target_waitstatus *ourstatus, int hoststatus)
+{
+  if (WIFEXITED (hoststatus))
+    {
+      ourstatus->kind = TARGET_WAITKIND_EXITED;
+      ourstatus->value.integer = WEXITSTATUS (hoststatus);
+    }
+  else if (!WIFSTOPPED (hoststatus))
+    {
+      ourstatus->kind = TARGET_WAITKIND_SIGNALLED;
+      ourstatus->value.sig = gdb_signal_from_host (WTERMSIG (hoststatus));
+    }
+  else
+    {
+      ourstatus->kind = TARGET_WAITKIND_STOPPED;
+      ourstatus->value.sig = gdb_signal_from_host (WSTOPSIG (hoststatus));
+    }
+}
+
+/* Implement a safe wrapper around waitpid().  */
+
+static pid_t
+netbsd_waitpid (ptid_t ptid, struct target_waitstatus *ourstatus, int options)
+{
+  int status;
+
+  pid_t pid
+    = gdb::handle_eintr<int> (-1, ::waitpid, ptid.pid (), &status, options);
+
+  if (pid == -1)
+    perror_with_name (_("Child process unexpectedly missing"));
+
+  netbsd_store_waitstatus (ourstatus, status);
+  return pid;
+}
+
+
+/* Implement the wait target_ops method.
+
+   Wait for the child specified by PTID to do something.  Return the
+   process ID of the child, or MINUS_ONE_PTID in case of error; store
+   the status in *OURSTATUS.  */
+
+static ptid_t
+netbsd_wait (ptid_t ptid, struct target_waitstatus *ourstatus,
+	     int target_options)
+{
+  pid_t pid = netbsd_waitpid (ptid, ourstatus, target_options);
+  ptid_t wptid = ptid_t (pid);
+
+  if (pid == 0)
+    {
+      gdb_assert (target_options & TARGET_WNOHANG);
+      ourstatus->kind = TARGET_WAITKIND_IGNORE;
+      return null_ptid;
+    }
+
+  gdb_assert (pid != -1);
+
+  /* If the child stopped, keep investigating its status.  */
+  if (ourstatus->kind != TARGET_WAITKIND_STOPPED)
+    return wptid;
+
+  /* Extract the event and thread that received a signal.  */
+  ptrace_siginfo_t psi;
+  if (ptrace (PT_GET_SIGINFO, pid, &psi, sizeof (psi)) == -1)
+    perror_with_name (("ptrace"));
+
+  /* Pick child's siginfo_t.  */
+  siginfo_t *si = &psi.psi_siginfo;
+
+  lwpid_t lwp = psi.psi_lwpid;
+
+  int signo = si->si_signo;
+  const int code = si->si_code;
+
+  /* Construct PTID with a specified thread that received the event.
+     If a signal was targeted to the whole process, lwp is 0.  */
+  wptid = ptid_t (pid, lwp, 0);
+
+  /* Bail out on non-debugger oriented signals.  */
+  if (signo != SIGTRAP)
+    return wptid;
+
+  /* Stop examining non-debugger oriented SIGTRAP codes.  */
+  if (code <= SI_USER || code == SI_NOINFO)
+    return wptid;
+
+  /* Process state for threading events.  */
+  ptrace_state_t pst = {};
+  if (code == TRAP_LWP)
+    if (ptrace (PT_GET_PROCESS_STATE, pid, &pst, sizeof (pst)) == -1)
+      perror_with_name (("ptrace"));
+
+  if (code == TRAP_LWP && pst.pe_report_event == PTRACE_LWP_EXIT)
+    {
+      /* If GDB attaches to a multi-threaded process, exiting
+	 threads might be skipped during post_attach that
+	 have not yet reported their PTRACE_LWP_EXIT event.
+	 Ignore exited events for an unknown LWP.  */
+      thread_info *thr = find_thread_ptid (wptid);
+      if (thr == nullptr)
+	  ourstatus->kind = TARGET_WAITKIND_SPURIOUS;
+      else
+	{
+	  ourstatus->kind = TARGET_WAITKIND_THREAD_EXITED;
+	  /* NetBSD does not store an LWP exit status.  */
+	  ourstatus->value.integer = 0;
+
+	  remove_thread (thr);
+	}
+      return wptid;
+    }
+
+  if (find_thread_ptid (ptid_t (pid)))
+    current_thread = find_thread_ptid (wptid);
+
+  if (code == TRAP_LWP && pst.pe_report_event == PTRACE_LWP_CREATE)
+    {
+      /* If GDB attaches to a multi-threaded process, newborn
+	 threads might be added by nbsd_add_threads that have
+	 not yet reported their PTRACE_LWP_CREATE event.  Ignore
+	 born events for an already-known LWP.  */
+      if (find_thread_ptid (wptid))
+	  ourstatus->kind = TARGET_WAITKIND_SPURIOUS;
+      else
+	{
+	  add_thread (wptid, NULL);
+	  ourstatus->kind = TARGET_WAITKIND_THREAD_CREATED;
+	}
+      return wptid;
+    }
+
+  if (code == TRAP_EXEC)
+    {
+      ourstatus->kind = TARGET_WAITKIND_EXECD;
+      ourstatus->value.execd_pathname
+	= xstrdup (netbsd_nat::pid_to_exec_file (pid));
+      return wptid;
+    }
+
+  if (code == TRAP_TRACE)
+      return wptid;
+
+  if (code == TRAP_SCE || code == TRAP_SCX)
+    {
+      int sysnum = si->si_sysnum;
+
+      if (!netbsd_catch_this_syscall(sysnum))
+	{
+	  /* If the core isn't interested in this event, ignore it.  */
+	  ourstatus->kind = TARGET_WAITKIND_SPURIOUS;
+	  return wptid;
+	}
+
+      ourstatus->kind
+	= ((code == TRAP_SCE) ? TARGET_WAITKIND_SYSCALL_ENTRY :
+	   TARGET_WAITKIND_SYSCALL_RETURN);
+      ourstatus->value.syscall_number = sysnum;
+      return wptid;
+    }
+
+  if (code == TRAP_BRKPT)
+    {
+#ifdef PTRACE_BREAKPOINT_ADJ
+      CORE_ADDR pc;
+      struct reg r;
+      ptrace (PT_GETREGS, pid, &r, psi.psi_lwpid);
+      pc = PTRACE_REG_PC (&r);
+      PTRACE_REG_SET_PC (&r, pc - PTRACE_BREAKPOINT_ADJ);
+      ptrace (PT_SETREGS, pid, &r, psi.psi_lwpid);
+#endif
+      return wptid;
+    }
+
+  /* Unclassified SIGTRAP event.  */
+  ourstatus->kind = TARGET_WAITKIND_SPURIOUS;
+  return wptid;
+}
+
+/* Implement the wait target_ops method.  */
+
+ptid_t
+netbsd_process_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
+			     int target_options)
+{
+  while (true)
+    {
+      ptid_t wptid = netbsd_wait (ptid, ourstatus, target_options);
+
+      /* Register thread in the gdbcore if a thread was not reported earlier.
+	 This is required after ::create_inferior, when the gdbcore does not
+	 know about the first internal thread.
+	 This may also happen on attach, when an event is registered on a thread
+	 that was not fully initialized during the attach stage.  */
+      if (wptid.lwp () != 0 && !find_thread_ptid (wptid)
+	  && ourstatus->kind != TARGET_WAITKIND_THREAD_EXITED)
+	add_thread (wptid, nullptr);
+
+      switch (ourstatus->kind)
+	{
+	case TARGET_WAITKIND_EXITED:
+	case TARGET_WAITKIND_STOPPED:
+	case TARGET_WAITKIND_SIGNALLED:
+	case TARGET_WAITKIND_FORKED:
+	case TARGET_WAITKIND_VFORKED:
+	case TARGET_WAITKIND_EXECD:
+	case TARGET_WAITKIND_VFORK_DONE:
+	case TARGET_WAITKIND_SYSCALL_ENTRY:
+	case TARGET_WAITKIND_SYSCALL_RETURN:
+	  /* Pass the result to the generic code.  */
+	  return wptid;
+	case TARGET_WAITKIND_THREAD_CREATED:
+	case TARGET_WAITKIND_THREAD_EXITED:
+	  /* The core needlessly stops on these events.  */
+	  /* FALLTHROUGH */
+	case TARGET_WAITKIND_SPURIOUS:
+	  /* Spurious events are unhandled by the gdbserver core.  */
+	  if (ptrace (PT_CONTINUE, current_process ()->pid, (void *) 1, 0)
+	      == -1)
+	    perror_with_name (("ptrace"));
+	  break;
+	default:
+	  error (("Unknown stopped status"));
+	}
+    }
+}
+
+/* Implement the kill target_ops method.  */
+
+int
+netbsd_process_target::kill (process_info *process)
+{
+  pid_t pid = process->pid;
+  if (ptrace (PT_KILL, pid, nullptr, 0) == -1)
+    return -1;
+
+  int status;
+  if (gdb::handle_eintr<int> (-1, ::waitpid, pid, &status, 0) == -1)
+    return -1;
+  mourn (process);
+  return 0;
+}
+
+/* Implement the detach target_ops method.  */
+
+int
+netbsd_process_target::detach (process_info *process)
+{
+  pid_t pid = process->pid;
+
+  ptrace (PT_DETACH, pid, (void *) 1, 0);
+  mourn (process);
+  return 0;
+}
+
+/* Implement the mourn target_ops method.  */
+
+void
+netbsd_process_target::mourn (struct process_info *proc)
+{
+  for_each_thread (proc->pid, remove_thread);
+
+  remove_process (proc);
+}
+
+/* Implement the join target_ops method.  */
+
+void
+netbsd_process_target::join (int pid)
+{
+  /* The PT_DETACH is sufficient to detach from the process.
+     So no need to do anything extra.  */
+}
+
+/* Implement the thread_alive target_ops method.  */
+
+bool
+netbsd_process_target::thread_alive (ptid_t ptid)
+{
+  return netbsd_nat::thread_alive (ptid);
+}
+
+/* Implement the fetch_registers target_ops method.  */
+
+void
+netbsd_process_target::fetch_registers (struct regcache *regcache, int regno)
+{
+  struct netbsd_regset_info *regset = netbsd_target_regsets;
+  ptid_t inferior_ptid = ptid_of (current_thread);
+
+  while (regset->size >= 0)
+    {
+      char *buf = (char *) xmalloc (regset->size);
+      int res = ptrace (regset->get_request, inferior_ptid.pid (), buf,
+			inferior_ptid.lwp ());
+      if (res == -1)
+	perror_with_name (("ptrace"));
+      regset->store_function (regcache, buf);
+      free (buf);
+      regset++;
+    }
+}
+
+/* Implement the store_registers target_ops method.  */
+
+void
+netbsd_process_target::store_registers (struct regcache *regcache, int regno)
+{
+  struct netbsd_regset_info *regset = netbsd_target_regsets;
+  ptid_t inferior_ptid = ptid_of (current_thread);
+
+  while (regset->size >= 0)
+    {
+      char *buf = (char *)xmalloc (regset->size);
+      int res = ptrace (regset->get_request, inferior_ptid.pid (), buf,
+			inferior_ptid.lwp ());
+      if (res == -1)
+	perror_with_name (("ptrace"));
+      if (res == 0)
+	{
+	  /* Then overlay our cached registers on that.  */
+	  regset->fill_function (regcache, buf);
+	  /* Only now do we write the register set.  */
+	  res = ptrace (regset->set_request, inferior_ptid.pid (), buf,
+			inferior_ptid.lwp ());
+	}
+      free (buf);
+      regset++;
+    }
+}
+
+/* Implement the read_memory target_ops method.  */
+
+int
+netbsd_process_target::read_memory (CORE_ADDR memaddr, unsigned char *myaddr,
+				    int size)
+{
+  struct ptrace_io_desc io;
+  io.piod_op = PIOD_READ_D;
+  io.piod_len = size;
+
+  pid_t pid = current_process ()->pid;
+
+  int bytes_read = 0;
+
+  if (size == 0)
+    {
+      /* Zero length write always succeeds.  */
+      return 0;
+    }
+  do
+    {
+      io.piod_offs = (void *)(memaddr + bytes_read);
+      io.piod_addr = myaddr + bytes_read;
+
+      int rv = ptrace (PT_IO, pid, &io, 0);
+      if (rv == -1)
+	return errno;
+      if (io.piod_len == 0)
+	return 0;
+
+      bytes_read += io.piod_len;
+      io.piod_len = size - bytes_read;
+    }
+  while (bytes_read < size);
+
+  return 0;
+}
+
+/* Implement the write_memory target_ops method.  */
+
+int
+netbsd_process_target::write_memory (CORE_ADDR memaddr,
+				     const unsigned char *myaddr, int size)
+{
+  struct ptrace_io_desc io;
+  io.piod_op = PIOD_WRITE_D;
+  io.piod_len = size;
+
+  pid_t pid = current_process ()->pid;
+
+  int bytes_written = 0;
+
+  if (size == 0)
+    {
+      /* Zero length write always succeeds.  */
+      return 0;
+    }
+
+  do
+    {
+      io.piod_addr = (void *)(myaddr + bytes_written);
+      io.piod_offs = (void *)(memaddr + bytes_written);
+
+      int rv = ptrace (PT_IO, pid, &io, 0);
+      if (rv == -1)
+	return errno;
+      if (io.piod_len == 0)
+	return 0;
+
+      bytes_written += io.piod_len;
+      io.piod_len = size - bytes_written;
+    }
+  while (bytes_written < size);
+
+  return 0;
+}
+
+/* Implement the request_interrupt target_ops method.  */
+
+void
+netbsd_process_target::request_interrupt ()
+{
+  ptid_t inferior_ptid = ptid_of (get_first_thread ());
+
+  ::kill (inferior_ptid.pid(), SIGINT);
+}
+
+/* Read the AUX Vector for the specified PID, wrapping the ptrace(2) call
+   with the PIOD_READ_AUXV operation and using the PT_IO standard input
+   and output arguments.  */
+
+static size_t
+netbsd_read_auxv(pid_t pid, void *offs, void *addr, size_t len)
+{
+  struct ptrace_io_desc pio;
+
+  pio.piod_op = PIOD_READ_AUXV;
+  pio.piod_offs = offs;
+  pio.piod_addr = addr;
+  pio.piod_len = len;
+
+  if (ptrace (PT_IO, pid, &pio, 0) == -1)
+    perror_with_name (("ptrace"));
+
+  return pio.piod_len;
+}
+
+/* Copy LEN bytes from inferior's auxiliary vector starting at OFFSET
+   to debugger memory starting at MYADDR.  */
+
+int
+netbsd_process_target::read_auxv (CORE_ADDR offset,
+				  unsigned char *myaddr, unsigned int len)
+{
+  pid_t pid = pid_of (current_thread);
+
+  return netbsd_read_auxv (pid, (void *) (intptr_t) offset, myaddr, len);
+}
+
+bool
+netbsd_process_target::supports_z_point_type (char z_type)
+{
+  switch (z_type)
+    {
+    case Z_PACKET_SW_BP:
+      return true;
+    case Z_PACKET_HW_BP:
+    case Z_PACKET_WRITE_WP:
+    case Z_PACKET_READ_WP:
+    case Z_PACKET_ACCESS_WP:
+    default:
+      return false; /* Not supported.  */
+    }
+}
+
+/* Insert {break/watch}point at address ADDR.  SIZE is not used.  */
+
+int
+netbsd_process_target::insert_point (enum raw_bkpt_type type, CORE_ADDR addr,
+		     int size, struct raw_breakpoint *bp)
+{
+  switch (type)
+    {
+    case raw_bkpt_type_sw:
+      return insert_memory_breakpoint (bp);
+    case raw_bkpt_type_hw:
+    case raw_bkpt_type_write_wp:
+    case raw_bkpt_type_read_wp:
+    case raw_bkpt_type_access_wp:
+    default:
+      return 1; /* Not supported.  */
+    }
+}
+
+/* Remove {break/watch}point at address ADDR.  SIZE is not used.  */
+
+int
+netbsd_process_target::remove_point (enum raw_bkpt_type type, CORE_ADDR addr,
+				     int size, struct raw_breakpoint *bp)
+{
+  switch (type)
+    {
+    case raw_bkpt_type_sw:
+      return remove_memory_breakpoint (bp);
+    case raw_bkpt_type_hw:
+    case raw_bkpt_type_write_wp:
+    case raw_bkpt_type_read_wp:
+    case raw_bkpt_type_access_wp:
+    default:
+      return 1; /* Not supported.  */
+    }
+}
+
+/* Implement the stopped_by_sw_breakpoint target_ops method.  */
+
+bool
+netbsd_process_target::stopped_by_sw_breakpoint ()
+{
+  ptrace_siginfo_t psi;
+  pid_t pid = current_process ()->pid;
+
+  if (ptrace (PT_GET_SIGINFO, pid, &psi, sizeof (psi)) == -1)
+    perror_with_name (("ptrace"));
+
+  return psi.psi_siginfo.si_signo == SIGTRAP &&
+	 psi.psi_siginfo.si_code == TRAP_BRKPT;
+}
+
+/* Implement the supports_stopped_by_sw_breakpoint target_ops method.  */
+
+bool
+netbsd_process_target::supports_stopped_by_sw_breakpoint ()
+{
+  return true;
+}
+
+/* Implement the supports_qxfer_siginfo target_ops method.  */
+
+bool
+netbsd_process_target::supports_qxfer_siginfo ()
+{
+  return true;
+}
+
+/* Implement the qxfer_siginfo target_ops method.  */
+
+int
+netbsd_process_target::qxfer_siginfo (const char *annex, unsigned char *readbuf,
+				      unsigned const char *writebuf,
+				      CORE_ADDR offset, int len)
+{
+  if (current_thread == nullptr)
+    return -1;
+
+  pid_t pid = current_process ()->pid;
+
+  return netbsd_nat::qxfer_siginfo(pid, annex, readbuf, writebuf, offset, len);
+}
+
+/* Implement the supports_non_stop target_ops method.  */
+
+bool
+netbsd_process_target::supports_non_stop ()
+{
+  return false;
+}
+
+/* Implement the supports_multi_process target_ops method.  */
+
+bool
+netbsd_process_target::supports_multi_process ()
+{
+  return true;
+}
+
+/* Check if fork events are supported.  */
+
+bool
+netbsd_process_target::supports_fork_events ()
+{
+  return false;
+}
+
+/* Check if vfork events are supported.  */
+
+bool
+netbsd_process_target::supports_vfork_events ()
+{
+  return false;
+}
+
+/* Check if exec events are supported.  */
+
+bool
+netbsd_process_target::supports_exec_events ()
+{
+  return true;
+}
+
+/* Implement the supports_disable_randomization target_ops method.  */
+
+bool
+netbsd_process_target::supports_disable_randomization ()
+{
+  return false;
+}
+
+/* Extract &phdr and num_phdr in the inferior.  Return 0 on success.  */
+
+template <typename T>
+int get_phdr_phnum_from_proc_auxv (const pid_t pid,
+				   CORE_ADDR *phdr_memaddr, int *num_phdr)
+{
+  typedef typename std::conditional<sizeof(T) == sizeof(int64_t),
+				    Aux64Info, Aux32Info>::type auxv_type;
+  const size_t auxv_size = sizeof (auxv_type);
+  const size_t auxv_buf_size = 128 * sizeof (auxv_type);
+
+  char *auxv_buf = (char *) xmalloc (auxv_buf_size);
+
+  netbsd_read_auxv (pid, nullptr, auxv_buf, auxv_buf_size);
+
+  *phdr_memaddr = 0;
+  *num_phdr = 0;
+
+  for (char *buf = auxv_buf; buf < (auxv_buf + auxv_buf_size); buf += auxv_size)
+    {
+      auxv_type *const aux = (auxv_type *) buf;
+
+      switch (aux->a_type)
+	{
+	case AT_PHDR:
+	  *phdr_memaddr = aux->a_v;
+	  break;
+	case AT_PHNUM:
+	  *num_phdr = aux->a_v;
+	  break;
+	}
+
+      if (*phdr_memaddr != 0 && *num_phdr != 0)
+	break;
+    }
+
+  xfree (auxv_buf);
+
+  if (*phdr_memaddr == 0 || *num_phdr == 0)
+    {
+      warning ("Unexpected missing AT_PHDR and/or AT_PHNUM: "
+	       "phdr_memaddr = %s, phdr_num = %d",
+	       core_addr_to_string (*phdr_memaddr), *num_phdr);
+      return 2;
+    }
+
+  return 0;
+}
+
+/* Return &_DYNAMIC (via PT_DYNAMIC) in the inferior, or 0 if not present.  */
+
+template <typename T>
+static CORE_ADDR
+get_dynamic (netbsd_process_target *target, const pid_t pid)
+{
+  typedef typename std::conditional<sizeof(T) == sizeof(int64_t),
+				    Elf64_Phdr, Elf32_Phdr>::type phdr_type;
+  const int phdr_size = sizeof (phdr_type);
+
+  CORE_ADDR phdr_memaddr;
+  int num_phdr;
+  if (get_phdr_phnum_from_proc_auxv<T> (pid, &phdr_memaddr, &num_phdr))
+    return 0;
+
+  std::vector<unsigned char> phdr_buf;
+  phdr_buf.resize (num_phdr * phdr_size);
+
+  if (target->read_memory (phdr_memaddr, phdr_buf.data (), phdr_buf.size ()))
+    return 0;
+
+  /* Compute relocation: it is expected to be 0 for "regular" executables,
+     non-zero for PIE ones.  */
+  CORE_ADDR relocation = -1;
+  for (int i = 0; relocation == -1 && i < num_phdr; i++)
+    {
+      phdr_type *const p = (phdr_type *) (phdr_buf.data() + i * phdr_size);
+
+      if (p->p_type == PT_PHDR)
+	relocation = phdr_memaddr - p->p_vaddr;
+    }
+
+  if (relocation == -1)
+    {
+      /* PT_PHDR is optional, but necessary for PIE in general.  Fortunately
+	 any real world executables, including PIE executables, have always
+	 PT_PHDR present.  PT_PHDR is not present in some shared libraries or
+	 in fpc (Free Pascal 2.4) binaries but neither of those have a need for
+	 or present DT_DEBUG anyway (fpc binaries are statically linked).
+
+	 Therefore if there exists DT_DEBUG there is always also PT_PHDR.
+
+	 GDB could find RELOCATION also from AT_ENTRY - e_entry.  */
+
+      return 0;
+    }
+
+  for (int i = 0; i < num_phdr; i++)
+    {
+      phdr_type *const p = (phdr_type *) (phdr_buf.data () + i * phdr_size);
+
+      if (p->p_type == PT_DYNAMIC)
+	return p->p_vaddr + relocation;
+    }
+
+  return 0;
+}
+
+/* Return &_r_debug in the inferior, or -1 if not present.  Return value
+   can be 0 if the inferior does not yet have the library list initialized.
+   We look for DT_MIPS_RLD_MAP first.  MIPS executables use this instead of
+   DT_DEBUG, although they sometimes contain an unused DT_DEBUG entry too.  */
+
+template <typename T>
+static CORE_ADDR
+get_r_debug (netbsd_process_target *target, const int pid)
+{
+  typedef typename std::conditional<sizeof(T) == sizeof(int64_t),
+				    Elf64_Dyn, Elf32_Dyn>::type dyn_type;
+  const int dyn_size = sizeof (dyn_type);
+  unsigned char buf[sizeof (dyn_type)];  /* The larger of the two.  */
+  CORE_ADDR map = -1;
+
+  CORE_ADDR dynamic_memaddr = get_dynamic<T> (target, pid);
+  if (dynamic_memaddr == 0)
+    return map;
+
+  while (target->read_memory (dynamic_memaddr, buf, dyn_size) == 0)
+    {
+      dyn_type *const dyn = (dyn_type *) buf;
+#if defined DT_MIPS_RLD_MAP
+      union
+      {
+	T map;
+	unsigned char buf[sizeof (T)];
+      }
+      rld_map;
+
+      if (dyn->d_tag == DT_MIPS_RLD_MAP)
+	{
+	  if (read_memory (dyn->d_un.d_val,
+			   rld_map.buf, sizeof (rld_map.buf)) == 0)
+	    return rld_map.map;
+	  else
+	    break;
+	}
+#endif  /* DT_MIPS_RLD_MAP */
+
+      if (dyn->d_tag == DT_DEBUG && map == -1)
+	map = dyn->d_un.d_val;
+
+      if (dyn->d_tag == DT_NULL)
+	break;
+
+      dynamic_memaddr += dyn_size;
+    }
+
+  return map;
+}
+
+/* Read one pointer from MEMADDR in the inferior.  */
+
+static int
+read_one_ptr (netbsd_process_target *target, CORE_ADDR memaddr, CORE_ADDR *ptr,
+	      int ptr_size)
+{
+  /* Go through a union so this works on either big or little endian
+     hosts, when the inferior's pointer size is smaller than the size
+     of CORE_ADDR.  It is assumed the inferior's endianness is the
+     same of the superior's.  */
+
+  union
+  {
+    CORE_ADDR core_addr;
+    unsigned int ui;
+    unsigned char uc;
+  } addr;
+
+  int ret = target->read_memory (memaddr, &addr.uc, ptr_size);
+  if (ret == 0)
+    {
+      if (ptr_size == sizeof (CORE_ADDR))
+	*ptr = addr.core_addr;
+      else if (ptr_size == sizeof (unsigned int))
+	*ptr = addr.ui;
+      else
+	gdb_assert_not_reached ("unhandled pointer size");
+    }
+  return ret;
+}
+
+/* Construct qXfer:libraries-svr4:read reply.  */
+
+template <typename T>
+int
+netbsd_qxfer_libraries_svr4 (netbsd_process_target *target,
+			     const pid_t pid, const char *annex,
+			     unsigned char *readbuf,
+			     unsigned const char *writebuf,
+			     CORE_ADDR offset, int len)
+{
+  struct link_map_offsets
+  {
+    /* Offset and size of r_debug.r_version.  */
+    int r_version_offset;
+
+    /* Offset and size of r_debug.r_map.  */
+    int r_map_offset;
+
+    /* Offset to l_addr field in struct link_map.  */
+    int l_addr_offset;
+
+    /* Offset to l_name field in struct link_map.  */
+    int l_name_offset;
+
+    /* Offset to l_ld field in struct link_map.  */
+    int l_ld_offset;
+
+    /* Offset to l_next field in struct link_map.  */
+    int l_next_offset;
+
+    /* Offset to l_prev field in struct link_map.  */
+    int l_prev_offset;
+  };
+
+  static const struct link_map_offsets lmo_32bit_offsets =
+    {
+      0,     /* r_version offset. */
+      4,     /* r_debug.r_map offset.  */
+      0,     /* l_addr offset in link_map.  */
+      4,     /* l_name offset in link_map.  */
+      8,     /* l_ld offset in link_map.  */
+      12,    /* l_next offset in link_map.  */
+      16     /* l_prev offset in link_map.  */
+    };
+
+  static const struct link_map_offsets lmo_64bit_offsets =
+    {
+      0,     /* r_version offset. */
+      8,     /* r_debug.r_map offset.  */
+      0,     /* l_addr offset in link_map.  */
+      8,     /* l_name offset in link_map.  */
+      16,    /* l_ld offset in link_map.  */
+      24,    /* l_next offset in link_map.  */
+      32     /* l_prev offset in link_map.  */
+    };
+
+  CORE_ADDR lm_addr = 0, lm_prev = 0;
+  CORE_ADDR l_name, l_addr, l_ld, l_next, l_prev;
+  int header_done = 0;
+
+  const struct link_map_offsets *lmo
+    = ((sizeof (T) == sizeof (int64_t))
+       ? &lmo_64bit_offsets : &lmo_32bit_offsets);
+  int ptr_size = sizeof (T);
+
+  while (annex[0] != '\0')
+    {
+      const char *sep = strchr (annex, '=');
+      if (sep == nullptr)
+	break;
+
+      int name_len = sep - annex;
+      CORE_ADDR *addrp;
+      if (name_len == 5 && startswith (annex, "start"))
+	addrp = &lm_addr;
+      else if (name_len == 4 && startswith (annex, "prev"))
+	addrp = &lm_prev;
+      else
+	{
+	  annex = strchr (sep, ';');
+	  if (annex == nullptr)
+	    break;
+	  annex++;
+	  continue;
+	}
+
+      annex = decode_address_to_semicolon (addrp, sep + 1);
+    }
+
+  if (lm_addr == 0)
+    {
+      CORE_ADDR r_debug = get_r_debug<T> (target, pid);
+
+      /* We failed to find DT_DEBUG.  Such situation will not change
+	 for this inferior - do not retry it.  Report it to GDB as
+	 E01, see for the reasons at the GDB solib-svr4.c side.  */
+      if (r_debug == (CORE_ADDR) -1)
+	return -1;
+
+      if (r_debug != 0)
+	{
+	  CORE_ADDR map_offset = r_debug + lmo->r_map_offset;
+	  if (read_one_ptr (target, map_offset, &lm_addr, ptr_size) != 0)
+	    warning ("unable to read r_map from %s",
+		     core_addr_to_string (map_offset));
+	}
+    }
+
+  std::string document = "<library-list-svr4 version=\"1.0\"";
+
+  while (lm_addr
+	 && read_one_ptr (target, lm_addr + lmo->l_name_offset,
+			  &l_name, ptr_size) == 0
+	 && read_one_ptr (target, lm_addr + lmo->l_addr_offset,
+			  &l_addr, ptr_size) == 0
+	 && read_one_ptr (target, lm_addr + lmo->l_ld_offset,
+			  &l_ld, ptr_size) == 0
+	 && read_one_ptr (target, lm_addr + lmo->l_prev_offset,
+			  &l_prev, ptr_size) == 0
+	 && read_one_ptr (target, lm_addr + lmo->l_next_offset,
+			  &l_next, ptr_size) == 0)
+    {
+      if (lm_prev != l_prev)
+	{
+	  warning ("Corrupted shared library list: 0x%lx != 0x%lx",
+		   (long) lm_prev, (long) l_prev);
+	  break;
+	}
+
+      /* Ignore the first entry even if it has valid name as the first entry
+	 corresponds to the main executable.  The first entry should not be
+	 skipped if the dynamic loader was loaded late by a static executable
+	 (see solib-svr4.c parameter ignore_first).  But in such case the main
+	 executable does not have PT_DYNAMIC present and this function already
+	 exited above due to failed get_r_debug.  */
+      if (lm_prev == 0)
+	string_appendf (document, " main-lm=\"0x%lx\"",
+			(unsigned long) lm_addr);
+      else
+	{
+	  unsigned char libname[PATH_MAX];
+
+	  /* Not checking for error because reading may stop before
+	     we've got PATH_MAX worth of characters.  */
+	  libname[0] = '\0';
+	  target->read_memory (l_name, libname, sizeof (libname) - 1);
+	  libname[sizeof (libname) - 1] = '\0';
+	  if (libname[0] != '\0')
+	    {
+	      if (!header_done)
+		{
+		  /* Terminate `<library-list-svr4'.  */
+		  document += '>';
+		  header_done = 1;
+		}
+
+	      string_appendf (document, "<library name=\"");
+	      xml_escape_text_append (&document, (char *) libname);
+	      string_appendf (document, "\" lm=\"0x%lx\" "
+			      "l_addr=\"0x%lx\" l_ld=\"0x%lx\"/>",
+			      (unsigned long) lm_addr, (unsigned long) l_addr,
+			      (unsigned long) l_ld);
+	    }
+	}
+
+      lm_prev = lm_addr;
+      lm_addr = l_next;
+    }
+
+  if (!header_done)
+    {
+      /* Empty list; terminate `<library-list-svr4'.  */
+      document += "/>";
+    }
+  else
+    document += "</library-list-svr4>";
+
+  int document_len = document.length ();
+  if (offset < document_len)
+    document_len -= offset;
+  else
+    document_len = 0;
+  if (len > document_len)
+    len = document_len;
+
+  memcpy (readbuf, document.data () + offset, len);
+
+  return len;
+}
+
+/* Return true if FILE is a 64-bit ELF file,
+   false if the file is not a 64-bit ELF file,
+   and error if the file is not accessible or doesn't exist.  */
+
+static bool
+elf_64_file_p (const char *file)
+{
+  int fd = gdb::handle_eintr<int> (-1, ::open, file, O_RDONLY);
+  if (fd < 0)
+    perror_with_name (("open"));
+
+  Elf64_Ehdr header;
+  ssize_t ret = gdb::handle_eintr<ssize_t> (-1, ::read, fd, &header, sizeof (header));
+  if (ret == -1)
+    perror_with_name (("read"));
+  gdb::handle_eintr<int> (-1, ::close, fd);
+  if (ret != sizeof (header))
+    error ("Cannot read ELF file header: %s", file);
+
+  if (header.e_ident[EI_MAG0] != ELFMAG0
+      || header.e_ident[EI_MAG1] != ELFMAG1
+      || header.e_ident[EI_MAG2] != ELFMAG2
+      || header.e_ident[EI_MAG3] != ELFMAG3)
+    error ("Unrecognized ELF file header: %s", file);
+
+  return header.e_ident[EI_CLASS] == ELFCLASS64;
+}
+
+/* Construct qXfer:libraries-svr4:read reply.  */
+
+int
+netbsd_process_target::qxfer_libraries_svr4 (const char *annex,
+					     unsigned char *readbuf,
+					     unsigned const char *writebuf,
+					     CORE_ADDR offset, int len)
+{
+  if (writebuf != nullptr)
+    return -2;
+  if (readbuf == nullptr)
+    return -1;
+
+  struct process_info *proc = current_process ();
+  pid_t pid = proc->pid;
+  bool is_elf64 = elf_64_file_p (netbsd_nat::pid_to_exec_file (pid));
+
+  if (is_elf64)
+    return netbsd_qxfer_libraries_svr4<int64_t> (this, pid, annex, readbuf,
+						 writebuf, offset, len);
+  else
+    return netbsd_qxfer_libraries_svr4<int32_t> (this, pid, annex, readbuf,
+						 writebuf, offset, len);
+}
+
+/* Implement the supports_qxfer_libraries_svr4 target_ops method.  */
+
+bool
+netbsd_process_target::supports_qxfer_libraries_svr4 ()
+{
+  return true;
+}
+
+/* Return the name of a file that can be opened to get the symbols for
+   the child process identified by PID.  */
+
+char *
+netbsd_process_target::pid_to_exec_file (pid_t pid)
+{
+  return netbsd_nat::pid_to_exec_file (pid);
+}
+
+/* Implementation of the target_ops method "supports_pid_to_exec_file".  */
+
+bool
+netbsd_process_target::supports_pid_to_exec_file ()
+{
+  return true;
+}
+
+/* Implementation of the target_ops method "supports_hardware_single_step".  */
+bool
+netbsd_process_target::supports_hardware_single_step ()
+{
+  return true;
+}
+
+/* Implementation of the target_ops method "sw_breakpoint_from_kind".  */
+
+const gdb_byte *
+netbsd_process_target::sw_breakpoint_from_kind (int kind, int *size)
+{
+  static gdb_byte brkpt[PTRACE_BREAKPOINT_SIZE] = {*PTRACE_BREAKPOINT};
+
+  *size = PTRACE_BREAKPOINT_SIZE;
+
+  return brkpt;
+}
+
+/* Implement the thread_name target_ops method.  */
+
+const char *
+netbsd_process_target::thread_name (ptid_t ptid)
+{
+  return netbsd_nat::thread_name (ptid);
+}
+
+/* Implement the supports_catch_syscall target_ops method.  */
+
+bool
+netbsd_process_target::supports_catch_syscall ()
+{
+  return true;
+}
+
+/* Implement the supports_read_auxv target_ops method.  */
+
+bool
+netbsd_process_target::supports_read_auxv ()
+{
+  return true;
+}
+
+/* The NetBSD target ops object.  */
+
+static netbsd_process_target the_netbsd_target;
+
+void
+initialize_low ()
+{
+  set_target_ops (&the_netbsd_target);
+  the_low_target.arch_setup ();
+}
diff --git a/gdbserver/netbsd-low.h b/gdbserver/netbsd-low.h
new file mode 100644
index 00000000000..3d2ec345a41
--- /dev/null
+++ b/gdbserver/netbsd-low.h
@@ -0,0 +1,154 @@
+/* Copyright (C) 2020 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef GDBSERVER_NETBSD_LOW_H
+#define GDBSERVER_NETBSD_LOW_H
+
+struct regcache;
+struct target_desc;
+
+/*  Some information relative to a given register set.   */
+
+struct netbsd_regset_info
+{
+  /* The ptrace request needed to get/set registers of this set.  */
+  int get_request, set_request;
+  /* The size of the register set.  */
+  int size;
+  /* Fill the buffer BUF from the contents of the given REGCACHE.  */
+  void (*fill_function) (struct regcache *regcache, char *buf);
+  /* Store the register value in BUF in the given REGCACHE.  */
+  void (*store_function) (struct regcache *regcache, const char *buf);
+};
+
+/* A list of regsets for the target being debugged, terminated by an entry
+   where the size is negative.
+
+   This list should be created by the target-specific code.  */
+
+extern struct netbsd_regset_info netbsd_target_regsets[];
+
+/* The target-specific operations for NetBSD support.  */
+
+struct netbsd_target_ops
+{
+  /* Architecture-specific setup.  */
+  void (*arch_setup) ();
+};
+
+/* Target ops definitions for a NetBSD target.  */
+
+class netbsd_process_target : public process_stratum_target
+{
+public:
+
+  int create_inferior (const char *program,
+		       const std::vector<char *> &program_args) override;
+
+  void post_create_inferior () override;
+
+  int attach (unsigned long pid) override;
+
+  int kill (process_info *proc) override;
+
+  int detach (process_info *proc) override;
+
+  void mourn (process_info *proc) override;
+
+  void join (int pid) override;
+
+  bool thread_alive (ptid_t pid) override;
+
+  void resume (thread_resume *resume_info, size_t n) override;
+
+  ptid_t wait (ptid_t ptid, target_waitstatus *status,
+	       int options) override;
+
+  void fetch_registers (regcache *regcache, int regno) override;
+
+  void store_registers (regcache *regcache, int regno) override;
+
+  int read_memory (CORE_ADDR memaddr, unsigned char *myaddr,
+		   int len) override;
+
+  int write_memory (CORE_ADDR memaddr, const unsigned char *myaddr,
+		    int len) override;
+
+  void request_interrupt () override;
+
+  bool supports_read_auxv () override;
+
+  int read_auxv (CORE_ADDR offset, unsigned char *myaddr,
+		 unsigned int len) override;
+
+  bool supports_hardware_single_step () override;
+
+  const gdb_byte *sw_breakpoint_from_kind (int kind, int *size) override;
+
+  bool supports_z_point_type (char z_type) override;
+
+  int insert_point (enum raw_bkpt_type type, CORE_ADDR addr,
+		    int size, struct raw_breakpoint *bp) override;
+
+  int remove_point (enum raw_bkpt_type type, CORE_ADDR addr,
+		    int size, struct raw_breakpoint *bp) override;
+
+  bool stopped_by_sw_breakpoint () override;
+
+  bool supports_qxfer_siginfo () override;
+
+  int qxfer_siginfo (const char *annex, unsigned char *readbuf,
+		     unsigned const char *writebuf, CORE_ADDR offset,
+		     int len) override;
+
+  bool supports_stopped_by_sw_breakpoint () override;
+
+  bool supports_non_stop () override;
+
+  bool supports_multi_process () override;
+
+  bool supports_fork_events () override;
+
+  bool supports_vfork_events () override;
+
+  bool supports_exec_events () override;
+
+  bool supports_disable_randomization () override;
+
+  bool supports_qxfer_libraries_svr4 () override;
+
+  int qxfer_libraries_svr4 (const char*, unsigned char*, const unsigned char*,
+			    CORE_ADDR, int) override;
+
+  bool supports_pid_to_exec_file () override;
+
+  char *pid_to_exec_file (int pid) override;
+
+  const char *thread_name (ptid_t thread) override;
+
+  bool supports_catch_syscall () override;
+};
+
+/* The inferior's target description.  This is a global because the
+   NetBSD ports support neither bi-arch nor multi-process.  */
+
+extern struct netbsd_target_ops the_low_target;
+
+/* XXX: multilib */
+extern const struct target_desc *netbsd_tdesc;
+
+#endif /* GDBSERVER_NETBSD_LOW_H */
diff --git a/gdbserver/netbsd-x86_64-low.cc b/gdbserver/netbsd-x86_64-low.cc
new file mode 100644
index 00000000000..9b8ea9b8aa6
--- /dev/null
+++ b/gdbserver/netbsd-x86_64-low.cc
@@ -0,0 +1,187 @@
+/* Copyright (C) 2020 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <sys/types.h>
+#include <sys/ptrace.h>
+#include <limits.h>
+
+#include "server.h"
+#include "netbsd-low.h"
+#include "gdbsupport/x86-xstate.h"
+#include "arch/amd64.h"
+#include "x86-tdesc.h"
+#include "tdesc.h"
+
+/* The index of various registers inside the regcache.  */
+
+enum netbsd_x86_64_gdb_regnum
+{
+  AMD64_RAX_REGNUM,	     /* %rax */
+  AMD64_RBX_REGNUM,	     /* %rbx */
+  AMD64_RCX_REGNUM,	     /* %rcx */
+  AMD64_RDX_REGNUM,	     /* %rdx */
+  AMD64_RSI_REGNUM,	     /* %rsi */
+  AMD64_RDI_REGNUM,	     /* %rdi */
+  AMD64_RBP_REGNUM,	     /* %rbp */
+  AMD64_RSP_REGNUM,	     /* %rsp */
+  AMD64_R8_REGNUM,	      /* %r8 */
+  AMD64_R9_REGNUM,	      /* %r9 */
+  AMD64_R10_REGNUM,	     /* %r10 */
+  AMD64_R11_REGNUM,	     /* %r11 */
+  AMD64_R12_REGNUM,	     /* %r12 */
+  AMD64_R13_REGNUM,	     /* %r13 */
+  AMD64_R14_REGNUM,	     /* %r14 */
+  AMD64_R15_REGNUM,	     /* %r15 */
+  AMD64_RIP_REGNUM,	     /* %rip */
+  AMD64_EFLAGS_REGNUM,	  /* %eflags */
+  AMD64_CS_REGNUM,	      /* %cs */
+  AMD64_SS_REGNUM,	      /* %ss */
+  AMD64_DS_REGNUM,	      /* %ds */
+  AMD64_ES_REGNUM,	      /* %es */
+  AMD64_FS_REGNUM,	      /* %fs */
+  AMD64_GS_REGNUM,	      /* %gs */
+  AMD64_ST0_REGNUM = 24,     /* %st0 */
+  AMD64_ST1_REGNUM,	     /* %st1 */
+  AMD64_FCTRL_REGNUM = AMD64_ST0_REGNUM + 8,
+  AMD64_FSTAT_REGNUM = AMD64_ST0_REGNUM + 9,
+  AMD64_FTAG_REGNUM = AMD64_ST0_REGNUM + 10,
+  AMD64_XMM0_REGNUM = 40,   /* %xmm0 */
+  AMD64_XMM1_REGNUM,	    /* %xmm1 */
+  AMD64_MXCSR_REGNUM = AMD64_XMM0_REGNUM + 16,
+  AMD64_YMM0H_REGNUM,	   /* %ymm0h */
+  AMD64_YMM15H_REGNUM = AMD64_YMM0H_REGNUM + 15,
+  AMD64_BND0R_REGNUM = AMD64_YMM15H_REGNUM + 1,
+  AMD64_BND3R_REGNUM = AMD64_BND0R_REGNUM + 3,
+  AMD64_BNDCFGU_REGNUM,
+  AMD64_BNDSTATUS_REGNUM,
+  AMD64_XMM16_REGNUM,
+  AMD64_XMM31_REGNUM = AMD64_XMM16_REGNUM + 15,
+  AMD64_YMM16H_REGNUM,
+  AMD64_YMM31H_REGNUM = AMD64_YMM16H_REGNUM + 15,
+  AMD64_K0_REGNUM,
+  AMD64_K7_REGNUM = AMD64_K0_REGNUM + 7,
+  AMD64_ZMM0H_REGNUM,
+  AMD64_ZMM31H_REGNUM = AMD64_ZMM0H_REGNUM + 31,
+  AMD64_PKRU_REGNUM,
+  AMD64_FSBASE_REGNUM,
+  AMD64_GSBASE_REGNUM
+};
+
+/* The fill_function for the general-purpose register set.  */
+
+static void
+netbsd_x86_64_fill_gregset (struct regcache *regcache, char *buf)
+{
+  struct reg *r = (struct reg *) buf;
+
+#define netbsd_x86_64_collect_gp(regnum, fld) do {		\
+    collect_register (regcache, regnum, &r->regs[_REG_##fld]);	\
+  } while (0)
+
+  netbsd_x86_64_collect_gp (AMD64_RAX_REGNUM, RAX);
+  netbsd_x86_64_collect_gp (AMD64_RBX_REGNUM, RBX);
+  netbsd_x86_64_collect_gp (AMD64_RCX_REGNUM, RCX);
+  netbsd_x86_64_collect_gp (AMD64_RDX_REGNUM, RDX);
+  netbsd_x86_64_collect_gp (AMD64_RSI_REGNUM, RSI);
+  netbsd_x86_64_collect_gp (AMD64_RDI_REGNUM, RDI);
+  netbsd_x86_64_collect_gp (AMD64_RBP_REGNUM, RBP);
+  netbsd_x86_64_collect_gp (AMD64_RSP_REGNUM, RSP);
+  netbsd_x86_64_collect_gp (AMD64_R8_REGNUM, R8);
+  netbsd_x86_64_collect_gp (AMD64_R9_REGNUM, R9);
+  netbsd_x86_64_collect_gp (AMD64_R10_REGNUM, R10);
+  netbsd_x86_64_collect_gp (AMD64_R11_REGNUM, R11);
+  netbsd_x86_64_collect_gp (AMD64_R12_REGNUM, R12);
+  netbsd_x86_64_collect_gp (AMD64_R13_REGNUM, R13);
+  netbsd_x86_64_collect_gp (AMD64_R14_REGNUM, R14);
+  netbsd_x86_64_collect_gp (AMD64_R15_REGNUM, R15);
+  netbsd_x86_64_collect_gp (AMD64_RIP_REGNUM, RIP);
+  netbsd_x86_64_collect_gp (AMD64_EFLAGS_REGNUM, RFLAGS);
+  netbsd_x86_64_collect_gp (AMD64_CS_REGNUM, CS);
+  netbsd_x86_64_collect_gp (AMD64_SS_REGNUM, SS);
+  netbsd_x86_64_collect_gp (AMD64_DS_REGNUM, DS);
+  netbsd_x86_64_collect_gp (AMD64_ES_REGNUM, ES);
+  netbsd_x86_64_collect_gp (AMD64_FS_REGNUM, FS);
+  netbsd_x86_64_collect_gp (AMD64_GS_REGNUM, GS);
+}
+
+/* The store_function for the general-purpose register set.  */
+
+static void
+netbsd_x86_64_store_gregset (struct regcache *regcache, const char *buf)
+{
+  struct reg *r = (struct reg *) buf;
+
+#define netbsd_x86_64_supply_gp(regnum, fld) do {		\
+    supply_register (regcache, regnum, &r->regs[_REG_##fld]);	\
+  } while(0)
+
+  netbsd_x86_64_supply_gp (AMD64_RAX_REGNUM, RAX);
+  netbsd_x86_64_supply_gp (AMD64_RBX_REGNUM, RBX);
+  netbsd_x86_64_supply_gp (AMD64_RCX_REGNUM, RCX);
+  netbsd_x86_64_supply_gp (AMD64_RDX_REGNUM, RDX);
+  netbsd_x86_64_supply_gp (AMD64_RSI_REGNUM, RSI);
+  netbsd_x86_64_supply_gp (AMD64_RDI_REGNUM, RDI);
+  netbsd_x86_64_supply_gp (AMD64_RBP_REGNUM, RBP);
+  netbsd_x86_64_supply_gp (AMD64_RSP_REGNUM, RSP);
+  netbsd_x86_64_supply_gp (AMD64_R8_REGNUM, R8);
+  netbsd_x86_64_supply_gp (AMD64_R9_REGNUM, R9);
+  netbsd_x86_64_supply_gp (AMD64_R10_REGNUM, R10);
+  netbsd_x86_64_supply_gp (AMD64_R11_REGNUM, R11);
+  netbsd_x86_64_supply_gp (AMD64_R12_REGNUM, R12);
+  netbsd_x86_64_supply_gp (AMD64_R13_REGNUM, R13);
+  netbsd_x86_64_supply_gp (AMD64_R14_REGNUM, R14);
+  netbsd_x86_64_supply_gp (AMD64_R15_REGNUM, R15);
+  netbsd_x86_64_supply_gp (AMD64_RIP_REGNUM, RIP);
+  netbsd_x86_64_supply_gp (AMD64_EFLAGS_REGNUM, RFLAGS);
+  netbsd_x86_64_supply_gp (AMD64_CS_REGNUM, CS);
+  netbsd_x86_64_supply_gp (AMD64_SS_REGNUM, SS);
+  netbsd_x86_64_supply_gp (AMD64_DS_REGNUM, DS);
+  netbsd_x86_64_supply_gp (AMD64_ES_REGNUM, ES);
+  netbsd_x86_64_supply_gp (AMD64_FS_REGNUM, FS);
+  netbsd_x86_64_supply_gp (AMD64_GS_REGNUM, GS);
+}
+
+/* Implements the netbsd_target_ops.arch_setup routine.  */
+
+static void
+netbsd_x86_64_arch_setup (void)
+{
+  struct target_desc *tdesc
+    = amd64_create_target_description (X86_XSTATE_SSE_MASK, false, false, false);
+
+  init_target_desc (tdesc, amd64_expedite_regs);
+
+  netbsd_tdesc = tdesc;
+}
+
+/* Description of all the x86-netbsd register sets.  */
+
+struct netbsd_regset_info netbsd_target_regsets[] =
+{
+ /* General Purpose Registers.  */
+ {PT_GETREGS, PT_SETREGS, sizeof (struct reg),
+  netbsd_x86_64_fill_gregset, netbsd_x86_64_store_gregset},
+ /* End of list marker.  */
+ {0, 0, -1, NULL, NULL }
+};
+
+/* The netbsd_target_ops vector for x86-netbsd.  */
+
+struct netbsd_target_ops the_low_target =
+{
+ netbsd_x86_64_arch_setup,
+};
--
2.28.0

Re: [PATCH v2 10/10] Add minimal and functional NetBSD/amd64 gdbserver

[Simon Marchi]

I did a superficial review, that's as much as I can, since I can't easily test this.  I also don't
know gdbserver by heart, so I can't tell off the top of my head if some method implementation doesn't
match what's expected, for example.

So I checked for obvious mistakes (didn't find any) and coding standards (looks pretty good).  Since
this is new code, I am not too worried, it won't break any existing use case.  The worst that can happen
is that there are bugs and we fix them later.

On 2020-09-03 8:29 p.m., Kamil Rytarowski wrote:
> Implement the following functionality: create_inferior,
> post_create_inferior, attach, kill, detach, mourn, join, thread_alive,
> resume, wait, fetch_registers, store_registers, read_memory, write_memory,
> request_interrupt, supports_read_auxv, read_auxv,
> supports_hardware_single_step, sw_breakpoint_from_kind,
> supports_z_point_type, insert_point, remove_point,
> stopped_by_sw_breakpoint, supports_qxfer_siginfo, qxfer_siginfo,
> supports_stopped_by_sw_breakpoint, supports_non_stop,
> supports_multi_process, supports_fork_events, supports_vfork_events,
> supports_exec_events, supports_disable_randomization,
> supports_qxfer_libraries_svr4, qxfer_libraries_svr4,
> supports_pid_to_exec_file, pid_to_exec_file, thread_name,
> supports_catch_syscall.
> 
> The only CPU architecture supported: x86_64.
> 
> Implement only support for hardware assisted single step and
> software breakpoint.
> 
> Implement support only for regular X86 registers, thus no FPU.
> 
> gdbserver/ChangeLog:
> 
>        * netbsd-low.cc: Add.
>        * netbsd-low.h: Likewise.
>        * netbsd-x86_64-low.cc: Likewise.

Let's name this new file netbsd-amd64-low.cc.  We use amd64 everywhere, so it's
better to stay consistent.

> +/* Implement the fetch_registers target_ops method.  */
> +
> +void
> +netbsd_process_target::fetch_registers (struct regcache *regcache, int regno)
> +{
> +  struct netbsd_regset_info *regset = netbsd_target_regsets;
> +  ptid_t inferior_ptid = ptid_of (current_thread);
> +
> +  while (regset->size >= 0)
> +    {
> +      char *buf = (char *) xmalloc (regset->size);
> +      int res = ptrace (regset->get_request, inferior_ptid.pid (), buf,
> +			inferior_ptid.lwp ());
> +      if (res == -1)
> +	perror_with_name (("ptrace"));
> +      regset->store_function (regcache, buf);
> +      free (buf);

The perror would leak `buf`.  Use some automatic memory management type.

> +      regset++;
> +    }
> +}
> +
> +/* Implement the store_registers target_ops method.  */
> +
> +void
> +netbsd_process_target::store_registers (struct regcache *regcache, int regno)
> +{
> +  struct netbsd_regset_info *regset = netbsd_target_regsets;
> +  ptid_t inferior_ptid = ptid_of (current_thread);
> +
> +  while (regset->size >= 0)
> +    {
> +      char *buf = (char *)xmalloc (regset->size);
> +      int res = ptrace (regset->get_request, inferior_ptid.pid (), buf,
> +			inferior_ptid.lwp ());
> +      if (res == -1)
> +	perror_with_name (("ptrace"));
> +      if (res == 0)
> +	{
> +	  /* Then overlay our cached registers on that.  */
> +	  regset->fill_function (regcache, buf);
> +	  /* Only now do we write the register set.  */
> +	  res = ptrace (regset->set_request, inferior_ptid.pid (), buf,
> +			inferior_ptid.lwp ());
> +	}
> +      free (buf);

Same here.

> +/* Extract &phdr and num_phdr in the inferior.  Return 0 on success.  */
> +
> +template <typename T>
> +int get_phdr_phnum_from_proc_auxv (const pid_t pid,
> +				   CORE_ADDR *phdr_memaddr, int *num_phdr)
> +{
> +  typedef typename std::conditional<sizeof(T) == sizeof(int64_t),
> +				    Aux64Info, Aux32Info>::type auxv_type;
> +  const size_t auxv_size = sizeof (auxv_type);
> +  const size_t auxv_buf_size = 128 * sizeof (auxv_type);
> +
> +  char *auxv_buf = (char *) xmalloc (auxv_buf_size);
> +
> +  netbsd_read_auxv (pid, nullptr, auxv_buf, auxv_buf_size);
> +
> +  *phdr_memaddr = 0;
> +  *num_phdr = 0;
> +
> +  for (char *buf = auxv_buf; buf < (auxv_buf + auxv_buf_size); buf += auxv_size)
> +    {
> +      auxv_type *const aux = (auxv_type *) buf;
> +
> +      switch (aux->a_type)
> +	{
> +	case AT_PHDR:
> +	  *phdr_memaddr = aux->a_v;
> +	  break;
> +	case AT_PHNUM:
> +	  *num_phdr = aux->a_v;
> +	  break;
> +	}
> +
> +      if (*phdr_memaddr != 0 && *num_phdr != 0)
> +	break;
> +    }
> +
> +  xfree (auxv_buf);

Same here.

> +struct regcache;
> +struct target_desc;
> +
> +/*  Some information relative to a given register set.   */
> +
> +struct netbsd_regset_info
> +{
> +  /* The ptrace request needed to get/set registers of this set.  */
> +  int get_request, set_request;
> +  /* The size of the register set.  */
> +  int size;
> +  /* Fill the buffer BUF from the contents of the given REGCACHE.  */
> +  void (*fill_function) (struct regcache *regcache, char *buf);
> +  /* Store the register value in BUF in the given REGCACHE.  */
> +  void (*store_function) (struct regcache *regcache, const char *buf);

I find the distinction between "fill" and "store" unclear, it doesn't really tell
which direction it's going (looking at the types helps though).  fill the regcache?
fill the buffer?  store in the regcache?  store in the buffer?

It's the same with the regcache::supply_regset and regcache::collect_regset methods
in GDB... I never know what supply and collect mean.  Are they from the point of view
of the regset?  Or from the point of view of the user of the regset?

What about:

  void (*buf_to_regcache) (struct regcache *regcache, char *buf);
  void (*regcache_to_buf) (struct regcache *regcache, const char *buf);

You could add the `_function` suffix if you prefer, though I don't think it's necessary.

Simon

Re: [PATCH v2 10/10] Add minimal and functional NetBSD/amd64 gdbserver

[Kamil Rytarowski]

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

On 07.09.2020 21:58, Simon Marchi wrote:
> I did a superficial review, that's as much as I can, since I can't easily test this.  I also don't
> know gdbserver by heart, so I can't tell off the top of my head if some method implementation doesn't
> match what's expected, for example.
> 
> So I checked for obvious mistakes (didn't find any) and coding standards (looks pretty good).  Since
> this is new code, I am not too worried, it won't break any existing use case.  The worst that can happen
> is that there are bugs and we fix them later.
> 

Thank you for the review! Right, there could be bugs and there is room
for improvement and adding new features. Nonetheless, this code already
works.

> On 2020-09-03 8:29 p.m., Kamil Rytarowski wrote:
>> Implement the following functionality: create_inferior,
>> post_create_inferior, attach, kill, detach, mourn, join, thread_alive,
>> resume, wait, fetch_registers, store_registers, read_memory, write_memory,
>> request_interrupt, supports_read_auxv, read_auxv,
>> supports_hardware_single_step, sw_breakpoint_from_kind,
>> supports_z_point_type, insert_point, remove_point,
>> stopped_by_sw_breakpoint, supports_qxfer_siginfo, qxfer_siginfo,
>> supports_stopped_by_sw_breakpoint, supports_non_stop,
>> supports_multi_process, supports_fork_events, supports_vfork_events,
>> supports_exec_events, supports_disable_randomization,
>> supports_qxfer_libraries_svr4, qxfer_libraries_svr4,
>> supports_pid_to_exec_file, pid_to_exec_file, thread_name,
>> supports_catch_syscall.
>>
>> The only CPU architecture supported: x86_64.
>>
>> Implement only support for hardware assisted single step and
>> software breakpoint.
>>
>> Implement support only for regular X86 registers, thus no FPU.
>>
>> gdbserver/ChangeLog:
>>
>>        * netbsd-low.cc: Add.
>>        * netbsd-low.h: Likewise.
>>        * netbsd-x86_64-low.cc: Likewise.
> 
> Let's name this new file netbsd-amd64-low.cc.  We use amd64 everywhere, so it's
> better to stay consistent.
> 

OK

>> +/* Implement the fetch_registers target_ops method.  */
>> +
>> +void
>> +netbsd_process_target::fetch_registers (struct regcache *regcache, int regno)
>> +{
>> +  struct netbsd_regset_info *regset = netbsd_target_regsets;
>> +  ptid_t inferior_ptid = ptid_of (current_thread);
>> +
>> +  while (regset->size >= 0)
>> +    {
>> +      char *buf = (char *) xmalloc (regset->size);
>> +      int res = ptrace (regset->get_request, inferior_ptid.pid (), buf,
>> +			inferior_ptid.lwp ());
>> +      if (res == -1)
>> +	perror_with_name (("ptrace"));
>> +      regset->store_function (regcache, buf);
>> +      free (buf);
> 
> The perror would leak `buf`.  Use some automatic memory management type.
> 

OK

>> +      regset++;
>> +    }
>> +}
>> +
>> +/* Implement the store_registers target_ops method.  */
>> +
>> +void
>> +netbsd_process_target::store_registers (struct regcache *regcache, int regno)
>> +{
>> +  struct netbsd_regset_info *regset = netbsd_target_regsets;
>> +  ptid_t inferior_ptid = ptid_of (current_thread);
>> +
>> +  while (regset->size >= 0)
>> +    {
>> +      char *buf = (char *)xmalloc (regset->size);
>> +      int res = ptrace (regset->get_request, inferior_ptid.pid (), buf,
>> +			inferior_ptid.lwp ());
>> +      if (res == -1)
>> +	perror_with_name (("ptrace"));
>> +      if (res == 0)
>> +	{
>> +	  /* Then overlay our cached registers on that.  */
>> +	  regset->fill_function (regcache, buf);
>> +	  /* Only now do we write the register set.  */
>> +	  res = ptrace (regset->set_request, inferior_ptid.pid (), buf,
>> +			inferior_ptid.lwp ());
>> +	}
>> +      free (buf);
> 
> Same here.
> 

OK

>> +/* Extract &phdr and num_phdr in the inferior.  Return 0 on success.  */
>> +
>> +template <typename T>
>> +int get_phdr_phnum_from_proc_auxv (const pid_t pid,
>> +				   CORE_ADDR *phdr_memaddr, int *num_phdr)
>> +{
>> +  typedef typename std::conditional<sizeof(T) == sizeof(int64_t),
>> +				    Aux64Info, Aux32Info>::type auxv_type;
>> +  const size_t auxv_size = sizeof (auxv_type);
>> +  const size_t auxv_buf_size = 128 * sizeof (auxv_type);
>> +
>> +  char *auxv_buf = (char *) xmalloc (auxv_buf_size);
>> +
>> +  netbsd_read_auxv (pid, nullptr, auxv_buf, auxv_buf_size);
>> +
>> +  *phdr_memaddr = 0;
>> +  *num_phdr = 0;
>> +
>> +  for (char *buf = auxv_buf; buf < (auxv_buf + auxv_buf_size); buf += auxv_size)
>> +    {
>> +      auxv_type *const aux = (auxv_type *) buf;
>> +
>> +      switch (aux->a_type)
>> +	{
>> +	case AT_PHDR:
>> +	  *phdr_memaddr = aux->a_v;
>> +	  break;
>> +	case AT_PHNUM:
>> +	  *num_phdr = aux->a_v;
>> +	  break;
>> +	}
>> +
>> +      if (*phdr_memaddr != 0 && *num_phdr != 0)
>> +	break;
>> +    }
>> +
>> +  xfree (auxv_buf);
> 
> Same here.
> 

OK

>> +struct regcache;
>> +struct target_desc;
>> +
>> +/*  Some information relative to a given register set.   */
>> +
>> +struct netbsd_regset_info
>> +{
>> +  /* The ptrace request needed to get/set registers of this set.  */
>> +  int get_request, set_request;
>> +  /* The size of the register set.  */
>> +  int size;
>> +  /* Fill the buffer BUF from the contents of the given REGCACHE.  */
>> +  void (*fill_function) (struct regcache *regcache, char *buf);
>> +  /* Store the register value in BUF in the given REGCACHE.  */
>> +  void (*store_function) (struct regcache *regcache, const char *buf);
> 
> I find the distinction between "fill" and "store" unclear, it doesn't really tell
> which direction it's going (looking at the types helps though).  fill the regcache?
> fill the buffer?  store in the regcache?  store in the buffer?
> 
> It's the same with the regcache::supply_regset and regcache::collect_regset methods
> in GDB... I never know what supply and collect mean.  Are they from the point of view
> of the regset?  Or from the point of view of the user of the regset?
> 
> What about:
> 
>   void (*buf_to_regcache) (struct regcache *regcache, char *buf);
>   void (*regcache_to_buf) (struct regcache *regcache, const char *buf);
> 
> You could add the `_function` suffix if you prefer, though I don't think it's necessary.
> 

I will leave the naming as it is in this patch as it exists in Linux and
some generic code. I have got no opinion on renaming, if we go for it,
it should happen independently.

> Simon
> 



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