Implement IP_STAT on NetBSD

Message ID 20200413181911.17133-1-n54@gmx.com
State New
Headers show
Series
  • Implement IP_STAT on NetBSD
Related show

Commit Message

Kamil Rytarowski April 13, 2020, 6:19 p.m.
NetBSD ships with a compatibility layer in a /proc virtual file system
and delivers optionally /proc/<pid>/stat. In case of missing
/proc/<pid>/stat fallback to emulating it with sysctl(3) APIs.

gdb/ChangeLog:

        * nbsd-nat.c: Include "sys/resource.h" and "machine/vmparam.h".
        (nbsd_pid_to_kinfo_proc2, nbsd_pid_to_rlimit_cur)
        (nbsd_pid_to_statstr): New.
        (nbsd_nat_target::info_proc): Add do_stat.
---
 gdb/ChangeLog  |   7 ++
 gdb/nbsd-nat.c | 278 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 285 insertions(+)

--
2.25.0

Comments

Simon Marchi April 13, 2020, 7:16 p.m. | #1
On 2020-04-13 2:19 p.m., Kamil Rytarowski wrote:
> NetBSD ships with a compatibility layer in a /proc virtual file system

> and delivers optionally /proc/<pid>/stat. In case of missing

> /proc/<pid>/stat fallback to emulating it with sysctl(3) APIs.

> 

> gdb/ChangeLog:

> 

>         * nbsd-nat.c: Include "sys/resource.h" and "machine/vmparam.h".

>         (nbsd_pid_to_kinfo_proc2, nbsd_pid_to_rlimit_cur)

>         (nbsd_pid_to_statstr): New.

>         (nbsd_nat_target::info_proc): Add do_stat.

> ---

>  gdb/ChangeLog  |   7 ++

>  gdb/nbsd-nat.c | 278 +++++++++++++++++++++++++++++++++++++++++++++++++

>  2 files changed, 285 insertions(+)

> 

> diff --git a/gdb/ChangeLog b/gdb/ChangeLog

> index 7d91abf6334..5ff76e85cb3 100644

> --- a/gdb/ChangeLog

> +++ b/gdb/ChangeLog

> @@ -1,3 +1,10 @@

> +2020-04-13  Kamil Rytarowski  <n54@gmx.com>

> +

> +	* nbsd-nat.c: Include "sys/resource.h" and "machine/vmparam.h".

> +	(nbsd_pid_to_kinfo_proc2, nbsd_pid_to_rlimit_cur)

> +	(nbsd_pid_to_statstr): New.

> +	(nbsd_nat_target::info_proc): Add do_stat.

> +

>  2020-04-12  Kamil Rytarowski  <n54@gmx.com>

> 

>  	* nbsd-nat.c (nbsd_nat_target::info_proc): Add IP_MINIMAL and

> diff --git a/gdb/nbsd-nat.c b/gdb/nbsd-nat.c

> index 5eaf9dec8af..b2a05d63122 100644

> --- a/gdb/nbsd-nat.c

> +++ b/gdb/nbsd-nat.c

> @@ -28,6 +28,8 @@

>  #include <sys/types.h>

>  #include <sys/ptrace.h>

>  #include <sys/sysctl.h>

> +#include <sys/resource.h>

> +#include <machine/vmparam.h>

> 

>  /* Return the name of a file that can be opened to get the symbols for

>     the child process identified by PID.  */

> @@ -58,6 +60,134 @@ nbsd_pid_to_cwd (int pid)

>    return buf;

>  }

> 

> +/* Return the kinfo_proc2 structure for the process identified by PID.  */

> +

> +static bool

> +nbsd_pid_to_kinfo_proc2(pid_t pid, struct kinfo_proc2 *ki)


Space.

> +{

> +  gdb_assert (ki != nullptr);

> +

> +  size_t size = sizeof (*ki);

> +  int mib[6] = {CTL_KERN, KERN_PROC2, KERN_PROC_PID, pid,

> +		static_cast<int> (size), 1};

> +  return sysctl (mib, ARRAY_SIZE (mib), ki, &size, NULL, 0);


That looks counter-intuitive.  sysctl returns 0 on success, which translates to
false as a bool.  WHen a function returns a bool it is more typical that true
means success.  This happens lower too.

> +}

> +

> +/* Return the RLIMIT value for the process identified by PID.  */

> +

> +static bool

> +nbsd_pid_to_rlimit_cur(pid_t pid, rlim_t *rlim, int type)


Space.

> +{

> +  gdb_assert (rlim != nullptr);

> +

> +  size_t size = sizeof (*rlim);

> +  int mib[5] = {CTL_PROC, pid, PROC_PID_LIMIT, type, PROC_PID_LIMIT_TYPE_SOFT};

> +  return sysctl (mib, ARRAY_SIZE (mib), rlim, &size, NULL, 0);

> +}

> +

> +/* Return the emulated /proc/#/stat string for the process identified by PID.

> +   NetBSD keeps the /proc filesystem that is optional and not recommended for

> +   new software, thus this call emulates it with sysctl(3).  */

> +

> +static gdb::unique_xmalloc_ptr<char>

> +nbsd_pid_to_statstr (int pid)

