[PATCH 0/5] Add a new 'info proc files' command

[PATCH 0/5] Add a new 'info proc files' command

[John Baldwin]

This series adds a new 'info proc files' subcommand of 'info proc'.
This subcommand lists information about the open file descriptors of a
process similar to how 'info proc mappings' lists information about
the active virtual memory mappings of a process.  The series includes
support for reading the list of file descriptors from both process
core dumps and live processes under FreeBSD.

I've included some sample output below from a live ssh process.  One
possibly odd thing to note is that FreeBSD includes some "special"
files in the list of open files that are not actual file descriptors,
but other files that each process references such as the current
working directory, the root directory (affected by chroot), the
controlling tty, etc.

(gdb) info proc files 22136
process 22136
Open files:

      FD   Type     Offset   Flags   Name
    text   file          - r-------- /usr/bin/slogin
    ctty    chr          - rw------- /dev/pts/20
     cwd    dir          - r-------- /usr/home/john
    root    dir          - r-------- /
       0    chr  0x32933a4 rw------- /dev/pts/20
       1    chr  0x32933a4 rw------- /dev/pts/20
       2    chr  0x32933a4 rw------- /dev/pts/20
       3 socket        0x0 rw----n-- tcp4 10.0.1.2:53014 -> 10.0.1.10:22
       4 socket        0x0 rw------- unix stream:/tmp/ssh-FIt89oAzOn5f/agent.2456
       5    chr  0x32933a4 rw------- /dev/pts/20
       6    chr  0x32933a4 rw------- /dev/pts/20
       7    chr  0x32933a4 rw------- /dev/pts/20


John Baldwin (5):
  Use KF_PATH to verify the size of a struct kinfo_file.
  Add a new 'info proc files' subcommand of 'info proc'.
  Add support for 'info proc files' on FreeBSD core dumps.
  Support 'info proc files' on live FreeBSD processes.
  Document the 'info proc files' command.

 gdb/ChangeLog       |  51 +++++
 gdb/NEWS            |   3 +
 gdb/defs.h          |   3 +
 gdb/doc/ChangeLog   |   5 +
 gdb/doc/gdb.texinfo |   8 +
 gdb/fbsd-nat.c      |  44 ++++-
 gdb/fbsd-tdep.c     | 443 +++++++++++++++++++++++++++++++++++++++++++-
 gdb/fbsd-tdep.h     |  31 ++++
 gdb/infcmd.c        |  12 ++
 9 files changed, 597 insertions(+), 3 deletions(-)

-- 
2.18.0

[PATCH 3/5] Add support for 'info proc files' on FreeBSD core dumps.

[John Baldwin]

Walk the list of struct kinfo_file objects in the
NT_FREEBSD_PROCSTAT_FILES core dump note outputting a description of
each open file descriptor.  For sockets, the local and remote socket
addresses are displayed in place of the file name field.  For UNIX
local domain sockets, only a single address is displayed since most
UNIX sockets only have one valid address and printing both pathnames
could be quite long.  The output format was somewhat inspired by the
output of the "procstat -f" command on FreeBSD, but with a few less
details and some fields were condensed.

gdb/ChangeLog:

	* fbsd-tdep.c (KF_FLAGS, KF_OFFSET, KF_VNODE_TYPE, KF_SOCK_DOMAIN)
	(KF_SOCK_TYPE, KF_SOCK_PROTOCOL, KF_SA_LOCAL, KF_SA_PEER)
	(KINFO_FILE_TYPE_SOCKET, KINFO_FILE_TYPE_PIPE)
	(KINFO_FILE_TYPE_FIFO, KINFO_FILE_TYPE_KQUEUE)
	(KINFO_FILE_TYPE_CRYPTO, KINFO_FILE_TYPE_MQUEUE)
	(KINFO_FILE_TYPE_SHM, KINFO_FILE_TYPE_SEM, KINFO_FILE_TYPE_PTS)
	(KINFO_FILE_TYPE_PROCDESC, KINFO_FILE_FD_TYPE_ROOT)
	(KINFO_FILE_FD_TYPE_JAIL, KINFO_FILE_FD_TYPE_TRACE)
	(KINFO_FILE_FD_TYPE_CTTY, KINFO_FILE_FLAG_READ)
	(KINFO_FILE_FLAG_WRITE, KINFO_FILE_FLAG_APPEND)
	(KINFO_FILE_FLAG_ASYNC, KINFO_FILE_FLAG_FSYNC)
	(KINFO_FILE_FLAG_NONBLOCK, KINFO_FILE_FLAG_DIRECT)
	(KINFO_FILE_FLAG_HASLOCK, KINFO_FILE_FLAG_EXEC)
	(KINFO_FILE_VTYPE_VREG, KINFO_FILE_VTYPE_VDIR)
	(KINFO_FILE_VTYPE_VCHR, KINFO_FILE_VTYPE_VLNK)
	(KINFO_FILE_VTYPE_VSOCK, KINFO_FILE_VTYPE_VFIFO, FBSD_AF_UNIX)
	(FBSD_AF_INET, FBSD_AF_INET6, FBSD_SOCK_STREAM, FBSD_SOCK_DGRAM)
	(FBSD_SOCK_SEQPACKET, FBSD_IPPROTO_ICMP, FBSD_IPPROTO_TCP)
	(FBSD_IPPROTO_UDP, FBSD_IPPROTO_SCTP): New defines.
	(struct fbsd_sockaddr_in, struct fbsd_sockaddr_in6)
	(struct fbsd_sockaddr_un): New types.
	(fbsd_file_fd, fbsd_file_type, fbsd_file_flags, fbsd_ipproto)
	(fbsd_print_sockaddr_in, fbsd_print_sockaddr_in6)
	(fbsd_print_socket, fbsd_core_info_proc_files): New functions.
	(fbsd_core_info_proc): List open file descriptors for IP_FILES and
	IP_ALL.
	* fbsd-tdep.h (fbsd_file_fd, fbsd_file_type, fbsd_file_flags)
	(fbsd_print_socket): New.
---
 gdb/ChangeLog   |  31 ++++
 gdb/fbsd-tdep.c | 439 ++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/fbsd-tdep.h |  31 ++++
 3 files changed, 501 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 2e5cd0a687..e3d9f40d02 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,34 @@
+2018-09-07  John Baldwin  <jhb@FreeBSD.org>
+
+	* fbsd-tdep.c (KF_FLAGS, KF_OFFSET, KF_VNODE_TYPE, KF_SOCK_DOMAIN)
+	(KF_SOCK_TYPE, KF_SOCK_PROTOCOL, KF_SA_LOCAL, KF_SA_PEER)
+	(KINFO_FILE_TYPE_SOCKET, KINFO_FILE_TYPE_PIPE)
+	(KINFO_FILE_TYPE_FIFO, KINFO_FILE_TYPE_KQUEUE)
+	(KINFO_FILE_TYPE_CRYPTO, KINFO_FILE_TYPE_MQUEUE)
+	(KINFO_FILE_TYPE_SHM, KINFO_FILE_TYPE_SEM, KINFO_FILE_TYPE_PTS)
+	(KINFO_FILE_TYPE_PROCDESC, KINFO_FILE_FD_TYPE_ROOT)
+	(KINFO_FILE_FD_TYPE_JAIL, KINFO_FILE_FD_TYPE_TRACE)
+	(KINFO_FILE_FD_TYPE_CTTY, KINFO_FILE_FLAG_READ)
+	(KINFO_FILE_FLAG_WRITE, KINFO_FILE_FLAG_APPEND)
+	(KINFO_FILE_FLAG_ASYNC, KINFO_FILE_FLAG_FSYNC)
+	(KINFO_FILE_FLAG_NONBLOCK, KINFO_FILE_FLAG_DIRECT)
+	(KINFO_FILE_FLAG_HASLOCK, KINFO_FILE_FLAG_EXEC)
+	(KINFO_FILE_VTYPE_VREG, KINFO_FILE_VTYPE_VDIR)
+	(KINFO_FILE_VTYPE_VCHR, KINFO_FILE_VTYPE_VLNK)
+	(KINFO_FILE_VTYPE_VSOCK, KINFO_FILE_VTYPE_VFIFO, FBSD_AF_UNIX)
+	(FBSD_AF_INET, FBSD_AF_INET6, FBSD_SOCK_STREAM, FBSD_SOCK_DGRAM)
+	(FBSD_SOCK_SEQPACKET, FBSD_IPPROTO_ICMP, FBSD_IPPROTO_TCP)
+	(FBSD_IPPROTO_UDP, FBSD_IPPROTO_SCTP): New defines.
+	(struct fbsd_sockaddr_in, struct fbsd_sockaddr_in6)
+	(struct fbsd_sockaddr_un): New types.
+	(fbsd_file_fd, fbsd_file_type, fbsd_file_flags, fbsd_ipproto)
+	(fbsd_print_sockaddr_in, fbsd_print_sockaddr_in6)
+	(fbsd_print_socket, fbsd_core_info_proc_files): New functions.
+	(fbsd_core_info_proc): List open file descriptors for IP_FILES and
+	IP_ALL.
+	* fbsd-tdep.h (fbsd_file_fd, fbsd_file_type, fbsd_file_flags)
+	(fbsd_print_socket): New.
+
 2018-09-07  John Baldwin  <jhb@FreeBSD.org>
 
 	* defs.h (enum info_proc_what) [IP_FILES]: New value.
diff --git a/gdb/fbsd-tdep.c b/gdb/fbsd-tdep.c
index 9e6d7276c4..a8b5b2f146 100644
--- a/gdb/fbsd-tdep.c
+++ b/gdb/fbsd-tdep.c
@@ -90,18 +90,115 @@
 #define	KF_STRUCTSIZE		0x0
 #define	KF_TYPE			0x4
 #define	KF_FD			0x8
+#define	KF_FLAGS		0x10
+#define	KF_OFFSET		0x18
+#define	KF_VNODE_TYPE		0x20
+#define	KF_SOCK_DOMAIN		0x24
+#define	KF_SOCK_TYPE		0x28
+#define	KF_SOCK_PROTOCOL	0x2c
+#define	KF_SA_LOCAL		0x30
+#define	KF_SA_PEER		0xb0
 #define	KF_PATH			0x170
 
 /* Constants for the 'kf_type' field in struct kinfo_file.  These
    match the KF_TYPE_* constants in <sys/user.h>.  */
 
 #define	KINFO_FILE_TYPE_VNODE	1
+#define	KINFO_FILE_TYPE_SOCKET	2
+#define	KINFO_FILE_TYPE_PIPE	3
+#define	KINFO_FILE_TYPE_FIFO	4
+#define	KINFO_FILE_TYPE_KQUEUE	5
+#define	KINFO_FILE_TYPE_CRYPTO	6
+#define	KINFO_FILE_TYPE_MQUEUE	7
+#define	KINFO_FILE_TYPE_SHM	8
+#define	KINFO_FILE_TYPE_SEM	9
+#define	KINFO_FILE_TYPE_PTS	10
+#define	KINFO_FILE_TYPE_PROCDESC 11
 
 /* Special values for the 'kf_fd' field in struct kinfo_file.  These
    match the KF_FD_TYPE_* constants in <sys/user.h>.  */
 
 #define	KINFO_FILE_FD_TYPE_CWD	-1
+#define	KINFO_FILE_FD_TYPE_ROOT	-2
+#define	KINFO_FILE_FD_TYPE_JAIL	-3
+#define	KINFO_FILE_FD_TYPE_TRACE -4
 #define	KINFO_FILE_FD_TYPE_TEXT	-5
+#define	KINFO_FILE_FD_TYPE_CTTY	-6
+
+/* Flags in the 'kf_flags' field in struct kinfo_file.  These match
+   the KF_FLAG_* constants in <sys/user.h>.  */
+
+#define	KINFO_FILE_FLAG_READ		0x00000001
+#define	KINFO_FILE_FLAG_WRITE		0x00000002
+#define	KINFO_FILE_FLAG_APPEND		0x00000004
+#define	KINFO_FILE_FLAG_ASYNC		0x00000008
+#define	KINFO_FILE_FLAG_FSYNC		0x00000010
+#define	KINFO_FILE_FLAG_NONBLOCK	0x00000020
+#define	KINFO_FILE_FLAG_DIRECT		0x00000040
+#define	KINFO_FILE_FLAG_HASLOCK		0x00000080
+#define	KINFO_FILE_FLAG_EXEC		0x00004000
+
+/* Constants for the 'kf_vnode_type' field in struct kinfo_file.
+   These match the KF_VTYPE_* constants in <sys/user.h>.  */
+
+#define	KINFO_FILE_VTYPE_VREG	1
+#define	KINFO_FILE_VTYPE_VDIR	2
+#define	KINFO_FILE_VTYPE_VCHR	4
+#define	KINFO_FILE_VTYPE_VLNK	5
+#define	KINFO_FILE_VTYPE_VSOCK	6
+#define	KINFO_FILE_VTYPE_VFIFO	7
+
+/* Constants for socket address families.  These match AF_* constants
+   in <sys/socket.h>.  */
+
+#define	FBSD_AF_UNIX		1
+#define	FBSD_AF_INET		2
+#define	FBSD_AF_INET6		28
+
+/* Constats for socket types.  These match SOCK_* constants in
+   <sys/socket.h>.  */
+
+#define	FBSD_SOCK_STREAM	1
+#define	FBSD_SOCK_DGRAM		2
+#define	FBSD_SOCK_SEQPACKET	5
+
+/* Constants for IP protocols.  These match IPPROTO_* constants in
+   <netinet/in.h>.  */
+
+#define	FBSD_IPPROTO_ICMP	1
+#define	FBSD_IPPROTO_TCP	6
+#define	FBSD_IPPROTO_UDP	17
+#define	FBSD_IPPROTO_SCTP	132
+
+/* Socket address structures.  These have the same layout on all
+   FreeBSD architectures.  In addition, multibyte fields such as IP
+   addresses are always stored in network byte order.  */
+
+struct fbsd_sockaddr_in
+{
+  uint8_t sin_len;
+  uint8_t sin_family;
+  uint8_t sin_port[2];
+  uint8_t sin_addr[4];
+  char sin_zero[8];
+};
+
+struct fbsd_sockaddr_in6
+{
+  uint8_t sin6_len;
+  uint8_t sin6_family;
+  uint8_t sin6_port[2];
+  uint32_t sin6_flowinfo;
+  uint8_t sin6_addr[16];
+  uint32_t sin6_scope_id;
+};
+
+struct fbsd_sockaddr_un
+{
+  uint8_t sun_len;
+  uint8_t sun_family;
+  char sun_path[104];
+};
 
 /* Number of 32-bit words in a signal set.  This matches _SIG_WORDS in
    <sys/_sigset.h> and is the same value on all architectures.  */
@@ -645,6 +742,341 @@ fbsd_make_corefile_notes (struct gdbarch *gdbarch, bfd *obfd, int *note_size)
   return note_data;
 }
 