> +{

> +  struct kinfo_proc2 ki;

> +  if (nbsd_pid_to_kinfo_proc2 (pid, &ki))

> +    return nullptr;

> +

> +  struct rusage cru = {};

> +  /* We can check only the current process.  */

> +  if (getpid() == pid)


Space.

> @@ -433,6 +568,149 @@ nbsd_nat_target::info_proc (const char *args, enum info_proc_what what)

>        else

>  	warning (_("unable to fetch virtual memory map"));

>      }

> +  if (do_stat)

> +    {

> +      /* First, try with Linux-compat /proc/<pid>/stat. */

> +      char filename[100];

> +      xsnprintf (filename, sizeof filename, "/proc/%d/stat", pid);


Use

  std::string filename = string_printf (...);

> +      gdb::unique_xmalloc_ptr<char> statstr

> +	= target_fileio_read_stralloc (NULL, filename);

> +

> +      /* Then fallback to emulating /proc/<pid>/stat with sysctl(3).  */

> +      if (statstr == nullptr)

> +	statstr = nbsd_pid_to_statstr (pid);


I find it a bit strange to take the route of formatting the contents of kinfo_proc2,
only to parse it here.  I think it would have been more natural to do:

- If /proc/#/stat is available: string -> struct some_internal_struct -> print function
- If /proc/#/stat is available: kinfo_proc2 -> struct some_internal_struct -> print function

> +

> +      if (statstr)


!= nullptr

> +	{

> +	  const char *p = statstr.get ();

> +

> +	  printf_filtered (_("Process: %s\n"),

> +			   pulongest (strtoulst (p, &p, 10)));

> +

> +	  p = skip_spaces (p);

> +	  if (*p == '(')

> +	    {

> +	      /* ps command also relies on no trailing fields

> +		 ever contain ')'.  */

> +	      const char *ep = strrchr (p, ')');

> +	      if (ep != NULL)

> +		{

> +		  printf_filtered ("Exec file: %.*s\n",

> +				   (int) (ep - p - 1), p + 1);

> +		  p = ep + 1;

> +		}

> +	    }

> +

> +	  p = skip_spaces (p);

> +	  if (*p)

> +	    printf_filtered (_("State: %c\n"), *p++);

> +

> +	  if (*p)

> +	    printf_filtered (_("Parent process: %s\n"),

> +			     pulongest (strtoulst (p, &p, 10)));

> +	  if (*p)

> +	    printf_filtered (_("Process group: %s\n"),

> +			     pulongest (strtoulst (p, &p, 10)));

> +	  if (*p)

> +	    printf_filtered (_("Session id: %s\n"),

> +			     pulongest (strtoulst (p, &p, 10)));

> +	  if (*p)

> +	    printf_filtered (_("TTY: %s\n"),

> +			     pulongest (strtoulst (p, &p, 10)));

> +	  if (*p)

> +	    printf_filtered (_("TTY owner process group: %s\n"),

> +			     pulongest (strtoulst (p, &p, 10)));

> +

> +	  if (*p)

> +	    printf_filtered (_("Flags: %s\n"),

> +			     hex_string (strtoulst (p, &p, 10)));

> +	  if (*p)

> +	    printf_filtered (_("Minor faults (no memory page): %s\n"),

> +			     pulongest (strtoulst (p, &p, 10)));

> +	  if (*p)

> +	    printf_filtered (_("Minor faults, children: %s\n"),

> +			     pulongest (strtoulst (p, &p, 10)));

> +	  if (*p)

> +	    printf_filtered (_("Major faults (memory page faults): %s\n"),

> +			     pulongest (strtoulst (p, &p, 10)));

> +	  if (*p)

> +	    printf_filtered (_("Major faults, children: %s\n"),

> +			     pulongest (strtoulst (p, &p, 10)));

> +	  if (*p)

> +	    printf_filtered (_("utime: %s\n"),

> +			     pulongest (strtoulst (p, &p, 10)));

> +	  if (*p)

> +	    printf_filtered (_("stime: %s\n"),

> +			     pulongest (strtoulst (p, &p, 10)));

> +	  if (*p)

> +	    printf_filtered (_("utime, children: %s\n"),

> +			     pulongest (strtoulst (p, &p, 10)));

> +	  if (*p)

> +	    printf_filtered (_("stime, children: %s\n"),

> +			     pulongest (strtoulst (p, &p, 10)));

> +	  if (*p)

> +	    printf_filtered (_("jiffies remaining in current "

> +			       "time slice: %s\n"),

> +			     pulongest (strtoulst (p, &p, 10)));

> +	  if (*p)

> +	    printf_filtered (_("'nice' value: %s\n"),

> +			     pulongest (strtoulst (p, &p, 10)));

> +	  if (*p)

> +	    printf_filtered (_("jiffies until next timeout: %s\n"),

> +			     pulongest (strtoulst (p, &p, 10)));

> +	  if (*p)

> +	    printf_filtered (_("jiffies until next SIGALRM: %s\n"),

> +			     pulongest (strtoulst (p, &p, 10)));

> +	  if (*p)

> +	    printf_filtered (_("start time (jiffies since "

> +			       "system boot): %s\n"),

> +			     pulongest (strtoulst (p, &p, 10)));

> +	  if (*p)

> +	    printf_filtered (_("Virtual memory size: %s\n"),

> +			     pulongest (strtoulst (p, &p, 10)));

> +	  if (*p)

> +	    printf_filtered (_("Resident set size: %s\n"),

> +			     pulongest (strtoulst (p, &p, 10)));

> +	  if (*p)

> +	    printf_filtered (_("rlim: %s\n"),

> +			     pulongest (strtoulst (p, &p, 10)));

> +	  if (*p)

> +	    printf_filtered (_("Start of text: %s\n"),

> +			     hex_string (strtoulst (p, &p, 10)));

> +	  if (*p)

> +	    printf_filtered (_("End of text: %s\n"),

> +			     hex_string (strtoulst (p, &p, 10)));

> +	  if (*p)

> +	    printf_filtered (_("Start of stack: %s\n"),

> +			     hex_string (strtoulst (p, &p, 10)));

> +#if 0	/* Don't know how architecture-dependent the rest is...

> +	   Anyway the signal bitmap info is available from "status".  */


Either figure out how to print this correctly, or let's just leave it out.

Simon
Kamil Rytarowski April 13, 2020, 7:43 p.m. | #2
On 13.04.2020 21:16, Simon Marchi wrote:
> On 2020-04-13 2:19 p.m., Kamil Rytarowski wrote:

>> NetBSD ships with a compatibility layer in a /proc virtual file system

>> and delivers optionally /proc/<pid>/stat. In case of missing

>> /proc/<pid>/stat fallback to emulating it with sysctl(3) APIs.

>>

>> gdb/ChangeLog:

>>

>>         * nbsd-nat.c: Include "sys/resource.h" and "machine/vmparam.h".

>>         (nbsd_pid_to_kinfo_proc2, nbsd_pid_to_rlimit_cur)

>>         (nbsd_pid_to_statstr): New.

>>         (nbsd_nat_target::info_proc): Add do_stat.

>> ---

>>  gdb/ChangeLog  |   7 ++

>>  gdb/nbsd-nat.c | 278 +++++++++++++++++++++++++++++++++++++++++++++++++

>>  2 files changed, 285 insertions(+)

>>

>> diff --git a/gdb/ChangeLog b/gdb/ChangeLog

>> index 7d91abf6334..5ff76e85cb3 100644

>> --- a/gdb/ChangeLog

>> +++ b/gdb/ChangeLog

>> @@ -1,3 +1,10 @@

>> +2020-04-13  Kamil Rytarowski  <n54@gmx.com>

>> +

>> +	* nbsd-nat.c: Include "sys/resource.h" and "machine/vmparam.h".

>> +	(nbsd_pid_to_kinfo_proc2, nbsd_pid_to_rlimit_cur)

>> +	(nbsd_pid_to_statstr): New.

>> +	(nbsd_nat_target::info_proc): Add do_stat.

>> +

>>  2020-04-12  Kamil Rytarowski  <n54@gmx.com>

>>

>>  	* nbsd-nat.c (nbsd_nat_target::info_proc): Add IP_MINIMAL and

>> diff --git a/gdb/nbsd-nat.c b/gdb/nbsd-nat.c

>> index 5eaf9dec8af..b2a05d63122 100644

>> --- a/gdb/nbsd-nat.c

>> +++ b/gdb/nbsd-nat.c

>> @@ -28,6 +28,8 @@

>>  #include <sys/types.h>

>>  #include <sys/ptrace.h>

>>  #include <sys/sysctl.h>

>> +#include <sys/resource.h>

>> +#include <machine/vmparam.h>

>>

>>  /* Return the name of a file that can be opened to get the symbols for

>>     the child process identified by PID.  */

>> @@ -58,6 +60,134 @@ nbsd_pid_to_cwd (int pid)

>>    return buf;

>>  }

>>

>> +/* Return the kinfo_proc2 structure for the process identified by PID.  */

>> +

>> +static bool

>> +nbsd_pid_to_kinfo_proc2(pid_t pid, struct kinfo_proc2 *ki)

>

> Space.

>


OK

>> +{

>> +  gdb_assert (ki != nullptr);

>> +

>> +  size_t size = sizeof (*ki);

>> +  int mib[6] = {CTL_KERN, KERN_PROC2, KERN_PROC_PID, pid,

>> +		static_cast<int> (size), 1};

>> +  return sysctl (mib, ARRAY_SIZE (mib), ki, &size, NULL, 0);

>

> That looks counter-intuitive.  sysctl returns 0 on success, which translates to

> false as a bool.  WHen a function returns a bool it is more typical that true

> means success.  This happens lower too.

>


OK

>> +}

>> +

>> +/* Return the RLIMIT value for the process identified by PID.  */

>> +

>> +static bool

>> +nbsd_pid_to_rlimit_cur(pid_t pid, rlim_t *rlim, int type)

>

> Space.

>


OK

>> +{

>> +  gdb_assert (rlim != nullptr);

>> +

>> +  size_t size = sizeof (*rlim);

>> +  int mib[5] = {CTL_PROC, pid, PROC_PID_LIMIT, type, PROC_PID_LIMIT_TYPE_SOFT};

>> +  return sysctl (mib, ARRAY_SIZE (mib), rlim, &size, NULL, 0);

>> +}

>> +

>> +/* Return the emulated /proc/#/stat string for the process identified by PID.

>> +   NetBSD keeps the /proc filesystem that is optional and not recommended for

>> +   new software, thus this call emulates it with sysctl(3).  */

>> +

>> +static gdb::unique_xmalloc_ptr<char>

>> +nbsd_pid_to_statstr (int pid)

>> +{

>> +  struct kinfo_proc2 ki;

>> +  if (nbsd_pid_to_kinfo_proc2 (pid, &ki))

>> +    return nullptr;

>> +

>> +  struct rusage cru = {};

>> +  /* We can check only the current process.  */

>> +  if (getpid() == pid)

>

> Space.

>


OK

>> @@ -433,6 +568,149 @@ nbsd_nat_target::info_proc (const char *args, enum info_proc_what what)

>>        else

>>  	warning (_("unable to fetch virtual memory map"));

>>      }

>> +  if (do_stat)

>> +    {

>> +      /* First, try with Linux-compat /proc/<pid>/stat. */

>> +      char filename[100];

>> +      xsnprintf (filename, sizeof filename, "/proc/%d/stat", pid);

>

> Use

>

>   std::string filename = string_printf (...);

>


OK. Meanwhile linux-tdep.c could be upgraded by a Linux maintainer.

>> +      gdb::unique_xmalloc_ptr<char> statstr

>> +	= target_fileio_read_stralloc (NULL, filename);

>> +

>> +      /* Then fallback to emulating /proc/<pid>/stat with sysctl(3).  */

>> +      if (statstr == nullptr)

>> +	statstr = nbsd_pid_to_statstr (pid);

>

> I find it a bit strange to take the route of formatting the contents of kinfo_proc2,

> only to parse it here.  I think it would have been more natural to do:

>

> - If /proc/#/stat is available: string -> struct some_internal_struct -> print function

> - If /proc/#/stat is available: kinfo_proc2 -> struct some_internal_struct -> print function

>



This compat shim is only to emulate Linux... it's possible to follow the
FreeBSD case and just print local kinfo_proc2 as-is (that is fully
OS-specific).

What do you think?

The same question applies to IP_STATUS.

>> +

>> +      if (statstr)

>

> != nullptr

>


OK

>> +	{

>> +	  const char *p = statstr.get ();

>> +

>> +	  printf_filtered (_("Process: %s\n"),

>> +			   pulongest (strtoulst (p, &p, 10)));

>> +

>> +	  p = skip_spaces (p);

>> +	  if (*p == '(')

>> +	    {

>> +	      /* ps command also relies on no trailing fields

>> +		 ever contain ')'.  */

>> +	      const char *ep = strrchr (p, ')');

>> +	      if (ep != NULL)

>> +		{

>> +		  printf_filtered ("Exec file: %.*s\n",

>> +				   (int) (ep - p - 1), p + 1);

>> +		  p = ep + 1;

>> +		}

>> +	    }

>> +

>> +	  p = skip_spaces (p);

>> +	  if (*p)

>> +	    printf_filtered (_("State: %c\n"), *p++);

>> +

>> +	  if (*p)

>> +	    printf_filtered (_("Parent process: %s\n"),

>> +			     pulongest (strtoulst (p, &p, 10)));

>> +	  if (*p)

>> +	    printf_filtered (_("Process group: %s\n"),

>> +			     pulongest (strtoulst (p, &p, 10)));

>> +	  if (*p)

>> +	    printf_filtered (_("Session id: %s\n"),

>> +			     pulongest (strtoulst (p, &p, 10)));

>> +	  if (*p)

>> +	    printf_filtered (_("TTY: %s\n"),

>> +			     pulongest (strtoulst (p, &p, 10)));

>> +	  if (*p)

>> +	    printf_filtered (_("TTY owner process group: %s\n"),

>> +			     pulongest (strtoulst (p, &p, 10)));

>> +

>> +	  if (*p)

>> +	    printf_filtered (_("Flags: %s\n"),

>> +			     hex_string (strtoulst (p, &p, 10)));

>> +	  if (*p)

>> +	    printf_filtered (_("Minor faults (no memory page): %s\n"),

>> +			     pulongest (strtoulst (p, &p, 10)));

>> +	  if (*p)

>> +	    printf_filtered (_("Minor faults, children: %s\n"),

>> +			     pulongest (strtoulst (p, &p, 10)));

>> +	  if (*p)

>> +	    printf_filtered (_("Major faults (memory page faults): %s\n"),

>> +			     pulongest (strtoulst (p, &p, 10)));

>> +	  if (*p)

>> +	    printf_filtered (_("Major faults, children: %s\n"),

>> +			     pulongest (strtoulst (p, &p, 10)));

>> +	  if (*p)

>> +	    printf_filtered (_("utime: %s\n"),

>> +			     pulongest (strtoulst (p, &p, 10)));

>> +	  if (*p)

>> +	    printf_filtered (_("stime: %s\n"),

>> +			     pulongest (strtoulst (p, &p, 10)));

>> +	  if (*p)

>> +	    printf_filtered (_("utime, children: %s\n"),

>> +			     pulongest (strtoulst (p, &p, 10)));

>> +	  if (*p)

>> +	    printf_filtered (_("stime, children: %s\n"),

>> +			     pulongest (strtoulst (p, &p, 10)));

>> +	  if (*p)

>> +	    printf_filtered (_("jiffies remaining in current "

>> +			       "time slice: %s\n"),

>> +			     pulongest (strtoulst (p, &p, 10)));

>> +	  if (*p)

>> +	    printf_filtered (_("'nice' value: %s\n"),

>> +			     pulongest (strtoulst (p, &p, 10)));

>> +	  if (*p)

>> +	    printf_filtered (_("jiffies until next timeout: %s\n"),

>> +			     pulongest (strtoulst (p, &p, 10)));

>> +	  if (*p)

>> +	    printf_filtered (_("jiffies until next SIGALRM: %s\n"),

>> +			     pulongest (strtoulst (p, &p, 10)));

>> +	  if (*p)

>> +	    printf_filtered (_("start time (jiffies since "

>> +			       "system boot): %s\n"),

>> +			     pulongest (strtoulst (p, &p, 10)));

>> +	  if (*p)

>> +	    printf_filtered (_("Virtual memory size: %s\n"),

>> +			     pulongest (strtoulst (p, &p, 10)));

>> +	  if (*p)

>> +	    printf_filtered (_("Resident set size: %s\n"),

>> +			     pulongest (strtoulst (p, &p, 10)));

>> +	  if (*p)

>> +	    printf_filtered (_("rlim: %s\n"),

>> +			     pulongest (strtoulst (p, &p, 10)));

>> +	  if (*p)

>> +	    printf_filtered (_("Start of text: %s\n"),

>> +			     hex_string (strtoulst (p, &p, 10)));

>> +	  if (*p)

>> +	    printf_filtered (_("End of text: %s\n"),

>> +			     hex_string (strtoulst (p, &p, 10)));

>> +	  if (*p)

>> +	    printf_filtered (_("Start of stack: %s\n"),

>> +			     hex_string (strtoulst (p, &p, 10)));

>> +#if 0	/* Don't know how architecture-dependent the rest is...

>> +	   Anyway the signal bitmap info is available from "status".  */

>

> Either figure out how to print this correctly, or let's just leave it out.

>


This comment was copied as-is from gdb/linux-tdep.c.

> Simon

>
Simon Marchi April 13, 2020, 8:11 p.m. | #3
On 2020-04-13 3:43 p.m., Kamil Rytarowski wrote:
>>> @@ -433,6 +568,149 @@ nbsd_nat_target::info_proc (const char *args, enum info_proc_what what)

>>>        else

>>>  	warning (_("unable to fetch virtual memory map"));

>>>      }

>>> +  if (do_stat)

>>> +    {

>>> +      /* First, try with Linux-compat /proc/<pid>/stat. */

>>> +      char filename[100];

>>> +      xsnprintf (filename, sizeof filename, "/proc/%d/stat", pid);

>>

>> Use

>>

>>   std::string filename = string_printf (...);

>>

> 

> OK. Meanwhile linux-tdep.c could be upgraded by a Linux maintainer.


Ok

>>> +      gdb::unique_xmalloc_ptr<char> statstr

>>> +	= target_fileio_read_stralloc (NULL, filename);

>>> +

>>> +      /* Then fallback to emulating /proc/<pid>/stat with sysctl(3).  */

>>> +      if (statstr == nullptr)

>>> +	statstr = nbsd_pid_to_statstr (pid);

>>

>> I find it a bit strange to take the route of formatting the contents of kinfo_proc2,

>> only to parse it here.  I think it would have been more natural to do:

>>

>> - If /proc/#/stat is available: string -> struct some_internal_struct -> print function

>> - If /proc/#/stat is available: kinfo_proc2 -> struct some_internal_struct -> print function

>>

> 

> 

> This compat shim is only to emulate Linux... it's possible to follow the

> FreeBSD case and just print local kinfo_proc2 as-is (that is fully

> OS-specific).

> 

> What do you think?

> 

> The same question applies to IP_STATUS.


When you put it like this... it just seems useless to read the info from /proc.

If sysctl is guaranteed to always be there and is the blessed way of getting the
information, then let's just use that.  The goal is not to emulate Linux, but to
use each OS's interface to get the required information.
>>> +#if 0	/* Don't know how architecture-dependent the rest is...

>>> +	   Anyway the signal bitmap info is available from "status".  */

>>

>> Either figure out how to print this correctly, or let's just leave it out.

>>

> 

> This comment was copied as-is from gdb/linux-tdep.c.


In my opinion it's not great to leave this as-is in linux-tdep.c either, so I'd
suggest to avoid spreading it.

Simon
Kamil Rytarowski April 13, 2020, 8:26 p.m. | #4
On 13.04.2020 22:11, Simon Marchi wrote:
>> This compat shim is only to emulate Linux... it's possible to follow the

>> FreeBSD case and just print local kinfo_proc2 as-is (that is fully

>> OS-specific).

>>

>> What do you think?

>>

>> The same question applies to IP_STATUS.

> When you put it like this... it just seems useless to read the info from /proc.

>

> If sysctl is guaranteed to always be there and is the blessed way of getting the

> information, then let's just use that.


As the kernel emulates Linux in /proc, the stat file is more complete in
the kernel as there are missing fields (sys/rusage.h ones, eip, esp,
stack position etc)

>  The goal is not to emulate Linux, but to

> use each OS's interface to get the required information.


Is it fine to print NetBSD specific internals from kinfo_proc2 in
NetBSD-specific format? Making up Linux-like output will be always
imperfect, but on the other hand end-users can be confused.
Simon Marchi April 13, 2020, 8:43 p.m. | #5
On 2020-04-13 4:26 p.m., Kamil Rytarowski wrote:
> On 13.04.2020 22:11, Simon Marchi wrote:

>>> This compat shim is only to emulate Linux... it's possible to follow the

>>> FreeBSD case and just print local kinfo_proc2 as-is (that is fully

>>> OS-specific).

>>>

>>> What do you think?

>>>

>>> The same question applies to IP_STATUS.

>> When you put it like this... it just seems useless to read the info from /proc.

>>

>> If sysctl is guaranteed to always be there and is the blessed way of getting the

>> information, then let's just use that.

> 

> As the kernel emulates Linux in /proc, the stat file is more complete in

> the kernel as there are missing fields (sys/rusage.h ones, eip, esp,

> stack position etc)


Ok, so if you'd like to have this extra information printed, then it could make
sense to try /proc first (if you think it's worth the extra complexity).

>>  The goal is not to emulate Linux, but to

>> use each OS's interface to get the required information.

> 

> Is it fine to print NetBSD specific internals from kinfo_proc2 in

> NetBSD-specific format? Making up Linux-like output will be always

> imperfect, but on the other hand end-users can be confused.


You would have to give some specific examples, but in general it's fine for different
OSes to print different information.  If some process property only exists in NetBSD,
then it should be printed here.  If some process property exists in Linux but not
NetBSD, then it should not be printed here.  Although when we print the same things, we
should strive to print them in a consistent way.

Simon

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 7d91abf6334..5ff76e85cb3 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@ 
+2020-04-13  Kamil Rytarowski  <n54@gmx.com>
+
+	* nbsd-nat.c: Include "sys/resource.h" and "machine/vmparam.h".
+	(nbsd_pid_to_kinfo_proc2, nbsd_pid_to_rlimit_cur)
+	(nbsd_pid_to_statstr): New.
+	(nbsd_nat_target::info_proc): Add do_stat.
+
 2020-04-12  Kamil Rytarowski  <n54@gmx.com>

 	* nbsd-nat.c (nbsd_nat_target::info_proc): Add IP_MINIMAL and
diff --git a/gdb/nbsd-nat.c b/gdb/nbsd-nat.c
index 5eaf9dec8af..b2a05d63122 100644
--- a/gdb/nbsd-nat.c
+++ b/gdb/nbsd-nat.c
@@ -28,6 +28,8 @@ 
 #include <sys/types.h>
 #include <sys/ptrace.h>
 #include <sys/sysctl.h>
+#include <sys/resource.h>
+#include <machine/vmparam.h>

 /* Return the name of a file that can be opened to get the symbols for
    the child process identified by PID.  */
@@ -58,6 +60,134 @@  nbsd_pid_to_cwd (int pid)
   return buf;
 }

+/* Return the kinfo_proc2 structure for the process identified by PID.  */
+
+static bool
+nbsd_pid_to_kinfo_proc2(pid_t pid, struct kinfo_proc2 *ki)
+{
+  gdb_assert (ki != nullptr);
+
+  size_t size = sizeof (*ki);
+  int mib[6] = {CTL_KERN, KERN_PROC2, KERN_PROC_PID, pid,
+		static_cast<int> (size), 1};
+  return sysctl (mib, ARRAY_SIZE (mib), ki, &size, NULL, 0);
+}
+
+/* Return the RLIMIT value for the process identified by PID.  */
+
+static bool
+nbsd_pid_to_rlimit_cur(pid_t pid, rlim_t *rlim, int type)
+{
+  gdb_assert (rlim != nullptr);
+
+  size_t size = sizeof (*rlim);
+  int mib[5] = {CTL_PROC, pid, PROC_PID_LIMIT, type, PROC_PID_LIMIT_TYPE_SOFT};
+  return sysctl (mib, ARRAY_SIZE (mib), rlim, &size, NULL, 0);
+}
+
+/* Return the emulated /proc/#/stat string for the process identified by PID.
+   NetBSD keeps the /proc filesystem that is optional and not recommended for
+   new software, thus this call emulates it with sysctl(3).  */
+
+static gdb::unique_xmalloc_ptr<char>
+nbsd_pid_to_statstr (int pid)
+{
+  struct kinfo_proc2 ki;
+  if (nbsd_pid_to_kinfo_proc2 (pid, &ki))
+    return nullptr;
+
+  struct rusage cru = {};
+  /* We can check only the current process.  */
+  if (getpid() == pid)
+    getrusage (RUSAGE_SELF, &cru);
+
+  rlim_t rsslim_cur;
+  if (nbsd_pid_to_rlimit_cur (pid, &rsslim_cur, PROC_PID_LIMIT_RSS))
+    return nullptr;
+
+  auto print_p_stat
+    = [] (int8_t state)
+      {
+	return "0RRSTZXR8"[(state > 8) ? 0 : state];
+      };
+
+  auto print_p_tdev
+    = [] (uint32_t dev)
+      {
+	return (dev != static_cast<uint32_t> (NODEV)) ? dev : 0;
+      };
+
+  auto print_utime2ticks
+    = [] (uint32_t sec, uint32_t usec)
+      {
+	return (static_cast<uint64_t> (sec) * 1000000 + (usec)) / 10000;
+      };
+
+  auto print_pgtokb
+    = [] (int32_t rsssize)
+      {
+	return static_cast<unsigned long int> (rsssize) << (PAGE_SHIFT - 10);
+      };
+
+  std::string statstr
+    = string_printf("%d (%s) %c %d %d %d %u %d "
+		    "%u "
+		    "%" PRIu64 " %lu %" PRIu64 " %lu %" PRIu64 " %" PRIu64 " "
+		    "%" PRIu64 " %" PRIu64 " "
+		    "%d %d %" PRIu64 " "
+		    "%lld %" PRIu64 " %" PRId64 " %lu %" PRIu64 " "
+		    "%lu %lu %lu "
+		    "%u %u "
+		    "%u %u %u %u "
+		    "%" PRIu64 " %" PRIu64 " %" PRIu64 " %d %" PRIu64 "\n",
+		    ki.p_pid,                               /* 1 pid */
+		    ki.p_comm,                              /* 2 tcomm */
+		    print_p_stat(ki.p_stat),                /* 3 state */
+		    ki.p_ppid,                              /* 4 ppid */
+		    ki.p__pgid,                             /* 5 pgrp */
+		    ki.p_sid,                               /* 6 sid */
+		    print_p_tdev(ki.p_tdev),                /* 7 tty_nr */
+		    ki.p_tpgid,                             /* 8 tty_pgrp */
+		    ki.p_flag,                              /* 9 flags */
+		    ki.p_uru_minflt,                        /* 10 min_flt */
+		    cru.ru_minflt,
+		    ki.p_uru_majflt,                        /* 12 maj_flt */
+		    cru.ru_majflt,
+		    print_utime2ticks(ki.p_uutime_sec,
+				      ki.p_uutime_usec),    /* 14 utime */
+		    print_utime2ticks(ki.p_ustime_sec,
+				      ki.p_ustime_usec),    /* 15 stime */
+		    print_utime2ticks(cru.ru_utime.tv_sec,
+				      cru.ru_utime.tv_usec),/* 16 cutime */
+		    print_utime2ticks(cru.ru_stime.tv_sec,
+				      cru.ru_stime.tv_usec),/* 17 cstime */
+		    ki.p_priority,                          /* 18 priority */
+		    ki.p_nice - NZERO,                      /* 19 nice */
+		    ki.p_nlwps,                             /* 20 num_threads */
+		    static_cast<long long int> (ki.p_rtime_sec),
+		    print_utime2ticks(ki.p_ustart_sec,
+				      ki.p_ustart_usec),    /* 22 start_time */
+		    ki.p_vm_msize,                          /* 23 vsize */
+		    print_pgtokb(ki.p_vm_rssize),           /* 24 rss */
+		    rsslim_cur,                             /* 25 rsslim */
+		    0L,                                     /* 26 start_code */
+		    0L,                                     /* 27 end_code */
+		    0L,                                     /* 28 start_stack */
+		    0,                                      /* 29 esp */
+		    0,                                      /* 30 eip */
+		    ki.p_siglist.__bits[0],                 /* 31 pending */
+		    0,                                      /* 32 blocked */
+		    ki.p_sigignore.__bits[0],               /* 33 sigign */
+		    ki.p_sigcatch.__bits[0],                /* 34 sigcatch */
+		    ki.p_wchan,                             /* 35 wchan */
+		    ki.p_uru_nvcsw,
+		    ki.p_uru_nivcsw,
+		    ki.p_exitsig,                           /* 38 exit_signal */
+		    ki.p_cpuid);                            /* 39 task_cpu */
+
+  return gdb::unique_xmalloc_ptr<char> (xstrprintf ("%s", statstr.c_str ()));
+}
+
 /* Return the command line for the process identified by PID.  */

 static gdb::unique_xmalloc_ptr<char[]>
@@ -344,6 +474,7 @@  nbsd_nat_target::info_proc (const char *args, enum info_proc_what what)
   bool do_cwd = false;
   bool do_exe = false;
   bool do_mappings = false;
+  bool do_stat = false;

   switch (what)
     {
@@ -352,6 +483,9 @@  nbsd_nat_target::info_proc (const char *args, enum info_proc_what what)
       do_cwd = true;
       do_exe = true;
       break;
+    case IP_STAT:
+      do_stat = true;
+      break;
     case IP_MAPPINGS:
       do_mappings = true;
       break;
@@ -369,6 +503,7 @@  nbsd_nat_target::info_proc (const char *args, enum info_proc_what what)
       do_cwd = true;
       do_exe = true;
       do_mappings = true;
+      do_stat = true;
       break;
     default:
       error (_("Not supported on this target."));
@@ -433,6 +568,149 @@  nbsd_nat_target::info_proc (const char *args, enum info_proc_what what)
       else
 	warning (_("unable to fetch virtual memory map"));
     }
+  if (do_stat)
+    {
+      /* First, try with Linux-compat /proc/<pid>/stat. */
+      char filename[100];
+      xsnprintf (filename, sizeof filename, "/proc/%d/stat", pid);
+      gdb::unique_xmalloc_ptr<char> statstr
+	= target_fileio_read_stralloc (NULL, filename);
+
+      /* Then fallback to emulating /proc/<pid>/stat with sysctl(3).  */
+      if (statstr == nullptr)
+	statstr = nbsd_pid_to_statstr (pid);
+
+      if (statstr)
+	{
+	  const char *p = statstr.get ();
+
+	  printf_filtered (_("Process: %s\n"),
+			   pulongest (strtoulst (p, &p, 10)));
+
+	  p = skip_spaces (p);
+	  if (*p == '(')
+	    {
+	      /* ps command also relies on no trailing fields
+		 ever contain ')'.  */
+	      const char *ep = strrchr (p, ')');
+	      if (ep != NULL)
+		{
+		  printf_filtered ("Exec file: %.*s\n",
+				   (int) (ep - p - 1), p + 1);
+		  p = ep + 1;
+		}
+	    }
+
+	  p = skip_spaces (p);
+	  if (*p)
+	    printf_filtered (_("State: %c\n"), *p++);
+
+	  if (*p)
+	    printf_filtered (_("Parent process: %s\n"),
+			     pulongest (strtoulst (p, &p, 10)));
+	  if (*p)
+	    printf_filtered (_("Process group: %s\n"),
+			     pulongest (strtoulst (p, &p, 10)));
+	  if (*p)
+	    printf_filtered (_("Session id: %s\n"),
+			     pulongest (strtoulst (p, &p, 10)));
+	  if (*p)
+	    printf_filtered (_("TTY: %s\n"),
+			     pulongest (strtoulst (p, &p, 10)));
+	  if (*p)
+	    printf_filtered (_("TTY owner process group: %s\n"),
+			     pulongest (strtoulst (p, &p, 10)));
+
+	  if (*p)
+	    printf_filtered (_("Flags: %s\n"),
+			     hex_string (strtoulst (p, &p, 10)));
+	  if (*p)
+	    printf_filtered (_("Minor faults (no memory page): %s\n"),
+			     pulongest (strtoulst (p, &p, 10)));
+	  if (*p)
+	    printf_filtered (_("Minor faults, children: %s\n"),
+			     pulongest (strtoulst (p, &p, 10)));
+	  if (*p)
+	    printf_filtered (_("Major faults (memory page faults): %s\n"),
+			     pulongest (strtoulst (p, &p, 10)));
+	  if (*p)
+	    printf_filtered (_("Major faults, children: %s\n"),
+			     pulongest (strtoulst (p, &p, 10)));
+	  if (*p)
+	    printf_filtered (_("utime: %s\n"),
+			     pulongest (strtoulst (p, &p, 10)));
+	  if (*p)
+	    printf_filtered (_("stime: %s\n"),
+			     pulongest (strtoulst (p, &p, 10)));
+	  if (*p)
+	    printf_filtered (_("utime, children: %s\n"),
+			     pulongest (strtoulst (p, &p, 10)));
+	  if (*p)
+	    printf_filtered (_("stime, children: %s\n"),
+			     pulongest (strtoulst (p, &p, 10)));
+	  if (*p)
+	    printf_filtered (_("jiffies remaining in current "
+			       "time slice: %s\n"),
+			     pulongest (strtoulst (p, &p, 10)));
+	  if (*p)
+	    printf_filtered (_("'nice' value: %s\n"),
+			     pulongest (strtoulst (p, &p, 10)));
+	  if (*p)
+	    printf_filtered (_("jiffies until next timeout: %s\n"),
+			     pulongest (strtoulst (p, &p, 10)));
+	  if (*p)
+	    printf_filtered (_("jiffies until next SIGALRM: %s\n"),
+			     pulongest (strtoulst (p, &p, 10)));
+	  if (*p)
+	    printf_filtered (_("start time (jiffies since "
+			       "system boot): %s\n"),
+			     pulongest (strtoulst (p, &p, 10)));
+	  if (*p)
+	    printf_filtered (_("Virtual memory size: %s\n"),
+			     pulongest (strtoulst (p, &p, 10)));
+	  if (*p)
+	    printf_filtered (_("Resident set size: %s\n"),
+			     pulongest (strtoulst (p, &p, 10)));
+	  if (*p)
+	    printf_filtered (_("rlim: %s\n"),
+			     pulongest (strtoulst (p, &p, 10)));
+	  if (*p)
+	    printf_filtered (_("Start of text: %s\n"),
+			     hex_string (strtoulst (p, &p, 10)));
+	  if (*p)
+	    printf_filtered (_("End of text: %s\n"),
+			     hex_string (strtoulst (p, &p, 10)));
+	  if (*p)
+	    printf_filtered (_("Start of stack: %s\n"),
+			     hex_string (strtoulst (p, &p, 10)));
+#if 0	/* Don't know how architecture-dependent the rest is...
+	   Anyway the signal bitmap info is available from "status".  */
+	  if (*p)
+	    printf_filtered (_("Kernel stack pointer: %s\n"),
+			     hex_string (strtoulst (p, &p, 10)));
+	  if (*p)
+	    printf_filtered (_("Kernel instr pointer: %s\n"),
+			     hex_string (strtoulst (p, &p, 10)));
+	  if (*p)
+	    printf_filtered (_("Pending signals bitmap: %s\n"),
+			     hex_string (strtoulst (p, &p, 10)));
+	  if (*p)
+	    printf_filtered (_("Blocked signals bitmap: %s\n"),
+			     hex_string (strtoulst (p, &p, 10)));
+	  if (*p)
+	    printf_filtered (_("Ignored signals bitmap: %s\n"),
+			     hex_string (strtoulst (p, &p, 10)));
+	  if (*p)
+	    printf_filtered (_("Catched signals bitmap: %s\n"),
+			     hex_string (strtoulst (p, &p, 10)));
+	  if (*p)
+	    printf_filtered (_("wchan (system call): %s\n"),
+			     hex_string (strtoulst (p, &p, 10)));
+#endif
+	}
+      else
+	warning (_("unable to open /proc file '%s'"), filename);
+    }

   return true;
 }