+/* Helper function to generate the file descriptor description for a
+   single open file in 'info proc files'.  */
+
+const char *
+fbsd_file_fd (int kf_fd)
+{
+  switch (kf_fd)
+    {
+    case KINFO_FILE_FD_TYPE_CWD:
+      return "cwd";
+    case KINFO_FILE_FD_TYPE_ROOT:
+      return "root";
+    case KINFO_FILE_FD_TYPE_JAIL:
+      return "jail";
+    case KINFO_FILE_FD_TYPE_TRACE:
+      return "trace";
+    case KINFO_FILE_FD_TYPE_TEXT:
+      return "text";
+    case KINFO_FILE_FD_TYPE_CTTY:
+      return "ctty";
+    default:
+      return int_string (kf_fd, 10, 1, 0, 0);
+    }
+}
+
+/* Helper function to generate the file type for a single open file in
+   'info proc files'.  */
+
+const char *
+fbsd_file_type (int kf_type, int kf_vnode_type)
+{
+  switch (kf_type)
+    {
+    case KINFO_FILE_TYPE_VNODE:
+      switch (kf_vnode_type)
+	{
+	case KINFO_FILE_VTYPE_VREG:
+	  return "file";
+	case KINFO_FILE_VTYPE_VDIR:
+	  return "dir";
+	case KINFO_FILE_VTYPE_VCHR:
+	  return "chr";
+	case KINFO_FILE_VTYPE_VLNK:
+	  return "link";
+	case KINFO_FILE_VTYPE_VSOCK:
+	  return "socket";
+	case KINFO_FILE_VTYPE_VFIFO:
+	  return "fifo";
+	default:
+	  {
+	    char *str = get_print_cell ();
+
+	    xsnprintf (str, PRINT_CELL_SIZE, "vn:%d", kf_vnode_type);
+	    return str;
+	  }
+	}
+    case KINFO_FILE_TYPE_SOCKET:
+      return "socket";
+    case KINFO_FILE_TYPE_PIPE:
+      return "pipe";
+    case KINFO_FILE_TYPE_FIFO:
+      return "fifo";
+    case KINFO_FILE_TYPE_KQUEUE:
+      return "kqueue";
+    case KINFO_FILE_TYPE_CRYPTO:
+      return "crypto";
+    case KINFO_FILE_TYPE_MQUEUE:
+      return "mqueue";
+    case KINFO_FILE_TYPE_SHM:
+      return "shm";
+    case KINFO_FILE_TYPE_SEM:
+      return "sem";
+    case KINFO_FILE_TYPE_PTS:
+      return "pts";
+    case KINFO_FILE_TYPE_PROCDESC:
+      return "proc";
+    default:
+      return int_string (kf_type, 10, 1, 0, 0);
+    }
+}
+
+/* Helper function to generate the file flags for a single open file in
+   'info proc files'.  */
+
+const char *
+fbsd_file_flags (int kf_flags)
+{
+  static char file_flags[10];
+
+  file_flags[0] = (kf_flags & KINFO_FILE_FLAG_READ) ? 'r' : '-';
+  file_flags[1] = (kf_flags & KINFO_FILE_FLAG_WRITE) ? 'w' : '-';
+  file_flags[2] = (kf_flags & KINFO_FILE_FLAG_EXEC) ? 'x' : '-';
+  file_flags[3] = (kf_flags & KINFO_FILE_FLAG_APPEND) ? 'a' : '-';
+  file_flags[4] = (kf_flags & KINFO_FILE_FLAG_ASYNC) ? 's' : '-';
+  file_flags[5] = (kf_flags & KINFO_FILE_FLAG_FSYNC) ? 'f' : '-';
+  file_flags[6] = (kf_flags & KINFO_FILE_FLAG_NONBLOCK) ? 'n' : '-';
+  file_flags[7] = (kf_flags & KINFO_FILE_FLAG_DIRECT) ? 'd' : '-';
+  file_flags[8] = (kf_flags & KINFO_FILE_FLAG_HASLOCK) ? 'l' : '-';
+  file_flags[9] = '\0';
+
+  return file_flags;
+}
+
+/* Helper function to generate the name of an IP protocol.  */
+
+static const char *
+fbsd_ipproto (int protocol)
+{
+  switch (protocol)
+    {
+    case FBSD_IPPROTO_ICMP:
+      return "icmp";
+    case FBSD_IPPROTO_TCP:
+      return "tcp";
+    case FBSD_IPPROTO_UDP:
+      return "udp";
+    case FBSD_IPPROTO_SCTP:
+      return "sctp";
+    default:
+      {
+	char *str = get_print_cell ();
+
+	xsnprintf (str, PRINT_CELL_SIZE, "ip<%d>", protocol);
+	return str;
+      }
+    }
+}
+
+/* Helper function to print out an IPv4 socket address.  The address
+   is formatted similar to inet_ntoa.  */
+
+static void
+fbsd_print_sockaddr_in (void *sockaddr)
+{
+  struct fbsd_sockaddr_in *sin =
+    reinterpret_cast<struct fbsd_sockaddr_in *>(sockaddr);
+
+  printf_filtered ("%u.%u.%u.%u:%u", sin->sin_addr[0], sin->sin_addr[1],
+		   sin->sin_addr[2], sin->sin_addr[3],
+		   (sin->sin_port[0] << 8) | sin->sin_port[1]);
+}
+
+/* Helper function to print out an IPv6 socket address.  The address
+   is formatted similar to inet_ntop.  */
+
+static void
+fbsd_print_sockaddr_in6 (void *sockaddr)
+{
+  struct fbsd_sockaddr_in6 *sin6 =
+    reinterpret_cast<struct fbsd_sockaddr_in6 *>(sockaddr);
+  uint16_t words[ARRAY_SIZE(sin6->sin6_addr) / 2];
+
+  /* Populate the array of 16-bit words from network-order bytes.  */
+  for (int i = 0; i < ARRAY_SIZE(words); i++)
+    words[i] = (sin6->sin6_addr[i * 2] << 8) | sin6->sin6_addr[i * 2 + 1];
+
+  /* Find the longest run of zero words.  */
+  int best, bestlen, current, len;
+
+  best = -1;
+  bestlen = 0;
+  current = -1;
+  len = 0;
+  for (int i = 0; i < ARRAY_SIZE(words); i++)
+    {
+      if (words[i] == 0)
+	{
+	  if (current >= 0)
+	    len++;
+	  else
+	    {
+	      current = i;
+	      len = 1;
+	    }
+	}
+      else
+	{
+	  if (current >= 0 && len > bestlen)
+	    {
+	      best = current;
+	      bestlen = len;
+	    }
+	  current = -1;
+	  len = 0;
+	}
+    }
+  if (current >= 0 && len > bestlen)
+    {
+      best = current;
+      bestlen = len;
+    }
+  if (bestlen < 2)
+    best = -1;
+
+  for (int i = 0; i < ARRAY_SIZE(words); i++)
+    {
+      if (best >= 0 && i >= best && i < best + bestlen)
+	{
+	  if (i == best || i == ARRAY_SIZE(words) - 1)
+	    printf_filtered (":");
+	}
+      else
+	{
+	  if (i != 0)
+	    printf_filtered (":");
+	  printf_filtered ("%x", words[i]);
+	}
+    }
+  printf_filtered (".%u", (sin6->sin6_port[0] << 8) | sin6->sin6_port[1]);
+}
+
+/* Helper function to print out a description of a socket for 'info
+   proc files'.  */
+
+void
+fbsd_print_socket (int kf_sock_domain, int kf_sock_type, int kf_sock_protocol,
+		   void *kf_sa_local, void *kf_sa_peer)
+{
+  switch (kf_sock_domain)
+    {
+    case FBSD_AF_UNIX:
+      {
+	switch (kf_sock_type)
+	  {
+	  case FBSD_SOCK_STREAM:
+	    printf_filtered ("unix stream:");
+	    break;
+	  case FBSD_SOCK_DGRAM:
+	    printf_filtered ("unix dgram:");
+	    break;
+	  case FBSD_SOCK_SEQPACKET:
+	    printf_filtered ("unix seqpacket:");
+	    break;
+	  default:
+	    printf_filtered ("unix <%d>:", kf_sock_type);
+	    break;
+	  }
+
+	/* For local sockets, print out the first non-nul path rather
+	   than both paths.  */
+	struct fbsd_sockaddr_un *sun =
+	  reinterpret_cast<struct fbsd_sockaddr_un *>(kf_sa_local);
+	if (sun->sun_path[0] == 0)
+	  sun = reinterpret_cast<struct fbsd_sockaddr_un *>(kf_sa_peer);
+	printf_filtered ("%s", sun->sun_path);
+	break;
+      }
+    case FBSD_AF_INET:
+      printf_filtered ("%s4 ", fbsd_ipproto (kf_sock_protocol));
+      fbsd_print_sockaddr_in (kf_sa_local);
+      printf_filtered (" -> ");
+      fbsd_print_sockaddr_in (kf_sa_peer);
+      break;
+    case FBSD_AF_INET6:
+      printf_filtered ("%s6 ", fbsd_ipproto (kf_sock_protocol));
+      fbsd_print_sockaddr_in6 (kf_sa_local);
+      printf_filtered (" -> ");
+      fbsd_print_sockaddr_in6 (kf_sa_peer);
+      break;
+    }
+}
+
+/* Implement "info proc files" for a corefile.  */
+
+static void
+fbsd_core_info_proc_files (struct gdbarch *gdbarch)
+{
+  asection *section;
+  unsigned char *descdata, *descend;
+  size_t note_size;
+
+  section = bfd_get_section_by_name (core_bfd, ".note.freebsdcore.files");
+  if (section == NULL)
+    {
+      warning (_("unable to find open files in core file"));
+      return;
+    }
+
+  note_size = bfd_get_section_size (section);
+  if (note_size < 4)
+    error (_("malformed core note - too short for header"));
+
+  gdb::def_vector<unsigned char> contents (note_size);
+  if (!bfd_get_section_contents (core_bfd, section, contents.data (),
+				 0, note_size))
+    error (_("could not get core note contents"));
+
+  descdata = contents.data ();
+  descend = descdata + note_size;
+
+  /* Skip over the structure size.  */
+  descdata += 4;
+
+  printf_filtered (_("Open files:\n\n"));
+  printf_filtered ("  %6s %6s %10s %9s %s\n",
+		   "FD", "Type", "Offset", "Flags  ", "Name");
+
+  while (descdata + KF_PATH < descend)
+    {
+      LONGEST fd, flags, offset, type, vnode_type;
+      ULONGEST structsize;
+
+      structsize = bfd_get_32 (core_bfd, descdata + KF_STRUCTSIZE);
+      if (structsize < KF_PATH)
+	error (_("malformed core note - vmmap entry too small"));
+
+      type = bfd_get_signed_32 (core_bfd, descdata + KF_TYPE);
+      fd = bfd_get_signed_32 (core_bfd, descdata + KF_FD);
+      flags = bfd_get_signed_32 (core_bfd, descdata + KF_FLAGS);
+      offset = bfd_get_signed_64 (core_bfd, descdata + KF_OFFSET);
+      vnode_type = bfd_get_signed_32 (core_bfd, descdata + KF_VNODE_TYPE);
+      printf_filtered ("  %6s %6s %10s %8s ",
+		       fbsd_file_fd (fd),
+		       fbsd_file_type (type, vnode_type),
+		       offset > -1 ? hex_string (offset) : "-",
+		       fbsd_file_flags (flags));
+      if (type == KINFO_FILE_TYPE_SOCKET)
+	{
+	  LONGEST sock_domain, sock_type, sock_protocol;
+
+	  sock_domain = bfd_get_signed_32 (core_bfd, descdata + KF_SOCK_DOMAIN);
+	  sock_type = bfd_get_signed_32 (core_bfd, descdata + KF_SOCK_TYPE);
+	  sock_protocol = bfd_get_signed_32 (core_bfd,
+					     descdata + KF_SOCK_PROTOCOL);
+	  fbsd_print_socket (sock_domain, sock_type, sock_protocol,
+			     descdata + KF_SA_LOCAL, descdata + KF_SA_PEER);
+	}
+      else
+	printf_filtered ("%s", descdata + KF_PATH);
+      printf_filtered ("\n");
+
+      descdata += structsize;
+    }
+}
+
 /* Helper function to generate mappings flags for a single VM map
    entry in 'info proc mappings'.  */
 
@@ -995,6 +1427,7 @@ fbsd_core_info_proc (struct gdbarch *gdbarch, const char *args,
   bool do_cmdline = false;
   bool do_cwd = false;
   bool do_exe = false;
+  bool do_files = false;
   bool do_mappings = false;
   bool do_status = false;
   int pid;
@@ -1022,10 +1455,14 @@ fbsd_core_info_proc (struct gdbarch *gdbarch, const char *args,
     case IP_CWD:
       do_cwd = true;
       break;
+    case IP_FILES:
+      do_files = true;
+      break;
     case IP_ALL:
       do_cmdline = true;
       do_cwd = true;
       do_exe = true;
+      do_files = true;
       do_mappings = true;
       do_status = true;
       break;
@@ -1065,6 +1502,8 @@ fbsd_core_info_proc (struct gdbarch *gdbarch, const char *args,
       else
 	warning (_("unable to read executable path name"));
     }
+  if (do_files)
+    fbsd_core_info_proc_files (gdbarch);
   if (do_mappings)
     fbsd_core_info_proc_mappings (gdbarch);
   if (do_status)
diff --git a/gdb/fbsd-tdep.h b/gdb/fbsd-tdep.h
index 0b293e5a25..0e0ba57be8 100644
--- a/gdb/fbsd-tdep.h
+++ b/gdb/fbsd-tdep.h
@@ -22,6 +22,37 @@
 
 extern void fbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch);
 
+/* Helper function to generate the file descriptor description for a
+   single open file in 'info proc files'.  The KF_FD parameter should
+   contain the value of the 'kf_fd' field from a 'struct
+   kinfo_file'.  */
+
+extern const char *fbsd_file_fd (int kf_fd);
+
+/* Helper function to generate the file type for a single open file in
+   'info proc files'.  The KF_TYPE and KF_VNODE_TYPE parameters should
+   contain the values of the corresponding fields in a 'struct
+   kinfo_file'.  */
+
+extern const char *fbsd_file_type (int kf_type, int kf_vnode_type);
+
+/* Helper function to generate the file flags for a single open file
+   in 'info proc files'.  The KF_FLAGS parameter should contain the
+   value of the 'kf_flags' field from a 'struct kinfo_file'.  */
+
+extern const char *fbsd_file_flags (int kf_flags);
+
+/* Helper function to print out a description of a socket for 'info
+   proc files'.  The KF_SOCK_DOMAIN, KF_SOCK_TYPE, and
+   KF_SOCK_PROTOCOL, parameters should contain the value of the
+   corresponding fields in a 'struct kinfo_file'.  The KF_SA_LOCAL and
+   KF_SA_PEER should contain pointers to the corresponding fields in a
+   'struct kinfo_file'.  */
+
+extern void fbsd_print_socket (int kf_sock_domain, int kf_sock_type,
+			       int kf_sock_protocol, void *kf_sa_local,
+			       void *kf_sa_peer);
+
 /* Helper function to generate mappings flags for a single VM map
    entry in 'info proc mappings'.  The KVE_FLAGS and KVE_PROTECTION
    parameters should contain the values of the corresponding fields in
-- 
2.18.0

Re: [PATCH 3/5] Add support for 'info proc files' on FreeBSD core dumps.

[Simon Marchi]

On 2018-09-08 01:36 AM, John Baldwin wrote:
> Walk the list of struct kinfo_file objects in the
> NT_FREEBSD_PROCSTAT_FILES core dump note outputting a description of
> each open file descriptor.  For sockets, the local and remote socket
> addresses are displayed in place of the file name field.  For UNIX
> local domain sockets, only a single address is displayed since most
> UNIX sockets only have one valid address and printing both pathnames
> could be quite long.  The output format was somewhat inspired by the
> output of the "procstat -f" command on FreeBSD, but with a few less
> details and some fields were condensed.

Just some nits, LGTM otherwise.
> diff --git a/gdb/fbsd-tdep.c b/gdb/fbsd-tdep.c
> index 9e6d7276c4..a8b5b2f146 100644
> --- a/gdb/fbsd-tdep.c
> +++ b/gdb/fbsd-tdep.c
> @@ -90,18 +90,115 @@
>  #define	KF_STRUCTSIZE		0x0
>  #define	KF_TYPE			0x4
>  #define	KF_FD			0x8
> +#define	KF_FLAGS		0x10
> +#define	KF_OFFSET		0x18

In sys/user.h, it says:

 /* XXX Hidden alignment padding here on amd64 */

Does that mean the field offsets can be different for other arches?

> +/* Constats for socket types.  These match SOCK_* constants in

"Constats"

> +/* Helper function to print out an IPv6 socket address.  The address
> +   is formatted similar to inet_ntop.  */
> +
> +static void
> +fbsd_print_sockaddr_in6 (void *sockaddr)
> +{
> +  struct fbsd_sockaddr_in6 *sin6 =
> +    reinterpret_cast<struct fbsd_sockaddr_in6 *>(sockaddr);
> +  uint16_t words[ARRAY_SIZE(sin6->sin6_addr) / 2];
> +
> +  /* Populate the array of 16-bit words from network-order bytes.  */
> +  for (int i = 0; i < ARRAY_SIZE(words); i++)
> +    words[i] = (sin6->sin6_addr[i * 2] << 8) | sin6->sin6_addr[i * 2 + 1];
> +
> +  /* Find the longest run of zero words.  */
> +  int best, bestlen, current, len;
> +
> +  best = -1;
> +  bestlen = 0;
> +  current = -1;
> +  len = 0;
> +  for (int i = 0; i < ARRAY_SIZE(words); i++)
> +    {
> +      if (words[i] == 0)
> +	{
> +	  if (current >= 0)
> +	    len++;
> +	  else
> +	    {
> +	      current = i;
> +	      len = 1;
> +	    }
> +	}
> +      else
> +	{
> +	  if (current >= 0 && len > bestlen)
> +	    {
> +	      best = current;
> +	      bestlen = len;
> +	    }
> +	  current = -1;
> +	  len = 0;
> +	}
> +    }
> +  if (current >= 0 && len > bestlen)
> +    {
> +      best = current;
> +      bestlen = len;
> +    }
> +  if (bestlen < 2)
> +    best = -1;
> +
> +  for (int i = 0; i < ARRAY_SIZE(words); i++)
> +    {
> +      if (best >= 0 && i >= best && i < best + bestlen)
> +	{
> +	  if (i == best || i == ARRAY_SIZE(words) - 1)
> +	    printf_filtered (":");
> +	}
> +      else
> +	{
> +	  if (i != 0)
> +	    printf_filtered (":");
> +	  printf_filtered ("%x", words[i]);
> +	}
> +    }
> +  printf_filtered (".%u", (sin6->sin6_port[0] << 8) | sin6->sin6_port[1]);
> +}

Hmm, I suppose we'll want to re-use this ipv6 address printing code for the Linux...
We can take care of moving it to a common file then.

> +/* Implement "info proc files" for a corefile.  */
> +
> +static void
> +fbsd_core_info_proc_files (struct gdbarch *gdbarch)
> +{
> +  asection *section;
> +  unsigned char *descdata, *descend;
> +  size_t note_size;

Don't hesitate to declare them variables at the point you use the the first time :).
> +
> +  section = bfd_get_section_by_name (core_bfd, ".note.freebsdcore.files");
> +  if (section == NULL)
> +    {
> +      warning (_("unable to find open files in core file"));
> +      return;
> +    }
> +
> +  note_size = bfd_get_section_size (section);
> +  if (note_size < 4)
> +    error (_("malformed core note - too short for header"));
> +
> +  gdb::def_vector<unsigned char> contents (note_size);
> +  if (!bfd_get_section_contents (core_bfd, section, contents.data (),
> +				 0, note_size))
> +    error (_("could not get core note contents"));
> +
> +  descdata = contents.data ();
> +  descend = descdata + note_size;
> +
> +  /* Skip over the structure size.  */
> +  descdata += 4;
> +
> +  printf_filtered (_("Open files:\n\n"));
> +  printf_filtered ("  %6s %6s %10s %9s %s\n",
> +		   "FD", "Type", "Offset", "Flags  ", "Name");
> +
> +  while (descdata + KF_PATH < descend)
> +    {
> +      LONGEST fd, flags, offset, type, vnode_type;
> +      ULONGEST structsize;
> +
> +      structsize = bfd_get_32 (core_bfd, descdata + KF_STRUCTSIZE);
> +      if (structsize < KF_PATH)
> +	error (_("malformed core note - vmmap entry too small"));

Copy pasta?  Actually, there seems to be the same mistake in fbsd_core_vnode_path.

Simon

Re: [PATCH 3/5] Add support for 'info proc files' on FreeBSD core dumps.

[John Baldwin]

On 9/8/18 3:54 PM, Simon Marchi wrote:
> On 2018-09-08 01:36 AM, John Baldwin wrote:
>> Walk the list of struct kinfo_file objects in the
>> NT_FREEBSD_PROCSTAT_FILES core dump note outputting a description of
>> each open file descriptor.  For sockets, the local and remote socket
>> addresses are displayed in place of the file name field.  For UNIX
>> local domain sockets, only a single address is displayed since most
>> UNIX sockets only have one valid address and printing both pathnames
>> could be quite long.  The output format was somewhat inspired by the
>> output of the "procstat -f" command on FreeBSD, but with a few less
>> details and some fields were condensed.
> 
> Just some nits, LGTM otherwise.
>> diff --git a/gdb/fbsd-tdep.c b/gdb/fbsd-tdep.c
>> index 9e6d7276c4..a8b5b2f146 100644
>> --- a/gdb/fbsd-tdep.c
>> +++ b/gdb/fbsd-tdep.c
>> @@ -90,18 +90,115 @@
>>  #define	KF_STRUCTSIZE		0x0
>>  #define	KF_TYPE			0x4
>>  #define	KF_FD			0x8
>> +#define	KF_FLAGS		0x10
>> +#define	KF_OFFSET		0x18
> 
> In sys/user.h, it says:
> 
>  /* XXX Hidden alignment padding here on amd64 */
> 
> Does that mean the field offsets can be different for other arches?

That comment is in the structure 'struct kinfo_ofile' which is for
an older format of this structure used in FreeBSD 7.0 kernels.  7.1
and later kernels use the 'struct kinfo_file' structure which has a
fixed layout (and includes an explicit kf_pad0 padding field).  FreeBSD
didn't add the NT_PROCSTAT_FILES note to process core dumps until FreeBSD
10.0 (also merged to 9.2), so process cores have only ever included the
newer format with a fixed layout.

If it is helpful I could add a comment.  I think I redid the same
archaeology when adding the initial 'info proc' core dump bits for
FreeBSD. :)

>> +/* Constats for socket types.  These match SOCK_* constants in
> 
> "Constats"

Oops, fixed.

>> +/* Helper function to print out an IPv6 socket address.  The address
>> +   is formatted similar to inet_ntop.  */
>> +
>> +static void
>> +fbsd_print_sockaddr_in6 (void *sockaddr)
>> +{
>> +  ...
>> +}
> 
> Hmm, I suppose we'll want to re-use this ipv6 address printing code for the Linux...
> We can take care of moving it to a common file then.

So I did hate to duplicate a lot of what is normally common code and even
POSIX (inet_ntoa() and inet_ntop()).  We currently don't use those routines
anywhere else in GDB, and I wasn't sure if we could depend on their always
existing.  gnulib doesn't seem to have existing fallbacks for those.  If
we could somehow require them or provide standard fallbacks then I'd be
happier using those instead.

>> +/* Implement "info proc files" for a corefile.  */
>> +
>> +static void
>> +fbsd_core_info_proc_files (struct gdbarch *gdbarch)
>> +{
>> +  asection *section;
>> +  unsigned char *descdata, *descend;
>> +  size_t note_size;
> 
> Don't hesitate to declare them variables at the point you use the the first time :).

Old C habits die hard (even with C99).  I've fixed for V2 though. :)

>> +  while (descdata + KF_PATH < descend)
>> +    {
>> +      LONGEST fd, flags, offset, type, vnode_type;
>> +      ULONGEST structsize;
>> +
>> +      structsize = bfd_get_32 (core_bfd, descdata + KF_STRUCTSIZE);
>> +      if (structsize < KF_PATH)
>> +	error (_("malformed core note - vmmap entry too small"));
> 
> Copy pasta?  Actually, there seems to be the same mistake in fbsd_core_vnode_path.

Yes, and I should fix the original mistake in the first patch of the series.

-- 
John Baldwin

                                                                            

                                                                            

Re: [PATCH 3/5] Add support for 'info proc files' on FreeBSD core dumps.

[Tom Tromey]

>>>>> "John" == John Baldwin <jhb@FreeBSD.org> writes:

John> That comment is in the structure 'struct kinfo_ofile' which is for
John> an older format of this structure used in FreeBSD 7.0 kernels.  7.1
John> and later kernels use the 'struct kinfo_file' structure which has a
John> fixed layout (and includes an explicit kf_pad0 padding field).  FreeBSD
John> didn't add the NT_PROCSTAT_FILES note to process core dumps until FreeBSD
John> 10.0 (also merged to 9.2), so process cores have only ever included the
John> newer format with a fixed layout.

John> If it is helpful I could add a comment.  I think I redid the same
John> archaeology when adding the initial 'info proc' core dump bits for
John> FreeBSD. :)

I think a comment sounds helpful for future developers.

Tom

Re: [PATCH 3/5] Add support for 'info proc files' on FreeBSD core dumps.

[John Baldwin]

On 9/13/18 8:07 AM, Tom Tromey wrote:
>>>>>> "John" == John Baldwin <jhb@FreeBSD.org> writes:
> 
> John> That comment is in the structure 'struct kinfo_ofile' which is for
> John> an older format of this structure used in FreeBSD 7.0 kernels.  7.1
> John> and later kernels use the 'struct kinfo_file' structure which has a
> John> fixed layout (and includes an explicit kf_pad0 padding field).  FreeBSD
> John> didn't add the NT_PROCSTAT_FILES note to process core dumps until FreeBSD
> John> 10.0 (also merged to 9.2), so process cores have only ever included the
> John> newer format with a fixed layout.
> 
> John> If it is helpful I could add a comment.  I think I redid the same
> John> archaeology when adding the initial 'info proc' core dump bits for
> John> FreeBSD. :)
> 
> I think a comment sounds helpful for future developers.

I added one in the V2 series.

-- 
John Baldwin

                                                                            

[PATCH 2/5] Add a new 'info proc files' subcommand of 'info proc'.

[John Baldwin]

This command displays a list of open file descriptors.

gdb/ChangeLog:

	* defs.h (enum info_proc_what) [IP_FILES]: New value.
	* infcmd.c (info_proc_cmd_files): New function.
	(_initialize_infcmd): Register 'info proc files' command.
---
 gdb/ChangeLog |  6 ++++++
 gdb/defs.h    |  3 +++
 gdb/infcmd.c  | 12 ++++++++++++
 3 files changed, 21 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index d32b390778..2e5cd0a687 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2018-09-07  John Baldwin  <jhb@FreeBSD.org>
+
+	* defs.h (enum info_proc_what) [IP_FILES]: New value.
+	* infcmd.c (info_proc_cmd_files): New function.
+	(_initialize_infcmd): Register 'info proc files' command.
+
 2018-09-07  John Baldwin  <jhb@FreeBSD.org>
 
 	* fbsd-tdep.c (fbsd_core_vnode_path): Use KF_PATH instead of
diff --git a/gdb/defs.h b/gdb/defs.h
index fc4217005a..6e3f4df116 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -389,6 +389,9 @@ enum info_proc_what
     /* * Display `info proc cwd'.  */
     IP_CWD,
 
+    /* * Display `info proc files'.  */
+    IP_FILES,
+
     /* * Display all of the above.  */
     IP_ALL
   };
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 860909f5e2..c6cfc10a49 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -3218,6 +3218,14 @@ info_proc_cmd_exe (const char *args, int from_tty)
   info_proc_cmd_1 (args, IP_EXE, from_tty);
 }
 
+/* Implement `info proc files'.  */
+
+static void
+info_proc_cmd_files (const char *args, int from_tty)
+{
+  info_proc_cmd_1 (args, IP_FILES, from_tty);
+}
+
 /* Implement `info proc all'.  */
 
 static void
@@ -3543,6 +3551,10 @@ List command line arguments of the process."),
 List absolute filename for executable of the process."),
 	   &info_proc_cmdlist);
 
+  add_cmd ("files", class_info, info_proc_cmd_files, _("\
+List of open files."),
+	   &info_proc_cmdlist);
+
   add_cmd ("all", class_info, info_proc_cmd_all, _("\
 List all available /proc info."),
 	   &info_proc_cmdlist);
-- 
2.18.0

Re: [PATCH 2/5] Add a new 'info proc files' subcommand of 'info proc'.

[Eli Zaretskii]

> From: John Baldwin <jhb@FreeBSD.org>
> Date: Fri,  7 Sep 2018 17:36:56 -0700
> 
> This command displays a list of open file descriptors.

Thanks.

> +  add_cmd ("files", class_info, info_proc_cmd_files, _("\
> +List of open files."),

IMO, this doc strings is too terse.  I suggest to expand it telling
that the command shows the files open by the process being debugged.

Re: [PATCH 2/5] Add a new 'info proc files' subcommand of 'info proc'.

[Simon Marchi]

On 2018-09-08 07:49 AM, Eli Zaretskii wrote:
>> From: John Baldwin <jhb@FreeBSD.org>
>> Date: Fri,  7 Sep 2018 17:36:56 -0700
>>
>> This command displays a list of open file descriptors.
> 
> Thanks.
> 
>> +  add_cmd ("files", class_info, info_proc_cmd_files, _("\
>> +List of open files."),
> 
> IMO, this doc strings is too terse.  I suggest to expand it telling
> that the command shows the files open by the process being debugged.

The info proc commands accept a pid, which allows you to refer to any
process, not only those debugged by GDB.  Other info proc commands use
use the generic form "of the process"

  info proc cmdline -- List command line arguments of the process

I think that's sufficient, especially that users are likely to see it
in the context of "help info proc", which describes what argument you
can pass:

(gdb) help info proc
Show /proc process information about any running process.
Specify any process id, or use the program being debugged by default.

List of info proc subcommands:

info proc all -- List all available /proc info
info proc cmdline -- List command line arguments of the process
info proc cwd -- List current working directory of the process
info proc exe -- List absolute filename for executable of the process
info proc mappings -- List of mapped memory regions
info proc stat -- List process info from /proc/PID/stat
info proc status -- List process info from /proc/PID/status

Simon

Re: [PATCH 2/5] Add a new 'info proc files' subcommand of 'info proc'.

[Eli Zaretskii]

> CC: <gdb-patches@sourceware.org>
> From: Simon Marchi <simon.marchi@ericsson.com>
> Date: Sat, 8 Sep 2018 23:30:36 +0100
> 
> >> +  add_cmd ("files", class_info, info_proc_cmd_files, _("\
> >> +List of open files."),
> > 
> > IMO, this doc strings is too terse.  I suggest to expand it telling
> > that the command shows the files open by the process being debugged.
> 
> The info proc commands accept a pid, which allows you to refer to any
> process, not only those debugged by GDB.

Then let's say

  List of files open by the specified process.

Re: [PATCH 2/5] Add a new 'info proc files' subcommand of 'info proc'.

[John Baldwin]

On 9/8/18 10:23 PM, Eli Zaretskii wrote:
>> CC: <gdb-patches@sourceware.org>
>> From: Simon Marchi <simon.marchi@ericsson.com>
>> Date: Sat, 8 Sep 2018 23:30:36 +0100
>>
>>>> +  add_cmd ("files", class_info, info_proc_cmd_files, _("\
>>>> +List of open files."),
>>>
>>> IMO, this doc strings is too terse.  I suggest to expand it telling
>>> that the command shows the files open by the process being debugged.
>>
>> The info proc commands accept a pid, which allows you to refer to any
>> process, not only those debugged by GDB.
> 
> Then let's say
> 
>   List of files open by the specified process.

I'm fine with that, but we should probably make the descriptions under
'info proc' a bit more consistent in general as a followup.  Most of them
use 'of the process' without including "specified", and 'info proc mappings'
(which I based the original description off of) doesn't include
"process" in its description.
-- 
John Baldwin

                                                                            

Re: [PATCH 2/5] Add a new 'info proc files' subcommand of 'info proc'.

[Eli Zaretskii]

> Cc: gdb-patches@sourceware.org
> From: John Baldwin <jhb@FreeBSD.org>
> Date: Mon, 10 Sep 2018 11:13:19 -0700
> 
> > Then let's say
> > 
> >   List of files open by the specified process.
> 
> I'm fine with that, but we should probably make the descriptions under
> 'info proc' a bit more consistent in general as a followup.

Yes.

Re: [PATCH 2/5] Add a new 'info proc files' subcommand of 'info proc'.

[Simon Marchi]

On 2018-09-08 01:36 AM, John Baldwin wrote:
> This command displays a list of open file descriptors.
> 
> gdb/ChangeLog:
> 
> 	* defs.h (enum info_proc_what) [IP_FILES]: New value.
> 	* infcmd.c (info_proc_cmd_files): New function.
> 	(_initialize_infcmd): Register 'info proc files' command.
> ---
>  gdb/ChangeLog |  6 ++++++
>  gdb/defs.h    |  3 +++
>  gdb/infcmd.c  | 12 ++++++++++++
>  3 files changed, 21 insertions(+)
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index d32b390778..2e5cd0a687 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,9 @@
> +2018-09-07  John Baldwin  <jhb@FreeBSD.org>
> +
> +	* defs.h (enum info_proc_what) [IP_FILES]: New value.
> +	* infcmd.c (info_proc_cmd_files): New function.
> +	(_initialize_infcmd): Register 'info proc files' command.
> +
>  2018-09-07  John Baldwin  <jhb@FreeBSD.org>
>  
>  	* fbsd-tdep.c (fbsd_core_vnode_path): Use KF_PATH instead of
> diff --git a/gdb/defs.h b/gdb/defs.h
> index fc4217005a..6e3f4df116 100644
> --- a/gdb/defs.h
> +++ b/gdb/defs.h
> @@ -389,6 +389,9 @@ enum info_proc_what
>      /* * Display `info proc cwd'.  */
>      IP_CWD,
>  
> +    /* * Display `info proc files'.  */
> +    IP_FILES,
> +
>      /* * Display all of the above.  */
>      IP_ALL
>    };
> diff --git a/gdb/infcmd.c b/gdb/infcmd.c
> index 860909f5e2..c6cfc10a49 100644
> --- a/gdb/infcmd.c
> +++ b/gdb/infcmd.c
> @@ -3218,6 +3218,14 @@ info_proc_cmd_exe (const char *args, int from_tty)
>    info_proc_cmd_1 (args, IP_EXE, from_tty);
>  }
>  
> +/* Implement `info proc files'.  */
> +
> +static void
> +info_proc_cmd_files (const char *args, int from_tty)
> +{
> +  info_proc_cmd_1 (args, IP_FILES, from_tty);
> +}
> +
>  /* Implement `info proc all'.  */
>  
>  static void
> @@ -3543,6 +3551,10 @@ List command line arguments of the process."),
>  List absolute filename for executable of the process."),
>  	   &info_proc_cmdlist);
>  
> +  add_cmd ("files", class_info, info_proc_cmd_files, _("\
> +List of open files."),
> +	   &info_proc_cmdlist);
> +
>    add_cmd ("all", class_info, info_proc_cmd_all, _("\
>  List all available /proc info."),
>  	   &info_proc_cmdlist);
The code LGTM, but please wait for Eli's reply on the doc bits.

Thanks,

Simon

[PATCH 1/5] Use KF_PATH to verify the size of a struct kinfo_file.

[John Baldwin]

fbsd_core_vnode_path needs to use the offset of the kf_path member of
struct kinfo_file as the minimum size of a struct kinfo_file object.
However, it was using KVE_PATH instead due to a copy and paste bug.

gdb/ChangeLog:

	* fbsd-tdep.c (fbsd_core_vnode_path): Use KF_PATH instead of
	KVE_PATH.
---
 gdb/ChangeLog   | 5 +++++
 gdb/fbsd-tdep.c | 4 ++--
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index e6f44a3ac2..d32b390778 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2018-09-07  John Baldwin  <jhb@FreeBSD.org>
+
+	* fbsd-tdep.c (fbsd_core_vnode_path): Use KF_PATH instead of
+	KVE_PATH.
+
 2018-09-06  Simon Ser  <contact@emersion.fr>
 
 	PR gdb/23105
diff --git a/gdb/fbsd-tdep.c b/gdb/fbsd-tdep.c
index ed43087169..9e6d7276c4 100644
--- a/gdb/fbsd-tdep.c
+++ b/gdb/fbsd-tdep.c
@@ -781,12 +781,12 @@ fbsd_core_vnode_path (struct gdbarch *gdbarch, int fd)
   /* Skip over the structure size.  */
   descdata += 4;
 
-  while (descdata + KVE_PATH < descend)
+  while (descdata + KF_PATH < descend)
     {
       ULONGEST structsize;
 
       structsize = bfd_get_32 (core_bfd, descdata + KF_STRUCTSIZE);
-      if (structsize < KVE_PATH)
+      if (structsize < KF_PATH)
 	error (_("malformed core note - vmmap entry too small"));
 
       if (bfd_get_32 (core_bfd, descdata + KF_TYPE) == KINFO_FILE_TYPE_VNODE
-- 
2.18.0

Re: [PATCH 1/5] Use KF_PATH to verify the size of a struct kinfo_file.

[Simon Marchi]

On 2018-09-08 01:36 AM, John Baldwin wrote:
> fbsd_core_vnode_path needs to use the offset of the kf_path member of
> struct kinfo_file as the minimum size of a struct kinfo_file object.
> However, it was using KVE_PATH instead due to a copy and paste bug.
> 
> gdb/ChangeLog:
> 
> 	* fbsd-tdep.c (fbsd_core_vnode_path): Use KF_PATH instead of
> 	KVE_PATH.
> ---
>  gdb/ChangeLog   | 5 +++++
>  gdb/fbsd-tdep.c | 4 ++--
>  2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index e6f44a3ac2..d32b390778 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,8 @@
> +2018-09-07  John Baldwin  <jhb@FreeBSD.org>
> +
> +	* fbsd-tdep.c (fbsd_core_vnode_path): Use KF_PATH instead of
> +	KVE_PATH.
> +
>  2018-09-06  Simon Ser  <contact@emersion.fr>
>  
>  	PR gdb/23105
> diff --git a/gdb/fbsd-tdep.c b/gdb/fbsd-tdep.c
> index ed43087169..9e6d7276c4 100644
> --- a/gdb/fbsd-tdep.c
> +++ b/gdb/fbsd-tdep.c
> @@ -781,12 +781,12 @@ fbsd_core_vnode_path (struct gdbarch *gdbarch, int fd)
>    /* Skip over the structure size.  */
>    descdata += 4;
>  
> -  while (descdata + KVE_PATH < descend)
> +  while (descdata + KF_PATH < descend)
>      {
>        ULONGEST structsize;
>  
>        structsize = bfd_get_32 (core_bfd, descdata + KF_STRUCTSIZE);
> -      if (structsize < KVE_PATH)
> +      if (structsize < KF_PATH)
>  	error (_("malformed core note - vmmap entry too small"));
>  
>        if (bfd_get_32 (core_bfd, descdata + KF_TYPE) == KINFO_FILE_TYPE_VNODE
> 


LGTM.

Simon

[PATCH 4/5] Support 'info proc files' on live FreeBSD processes.

[John Baldwin]

This walks the list of struct kinfo_file objects returned by a call to
kinfo_getfile outputting a description of each open file descriptor.

gdb/ChangeLog:

	* fbsd-nat.c (fbsd_nat_target::info_proc): List open file
	descriptors for IP_FILES and IP_ALL.
---
 gdb/ChangeLog  |  5 +++++
 gdb/fbsd-nat.c | 44 +++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index e3d9f40d02..fb62bc55ea 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2018-09-07  John Baldwin  <jhb@FreeBSD.org>
+
+	* fbsd-nat.c (fbsd_nat_target::info_proc): List open file
+	descriptors for IP_FILES and IP_ALL.
+
 2018-09-07  John Baldwin  <jhb@FreeBSD.org>
 
 	* fbsd-tdep.c (KF_FLAGS, KF_OFFSET, KF_VNODE_TYPE, KF_SOCK_DOMAIN)
diff --git a/gdb/fbsd-nat.c b/gdb/fbsd-nat.c
index a255318d14..d7db03336e 100644
--- a/gdb/fbsd-nat.c
+++ b/gdb/fbsd-nat.c
@@ -266,6 +266,9 @@ fbsd_nat_target::info_proc (const char *args, enum info_proc_what what)
   bool do_cmdline = false;
   bool do_cwd = false;
   bool do_exe = false;
+#ifdef HAVE_KINFO_GETFILE
+  bool do_files = false;
+#endif
 #ifdef HAVE_KINFO_GETVMMAP
   bool do_mappings = false;
 #endif
@@ -296,10 +299,18 @@ fbsd_nat_target::info_proc (const char *args, enum info_proc_what what)
     case IP_CWD:
       do_cwd = true;
       break;
+#ifdef HAVE_KINFO_GETFILE
+    case IP_FILES:
+      do_files = true;
+      break;
+#endif
     case IP_ALL:
       do_cmdline = true;
       do_cwd = true;
       do_exe = true;
+#ifdef HAVE_KINFO_GETFILE
+      do_files = true;
+#endif
 #ifdef HAVE_KINFO_GETVMMAP
       do_mappings = true;
 #endif
@@ -323,7 +334,7 @@ fbsd_nat_target::info_proc (const char *args, enum info_proc_what what)
 
   printf_filtered (_("process %d\n"), pid);
 #ifdef HAVE_KINFO_GETFILE
-  if (do_cwd || do_exe)
+  if (do_cwd || do_exe || do_files)
     fdtbl.reset (kinfo_getfile (pid, &nfd));
 #endif
 
@@ -375,6 +386,37 @@ fbsd_nat_target::info_proc (const char *args, enum info_proc_what what)
       else
 	warning (_("unable to fetch executable path name"));
     }
+#ifdef HAVE_KINFO_GETFILE
+  if (do_files)
+    {
+      struct kinfo_file *kf = fdtbl.get ();
+
+      if (nfd > 0)
+	{
+	  printf_filtered (_("Open files:\n\n"));
+	  printf_filtered ("  %6s %6s %10s %9s %s\n",
+			   "FD", "Type", "Offset", "Flags  ", "Name");
+	  for (int i = 0; i < nfd; i++, kf++)
+	    {
+	      printf_filtered ("  %6s %6s %10s %8s ",
+			       fbsd_file_fd (kf->kf_fd),
+			       fbsd_file_type (kf->kf_type, kf->kf_vnode_type),
+			       kf->kf_offset > -1 ? hex_string (kf->kf_offset)
+			       : "-",
+			       fbsd_file_flags (kf->kf_flags));
+	      if (kf->kf_type == KF_TYPE_SOCKET)
+		fbsd_print_socket (kf->kf_sock_domain, kf->kf_sock_type,
+				   kf->kf_sock_protocol, &kf->kf_sa_local,
+				   &kf->kf_sa_peer);
+	      else
+		printf_filtered ("%s", kf->kf_path);
+	      printf_filtered ("\n");
+	    }
+	}
+      else
+	warning (_("unable to fetch list of open files"));
+    }
+#endif
 #ifdef HAVE_KINFO_GETVMMAP
   if (do_mappings)
     {
-- 
2.18.0

Re: [PATCH 4/5] Support 'info proc files' on live FreeBSD processes.

[Simon Marchi]

On 2018-09-08 01:36 AM, John Baldwin wrote:
> This walks the list of struct kinfo_file objects returned by a call to
> kinfo_getfile outputting a description of each open file descriptor.

LGTM.

It would be nice to share the printing of the information between core
and live process, so that we can't forget to change one if we change the
other.  But if there are some subtle differences between both loops that
would make sharing more annoying than anything, I don't mind.

Simon

Re: [PATCH 4/5] Support 'info proc files' on live FreeBSD processes.

[John Baldwin]

On 9/8/18 4:00 PM, Simon Marchi wrote:
> On 2018-09-08 01:36 AM, John Baldwin wrote:
>> This walks the list of struct kinfo_file objects returned by a call to
>> kinfo_getfile outputting a description of each open file descriptor.
> 
> LGTM.
> 
> It would be nice to share the printing of the information between core
> and live process, so that we can't forget to change one if we change the
> other.  But if there are some subtle differences between both loops that
> would make sharing more annoying than anything, I don't mind.

I've followed the same approach I used for 'info proc mappings' which is
to use shared helper routines as much as possible.

What I could perhaps do to share code is add new TARGET_FREEBSD_<foo>
target objects, but this would entail reworking the code quite a bit I
think.  It would mean that I would need a way to let a gdbarch hook into
the core target's xfer_partial method more generically (right now there is
a hook just for siginfo, but I think we'd want a hook for arbitrary
objects).  I would then rewrite the info proc bits in fbsd-tdep.c in terms
of fetching target objects and always parsing them in the core dump
format.  I think there were a few things in some of the other 'info proc'
methods that weren't quite as straightforward as for the 'files' and
'mappings' case though.

-- 
John Baldwin

                                                                            

Re: [PATCH 4/5] Support 'info proc files' on live FreeBSD processes.

[Simon Marchi]

On 2018-09-10 19:30, John Baldwin wrote:
> On 9/8/18 4:00 PM, Simon Marchi wrote:
>> On 2018-09-08 01:36 AM, John Baldwin wrote:
>>> This walks the list of struct kinfo_file objects returned by a call 
>>> to
>>> kinfo_getfile outputting a description of each open file descriptor.
>> 
>> LGTM.
>> 
>> It would be nice to share the printing of the information between core
>> and live process, so that we can't forget to change one if we change 
>> the
>> other.  But if there are some subtle differences between both loops 
>> that
>> would make sharing more annoying than anything, I don't mind.
> 
> I've followed the same approach I used for 'info proc mappings' which 
> is
> to use shared helper routines as much as possible.
> 
> What I could perhaps do to share code is add new TARGET_FREEBSD_<foo>
> target objects, but this would entail reworking the code quite a bit I
> think.  It would mean that I would need a way to let a gdbarch hook 
> into
> the core target's xfer_partial method more generically (right now there 
> is
> a hook just for siginfo, but I think we'd want a hook for arbitrary
> objects).  I would then rewrite the info proc bits in fbsd-tdep.c in 
> terms
> of fetching target objects and always parsing them in the core dump
> format.  I think there were a few things in some of the other 'info 
> proc'
> methods that weren't quite as straightforward as for the 'files' and
> 'mappings' case though.

I was thinking more about how to share the code that formats and print 
one entry.  Let's say you realize later that a column needs to be wider, 
you have to remember to update both the live and core versions.  Not 
really a big deal though.

Simon

Re: [PATCH 4/5] Support 'info proc files' on live FreeBSD processes.

[John Baldwin]

On 9/10/18 12:03 PM, Simon Marchi wrote:
> I was thinking more about how to share the code that formats and print 
> one entry.  Let's say you realize later that a column needs to be wider, 
> you have to remember to update both the live and core versions.  Not 
> really a big deal though.

Hmm, ok.  I think I've managed to do this in the V2 I will post in a bit.
I've made the helper routines fbsd-tdep.c exports more abstract ("print
header" and "print entry" rather than for individual fields).

-- 
John Baldwin

                                                                            

[PATCH 5/5] Document the 'info proc files' command.

[John Baldwin]

gdb/ChangeLog:

	* NEWS: Mention 'info proc files' command.

gdb/doc/ChangeLog:

	* gdb.texinfo (Process Information): Document "info proc files"
	command.
---
 gdb/ChangeLog       | 4 ++++
 gdb/NEWS            | 3 +++
 gdb/doc/ChangeLog   | 5 +++++
 gdb/doc/gdb.texinfo | 8 ++++++++
 4 files changed, 20 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index fb62bc55ea..720ae10ca9 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,7 @@
+2018-09-07  John Baldwin  <jhb@FreeBSD.org>
+
+	* NEWS: Mention 'info proc files' command.
+
 2018-09-07  John Baldwin  <jhb@FreeBSD.org>
 
 	* fbsd-nat.c (fbsd_nat_target::info_proc): List open file
diff --git a/gdb/NEWS b/gdb/NEWS
index 75436b0fc3..f5ea98ac52 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -51,6 +51,9 @@ maint set dwarf unwinders (on|off)
 maint show dwarf unwinders
   Control whether DWARF unwinders can be used.
 
+info proc files
+  Display a list of open files for a process.
+
 * Changed commands
 
 thread apply [all | COUNT | -COUNT] [FLAG]... COMMAND
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index 58e01e4f59..a9ee6834dc 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,8 @@
+2018-09-07  John Baldwin  <jhb@FreeBSD.org>
+
+	* gdb.texinfo (Process Information): Document "info proc files"
+	command.
+
 2018-08-29  Keith Seitz  <keiths@redhat.com>
 
 	* gdb.texinfo (Compiling and injecting code in GDB): Document
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index f2d1155b4d..42c077aa69 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -22236,6 +22236,14 @@ supported on @sc{gnu}/Linux and FreeBSD.
 Show the name of executable of the process.  This command is supported
 on @sc{gnu}/Linux and FreeBSD.
 
+@item info proc files
+@cindex info proc files
+Report the open file descriptors accessible in the program.  Each
+entry displays the index and type of each descriptor.  The file name
+is also listed for descriptors with an associated file name.  Network
+socket descriptors display the socket addresses in place of the file
+name.  This command is supported on FreeBSD.
+
 @item info proc mappings
 @cindex memory address space mappings
 Report the memory address space ranges accessible in the program.  On
-- 
2.18.0

Re: [PATCH 5/5] Document the 'info proc files' command.

[Eli Zaretskii]

> From: John Baldwin <jhb@FreeBSD.org>
> Date: Fri,  7 Sep 2018 17:36:59 -0700
> 
> diff --git a/gdb/NEWS b/gdb/NEWS
> index 75436b0fc3..f5ea98ac52 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -51,6 +51,9 @@ maint set dwarf unwinders (on|off)
>  maint show dwarf unwinders
>    Control whether DWARF unwinders can be used.
>  
> +info proc files
> +  Display a list of open files for a process.
> +

This is OK.

> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index f2d1155b4d..42c077aa69 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -22236,6 +22236,14 @@ supported on @sc{gnu}/Linux and FreeBSD.
>  Show the name of executable of the process.  This command is supported
>  on @sc{gnu}/Linux and FreeBSD.
>  
> +@item info proc files
> +@cindex info proc files
> +Report the open file descriptors accessible in the program.  Each

Not "accessible in", "open by", right?  "Accessible" is ambiguous, it
could mean "can potentially be accessed", and that is not what is
shown here, AFAIU.

> +entry displays the index and type of each descriptor.  The file name

By "index", I guess you meant the value of the descriptor, is that
right?

> +is also listed for descriptors with an associated file name.  Network
> +socket descriptors display the socket addresses in place of the file
> +name.

From the example you have shown (btw, why not show it in the manual?),
I understand that the Name field is always present, and is either a
file or directory name, or the protocol and socket address.  If that
is indeed correct, I suggest to reword the description:

  Show the file descriptors open by the process.  For each open file
  descriptor, @value{GDBN} shows its number, type (file, directory,
  character device, socket), offset, and the name of the resource open
  on the descriptor.  The resource name can be a file name (for files,
  directories, and devices) or a protocol followed by socket address
  (for network connections).

This lacks the details about "offset", which you didn't describe, and
I couldn't guess.

>        This command is supported on FreeBSD.

Only on FreeBSD?

I actually suggest to say something more future-proof, like "this
command is supported only on some systems".

Thanks.

Re: [PATCH 5/5] Document the 'info proc files' command.

[John Baldwin]

On 9/8/18 12:01 AM, Eli Zaretskii wrote:
>> From: John Baldwin <jhb@FreeBSD.org>
>> Date: Fri,  7 Sep 2018 17:36:59 -0700
>>
>> diff --git a/gdb/NEWS b/gdb/NEWS
>> index 75436b0fc3..f5ea98ac52 100644
>> --- a/gdb/NEWS
>> +++ b/gdb/NEWS
>> @@ -51,6 +51,9 @@ maint set dwarf unwinders (on|off)
>>  maint show dwarf unwinders
>>    Control whether DWARF unwinders can be used.
>>  
>> +info proc files
>> +  Display a list of open files for a process.
>> +
> 
> This is OK.
> 
>> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
>> index f2d1155b4d..42c077aa69 100644
>> --- a/gdb/doc/gdb.texinfo
>> +++ b/gdb/doc/gdb.texinfo
>> @@ -22236,6 +22236,14 @@ supported on @sc{gnu}/Linux and FreeBSD.
>>  Show the name of executable of the process.  This command is supported
>>  on @sc{gnu}/Linux and FreeBSD.
>>  
>> +@item info proc files
>> +@cindex info proc files
>> +Report the open file descriptors accessible in the program.  Each
> 
> Not "accessible in", "open by", right?  "Accessible" is ambiguous, it
> could mean "can potentially be accessed", and that is not what is
> shown here, AFAIU.

Ok.  I was probably trying to match the text from 'info proc mappings'
too closely.  Also, in the same vein, this should really say "process"
instead of "program" I think.  (The mappings one says program currently
but should probably also say "process" instead.)  (Oh, you already
included the "process" fix below as well.)

>> +entry displays the index and type of each descriptor.  The file name
> 
> By "index", I guess you meant the value of the descriptor, is that
> right?

Yes, not sure if there's a better way to describe that.  Looks like you
used "number" below and that is probably fine.

>> +is also listed for descriptors with an associated file name.  Network
>> +socket descriptors display the socket addresses in place of the file
>> +name.
> 
> From the example you have shown (btw, why not show it in the manual?),
> I understand that the Name field is always present, and is either a
> file or directory name, or the protocol and socket address.  If that
> is indeed correct, I suggest to reword the description:
> 
>   Show the file descriptors open by the process.  For each open file
>   descriptor, @value{GDBN} shows its number, type (file, directory,
>   character device, socket), offset, and the name of the resource open
>   on the descriptor.  The resource name can be a file name (for files,
>   directories, and devices) or a protocol followed by socket address
>   (for network connections).

This looks better to me, thanks.

> This lacks the details about "offset", which you didn't describe, and
> I couldn't guess.

Some file descriptors (for files for example) include a read/write offset
(the thing lseek() changes) used as the starting offset of read() and
write() (but not for syscalls like pread() and pwrite() that take an
explicit offset).  This value is usually referred to as the "offset" of
the file descriptor (e.g. in man pages for lseek).

>>        This command is supported on FreeBSD.
> 
> Only on FreeBSD?

My patch series only adds support for FreeBSD.  I expect someone else will
probably implement this on at least Linux.  I used this language to
match existing language of other 'info proc' subcommands in the manual
(e.g. 'info proc cmdline/cwd/exe/status' all include a sentence saying
"This command is supported on @sc{gnu}/Linux and FreeBSD.")

I don't mind adjusting it, I just think we should be consistent in the
language we use.  If we want to use something more future-proof we might
add a blanket statement to the "info proc" node to say that individual
subcommands are only supported on some systems or something to that
effect.

-- 
John Baldwin

                                                                            

Re: [PATCH 5/5] Document the 'info proc files' command.

[Eli Zaretskii]

> Cc: gdb-patches@sourceware.org
> From: John Baldwin <jhb@FreeBSD.org>
> Date: Mon, 10 Sep 2018 11:43:16 -0700
> 
> >   Show the file descriptors open by the process.  For each open file
> >   descriptor, @value{GDBN} shows its number, type (file, directory,
> >   character device, socket), offset, and the name of the resource open
> >   on the descriptor.  The resource name can be a file name (for files,
> >   directories, and devices) or a protocol followed by socket address
> >   (for network connections).
> 
> This looks better to me, thanks.
> 
> > This lacks the details about "offset", which you didn't describe, and
> > I couldn't guess.
> 
> Some file descriptors (for files for example) include a read/write offset
> (the thing lseek() changes) used as the starting offset of read() and
> write() (but not for syscalls like pread() and pwrite() that take an
> explicit offset).  This value is usually referred to as the "offset" of
> the file descriptor (e.g. in man pages for lseek).

I'd say "file pointer offset", then.

Re: [PATCH 5/5] Document the 'info proc files' command.

[John Baldwin]

On 9/8/18 12:01 AM, Eli Zaretskii wrote:
>> +is also listed for descriptors with an associated file name.  Network
>> +socket descriptors display the socket addresses in place of the file
>> +name.
> 
> From the example you have shown (btw, why not show it in the manual?),

Sorry, missed replying to this point.  I'm fine with adding examples to
the manual if it would be helpful.  We don't have existing examples for
other 'info proc' commands, so I was just matching the existing practice.

-- 
John Baldwin