public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 3/4] Support 'info proc' for native FreeBSD processes.
  2017-12-22 22:05 [PATCH 0/4] Support for 'info proc' on FreeBSD cores and native John Baldwin
  2017-12-22 22:05 ` [PATCH 2/4] Support 'info proc' for FreeBSD process core dumps John Baldwin
@ 2017-12-22 22:05 ` John Baldwin
  2017-12-27  2:23   ` Simon Marchi
  2017-12-22 22:05 ` [PATCH 1/4] Create psuedo sections for FreeBSD NT_PROCSTAT_(PROC|FILES|VMMAP) notes John Baldwin
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 18+ messages in thread
From: John Baldwin @ 2017-12-22 22:05 UTC (permalink / raw)
  To: gdb-patches, binutils

- Command line arguments are fetched via the kern.proc.args.<pid>
  sysctl.
- The 'cwd' and 'exe' values are obtained from the per-process
  file descriptor table returned by kinfo_getfile() from libutil.
- 'mappings' is implemented by walking the array of VM map entries
  returned by kinfo_getvmmap() from libutil.
- 'stat' and 'status' output is generated by outputting fields from
  the structure returned by the kern.proc.pid.<pid> sysctl.

gdb/ChangeLog:

	* configure.ac: Check for kinfo_getfile in libutil.
	* configure: Regenerate.
	* config.in: Regenerate.
	* fbsd-nat.c: Include "fbsd-tdep.h".
	(fbsd_fetch_cmdline): New.
	(fbsd_fetch_kinfo_proc): Move earlier and change to return a bool
	rather than calling error.
	(fbsd_info_proc): New.
	(fbsd_thread_name): Report error if fbsd_fetch_kinfo_proc fails.
	(fbsd_wait): Report warning if fbsd_fetch_kinfo_proc fails.
	(fbsd_nat_add_target): Set "to_info_proc" to "fbsd_info_proc".
---
 gdb/ChangeLog    |  14 ++
 gdb/config.in    |   3 +
 gdb/configure    |  60 +++++++++
 gdb/configure.ac |   5 +
 gdb/fbsd-nat.c   | 404 +++++++++++++++++++++++++++++++++++++++++++++++++++----
 5 files changed, 462 insertions(+), 24 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 2311749de7..3d580c1c9e 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,17 @@
+2017-12-22  John Baldwin  <jhb@FreeBSD.org>
+
+	* configure.ac: Check for kinfo_getfile in libutil.
+	* configure: Regenerate.
+	* config.in: Regenerate.
+	* fbsd-nat.c: Include "fbsd-tdep.h".
+	(fbsd_fetch_cmdline): New.
+	(fbsd_fetch_kinfo_proc): Move earlier and change to return a bool
+	rather than calling error.
+	(fbsd_info_proc): New.
+	(fbsd_thread_name): Report error if fbsd_fetch_kinfo_proc fails.
+	(fbsd_wait): Report warning if fbsd_fetch_kinfo_proc fails.
+	(fbsd_nat_add_target): Set "to_info_proc" to "fbsd_info_proc".
+
 2017-12-22  John Baldwin  <jhb@FreeBSD.org>
 
 	* fbsd-tdep.c (KVE_STRUCTSIZE, KVE_START, KVE_END, KVE_OFFSET)
diff --git a/gdb/config.in b/gdb/config.in
index 1d11a97080..ad2cc1754e 100644
--- a/gdb/config.in
+++ b/gdb/config.in
@@ -219,6 +219,9 @@
 /* Define to 1 if you have the <inttypes.h> header file. */
 #undef HAVE_INTTYPES_H
 
+/* Define to 1 if your system has the kinfo_getfile function. */
+#undef HAVE_KINFO_GETFILE
+
 /* Define to 1 if your system has the kinfo_getvmmap function. */
 #undef HAVE_KINFO_GETVMMAP
 
diff --git a/gdb/configure b/gdb/configure
index 7b250079de..a0baa7a53a 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -7927,6 +7927,66 @@ $as_echo "#define HAVE_KINFO_GETVMMAP 1" >>confdefs.h
 fi
 
 
+# fbsd-nat.c can also use kinfo_getfile.
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for library containing kinfo_getfile" >&5
+$as_echo_n "checking for library containing kinfo_getfile... " >&6; }
+if test "${ac_cv_search_kinfo_getfile+set}" = set; then :
+  $as_echo_n "(cached) " >&6
+else
+  ac_func_search_save_LIBS=$LIBS
+cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h.  */
+
+/* Override any GCC internal prototype to avoid an error.
+   Use char because int might match the return type of a GCC
+   builtin and then its argument prototype would still apply.  */
+#ifdef __cplusplus
+extern "C"
+#endif
+char kinfo_getfile ();
+int
+main ()
+{
+return kinfo_getfile ();
+  ;
+  return 0;
+}
+_ACEOF
+for ac_lib in '' util util-freebsd; do
+  if test -z "$ac_lib"; then
+    ac_res="none required"
+  else
+    ac_res=-l$ac_lib
+    LIBS="-l$ac_lib  $ac_func_search_save_LIBS"
+  fi
+  if ac_fn_c_try_link "$LINENO"; then :
+  ac_cv_search_kinfo_getfile=$ac_res
+fi
+rm -f core conftest.err conftest.$ac_objext \
+    conftest$ac_exeext
+  if test "${ac_cv_search_kinfo_getfile+set}" = set; then :
+  break
+fi
+done
+if test "${ac_cv_search_kinfo_getfile+set}" = set; then :
+
+else
+  ac_cv_search_kinfo_getfile=no
+fi
+rm conftest.$ac_ext
+LIBS=$ac_func_search_save_LIBS
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_search_kinfo_getfile" >&5
+$as_echo "$ac_cv_search_kinfo_getfile" >&6; }
+ac_res=$ac_cv_search_kinfo_getfile
+if test "$ac_res" != no; then :
+  test "$ac_res" = "none required" || LIBS="$ac_res $LIBS"
+
+$as_echo "#define HAVE_KINFO_GETFILE 1" >>confdefs.h
+
+fi
+
+
 
       if test "X$prefix" = "XNONE"; then
     acl_final_prefix="$ac_default_prefix"
diff --git a/gdb/configure.ac b/gdb/configure.ac
index 8e706b6e27..96ec77e0af 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -523,6 +523,11 @@ AC_SEARCH_LIBS(kinfo_getvmmap, util util-freebsd,
   [AC_DEFINE(HAVE_KINFO_GETVMMAP, 1,
             [Define to 1 if your system has the kinfo_getvmmap function. ])])
 
+# fbsd-nat.c can also use kinfo_getfile.
+AC_SEARCH_LIBS(kinfo_getfile, util util-freebsd,
+  [AC_DEFINE(HAVE_KINFO_GETFILE, 1,
+            [Define to 1 if your system has the kinfo_getfile function. ])])
+
 AM_ICONV
 
 # GDB may fork/exec the iconv program to get the list of supported character
diff --git a/gdb/fbsd-nat.c b/gdb/fbsd-nat.c
index 29b7ee5e33..e460c23d95 100644
--- a/gdb/fbsd-nat.c
+++ b/gdb/fbsd-nat.c
@@ -32,14 +32,16 @@
 #include <sys/signal.h>
 #include <sys/sysctl.h>
 #include <sys/user.h>
-#ifdef HAVE_KINFO_GETVMMAP
+#if defined(HAVE_KINFO_GETFILE) || defined(HAVE_KINFO_GETVMMAP)
 #include <libutil.h>
-#else
+#endif
+#if !defined(HAVE_KINFO_GETVMMAP)
 #include "filestuff.h"
 #endif
 
 #include "elf-bfd.h"
 #include "fbsd-nat.h"
+#include "fbsd-tdep.h"
 
 #include <list>
 
@@ -77,7 +79,7 @@ fbsd_pid_to_exec_file (struct target_ops *self, int pid)
   return NULL;
 }
 
-#ifdef HAVE_KINFO_GETVMMAP
+#if defined(HAVE_KINFO_GETFILE) || defined(HAVE_KINFO_GETVMMAP)
 /* Deleter for std::unique_ptr that invokes free.  */
 
 template <typename T>
@@ -85,7 +87,9 @@ struct free_deleter
 {
   void operator() (T *ptr) const { free (ptr); }
 };
+#endif
 
+#ifdef HAVE_KINFO_GETVMMAP
 /* Iterate over all the memory regions in the current inferior,
    calling FUNC for each memory region.  OBFD is passed as the last
    argument to FUNC.  */
@@ -210,6 +214,369 @@ fbsd_find_memory_regions (struct target_ops *self,
 }
 #endif
 
+/* Fetch the command line for a running process.  */
+
+static gdb::unique_xmalloc_ptr<char>
+fbsd_fetch_cmdline (pid_t pid)
+{
+  size_t len;
+  int mib[4];
+
+  len = 0;
+  mib[0] = CTL_KERN;
+  mib[1] = KERN_PROC;
+  mib[2] = KERN_PROC_ARGS;
+  mib[3] = pid;
+  if (sysctl (mib, 4, NULL, &len, NULL, 0) == -1)
+    return nullptr;
+
+  gdb::unique_xmalloc_ptr<char> cmdline ((char *) xmalloc (len));
+  if (sysctl (mib, 4, cmdline.get (), &len, NULL, 0) == -1)
+    return nullptr;
+
+  return cmdline;
+}
+
+/* Fetch the external variant of the kernel's internal process
+   structure for the process PID into KP.  */
+
+static bool
+fbsd_fetch_kinfo_proc (pid_t pid, struct kinfo_proc *kp)
+{
+  size_t len;
+  int mib[4];
+
+  len = sizeof *kp;
+  mib[0] = CTL_KERN;
+  mib[1] = KERN_PROC;
+  mib[2] = KERN_PROC_PID;
+  mib[3] = pid;
+  return (sysctl (mib, 4, kp, &len, NULL, 0) == 0);
+}
+
+/* Implement the "to_info_proc target_ops" method.  */
+
+static void
+fbsd_info_proc (struct target_ops *ops, const char *args,
+		enum info_proc_what what)
+{
+#ifdef HAVE_KINFO_GETFILE
+  std::unique_ptr<struct kinfo_file, free_deleter<struct kinfo_file>> fdtbl;
+  int nfd = 0;
+#endif
+  struct kinfo_proc kp;
+  char *tmp;
+  pid_t pid;
+  bool do_cmdline = false;
+  bool do_cwd = false;
+  bool do_exe = false;
+#ifdef HAVE_KINFO_GETVMMAP
+  bool do_mappings = false;
+#endif
+  bool do_status = false;
+  bool do_stat = false;
+  bool kp_valid = false;
+
+  switch (what)
+    {
+    case IP_MINIMAL:
+      do_cmdline = true;
+      do_cwd = true;
+      do_exe = true;
+      break;
+#ifdef HAVE_KINFO_GETVMMAP
+    case IP_MAPPINGS:
+      do_mappings = true;
+      break;
+#endif
+    case IP_STATUS:
+      do_status = true;
+      break;
+    case IP_STAT:
+      do_stat = true;
+      break;
+    case IP_CMDLINE:
+      do_cmdline = true;
+      break;
+    case IP_EXE:
+      do_exe = true;
+      break;
+    case IP_CWD:
+      do_cwd = true;
+      break;
+    case IP_ALL:
+      do_cmdline = true;
+      do_cwd = true;
+      do_exe = true;
+#ifdef HAVE_KINFO_GETVMMAP
+      do_mappings = true;
+#endif
+      do_status = true;
+      do_stat = true;
+      break;
+    default:
+      error (_("Not supported on this target."));
+    }
+
+  gdb_argv built_argv (args);
+  if (built_argv.count () == 0)
+    {
+      pid = ptid_get_pid (inferior_ptid);
+      if (pid == 0)
+	error (_("No current process: you must name one."));
+    }
+  else
+    {
+      pid = strtol (built_argv[0], NULL, 10);
+    }
+
+  printf_filtered (_("process %d\n"), pid);
+#ifdef HAVE_KINFO_GETFILE
+  if (do_cwd || do_exe)
+    fdtbl.reset (kinfo_getfile (pid, &nfd));
+#endif
+  if (do_stat || do_status)
+    {
+      kp_valid = fbsd_fetch_kinfo_proc(pid, &kp);
+      if (!kp_valid)
+	warning (_("Failed to fetch process information"));
+    }
+
+  if (do_cmdline)
+    {
+      gdb::unique_xmalloc_ptr<char> cmdline = fbsd_fetch_cmdline (pid);
+      if (cmdline)
+	printf_filtered ("cmdline = '%s'\n", cmdline.get ());
+      else
+	warning (_("unable to fetch command line"));
+    }
+  if (do_cwd)
+    {
+      const char *cwd = NULL;
+#ifdef HAVE_KINFO_GETFILE
+      struct kinfo_file *kf = fdtbl.get ();
+      for (int i = 0; i < nfd; i++, kf++)
+	{
+	  if (kf->kf_type == KF_TYPE_VNODE && kf->kf_fd == KF_FD_TYPE_CWD)
+	    {
+	      cwd = kf->kf_path;
+	      break;
+	    }
+	}
+#endif
+      if (cwd)
+	printf_filtered ("cwd = '%s'\n", cwd);
+      else
+	warning (_("unable to fetch current working directory"));
+    }
+  if (do_exe)
+    {
+      const char *exe = NULL;
+#ifdef HAVE_KINFO_GETFILE
+      struct kinfo_file *kf = fdtbl.get ();
+      for (int i = 0; i < nfd; i++, kf++)
+	{
+	  if (kf->kf_type == KF_TYPE_VNODE && kf->kf_fd == KF_FD_TYPE_TEXT)
+	    {
+	      exe = kf->kf_path;
+	      break;
+	    }
+	}
+#endif
+      if (exe == NULL)
+	exe = fbsd_pid_to_exec_file (ops, pid);
+      if (exe)
+	printf_filtered ("exe = '%s'\n", exe);
+      else
+	warning (_("unable to fetch executable path name"));
+    }
+#ifdef HAVE_KINFO_GETVMMAP
+  if (do_mappings)
+    {
+      int nvment;
+      std::unique_ptr<struct kinfo_vmentry, free_deleter<struct kinfo_vmentry>>
+	vmentl (kinfo_getvmmap (pid, &nvment));
+
+      if (vmentl)
+	{
+	  printf_filtered (_("Mapped address spaces:\n\n"));
+#ifdef __LP64__
+	  printf_filtered ("  %18s %18s %10s %10s %9s %s\n",
+			   "Start Addr",
+			   "  End Addr",
+			   "      Size", "    Offset", "Flags  ", "File");
+#else
+	  printf_filtered ("\t%10s %10s %10s %10s %9s %s\n",
+			   "Start Addr",
+			   "  End Addr",
+			   "      Size", "    Offset", "Flags  ", "File");
+#endif
+
+	  struct kinfo_vmentry *kve = vmentl.get ();
+	  for (int i = 0; i < nvment; i++, kve++)
+	    {
+	      ULONGEST start, end;
+
+	      start = kve->kve_start;
+	      end = kve->kve_end;
+#ifdef __LP64__
+	      printf_filtered ("  %18s %18s %10s %10s %9s %s\n",
+			       hex_string (start),
+			       hex_string (end),
+			       hex_string (end - start),
+			       hex_string (kve->kve_offset),
+			       fbsd_vm_map_entry_flags (kve->kve_flags,
+							kve->kve_protection),
+			       kve->kve_path);
+#else
+	      printf_filtered ("\t%10s %10s %10s %10s %9s %s\n",
+			       hex_string (start),
+			       hex_string (end),
+			       hex_string (end - start),
+			       hex_string (kve->kve_offset),
+			       fbsd_vm_map_entry_flags (kve->kve_flags,
+							kve->kve_protection),
+			       kve->kve_path);
+#endif
+	    }
+	}
+      else
+	warning (_("unable to fetch virtual memory map"));
+    }
+#endif
+  if (do_status && kp_valid)
+    {
+      const char *state;
+      int pgtok;
+
+      printf_filtered ("Name:\t%s\n", kp.ki_comm);
+      switch (kp.ki_stat)
+	{
+	case SIDL:
+	  state = "I (idle)";
+	  break;
+	case SRUN:
+	  state = "R (running)";
+	  break;
+	case SSTOP:
+	  state = "T (stopped)";
+	  break;
+	case SZOMB:
+	  state = "Z (zombie)";
+	  break;
+	case SSLEEP:
+	  state = "S (sleeping)";
+	  break;
+	case SWAIT:
+	  state = "W (interrupt wait)";
+	  break;
+	case SLOCK:
+	  state = "L (blocked on lock)";
+	  break;
+	default:
+	  state = "? (unknown)";
+	  break;
+	}
+      printf_filtered ("State:\t%s\n", state);
+      printf_filtered ("Pid:\t%d\n", kp.ki_pid);
+      printf_filtered ("PPid:\t%d\n", kp.ki_ppid);
+      printf_filtered ("Uid:\t%d %d %d\n", kp.ki_ruid, kp.ki_uid, kp.ki_svuid);
+      printf_filtered ("Gid:\t%d %d %d\n", kp.ki_rgid, kp.ki_groups[0],
+		       kp.ki_svgid);
+      printf_filtered ("Groups:\t");
+      for (int i = 0; i < kp.ki_ngroups; i++)
+	printf_filtered ("%d ", kp.ki_groups[i]);
+      printf_filtered ("\n");
+      pgtok = getpagesize () / 1024;
+      printf_filtered ("VmSize:\t%8ju kB\n", (uintmax_t) kp.ki_size / 1024);
+      printf_filtered ("VmRSS:\t%8ju kB\n", (uintmax_t) kp.ki_rssize * pgtok);
+      printf_filtered ("VmData:\t%8ju kB\n", (uintmax_t) kp.ki_dsize * pgtok);
+      printf_filtered ("VmStk:\t%8ju kB\n", (uintmax_t) kp.ki_ssize * pgtok);
+      printf_filtered ("VmExe:\t%8ju kB\n", (uintmax_t) kp.ki_tsize * pgtok);
+      printf_filtered ("SigPnd:\t");
+      for (int i = 0; i < _SIG_WORDS; i++)
+	printf_filtered ("%08x ", kp.ki_siglist.__bits[i]);
+      printf_filtered ("\n");
+      printf_filtered ("SigIgn:\t");
+      for (int i = 0; i < _SIG_WORDS; i++)
+	printf_filtered ("%08x ", kp.ki_sigignore.__bits[i]);
+      printf_filtered ("\n");
+      printf_filtered ("SigCgt:\t");
+      for (int i = 0; i < _SIG_WORDS; i++)
+	printf_filtered ("%08x ", kp.ki_sigcatch.__bits[i]);
+      printf_filtered ("\n");
+    }
+  if (do_stat && kp_valid)
+    {
+      char state;
+      int pgtok;
+
+      printf_filtered ("Exec file: %s\n", kp.ki_comm);
+      switch (kp.ki_stat)
+	{
+	case SIDL:
+	  state = 'I';
+	  break;
+	case SRUN:
+	  state = 'R';
+	  break;
+	case SSTOP:
+	  state = 'T';
+	  break;
+	case SZOMB:
+	  state = 'Z';
+	  break;
+	case SSLEEP:
+	  state = 'S';
+	  break;
+	case SWAIT:
+	  state = 'W';
+	  break;
+	case SLOCK:
+	  state = 'L';
+	  break;
+	default:
+	  state = '?';
+	  break;
+	}
+      printf_filtered ("State: %c\n", state);
+      printf_filtered ("Parent process: %d\n", kp.ki_ppid);
+      printf_filtered ("Process group: %d\n", kp.ki_pgid);
+      printf_filtered ("Session id: %d\n", kp.ki_sid);
+      printf_filtered ("TTY: %ju\n", (uintmax_t) kp.ki_tdev);
+      printf_filtered ("TTY owner process group: %d\n", kp.ki_tpgid);
+      printf_filtered ("Minor faults (no memory page): %ld\n",
+		       kp.ki_rusage.ru_minflt);
+      printf_filtered ("Minor faults, children: %ld\n",
+		       kp.ki_rusage_ch.ru_minflt);
+      printf_filtered ("Major faults (memory page faults): %ld\n",
+		       kp.ki_rusage.ru_majflt);
+      printf_filtered ("Major faults, children: %ld\n",
+		       kp.ki_rusage_ch.ru_majflt);
+      printf_filtered ("utime: %jd.%06ld\n",
+		       (intmax_t) kp.ki_rusage.ru_utime.tv_sec,
+		       kp.ki_rusage.ru_utime.tv_usec);
+      printf_filtered ("stime: %jd.%06ld\n",
+		       (intmax_t) kp.ki_rusage.ru_stime.tv_sec,
+		       kp.ki_rusage.ru_stime.tv_usec);
+      printf_filtered ("utime, children: %jd.%06ld\n",
+		       (intmax_t) kp.ki_rusage_ch.ru_utime.tv_sec,
+		       kp.ki_rusage_ch.ru_utime.tv_usec);
+      printf_filtered ("stime, children: %jd.%06ld\n",
+		       (intmax_t) kp.ki_rusage_ch.ru_stime.tv_sec,
+		       kp.ki_rusage_ch.ru_stime.tv_usec);
+      printf_filtered ("'nice' value: %d\n", kp.ki_nice);
+      printf_filtered ("start time: %jd.%06ld\n", kp.ki_start.tv_sec,
+		       kp.ki_start.tv_usec);
+      pgtok = getpagesize () / 1024;
+      printf_filtered ("Virtual memory size: %ju kB\n",
+		       (uintmax_t) kp.ki_size / 1024);
+      printf_filtered ("Resident set size: %ju kB\n",
+		       (uintmax_t) kp.ki_rssize * pgtok);
+      printf_filtered ("rlim: %ju kB\n", (uintmax_t) kp.ki_rusage.ru_maxrss);
+    }
+}
+
 #ifdef KERN_PROC_AUXV
 static enum target_xfer_status (*super_xfer_partial) (struct target_ops *ops,
 						      enum target_object object,
@@ -461,23 +828,6 @@ show_fbsd_lwp_debug (struct ui_file *file, int from_tty,
 }
 
 #if defined(TDP_RFPPWAIT) || defined(HAVE_STRUCT_PTRACE_LWPINFO_PL_TDNAME)
-/* Fetch the external variant of the kernel's internal process
-   structure for the process PID into KP.  */
-
-static void
-fbsd_fetch_kinfo_proc (pid_t pid, struct kinfo_proc *kp)
-{
-  size_t len;
-  int mib[4];
-
-  len = sizeof *kp;
-  mib[0] = CTL_KERN;
-  mib[1] = KERN_PROC;
-  mib[2] = KERN_PROC_PID;
-  mib[3] = pid;
-  if (sysctl (mib, 4, kp, &len, NULL, 0) == -1)
-    perror_with_name (("sysctl"));
-}
 #endif
 
 /*
@@ -565,7 +915,8 @@ fbsd_thread_name (struct target_ops *self, struct thread_info *thr)
   /* Note that ptrace_lwpinfo returns the process command in pl_tdname
      if a name has not been set explicitly.  Return a NULL name in
      that case.  */
-  fbsd_fetch_kinfo_proc (pid, &kp);
+  if (!fbsd_fetch_kinfo_proc (pid, &kp))
+    perror_with_name (_("Failed to fetch process information"));
   if (ptrace (PT_LWPINFO, lwp, (caddr_t) &pl, sizeof pl) == -1)
     perror_with_name (("ptrace"));
   if (strcmp (kp.ki_comm, pl.pl_tdname) == 0)
@@ -975,9 +1326,13 @@ fbsd_wait (struct target_ops *ops,
 #ifndef PTRACE_VFORK
 	      /* For vfork, the child process will have the P_PPWAIT
 		 flag set.  */
-	      fbsd_fetch_kinfo_proc (child, &kp);
-	      if (kp.ki_flag & P_PPWAIT)
-		ourstatus->kind = TARGET_WAITKIND_VFORKED;
+	      if (fbsd_fetch_kinfo_proc (child, &kp))
+		{
+		  if (kp.ki_flag & P_PPWAIT)
+		    ourstatus->kind = TARGET_WAITKIND_VFORKED;
+		}
+	      else
+		warning (_("Failed to fetch process information"));
 #endif
 	      ourstatus->value.related_pid = child_ptid;
 
@@ -1181,6 +1536,7 @@ fbsd_nat_add_target (struct target_ops *t)
 {
   t->to_pid_to_exec_file = fbsd_pid_to_exec_file;
   t->to_find_memory_regions = fbsd_find_memory_regions;
+  t->to_info_proc = fbsd_info_proc;
 #ifdef KERN_PROC_AUXV
   super_xfer_partial = t->to_xfer_partial;
   t->to_xfer_partial = fbsd_xfer_partial;
-- 
2.15.1

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/4] Create psuedo sections for FreeBSD NT_PROCSTAT_(PROC|FILES|VMMAP) notes.
  2017-12-22 22:05 [PATCH 0/4] Support for 'info proc' on FreeBSD cores and native John Baldwin
  2017-12-22 22:05 ` [PATCH 2/4] Support 'info proc' for FreeBSD process core dumps John Baldwin
  2017-12-22 22:05 ` [PATCH 3/4] Support 'info proc' for native FreeBSD processes John Baldwin
@ 2017-12-22 22:05 ` John Baldwin
  2017-12-27  1:18   ` Simon Marchi
  2018-01-02 11:49   ` Nick Clifton
  2017-12-22 22:13 ` [PATCH 4/4] Document support for 'info proc' on FreeBSD John Baldwin
  2017-12-27  1:53 ` [PATCH 0/4] Support for 'info proc' on FreeBSD cores and native Simon Marchi
  4 siblings, 2 replies; 18+ messages in thread
From: John Baldwin @ 2017-12-22 22:05 UTC (permalink / raw)
  To: gdb-patches, binutils

bfd/ChangeLog:

	* elf.c (elfcore_grok_freebsd_note): Handle
	NT_FREEBSD_PROCSTAT_PROC, NT_FREEBSD_PROCSTAT_FILES, and
	NT_FREEBSD_PROCSTAT_VMMAP.
---
 bfd/ChangeLog |  6 ++++++
 bfd/elf.c     | 12 ++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index e994da3ed6..6a3f44b23d 100644
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,3 +1,9 @@
+2017-12-22  John Baldwin  <jhb@FreeBSD.org>
+
+	* elf.c (elfcore_grok_freebsd_note): Handle
+	NT_FREEBSD_PROCSTAT_PROC, NT_FREEBSD_PROCSTAT_FILES, and
+	NT_FREEBSD_PROCSTAT_VMMAP.
+
 2017-12-19  Alan Modra  <amodra@gmail.com>
 
 	PR 22626
diff --git a/bfd/elf.c b/bfd/elf.c
index fa70a94975..b635ba29b1 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -10014,6 +10014,18 @@ elfcore_grok_freebsd_note (bfd *abfd, Elf_Internal_Note *note)
       else
 	return TRUE;
 
+    case NT_FREEBSD_PROCSTAT_PROC:
+      return elfcore_make_note_pseudosection (abfd, ".note.freebsdcore.proc",
+					      note);
+
+    case NT_FREEBSD_PROCSTAT_FILES:
+      return elfcore_make_note_pseudosection (abfd, ".note.freebsdcore.files",
+					      note);
+
+    case NT_FREEBSD_PROCSTAT_VMMAP:
+      return elfcore_make_note_pseudosection (abfd, ".note.freebsdcore.vmmap",
+					      note);
+
     case NT_FREEBSD_PROCSTAT_AUXV:
       {
 	asection *sect = bfd_make_section_anyway_with_flags (abfd, ".auxv",
-- 
2.15.1

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 0/4] Support for 'info proc' on FreeBSD cores and native
@ 2017-12-22 22:05 John Baldwin
  2017-12-22 22:05 ` [PATCH 2/4] Support 'info proc' for FreeBSD process core dumps John Baldwin
                   ` (4 more replies)
  0 siblings, 5 replies; 18+ messages in thread
From: John Baldwin @ 2017-12-22 22:05 UTC (permalink / raw)
  To: gdb-patches, binutils

This series adds initial support for the 'info proc' command on
FreeBSD native processes and process cores.  FreeBSD generally does
not use the /proc filesystem, but instead exports data structures
containing process information either via kernel system control nodes
(for live processes), or in core dump notes.

My assumption is that the format of 'info proc' is expected to be
somewhat OS-specific though probably not gratuitously so.

For 'info proc mappings' I choose to include both mapping attributes
(such as permissions) along with the object file name.

I did choose to implement versions of 'info proc stat' and 'info proc
status' that are similar to the output on Linux for now.  However,
given that the output on FreeBSD is not tied to the output of files in
/proc and that having both 'stat' and 'status' with overlapping
content seems ambiguous, I do wonder if it wouldn't be better to just
have a single command that includes one copy of the information (and
perhaps treat 'stat' as an alias of 'status' on FreeBSD)?  I also
noticed in the document that there are older commands such as 'info
proc id' and 'info proc time' that if implemented would contain a
subset of the info in the 'stat' commands.  I would possibly prefer to
resurrect these commands on FreeBSD as subsets of 'stat/status'?  What
do you all think?

I do eventually plan on adding a 'info proc files' that outputs a
table of open file descriptors.

For the documentation I made minimal changes to the existing
documentation for 'info proc' to not state that it requires /proc, but
the wording could probably use improvement.  I have also not yet
documented that FreeBSD supports 'proc stat' and 'proc status' due to
the question above.

John Baldwin (4):
  Create psuedo sections for FreeBSD NT_PROCSTAT_(PROC|FILES|VMMAP)
    notes.
  Support 'info proc' for FreeBSD process core dumps.
  Support 'info proc' for native FreeBSD processes.
  Document support for 'info proc' on FreeBSD.

 bfd/ChangeLog       |   6 +
 bfd/elf.c           |  12 +
 gdb/ChangeLog       |  33 +++
 gdb/config.in       |   3 +
 gdb/configure       |  60 +++++
 gdb/configure.ac    |   5 +
 gdb/doc/ChangeLog   |   7 +
 gdb/doc/gdb.texinfo |  19 +-
 gdb/fbsd-nat.c      | 404 ++++++++++++++++++++++++++++--
 gdb/fbsd-tdep.c     | 698 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/fbsd-tdep.h     |   1 +
 11 files changed, 1217 insertions(+), 31 deletions(-)

-- 
2.15.1

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 2/4] Support 'info proc' for FreeBSD process core dumps.
  2017-12-22 22:05 [PATCH 0/4] Support for 'info proc' on FreeBSD cores and native John Baldwin
@ 2017-12-22 22:05 ` John Baldwin
  2017-12-27  1:56   ` Simon Marchi
  2017-12-22 22:05 ` [PATCH 3/4] Support 'info proc' for native FreeBSD processes John Baldwin
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 18+ messages in thread
From: John Baldwin @ 2017-12-22 22:05 UTC (permalink / raw)
  To: gdb-patches, binutils

- Command line arguments are obtained from the pr_psargs[] array
  saved in the NT_PRPSINFO note.
- The 'cwd' and 'exe' values are obtained from the per-process file
  descriptor table stored in the NT_PROCSTAT_FILES core note.
- 'mappings' is implemented by walking the array of VM map entries
  stored in the NT_PROCSTAT_VMMAP core note.
- 'stat' and 'status' output is generated by outputting fields from
  the first structure stored in the NT_PROCSTAT_PROC core note.

gdb/ChangeLog:

	* fbsd-tdep.c (KVE_STRUCTSIZE, KVE_START, KVE_END, KVE_OFFSET)
	(KVE_FLAGS, KVE_PROTECTION, KVE_PATH, KINFO_VME_PROT_READ)
	(KINFO_VME_PROT_WRITE, KINFO_VME_PROT_EXEC, KINFO_VME_FLAG_COW)
	(KINFO_VME_FLAG_NEEDS_COPY, KINFO_VME_FLAG_NOCOREDUMP)
	(KINFO_VME_FLAG_SUPER, KINFO_VME_FLAG_GROWS_UP)
	(KINFO_VME_FLAG_GROWS_DOWN, KF_STRUCTSIZE, KF_TYPE, KF_FD)
	(KF_PATH, KINFO_FILE_TYPE_VNODE, KINFO_FILE_FD_TYPE_CWD)
	(KINFO_FILE_FD_TYPE_TEXT, struct kinfo_proc_layout)
	(kinfo_proc_layout_32, kinfo_proc_layout_i386)
	(kinfo_proc_layout_64, fbsd_vm_map_entry_flags)
	(fbsd_core_info_proc_mappings)
	(fbsd_core_vnode_path, fbsd_print_sigset)
	(fbsd_core_info_proc_status, fbsd_core_fetch_timeval)
	(fbsd_core_info_proc_stat, fbsd_core_info_proc): New.
	(fbsd_init_abi):  Install gdbarch "core_info_proc" method.
	* fbsd-tdep.h (fbsd_vm_map_entry_flags): New.
---
 gdb/ChangeLog   |  19 ++
 gdb/fbsd-tdep.c | 698 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 gdb/fbsd-tdep.h |   1 +
 3 files changed, 718 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 1b4b13aba9..2311749de7 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,22 @@
+2017-12-22  John Baldwin  <jhb@FreeBSD.org>
+
+	* fbsd-tdep.c (KVE_STRUCTSIZE, KVE_START, KVE_END, KVE_OFFSET)
+	(KVE_FLAGS, KVE_PROTECTION, KVE_PATH, KINFO_VME_PROT_READ)
+	(KINFO_VME_PROT_WRITE, KINFO_VME_PROT_EXEC, KINFO_VME_FLAG_COW)
+	(KINFO_VME_FLAG_NEEDS_COPY, KINFO_VME_FLAG_NOCOREDUMP)
+	(KINFO_VME_FLAG_SUPER, KINFO_VME_FLAG_GROWS_UP)
+	(KINFO_VME_FLAG_GROWS_DOWN, KF_STRUCTSIZE, KF_TYPE, KF_FD)
+	(KF_PATH, KINFO_FILE_TYPE_VNODE, KINFO_FILE_FD_TYPE_CWD)
+	(KINFO_FILE_FD_TYPE_TEXT, struct kinfo_proc_layout)
+	(kinfo_proc_layout_32, kinfo_proc_layout_i386)
+	(kinfo_proc_layout_64, fbsd_vm_map_entry_flags)
+	(fbsd_core_info_proc_mappings)
+	(fbsd_core_vnode_path, fbsd_print_sigset)
+	(fbsd_core_info_proc_status, fbsd_core_fetch_timeval)
+	(fbsd_core_info_proc_stat, fbsd_core_info_proc): New.
+	(fbsd_init_abi):  Install gdbarch "core_info_proc" method.
+	* fbsd-tdep.h (fbsd_vm_map_entry_flags): New.
+
 2017-12-21  Simon Marchi  <simon.marchi@ericsson.com>
 	    Sergio Durigan Junior  <sergiodj@redhat.com>
 
diff --git a/gdb/fbsd-tdep.c b/gdb/fbsd-tdep.c
index f89b520c5f..454036dcac 100644
--- a/gdb/fbsd-tdep.c
+++ b/gdb/fbsd-tdep.c
@@ -52,6 +52,223 @@
 #define	SIZE64_SIGINFO_T	80
 #define	SIZE32_SIGINFO_T	64
 
+/* Offsets in data structure used in NT_FREEBSD_PROCSTAT_VMMAP core
+   dump notes.  See <sys/user.h> for the definition of struct
+   kinfo_vmentry.  This data structure should have the same layout on
+   all architectures.  */
+
+#define	KVE_STRUCTSIZE		0x0
+#define	KVE_START		0x8
+#define	KVE_END			0x10
+#define	KVE_OFFSET		0x18
+#define	KVE_FLAGS		0x2c
+#define	KVE_PROTECTION		0x56
+#define	KVE_PATH		0x88
+
+/* Flags in the 'kve_protection' field in struct kinfo_vmentry.  These
+   match the KVME_PROT_* constants in <sys/user.h>.  */
+
+#define	KINFO_VME_PROT_READ	0x00000001
+#define	KINFO_VME_PROT_WRITE	0x00000002
+#define	KINFO_VME_PROT_EXEC	0x00000004
+
+/* Flags in the 'kve_flags' field in struct kinfo_vmentry.  These
+   match the KVME_FLAG_* constants in <sys/user.h>.  */
+
+#define	KINFO_VME_FLAG_COW		0x00000001
+#define	KINFO_VME_FLAG_NEEDS_COPY	0x00000002
+#define	KINFO_VME_FLAG_NOCOREDUMP	0x00000004
+#define	KINFO_VME_FLAG_SUPER		0x00000008
+#define	KINFO_VME_FLAG_GROWS_UP		0x00000010
+#define	KINFO_VME_FLAG_GROWS_DOWN	0x00000020
+
+/* Offsets in data structure used in NT_FREEBSD_PROCSTAT_FILES core
+   dump notes.  See <sys/user.h> for the definition of struct
+   kinfo_file.  This data structure should have the same layout on all
+   architectures.  */
+
+#define	KF_STRUCTSIZE		0x0
+#define	KF_TYPE			0x4
+#define	KF_FD			0x8
+#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
+
+/* 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_TEXT	-5
+
+/* Offsets in data structure used in NT_FREEBSD_PROCSTAT_PROC core
+   dump notes.  See <sys/user.h> for the definition of struct
+   kinfo_proc.  This data structure has different layouts on different
+   architectures mostly due to ILP32 vs LP64.  However, FreeBSD/i386
+   uses a 32-bit time_t while all other architectures use a 64-bit
+   time_t.
+
+   The core dump note actually contains one kinfo_proc structure for
+   each thread, but all of the process-wide data can be obtained from
+   the first structure.  One result of this note's format is that some
+   of the process-wide status available in the native target method
+   from the kern.proc.pid.<pid> sysctl such as ki_stat and ki_siglist
+   is not available from a core dump.  Instead, the per-thread data
+   structures contain the value of these fields for individual
+   threads.  */
+
+struct kinfo_proc_layout
+  {
+    /* Offsets of struct kinfo_proc members.  */
+    int ki_layout;
+    int ki_pid;
+    int ki_ppid;
+    int ki_pgid;
+    int ki_tpgid;
+    int ki_sid;
+    int ki_tdev_freebsd11;
+    int ki_sigignore;
+    int ki_sigcatch;
+    int ki_uid;
+    int ki_ruid;
+    int ki_svuid;
+    int ki_rgid;
+    int ki_svgid;
+    int ki_ngroups;
+    int ki_groups;
+    int ki_size;
+    int ki_rssize;
+    int ki_tsize;
+    int ki_dsize;
+    int ki_ssize;
+    int ki_start;
+    int ki_nice;
+    int ki_comm;
+    int ki_tdev;
+    int ki_rusage;
+    int ki_rusage_ch;
+
+    /* Offsets of struct rusage members.  */
+    int ru_utime;
+    int ru_stime;
+    int ru_maxrss;
+    int ru_minflt;
+    int ru_majflt;
+  };
+
+struct kinfo_proc_layout kinfo_proc_layout_32 =
+  {
+    .ki_layout = 0x4,
+    .ki_pid = 0x28,
+    .ki_ppid = 0x2c,
+    .ki_pgid = 0x30,
+    .ki_tpgid = 0x34,
+    .ki_sid = 0x38,
+    .ki_tdev_freebsd11 = 0x44,
+    .ki_sigignore = 0x68,
+    .ki_sigcatch = 0x78,
+    .ki_uid = 0x88,
+    .ki_ruid = 0x8c,
+    .ki_svuid = 0x90,
+    .ki_rgid = 0x94,
+    .ki_svgid = 0x98,
+    .ki_ngroups = 0x9c,
+    .ki_groups = 0xa0,
+    .ki_size = 0xe0,
+    .ki_rssize = 0xe4,
+    .ki_tsize = 0xec,
+    .ki_dsize = 0xf0,
+    .ki_ssize = 0xf4,
+    .ki_start = 0x118,
+    .ki_nice = 0x145,
+    .ki_comm = 0x17f,
+    .ki_tdev = 0x1f0,
+    .ki_rusage = 0x220,
+    .ki_rusage_ch = 0x278,
+
+    .ru_utime = 0x0,
+    .ru_stime = 0x10,
+    .ru_maxrss = 0x20,
+    .ru_minflt = 0x30,
+    .ru_majflt = 0x34,
+  };
+
+struct kinfo_proc_layout kinfo_proc_layout_i386 =
+  {
+    .ki_layout = 0x4,
+    .ki_pid = 0x28,
+    .ki_ppid = 0x2c,
+    .ki_pgid = 0x30,
+    .ki_tpgid = 0x34,
+    .ki_sid = 0x38,
+    .ki_tdev_freebsd11 = 0x44,
+    .ki_sigignore = 0x68,
+    .ki_sigcatch = 0x78,
+    .ki_uid = 0x88,
+    .ki_ruid = 0x8c,
+    .ki_svuid = 0x90,
+    .ki_rgid = 0x94,
+    .ki_svgid = 0x98,
+    .ki_ngroups = 0x9c,
+    .ki_groups = 0xa0,
+    .ki_size = 0xe0,
+    .ki_rssize = 0xe4,
+    .ki_tsize = 0xec,
+    .ki_dsize = 0xf0,
+    .ki_ssize = 0xf4,
+    .ki_start = 0x118,
+    .ki_nice = 0x135,
+    .ki_comm = 0x16f,
+    .ki_tdev = 0x1e0,
+    .ki_rusage = 0x210,
+    .ki_rusage_ch = 0x258,
+
+    .ru_utime = 0x0,
+    .ru_stime = 0x8,
+    .ru_maxrss = 0x10,
+    .ru_minflt = 0x20,
+    .ru_majflt = 0x24,
+  };
+
+struct kinfo_proc_layout kinfo_proc_layout_64 =
+  {
+    .ki_layout = 0x4,
+    .ki_pid = 0x48,
+    .ki_ppid = 0x4c,
+    .ki_pgid = 0x50,
+    .ki_tpgid = 0x54,
+    .ki_sid = 0x58,
+    .ki_tdev_freebsd11 = 0x64,
+    .ki_sigignore = 0x88,
+    .ki_sigcatch = 0x98,
+    .ki_uid = 0xa8,
+    .ki_ruid = 0xac,
+    .ki_svuid = 0xb0,
+    .ki_rgid = 0xb4,
+    .ki_svgid = 0xb8,
+    .ki_ngroups = 0xbc,
+    .ki_groups = 0xc0,
+    .ki_size = 0x100,
+    .ki_rssize = 0x108,
+    .ki_tsize = 0x118,
+    .ki_dsize = 0x120,
+    .ki_ssize = 0x128,
+    .ki_start = 0x150,
+    .ki_nice = 0x185,
+    .ki_comm = 0x1bf,
+    .ki_tdev = 0x230,
+    .ki_rusage = 0x260,
+    .ki_rusage_ch = 0x2f0,
+
+    .ru_utime = 0x0,
+    .ru_stime = 0x10,
+    .ru_maxrss = 0x20,
+    .ru_minflt = 0x40,
+    .ru_majflt = 0x48,
+  };
+
 static struct gdbarch_data *fbsd_gdbarch_data_handle;
 
 struct fbsd_gdbarch_data
@@ -367,6 +584,486 @@ fbsd_make_corefile_notes (struct gdbarch *gdbarch, bfd *obfd, int *note_size)
   return note_data;
 }
 
+/* Helper function to generate mappings flags for a single VM map entry.  */
+
+const char *
+fbsd_vm_map_entry_flags (int kve_flags, int kve_protection)
+{
+  static char vm_flags[9];
+
+  vm_flags[0] = (kve_protection & KINFO_VME_PROT_READ) ? 'r' : '-';
+  vm_flags[1] = (kve_protection & KINFO_VME_PROT_WRITE) ? 'w' : '-';
+  vm_flags[2] = (kve_protection & KINFO_VME_PROT_EXEC) ? 'x' : '-';
+  vm_flags[3] = ' ';
+  vm_flags[4] = (kve_flags & KINFO_VME_FLAG_COW) ? 'C' : '-';
+  vm_flags[5] = (kve_flags & KINFO_VME_FLAG_NEEDS_COPY) ? 'N' : '-';
+  vm_flags[6] = (kve_flags & KINFO_VME_FLAG_SUPER) ? 'S' : '-';
+  vm_flags[7] = (kve_flags & KINFO_VME_FLAG_GROWS_UP) ? 'U'
+    : (kve_flags & KINFO_VME_FLAG_GROWS_DOWN) ? 'D' : '-';
+  vm_flags[8] = '\0';
+
+  return vm_flags;
+}
+
+/* Implement "info proc mappings" for a corefile.  */
+
+static void
+fbsd_core_info_proc_mappings (struct gdbarch *gdbarch)
+{
+  asection *section;
+  unsigned char *descdata, *descend;
+  size_t note_size;
+
+  section = bfd_get_section_by_name (core_bfd, ".note.freebsdcore.vmmap");
+  if (section == NULL)
+    {
+      warning (_("unable to find mappings 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 (_("Mapped address spaces:\n\n"));
+  if (gdbarch_addr_bit (gdbarch) == 64)
+    {
+      printf_filtered ("  %18s %18s %10s %10s %9s %s\n",
+		       "Start Addr",
+		       "  End Addr",
+		       "      Size", "    Offset", "Flags  ", "File");
+    }
+  else
+    {
+      printf_filtered ("\t%10s %10s %10s %10s %9s %s\n",
+		       "Start Addr",
+		       "  End Addr",
+		       "      Size", "    Offset", "Flags  ", "File");
+    }
+
+  while (descdata + KVE_PATH < descend)
+    {
+      ULONGEST start, end, offset, flags, prot, structsize;
+
+      structsize = bfd_get_32 (core_bfd, descdata + KVE_STRUCTSIZE);
+      if (structsize < KVE_PATH)
+	error (_("malformed core note - vmmap entry too small"));
+
+      start = bfd_get_64 (core_bfd, descdata + KVE_START);
+      end = bfd_get_64 (core_bfd, descdata + KVE_END);
+      offset = bfd_get_64 (core_bfd, descdata + KVE_OFFSET);
+      flags = bfd_get_32 (core_bfd, descdata + KVE_FLAGS);
+      prot = bfd_get_32 (core_bfd, descdata + KVE_PROTECTION);
+      if (gdbarch_addr_bit (gdbarch) == 64)
+	{
+	  printf_filtered ("  %18s %18s %10s %10s %9s %s\n",
+			   paddress (gdbarch, start),
+			   paddress (gdbarch, end),
+			   hex_string (end - start),
+			   hex_string (offset),
+			   fbsd_vm_map_entry_flags (flags, prot),
+			   descdata + KVE_PATH);
+	}
+      else
+	{
+	  printf_filtered ("\t%10s %10s %10s %10s %9s %s\n",
+			   paddress (gdbarch, start),
+			   paddress (gdbarch, end),
+			   hex_string (end - start),
+			   hex_string (offset),
+			   fbsd_vm_map_entry_flags (flags, prot),
+			   descdata + KVE_PATH);
+	}
+
+      descdata += structsize;
+    }
+}
+
+/* Fetch the pathname of a vnode for a single file descriptor from the
+   file table core note.  */
+
+static gdb::unique_xmalloc_ptr<char>
+fbsd_core_vnode_path (struct gdbarch *gdbarch, int fd)
+{
+  asection *section;
+  unsigned char *descdata, *descend;
+  size_t note_size;
+
+  section = bfd_get_section_by_name (core_bfd, ".note.freebsdcore.files");
+  if (section == NULL)
+    return nullptr;
+
+  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;
+
+  while (descdata + KVE_PATH < descend)
+    {
+      ULONGEST structsize;
+
+      structsize = bfd_get_32 (core_bfd, descdata + KF_STRUCTSIZE);
+      if (structsize < KVE_PATH)
+	error (_("malformed core note - vmmap entry too small"));
+
+      if (bfd_get_32 (core_bfd, descdata + KF_TYPE) == KINFO_FILE_TYPE_VNODE
+	  && bfd_get_signed_32 (core_bfd, descdata + KF_FD) == fd)
+	{
+	  char *path = (char *) descdata + KF_PATH;
+	  return gdb::unique_xmalloc_ptr<char> (xstrdup (path));
+	}
+
+      descdata += structsize;
+    }
+  return nullptr;
+}
+
+/* Print out the contents of a signal set.  */
+
+static void
+fbsd_print_sigset (const char *descr, unsigned char *sigset)
+{
+  printf_filtered ("%s:\t", descr);
+  for (int i = 0; i < _SIG_WORDS; i++)
+    printf_filtered ("%08x ",
+		     (unsigned int) bfd_get_32 (core_bfd, sigset + i * 4));
+  printf_filtered ("\n");
+}
+
+/* Implement "info proc status" for a corefile.  */
+
+static void
+fbsd_core_info_proc_status (struct gdbarch *gdbarch)
+{
+  struct kinfo_proc_layout *kp;
+  asection *section;
+  const char *state;
+  unsigned char *descdata, *descend;
+  int addr_bit, long_bit;
+  size_t note_size;
+  ULONGEST value;
+
+  section = bfd_get_section_by_name (core_bfd, ".note.freebsdcore.proc");
+  if (section == NULL)
+    {
+      warning (_("unable to find process info in core file"));
+      return;
+    }
+
+  addr_bit = gdbarch_addr_bit (gdbarch);
+  if (addr_bit == 64)
+    kp = &kinfo_proc_layout_64;
+  else if (bfd_get_arch (core_bfd) == bfd_arch_i386)
+    kp = &kinfo_proc_layout_i386;
+  else
+    kp = &kinfo_proc_layout_32;
+
+  note_size = bfd_get_section_size (section);
+  if (note_size < 4 + kp->ki_rusage_ch + kp->ru_majflt + addr_bit)
+    error (_("malformed core note - too short"));
+
+  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;
+
+  /* Verify 'ki_layout' is 0.  */
+  if (bfd_get_32 (core_bfd, descdata + kp->ki_layout) != 0)
+    {
+      warning (_("unsupported process information in core file"));
+      return;
+    }
+
+  printf_filtered ("Name:\t%.19s\n", descdata + kp->ki_comm);
+  printf_filtered ("Pid:\t%s\n",
+		   pulongest(bfd_get_32 (core_bfd, descdata + kp->ki_pid)));
+  printf_filtered ("PPid:\t%s\n",
+		   pulongest(bfd_get_32 (core_bfd, descdata + kp->ki_ppid)));
+  printf_filtered ("Uid:\t%s %s %s\n",
+		   pulongest(bfd_get_32 (core_bfd, descdata + kp->ki_ruid)),
+		   pulongest(bfd_get_32 (core_bfd, descdata + kp->ki_uid)),
+		   pulongest(bfd_get_32 (core_bfd, descdata + kp->ki_svuid)));
+  printf_filtered ("Gid:\t%s %s %s\n",
+		   pulongest(bfd_get_32 (core_bfd, descdata + kp->ki_rgid)),
+		   pulongest(bfd_get_32 (core_bfd, descdata + kp->ki_groups)),
+		   pulongest(bfd_get_32 (core_bfd, descdata + kp->ki_svgid)));
+  printf_filtered ("Groups:\t");
+  uint16_t ngroups = bfd_get_16 (core_bfd, descdata + kp->ki_ngroups);
+  for (int i = 0; i < ngroups; i++)
+    printf_filtered ("%s ",
+		     pulongest(bfd_get_32 (core_bfd,
+					   descdata + kp->ki_groups + i * 4)));
+  printf_filtered ("\n");
+  value = bfd_get (addr_bit, core_bfd, descdata + kp->ki_size);
+  printf_filtered ("VmSize:\t%8ju kB\n", (uintmax_t) value / 1024);
+  value = bfd_get (addr_bit, core_bfd, descdata + kp->ki_rssize);
+  printf_filtered ("VmRSS:\t%8ju pages\n", (uintmax_t) value);
+  value = bfd_get (addr_bit, core_bfd, descdata + kp->ki_dsize);
+  printf_filtered ("VmData:\t%8ju pages\n", (uintmax_t) value);
+  value = bfd_get (addr_bit, core_bfd, descdata + kp->ki_ssize);
+  printf_filtered ("VmStk:\t%8ju pages\n", (uintmax_t) value);
+  value = bfd_get (addr_bit, core_bfd, descdata + kp->ki_tsize);
+  printf_filtered ("VmExe:\t%8ju pages\n", (uintmax_t) value);
+  fbsd_print_sigset ("SigIgn", descdata + kp->ki_sigignore);
+  fbsd_print_sigset ("SigCgt", descdata + kp->ki_sigcatch);
+}
+
+/* Helper function to read a struct timeval.  */
+
+static void
+fbsd_core_fetch_timeval (struct gdbarch *gdbarch, unsigned char *data,
+			 LONGEST &sec, ULONGEST &usec)
+{
+  if (gdbarch_addr_bit (gdbarch) == 64)
+    {
+      sec = bfd_get_signed_64 (core_bfd, data);
+      usec = bfd_get_64 (core_bfd, data + 8);
+    }
+  else if (bfd_get_arch (core_bfd) == bfd_arch_i386)
+    {
+      sec = bfd_get_signed_32 (core_bfd, data);
+      usec = bfd_get_32 (core_bfd, data + 4);
+    }
+  else
+    {
+      sec = bfd_get_signed_64 (core_bfd, data);
+      usec = bfd_get_32 (core_bfd, data + 8);
+    }
+}
+
+/* Implement "info proc stat" for a corefile.  */
+
+static void
+fbsd_core_info_proc_stat (struct gdbarch *gdbarch)
+{
+  struct kinfo_proc_layout *kp;
+  asection *section;
+  char state;
+  unsigned char *descdata, *descend;
+  int addr_bit;
+  size_t note_size;
+  ULONGEST value;
+  LONGEST sec;
+
+  section = bfd_get_section_by_name (core_bfd, ".note.freebsdcore.proc");
+  if (section == NULL)
+    {
+      warning (_("unable to find process info in core file"));
+      return;
+    }
+
+  addr_bit = gdbarch_addr_bit (gdbarch);
+  if (addr_bit == 64)
+    kp = &kinfo_proc_layout_64;
+  else if (bfd_get_arch (core_bfd) == bfd_arch_i386)
+    kp = &kinfo_proc_layout_i386;
+  else
+    kp = &kinfo_proc_layout_32;
+
+  note_size = bfd_get_section_size (section);
+  if (note_size < 4 + kp->ki_rusage_ch + kp->ru_majflt + addr_bit)
+    error (_("malformed core note - too short"));
+
+  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;
+
+  /* Verify 'ki_layout' is 0.  */
+  if (bfd_get_32 (core_bfd, descdata + kp->ki_layout) != 0)
+    {
+      warning (_("unsupported process information in core file"));
+      return;
+    }
+
+  printf_filtered ("Exec file: %.19s\n", descdata + kp->ki_comm);
+  printf_filtered ("Parent process: %s\n",
+		   pulongest (bfd_get_32 (core_bfd, descdata + kp->ki_ppid)));
+  printf_filtered ("Process group: %s\n",
+		   pulongest (bfd_get_32 (core_bfd, descdata + kp->ki_pgid)));
+  printf_filtered ("Session id: %s\n",
+		   pulongest (bfd_get_32 (core_bfd, descdata + kp->ki_sid)));
+
+  /* FreeBSD 12.0 and later store a 64-bit dev_t at 'ki_tdev'.  Older
+     kernels store a 32-bit dev_t at 'ki_tdev_freebsd11'.  In older
+     kernels the 64-bit 'ki_tdev' field is in a reserved section of
+     the structure that is cleared to zero.  Assume that a zero value
+     in ki_tdev indicates a core dump from an older kernel and use the
+     value in 'ki_tdev_freebsd11' instead.  */
+  value = bfd_get_64 (core_bfd, descdata + kp->ki_tdev);
+  if (value == 0)
+    value = bfd_get_32 (core_bfd, descdata + kp->ki_tdev_freebsd11);
+  printf_filtered ("TTY: %s\n", pulongest (value));
+  printf_filtered ("TTY owner process group: %s\n",
+		   pulongest (bfd_get_32 (core_bfd, descdata + kp->ki_tpgid)));
+  value = bfd_get (addr_bit, core_bfd,
+		   descdata + kp->ki_rusage + kp->ru_minflt);
+  printf_filtered ("Minor faults (no memory page): %s\n", pulongest (value));
+  value = bfd_get (addr_bit, core_bfd,
+		   descdata + kp->ki_rusage_ch + kp->ru_minflt);
+  printf_filtered ("Minor faults, children: %s\n", pulongest (value));
+  value = bfd_get (addr_bit, core_bfd,
+		   descdata + kp->ki_rusage + kp->ru_majflt);
+  printf_filtered ("Major faults (memory page faults): %s\n",
+		   pulongest (value));
+  value = bfd_get (addr_bit, core_bfd,
+		   descdata + kp->ki_rusage_ch + kp->ru_majflt);
+  printf_filtered ("Major faults, children: %s\n", pulongest (value));
+  fbsd_core_fetch_timeval (gdbarch,
+			   descdata + kp->ki_rusage + kp->ru_utime,
+			   sec, value);
+  printf_filtered ("utime: %s.%06d\n", plongest (sec), (int) value);
+  fbsd_core_fetch_timeval (gdbarch,
+			   descdata + kp->ki_rusage + kp->ru_stime,
+			   sec, value);
+  printf_filtered ("stime: %s.%06d\n", plongest (sec), (int) value);
+  fbsd_core_fetch_timeval (gdbarch,
+			   descdata + kp->ki_rusage_ch + kp->ru_utime,
+			   sec, value);
+  printf_filtered ("utime, children: %s.%06d\n", plongest (sec), (int) value);
+  fbsd_core_fetch_timeval (gdbarch,
+			   descdata + kp->ki_rusage_ch + kp->ru_stime,
+			   sec, value);
+  printf_filtered ("stime, children: %s.%06d\n", plongest (sec), (int) value);
+  printf_filtered ("'nice' value: %d\n",
+		   bfd_get_signed_8 (core_bfd, descdata + kp->ki_nice));
+  fbsd_core_fetch_timeval (gdbarch, descdata + kp->ki_start, sec, value);
+  printf_filtered ("start time: %s.%06d\n", plongest (sec), (int) value);
+  printf_filtered ("Virtual memory size: %s kB\n",
+		   pulongest (bfd_get (addr_bit, core_bfd,
+				       descdata + kp->ki_start) / 1024));
+  printf_filtered ("Resident set size: %s pages\n",
+		   pulongest (bfd_get (addr_bit, core_bfd,
+				       descdata + kp->ki_rssize)));
+  value = bfd_get (addr_bit, core_bfd,
+		   descdata + kp->ki_rusage + kp->ru_maxrss);
+  printf_filtered ("rlim: %s kB\n", pulongest (value));
+}
+
+/* Implement the "core_info_proc" gdbarch method.  */
+
+static void
+fbsd_core_info_proc (struct gdbarch *gdbarch, const char *args,
+		     enum info_proc_what what)
+{
+  bool do_cmdline = false;
+  bool do_cwd = false;
+  bool do_exe = false;
+  bool do_mappings = false;
+  bool do_status = false;
+  bool do_stat = false;
+  int pid;
+
+  switch (what)
+    {
+    case IP_MINIMAL:
+      do_cmdline = true;
+      do_cwd = true;
+      do_exe = true;
+      break;
+    case IP_MAPPINGS:
+      do_mappings = true;
+      break;
+    case IP_STATUS:
+      do_status = true;
+      break;
+    case IP_STAT:
+      do_stat = true;
+      break;
+    case IP_CMDLINE:
+      do_cmdline = true;
+      break;
+    case IP_EXE:
+      do_exe = true;
+      break;
+    case IP_CWD:
+      do_cwd = true;
+      break;
+    case IP_ALL:
+      do_cmdline = true;
+      do_cwd = true;
+      do_exe = true;
+      do_mappings = true;
+      do_status = true;
+      do_stat = true;
+      break;
+    default:
+      error (_("Not supported on this architecture."));
+    }
+
+  pid = bfd_core_file_pid (core_bfd);
+  if (pid != 0)
+    printf_filtered (_("process %d\n"), pid);
+
+  if (do_cmdline)
+    {
+      const char *cmdline;
+
+      cmdline = bfd_core_file_failing_command (core_bfd);
+      if (cmdline)
+	printf_filtered ("cmdline = '%s'\n", cmdline);
+      else
+	warning (_("Command line unavailable"));
+    }
+  if (do_cwd)
+    {
+      gdb::unique_xmalloc_ptr<char> cwd =
+	fbsd_core_vnode_path (gdbarch, KINFO_FILE_FD_TYPE_CWD);
+      if (cwd)
+	printf_filtered ("cwd = '%s'\n", cwd.get ());
+      else
+	warning (_("unable to read current working directory"));
+    }
+  if (do_exe)
+    {
+      gdb::unique_xmalloc_ptr<char> exe =
+	fbsd_core_vnode_path (gdbarch, KINFO_FILE_FD_TYPE_TEXT);
+      if (exe)
+	printf_filtered ("exe = '%s'\n", exe.get ());
+      else
+	warning (_("unable to read executable path name"));
+    }
+  if (do_mappings)
+    fbsd_core_info_proc_mappings (gdbarch);
+  if (do_status)
+    fbsd_core_info_proc_status (gdbarch);
+  if (do_stat)
+    fbsd_core_info_proc_stat (gdbarch);
+}
+
 /* Print descriptions of FreeBSD-specific AUXV entries to FILE.  */
 
 static void
@@ -519,6 +1216,7 @@ fbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   set_gdbarch_core_thread_name (gdbarch, fbsd_core_thread_name);
   set_gdbarch_core_xfer_siginfo (gdbarch, fbsd_core_xfer_siginfo);
   set_gdbarch_make_corefile_notes (gdbarch, fbsd_make_corefile_notes);
+  set_gdbarch_core_info_proc (gdbarch, fbsd_core_info_proc);
   set_gdbarch_print_auxv_entry (gdbarch, fbsd_print_auxv_entry);
   set_gdbarch_get_siginfo_type (gdbarch, fbsd_get_siginfo_type);
 
diff --git a/gdb/fbsd-tdep.h b/gdb/fbsd-tdep.h
index ff2e207aae..0029e03d41 100644
--- a/gdb/fbsd-tdep.h
+++ b/gdb/fbsd-tdep.h
@@ -21,5 +21,6 @@
 #define FBSD_TDEP_H
 
 extern void fbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch);
+extern const char *fbsd_vm_map_entry_flags (int kve_flags, int kve_protection);
 
 #endif /* fbsd-tdep.h */
-- 
2.15.1

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 4/4] Document support for 'info proc' on FreeBSD.
  2017-12-22 22:05 [PATCH 0/4] Support for 'info proc' on FreeBSD cores and native John Baldwin
                   ` (2 preceding siblings ...)
  2017-12-22 22:05 ` [PATCH 1/4] Create psuedo sections for FreeBSD NT_PROCSTAT_(PROC|FILES|VMMAP) notes John Baldwin
@ 2017-12-22 22:13 ` John Baldwin
  2017-12-23  8:46   ` Eli Zaretskii
  2017-12-27  1:53 ` [PATCH 0/4] Support for 'info proc' on FreeBSD cores and native Simon Marchi
  4 siblings, 1 reply; 18+ messages in thread
From: John Baldwin @ 2017-12-22 22:13 UTC (permalink / raw)
  To: gdb-patches, binutils

gdb/doc/ChangeLog:

	* gdb.texinfo (Native): Rename subsection from SVR4 Process
	Information to Process Information.
	(Process Information): Document support for "info proc" on
	FreeBSD.
---
 gdb/doc/ChangeLog   |  7 +++++++
 gdb/doc/gdb.texinfo | 19 ++++++++++++-------
 2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index 319e0c3cad..749771165a 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,3 +1,10 @@
+2017-12-22  John Baldwin  <jhb@FreeBSD.org>
+
+	* gdb.texinfo (Native): Rename subsection from SVR4 Process
+	Information to Process Information.
+	(Process Information): Document support for "info proc" on
+	FreeBSD.
+
 2017-12-15  Sergio Durigan Junior  <sergiodj@redhat.com>
 
 	PR cli/16224
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 60ed80c363..cc70d2b249 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -21734,8 +21734,8 @@ Set current context from proc address.  This command isn't available on
 modern FreeBSD systems.
 @end table
 
-@node SVR4 Process Information
-@subsection SVR4 Process Information
+@node Process Information
+@subsection Process Information
 @cindex /proc
 @cindex examine process image
 @cindex process info via @file{/proc}
@@ -21750,8 +21750,12 @@ information about the process running your program, or about any
 process running on your system.  This includes, as of this writing,
 @sc{gnu}/Linux and Solaris, for example.
 
-This command may also work on core files that were created on a system
-that has the @samp{/proc} facility.
+This command may also work on other systems that provide process
+information via other means.  For example, FreeBSD systems use system
+control nodes to provide process information.
+
+This command may also work on core files that were created on
+@sc{gnu}/Linux and FreeBSD systems.
 
 @table @code
 @kindex info proc
@@ -21775,12 +21779,12 @@ a process ID rather than a thread ID).
 @item info proc cmdline
 @cindex info proc cmdline
 Show the original command line of the process.  This command is
-specific to @sc{gnu}/Linux.
+supported on @sc{gnu}/Linux and FreeBSD.
 
 @item info proc cwd
 @cindex info proc cwd
 Show the current working directory of the process.  This command is
-specific to @sc{gnu}/Linux.
+supported on @sc{gnu}/Linux and FreeBSD.
 
 @item info proc exe
 @cindex info proc exe
@@ -21793,7 +21797,8 @@ Report the memory address space ranges accessible in the program, with
 information on whether the process has read, write, or execute access
 rights to each range.  On @sc{gnu}/Linux systems, each memory range
 includes the object file which is mapped to that range, instead of the
-memory access rights to that range.
+memory access rights to that range.  On FreeBSD systems, each memory
+range includes the memory access rights and object file.
 
 @item info proc stat
 @itemx info proc status
-- 
2.15.1

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 4/4] Document support for 'info proc' on FreeBSD.
  2017-12-22 22:13 ` [PATCH 4/4] Document support for 'info proc' on FreeBSD John Baldwin
@ 2017-12-23  8:46   ` Eli Zaretskii
  0 siblings, 0 replies; 18+ messages in thread
From: Eli Zaretskii @ 2017-12-23  8:46 UTC (permalink / raw)
  To: John Baldwin; +Cc: gdb-patches, binutils

> From: John Baldwin <jhb@FreeBSD.org>
> Date: Fri, 22 Dec 2017 14:05:13 -0800
> 
> gdb/doc/ChangeLog:
> 
> 	* gdb.texinfo (Native): Rename subsection from SVR4 Process
> 	Information to Process Information.
> 	(Process Information): Document support for "info proc" on
> 	FreeBSD.

OK, with one comment:

> -@node SVR4 Process Information
> -@subsection SVR4 Process Information
> +@node Process Information
> +@subsection Process Information

This renaming must be done in the entire gdb.texinfo file, or else you
will get errors while producing the Info manual.  I see at least one
more instance of the old name.

Thanks.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/4] Create psuedo sections for FreeBSD NT_PROCSTAT_(PROC|FILES|VMMAP) notes.
  2017-12-22 22:05 ` [PATCH 1/4] Create psuedo sections for FreeBSD NT_PROCSTAT_(PROC|FILES|VMMAP) notes John Baldwin
@ 2017-12-27  1:18   ` Simon Marchi
  2018-01-02 11:49   ` Nick Clifton
  1 sibling, 0 replies; 18+ messages in thread
From: Simon Marchi @ 2017-12-27  1:18 UTC (permalink / raw)
  To: John Baldwin, gdb-patches, binutils

On 2017-12-22 05:05 PM, John Baldwin wrote:
> bfd/ChangeLog:
> 
> 	* elf.c (elfcore_grok_freebsd_note): Handle
> 	NT_FREEBSD_PROCSTAT_PROC, NT_FREEBSD_PROCSTAT_FILES, and
> 	NT_FREEBSD_PROCSTAT_VMMAP.

There's a typo in the title, psuedo -> pseudo.

Simon

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/4] Support for 'info proc' on FreeBSD cores and native
  2017-12-22 22:05 [PATCH 0/4] Support for 'info proc' on FreeBSD cores and native John Baldwin
                   ` (3 preceding siblings ...)
  2017-12-22 22:13 ` [PATCH 4/4] Document support for 'info proc' on FreeBSD John Baldwin
@ 2017-12-27  1:53 ` Simon Marchi
  2018-01-03 19:05   ` John Baldwin
  4 siblings, 1 reply; 18+ messages in thread
From: Simon Marchi @ 2017-12-27  1:53 UTC (permalink / raw)
  To: John Baldwin, gdb-patches, binutils

On 2017-12-22 05:05 PM, John Baldwin wrote:
> This series adds initial support for the 'info proc' command on
> FreeBSD native processes and process cores.  FreeBSD generally does
> not use the /proc filesystem, but instead exports data structures
> containing process information either via kernel system control nodes
> (for live processes), or in core dump notes.
> 
> My assumption is that the format of 'info proc' is expected to be
> somewhat OS-specific though probably not gratuitously so.
> 
> For 'info proc mappings' I choose to include both mapping attributes
> (such as permissions) along with the object file name.
> 
> I did choose to implement versions of 'info proc stat' and 'info proc
> status' that are similar to the output on Linux for now.  However,
> given that the output on FreeBSD is not tied to the output of files in
> /proc and that having both 'stat' and 'status' with overlapping
> content seems ambiguous, I do wonder if it wouldn't be better to just
> have a single command that includes one copy of the information (and
> perhaps treat 'stat' as an alias of 'status' on FreeBSD)?  I also
> noticed in the document that there are older commands such as 'info
> proc id' and 'info proc time' that if implemented would contain a
> subset of the info in the 'stat' commands.  I would possibly prefer to
> resurrect these commands on FreeBSD as subsets of 'stat/status'?  What
> do you all think?
> 
> I do eventually plan on adding a 'info proc files' that outputs a
> table of open file descriptors.
> 
> For the documentation I made minimal changes to the existing
> documentation for 'info proc' to not state that it requires /proc, but
> the wording could probably use improvement.  I have also not yet
> documented that FreeBSD supports 'proc stat' and 'proc status' due to
> the question above.

Hi John,

From reading the documentation, "info proc" seems to have been introduced
specifically to print things from /proc.  I find it too bad however that
the command line interface is based so closely on the /proc interface,
since it brings all of its quirks with it (e.g. stat vs status).  Also,
the important thing to the user is the information, regardless of where
it comes from.

With your patch, it moves "info proc" a little bit from "printing /proc"
to "print things about a process", which I think is totally fine.  I think
you could change the doc to put even less emphasis on the fact that the info
comes from /proc.

I'm fine with what you suggested above.

Simon

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/4] Support 'info proc' for FreeBSD process core dumps.
  2017-12-22 22:05 ` [PATCH 2/4] Support 'info proc' for FreeBSD process core dumps John Baldwin
@ 2017-12-27  1:56   ` Simon Marchi
  2018-01-03 19:05     ` John Baldwin
  0 siblings, 1 reply; 18+ messages in thread
From: Simon Marchi @ 2017-12-27  1:56 UTC (permalink / raw)
  To: John Baldwin, gdb-patches, binutils

On 2017-12-22 05:05 PM, John Baldwin wrote:
> - Command line arguments are obtained from the pr_psargs[] array
>   saved in the NT_PRPSINFO note.
> - The 'cwd' and 'exe' values are obtained from the per-process file
>   descriptor table stored in the NT_PROCSTAT_FILES core note.
> - 'mappings' is implemented by walking the array of VM map entries
>   stored in the NT_PROCSTAT_VMMAP core note.
> - 'stat' and 'status' output is generated by outputting fields from
>   the first structure stored in the NT_PROCSTAT_PROC core note.
> 
> gdb/ChangeLog:
> 
> 	* fbsd-tdep.c (KVE_STRUCTSIZE, KVE_START, KVE_END, KVE_OFFSET)
> 	(KVE_FLAGS, KVE_PROTECTION, KVE_PATH, KINFO_VME_PROT_READ)
> 	(KINFO_VME_PROT_WRITE, KINFO_VME_PROT_EXEC, KINFO_VME_FLAG_COW)
> 	(KINFO_VME_FLAG_NEEDS_COPY, KINFO_VME_FLAG_NOCOREDUMP)
> 	(KINFO_VME_FLAG_SUPER, KINFO_VME_FLAG_GROWS_UP)
> 	(KINFO_VME_FLAG_GROWS_DOWN, KF_STRUCTSIZE, KF_TYPE, KF_FD)
> 	(KF_PATH, KINFO_FILE_TYPE_VNODE, KINFO_FILE_FD_TYPE_CWD)
> 	(KINFO_FILE_FD_TYPE_TEXT, struct kinfo_proc_layout)
> 	(kinfo_proc_layout_32, kinfo_proc_layout_i386)
> 	(kinfo_proc_layout_64, fbsd_vm_map_entry_flags)
> 	(fbsd_core_info_proc_mappings)
> 	(fbsd_core_vnode_path, fbsd_print_sigset)
> 	(fbsd_core_info_proc_status, fbsd_core_fetch_timeval)
> 	(fbsd_core_info_proc_stat, fbsd_core_info_proc): New.
> 	(fbsd_init_abi):  Install gdbarch "core_info_proc" method.
> 	* fbsd-tdep.h (fbsd_vm_map_entry_flags): New.
> ---
>  gdb/ChangeLog   |  19 ++
>  gdb/fbsd-tdep.c | 698 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  gdb/fbsd-tdep.h |   1 +
>  3 files changed, 718 insertions(+)
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 1b4b13aba9..2311749de7 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,22 @@
> +2017-12-22  John Baldwin  <jhb@FreeBSD.org>
> +
> +	* fbsd-tdep.c (KVE_STRUCTSIZE, KVE_START, KVE_END, KVE_OFFSET)
> +	(KVE_FLAGS, KVE_PROTECTION, KVE_PATH, KINFO_VME_PROT_READ)
> +	(KINFO_VME_PROT_WRITE, KINFO_VME_PROT_EXEC, KINFO_VME_FLAG_COW)
> +	(KINFO_VME_FLAG_NEEDS_COPY, KINFO_VME_FLAG_NOCOREDUMP)
> +	(KINFO_VME_FLAG_SUPER, KINFO_VME_FLAG_GROWS_UP)
> +	(KINFO_VME_FLAG_GROWS_DOWN, KF_STRUCTSIZE, KF_TYPE, KF_FD)
> +	(KF_PATH, KINFO_FILE_TYPE_VNODE, KINFO_FILE_FD_TYPE_CWD)
> +	(KINFO_FILE_FD_TYPE_TEXT, struct kinfo_proc_layout)
> +	(kinfo_proc_layout_32, kinfo_proc_layout_i386)
> +	(kinfo_proc_layout_64, fbsd_vm_map_entry_flags)
> +	(fbsd_core_info_proc_mappings)
> +	(fbsd_core_vnode_path, fbsd_print_sigset)
> +	(fbsd_core_info_proc_status, fbsd_core_fetch_timeval)
> +	(fbsd_core_info_proc_stat, fbsd_core_info_proc): New.
> +	(fbsd_init_abi):  Install gdbarch "core_info_proc" method.
> +	* fbsd-tdep.h (fbsd_vm_map_entry_flags): New.
> +
>  2017-12-21  Simon Marchi  <simon.marchi@ericsson.com>
>  	    Sergio Durigan Junior  <sergiodj@redhat.com>
>  
> diff --git a/gdb/fbsd-tdep.c b/gdb/fbsd-tdep.c
> index f89b520c5f..454036dcac 100644
> --- a/gdb/fbsd-tdep.c
> +++ b/gdb/fbsd-tdep.c
> @@ -52,6 +52,223 @@
>  #define	SIZE64_SIGINFO_T	80
>  #define	SIZE32_SIGINFO_T	64
>  
> +/* Offsets in data structure used in NT_FREEBSD_PROCSTAT_VMMAP core
> +   dump notes.  See <sys/user.h> for the definition of struct
> +   kinfo_vmentry.  This data structure should have the same layout on
> +   all architectures.  */
> +
> +#define	KVE_STRUCTSIZE		0x0
> +#define	KVE_START		0x8
> +#define	KVE_END			0x10
> +#define	KVE_OFFSET		0x18
> +#define	KVE_FLAGS		0x2c
> +#define	KVE_PROTECTION		0x56
> +#define	KVE_PATH		0x88
> +
> +/* Flags in the 'kve_protection' field in struct kinfo_vmentry.  These
> +   match the KVME_PROT_* constants in <sys/user.h>.  */
> +
> +#define	KINFO_VME_PROT_READ	0x00000001
> +#define	KINFO_VME_PROT_WRITE	0x00000002
> +#define	KINFO_VME_PROT_EXEC	0x00000004
> +
> +/* Flags in the 'kve_flags' field in struct kinfo_vmentry.  These
> +   match the KVME_FLAG_* constants in <sys/user.h>.  */
> +
> +#define	KINFO_VME_FLAG_COW		0x00000001
> +#define	KINFO_VME_FLAG_NEEDS_COPY	0x00000002
> +#define	KINFO_VME_FLAG_NOCOREDUMP	0x00000004
> +#define	KINFO_VME_FLAG_SUPER		0x00000008
> +#define	KINFO_VME_FLAG_GROWS_UP		0x00000010
> +#define	KINFO_VME_FLAG_GROWS_DOWN	0x00000020
> +
> +/* Offsets in data structure used in NT_FREEBSD_PROCSTAT_FILES core
> +   dump notes.  See <sys/user.h> for the definition of struct
> +   kinfo_file.  This data structure should have the same layout on all
> +   architectures.  */
> +
> +#define	KF_STRUCTSIZE		0x0
> +#define	KF_TYPE			0x4
> +#define	KF_FD			0x8
> +#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
> +
> +/* 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_TEXT	-5
> +
> +/* Offsets in data structure used in NT_FREEBSD_PROCSTAT_PROC core
> +   dump notes.  See <sys/user.h> for the definition of struct
> +   kinfo_proc.  This data structure has different layouts on different
> +   architectures mostly due to ILP32 vs LP64.  However, FreeBSD/i386
> +   uses a 32-bit time_t while all other architectures use a 64-bit
> +   time_t.
> +
> +   The core dump note actually contains one kinfo_proc structure for
> +   each thread, but all of the process-wide data can be obtained from
> +   the first structure.  One result of this note's format is that some
> +   of the process-wide status available in the native target method
> +   from the kern.proc.pid.<pid> sysctl such as ki_stat and ki_siglist
> +   is not available from a core dump.  Instead, the per-thread data
> +   structures contain the value of these fields for individual
> +   threads.  */
> +
> +struct kinfo_proc_layout

struct definitions should not be indented, they should look like:

struct kinfo_proc_layout
{
  int ki_layout;
  ...
}

> +  {
> +    /* Offsets of struct kinfo_proc members.  */
> +    int ki_layout;
> +    int ki_pid;
> +    int ki_ppid;
> +    int ki_pgid;
> +    int ki_tpgid;
> +    int ki_sid;
> +    int ki_tdev_freebsd11;
> +    int ki_sigignore;
> +    int ki_sigcatch;
> +    int ki_uid;
> +    int ki_ruid;
> +    int ki_svuid;
> +    int ki_rgid;
> +    int ki_svgid;
> +    int ki_ngroups;
> +    int ki_groups;
> +    int ki_size;
> +    int ki_rssize;
> +    int ki_tsize;
> +    int ki_dsize;
> +    int ki_ssize;
> +    int ki_start;
> +    int ki_nice;
> +    int ki_comm;
> +    int ki_tdev;
> +    int ki_rusage;
> +    int ki_rusage_ch;
> +
> +    /* Offsets of struct rusage members.  */
> +    int ru_utime;
> +    int ru_stime;
> +    int ru_maxrss;
> +    int ru_minflt;
> +    int ru_majflt;
> +  };
> +
> +struct kinfo_proc_layout kinfo_proc_layout_32 =

I would suggest making these const.

> +  {
> +    .ki_layout = 0x4,
> +    .ki_pid = 0x28,
> +    .ki_ppid = 0x2c,
> +    .ki_pgid = 0x30,
> +    .ki_tpgid = 0x34,
> +    .ki_sid = 0x38,
> +    .ki_tdev_freebsd11 = 0x44,
> +    .ki_sigignore = 0x68,
> +    .ki_sigcatch = 0x78,
> +    .ki_uid = 0x88,
> +    .ki_ruid = 0x8c,
> +    .ki_svuid = 0x90,
> +    .ki_rgid = 0x94,
> +    .ki_svgid = 0x98,
> +    .ki_ngroups = 0x9c,
> +    .ki_groups = 0xa0,
> +    .ki_size = 0xe0,
> +    .ki_rssize = 0xe4,
> +    .ki_tsize = 0xec,
> +    .ki_dsize = 0xf0,
> +    .ki_ssize = 0xf4,
> +    .ki_start = 0x118,
> +    .ki_nice = 0x145,
> +    .ki_comm = 0x17f,
> +    .ki_tdev = 0x1f0,
> +    .ki_rusage = 0x220,
> +    .ki_rusage_ch = 0x278,
> +
> +    .ru_utime = 0x0,
> +    .ru_stime = 0x10,
> +    .ru_maxrss = 0x20,
> +    .ru_minflt = 0x30,
> +    .ru_majflt = 0x34,
> +  };
> +
> +struct kinfo_proc_layout kinfo_proc_layout_i386 =
> +  {
> +    .ki_layout = 0x4,
> +    .ki_pid = 0x28,
> +    .ki_ppid = 0x2c,
> +    .ki_pgid = 0x30,
> +    .ki_tpgid = 0x34,
> +    .ki_sid = 0x38,
> +    .ki_tdev_freebsd11 = 0x44,
> +    .ki_sigignore = 0x68,
> +    .ki_sigcatch = 0x78,
> +    .ki_uid = 0x88,
> +    .ki_ruid = 0x8c,
> +    .ki_svuid = 0x90,
> +    .ki_rgid = 0x94,
> +    .ki_svgid = 0x98,
> +    .ki_ngroups = 0x9c,
> +    .ki_groups = 0xa0,
> +    .ki_size = 0xe0,
> +    .ki_rssize = 0xe4,
> +    .ki_tsize = 0xec,
> +    .ki_dsize = 0xf0,
> +    .ki_ssize = 0xf4,
> +    .ki_start = 0x118,
> +    .ki_nice = 0x135,
> +    .ki_comm = 0x16f,
> +    .ki_tdev = 0x1e0,
> +    .ki_rusage = 0x210,
> +    .ki_rusage_ch = 0x258,
> +
> +    .ru_utime = 0x0,
> +    .ru_stime = 0x8,
> +    .ru_maxrss = 0x10,
> +    .ru_minflt = 0x20,
> +    .ru_majflt = 0x24,
> +  };
> +
> +struct kinfo_proc_layout kinfo_proc_layout_64 =
> +  {
> +    .ki_layout = 0x4,
> +    .ki_pid = 0x48,
> +    .ki_ppid = 0x4c,
> +    .ki_pgid = 0x50,
> +    .ki_tpgid = 0x54,
> +    .ki_sid = 0x58,
> +    .ki_tdev_freebsd11 = 0x64,
> +    .ki_sigignore = 0x88,
> +    .ki_sigcatch = 0x98,
> +    .ki_uid = 0xa8,
> +    .ki_ruid = 0xac,
> +    .ki_svuid = 0xb0,
> +    .ki_rgid = 0xb4,
> +    .ki_svgid = 0xb8,
> +    .ki_ngroups = 0xbc,
> +    .ki_groups = 0xc0,
> +    .ki_size = 0x100,
> +    .ki_rssize = 0x108,
> +    .ki_tsize = 0x118,
> +    .ki_dsize = 0x120,
> +    .ki_ssize = 0x128,
> +    .ki_start = 0x150,
> +    .ki_nice = 0x185,
> +    .ki_comm = 0x1bf,
> +    .ki_tdev = 0x230,
> +    .ki_rusage = 0x260,
> +    .ki_rusage_ch = 0x2f0,
> +
> +    .ru_utime = 0x0,
> +    .ru_stime = 0x10,
> +    .ru_maxrss = 0x20,
> +    .ru_minflt = 0x40,
> +    .ru_majflt = 0x48,
> +  };
> +
>  static struct gdbarch_data *fbsd_gdbarch_data_handle;
>  
>  struct fbsd_gdbarch_data
> @@ -367,6 +584,486 @@ fbsd_make_corefile_notes (struct gdbarch *gdbarch, bfd *obfd, int *note_size)
>    return note_data;
>  }
>  
> +/* Helper function to generate mappings flags for a single VM map entry.  */
> +
> +const char *
> +fbsd_vm_map_entry_flags (int kve_flags, int kve_protection)
> +{
> +  static char vm_flags[9];
> +
> +  vm_flags[0] = (kve_protection & KINFO_VME_PROT_READ) ? 'r' : '-';
> +  vm_flags[1] = (kve_protection & KINFO_VME_PROT_WRITE) ? 'w' : '-';
> +  vm_flags[2] = (kve_protection & KINFO_VME_PROT_EXEC) ? 'x' : '-';
> +  vm_flags[3] = ' ';
> +  vm_flags[4] = (kve_flags & KINFO_VME_FLAG_COW) ? 'C' : '-';
> +  vm_flags[5] = (kve_flags & KINFO_VME_FLAG_NEEDS_COPY) ? 'N' : '-';
> +  vm_flags[6] = (kve_flags & KINFO_VME_FLAG_SUPER) ? 'S' : '-';
> +  vm_flags[7] = (kve_flags & KINFO_VME_FLAG_GROWS_UP) ? 'U'
> +    : (kve_flags & KINFO_VME_FLAG_GROWS_DOWN) ? 'D' : '-';
> +  vm_flags[8] = '\0';
> +
> +  return vm_flags;
> +}
> +
> +/* Implement "info proc mappings" for a corefile.  */
> +
> +static void
> +fbsd_core_info_proc_mappings (struct gdbarch *gdbarch)
> +{
> +  asection *section;
> +  unsigned char *descdata, *descend;
> +  size_t note_size;
> +
> +  section = bfd_get_section_by_name (core_bfd, ".note.freebsdcore.vmmap");
> +  if (section == NULL)
> +    {
> +      warning (_("unable to find mappings 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 (_("Mapped address spaces:\n\n"));
> +  if (gdbarch_addr_bit (gdbarch) == 64)
> +    {
> +      printf_filtered ("  %18s %18s %10s %10s %9s %s\n",
> +		       "Start Addr",
> +		       "  End Addr",
> +		       "      Size", "    Offset", "Flags  ", "File");
> +    }
> +  else
> +    {
> +      printf_filtered ("\t%10s %10s %10s %10s %9s %s\n",
> +		       "Start Addr",
> +		       "  End Addr",
> +		       "      Size", "    Offset", "Flags  ", "File");
> +    }
> +
> +  while (descdata + KVE_PATH < descend)
> +    {
> +      ULONGEST start, end, offset, flags, prot, structsize;
> +
> +      structsize = bfd_get_32 (core_bfd, descdata + KVE_STRUCTSIZE);
> +      if (structsize < KVE_PATH)
> +	error (_("malformed core note - vmmap entry too small"));
> +
> +      start = bfd_get_64 (core_bfd, descdata + KVE_START);
> +      end = bfd_get_64 (core_bfd, descdata + KVE_END);
> +      offset = bfd_get_64 (core_bfd, descdata + KVE_OFFSET);
> +      flags = bfd_get_32 (core_bfd, descdata + KVE_FLAGS);
> +      prot = bfd_get_32 (core_bfd, descdata + KVE_PROTECTION);
> +      if (gdbarch_addr_bit (gdbarch) == 64)
> +	{
> +	  printf_filtered ("  %18s %18s %10s %10s %9s %s\n",
> +			   paddress (gdbarch, start),
> +			   paddress (gdbarch, end),
> +			   hex_string (end - start),
> +			   hex_string (offset),
> +			   fbsd_vm_map_entry_flags (flags, prot),
> +			   descdata + KVE_PATH);
> +	}
> +      else
> +	{
> +	  printf_filtered ("\t%10s %10s %10s %10s %9s %s\n",
> +			   paddress (gdbarch, start),
> +			   paddress (gdbarch, end),
> +			   hex_string (end - start),
> +			   hex_string (offset),
> +			   fbsd_vm_map_entry_flags (flags, prot),
> +			   descdata + KVE_PATH);
> +	}
> +
> +      descdata += structsize;
> +    }
> +}
> +
> +/* Fetch the pathname of a vnode for a single file descriptor from the
> +   file table core note.  */
> +
> +static gdb::unique_xmalloc_ptr<char>
> +fbsd_core_vnode_path (struct gdbarch *gdbarch, int fd)
> +{
> +  asection *section;
> +  unsigned char *descdata, *descend;
> +  size_t note_size;
> +
> +  section = bfd_get_section_by_name (core_bfd, ".note.freebsdcore.files");
> +  if (section == NULL)
> +    return nullptr;
> +
> +  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;
> +
> +  while (descdata + KVE_PATH < descend)
> +    {
> +      ULONGEST structsize;
> +
> +      structsize = bfd_get_32 (core_bfd, descdata + KF_STRUCTSIZE);
> +      if (structsize < KVE_PATH)
> +	error (_("malformed core note - vmmap entry too small"));
> +
> +      if (bfd_get_32 (core_bfd, descdata + KF_TYPE) == KINFO_FILE_TYPE_VNODE
> +	  && bfd_get_signed_32 (core_bfd, descdata + KF_FD) == fd)
> +	{
> +	  char *path = (char *) descdata + KF_PATH;
> +	  return gdb::unique_xmalloc_ptr<char> (xstrdup (path));
> +	}
> +
> +      descdata += structsize;
> +    }
> +  return nullptr;
> +}
> +
> +/* Print out the contents of a signal set.  */
> +
> +static void
> +fbsd_print_sigset (const char *descr, unsigned char *sigset)
> +{
> +  printf_filtered ("%s:\t", descr);
> +  for (int i = 0; i < _SIG_WORDS; i++)

_SIG_WORDS seems to be FreeBSD-specific, so shouldn't be used in the tdep file,
unless we redefine it.

> +    printf_filtered ("%08x ",
> +		     (unsigned int) bfd_get_32 (core_bfd, sigset + i * 4));
> +  printf_filtered ("\n");
> +}
> +
> +/* Implement "info proc status" for a corefile.  */
> +
> +static void
> +fbsd_core_info_proc_status (struct gdbarch *gdbarch)
> +{
> +  struct kinfo_proc_layout *kp;
> +  asection *section;
> +  const char *state;
> +  unsigned char *descdata, *descend;
> +  int addr_bit, long_bit;
> +  size_t note_size;
> +  ULONGEST value;
> +
> +  section = bfd_get_section_by_name (core_bfd, ".note.freebsdcore.proc");
> +  if (section == NULL)
> +    {
> +      warning (_("unable to find process info in core file"));
> +      return;
> +    }
> +
> +  addr_bit = gdbarch_addr_bit (gdbarch);
> +  if (addr_bit == 64)
> +    kp = &kinfo_proc_layout_64;
> +  else if (bfd_get_arch (core_bfd) == bfd_arch_i386)
> +    kp = &kinfo_proc_layout_i386;
> +  else
> +    kp = &kinfo_proc_layout_32;
> +
> +  note_size = bfd_get_section_size (section);
> +  if (note_size < 4 + kp->ki_rusage_ch + kp->ru_majflt + addr_bit)
> +    error (_("malformed core note - too short"));
> +
> +  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;
> +
> +  /* Verify 'ki_layout' is 0.  */
> +  if (bfd_get_32 (core_bfd, descdata + kp->ki_layout) != 0)
> +    {
> +      warning (_("unsupported process information in core file"));
> +      return;
> +    }
> +
> +  printf_filtered ("Name:\t%.19s\n", descdata + kp->ki_comm);
> +  printf_filtered ("Pid:\t%s\n",
> +		   pulongest(bfd_get_32 (core_bfd, descdata + kp->ki_pid)));

Missing a few spaces before parentheses here and there.

> +  printf_filtered ("PPid:\t%s\n",
> +		   pulongest(bfd_get_32 (core_bfd, descdata + kp->ki_ppid)));
> +  printf_filtered ("Uid:\t%s %s %s\n",
> +		   pulongest(bfd_get_32 (core_bfd, descdata + kp->ki_ruid)),
> +		   pulongest(bfd_get_32 (core_bfd, descdata + kp->ki_uid)),
> +		   pulongest(bfd_get_32 (core_bfd, descdata + kp->ki_svuid)));
> +  printf_filtered ("Gid:\t%s %s %s\n",
> +		   pulongest(bfd_get_32 (core_bfd, descdata + kp->ki_rgid)),
> +		   pulongest(bfd_get_32 (core_bfd, descdata + kp->ki_groups)),
> +		   pulongest(bfd_get_32 (core_bfd, descdata + kp->ki_svgid)));
> +  printf_filtered ("Groups:\t");
> +  uint16_t ngroups = bfd_get_16 (core_bfd, descdata + kp->ki_ngroups);
> +  for (int i = 0; i < ngroups; i++)
> +    printf_filtered ("%s ",
> +		     pulongest(bfd_get_32 (core_bfd,
> +					   descdata + kp->ki_groups + i * 4)));
> +  printf_filtered ("\n");
> +  value = bfd_get (addr_bit, core_bfd, descdata + kp->ki_size);
> +  printf_filtered ("VmSize:\t%8ju kB\n", (uintmax_t) value / 1024);
> +  value = bfd_get (addr_bit, core_bfd, descdata + kp->ki_rssize);
> +  printf_filtered ("VmRSS:\t%8ju pages\n", (uintmax_t) value);
> +  value = bfd_get (addr_bit, core_bfd, descdata + kp->ki_dsize);
> +  printf_filtered ("VmData:\t%8ju pages\n", (uintmax_t) value);
> +  value = bfd_get (addr_bit, core_bfd, descdata + kp->ki_ssize);
> +  printf_filtered ("VmStk:\t%8ju pages\n", (uintmax_t) value);
> +  value = bfd_get (addr_bit, core_bfd, descdata + kp->ki_tsize);
> +  printf_filtered ("VmExe:\t%8ju pages\n", (uintmax_t) value);
> +  fbsd_print_sigset ("SigIgn", descdata + kp->ki_sigignore);
> +  fbsd_print_sigset ("SigCgt", descdata + kp->ki_sigcatch);
> +}
> +
> +/* Helper function to read a struct timeval.  */
> +
> +static void
> +fbsd_core_fetch_timeval (struct gdbarch *gdbarch, unsigned char *data,
> +			 LONGEST &sec, ULONGEST &usec)
> +{
> +  if (gdbarch_addr_bit (gdbarch) == 64)
> +    {
> +      sec = bfd_get_signed_64 (core_bfd, data);
> +      usec = bfd_get_64 (core_bfd, data + 8);
> +    }
> +  else if (bfd_get_arch (core_bfd) == bfd_arch_i386)
> +    {
> +      sec = bfd_get_signed_32 (core_bfd, data);
> +      usec = bfd_get_32 (core_bfd, data + 4);
> +    }
> +  else
> +    {
> +      sec = bfd_get_signed_64 (core_bfd, data);
> +      usec = bfd_get_32 (core_bfd, data + 8);
> +    }
> +}
> +
> +/* Implement "info proc stat" for a corefile.  */
> +
> +static void
> +fbsd_core_info_proc_stat (struct gdbarch *gdbarch)
> +{
> +  struct kinfo_proc_layout *kp;
> +  asection *section;
> +  char state;
> +  unsigned char *descdata, *descend;
> +  int addr_bit;
> +  size_t note_size;
> +  ULONGEST value;
> +  LONGEST sec;
> +
> +  section = bfd_get_section_by_name (core_bfd, ".note.freebsdcore.proc");
> +  if (section == NULL)
> +    {
> +      warning (_("unable to find process info in core file"));
> +      return;
> +    }
> +
> +  addr_bit = gdbarch_addr_bit (gdbarch);
> +  if (addr_bit == 64)
> +    kp = &kinfo_proc_layout_64;
> +  else if (bfd_get_arch (core_bfd) == bfd_arch_i386)
> +    kp = &kinfo_proc_layout_i386;
> +  else
> +    kp = &kinfo_proc_layout_32;
> +
> +  note_size = bfd_get_section_size (section);
> +  if (note_size < 4 + kp->ki_rusage_ch + kp->ru_majflt + addr_bit)
> +    error (_("malformed core note - too short"));
> +
> +  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;
> +
> +  /* Verify 'ki_layout' is 0.  */
> +  if (bfd_get_32 (core_bfd, descdata + kp->ki_layout) != 0)
> +    {
> +      warning (_("unsupported process information in core file"));
> +      return;
> +    }
> +
> +  printf_filtered ("Exec file: %.19s\n", descdata + kp->ki_comm);
> +  printf_filtered ("Parent process: %s\n",
> +		   pulongest (bfd_get_32 (core_bfd, descdata + kp->ki_ppid)));
> +  printf_filtered ("Process group: %s\n",
> +		   pulongest (bfd_get_32 (core_bfd, descdata + kp->ki_pgid)));
> +  printf_filtered ("Session id: %s\n",
> +		   pulongest (bfd_get_32 (core_bfd, descdata + kp->ki_sid)));
> +
> +  /* FreeBSD 12.0 and later store a 64-bit dev_t at 'ki_tdev'.  Older
> +     kernels store a 32-bit dev_t at 'ki_tdev_freebsd11'.  In older
> +     kernels the 64-bit 'ki_tdev' field is in a reserved section of
> +     the structure that is cleared to zero.  Assume that a zero value
> +     in ki_tdev indicates a core dump from an older kernel and use the
> +     value in 'ki_tdev_freebsd11' instead.  */
> +  value = bfd_get_64 (core_bfd, descdata + kp->ki_tdev);
> +  if (value == 0)
> +    value = bfd_get_32 (core_bfd, descdata + kp->ki_tdev_freebsd11);
> +  printf_filtered ("TTY: %s\n", pulongest (value));
> +  printf_filtered ("TTY owner process group: %s\n",
> +		   pulongest (bfd_get_32 (core_bfd, descdata + kp->ki_tpgid)));
> +  value = bfd_get (addr_bit, core_bfd,
> +		   descdata + kp->ki_rusage + kp->ru_minflt);
> +  printf_filtered ("Minor faults (no memory page): %s\n", pulongest (value));
> +  value = bfd_get (addr_bit, core_bfd,
> +		   descdata + kp->ki_rusage_ch + kp->ru_minflt);
> +  printf_filtered ("Minor faults, children: %s\n", pulongest (value));
> +  value = bfd_get (addr_bit, core_bfd,
> +		   descdata + kp->ki_rusage + kp->ru_majflt);
> +  printf_filtered ("Major faults (memory page faults): %s\n",
> +		   pulongest (value));
> +  value = bfd_get (addr_bit, core_bfd,
> +		   descdata + kp->ki_rusage_ch + kp->ru_majflt);
> +  printf_filtered ("Major faults, children: %s\n", pulongest (value));
> +  fbsd_core_fetch_timeval (gdbarch,
> +			   descdata + kp->ki_rusage + kp->ru_utime,
> +			   sec, value);
> +  printf_filtered ("utime: %s.%06d\n", plongest (sec), (int) value);
> +  fbsd_core_fetch_timeval (gdbarch,
> +			   descdata + kp->ki_rusage + kp->ru_stime,
> +			   sec, value);
> +  printf_filtered ("stime: %s.%06d\n", plongest (sec), (int) value);
> +  fbsd_core_fetch_timeval (gdbarch,
> +			   descdata + kp->ki_rusage_ch + kp->ru_utime,
> +			   sec, value);
> +  printf_filtered ("utime, children: %s.%06d\n", plongest (sec), (int) value);
> +  fbsd_core_fetch_timeval (gdbarch,
> +			   descdata + kp->ki_rusage_ch + kp->ru_stime,
> +			   sec, value);
> +  printf_filtered ("stime, children: %s.%06d\n", plongest (sec), (int) value);
> +  printf_filtered ("'nice' value: %d\n",
> +		   bfd_get_signed_8 (core_bfd, descdata + kp->ki_nice));
> +  fbsd_core_fetch_timeval (gdbarch, descdata + kp->ki_start, sec, value);
> +  printf_filtered ("start time: %s.%06d\n", plongest (sec), (int) value);
> +  printf_filtered ("Virtual memory size: %s kB\n",
> +		   pulongest (bfd_get (addr_bit, core_bfd,
> +				       descdata + kp->ki_start) / 1024));
> +  printf_filtered ("Resident set size: %s pages\n",
> +		   pulongest (bfd_get (addr_bit, core_bfd,
> +				       descdata + kp->ki_rssize)));
> +  value = bfd_get (addr_bit, core_bfd,
> +		   descdata + kp->ki_rusage + kp->ru_maxrss);
> +  printf_filtered ("rlim: %s kB\n", pulongest (value));
> +}
> +
> +/* Implement the "core_info_proc" gdbarch method.  */
> +
> +static void
> +fbsd_core_info_proc (struct gdbarch *gdbarch, const char *args,
> +		     enum info_proc_what what)
> +{
> +  bool do_cmdline = false;
> +  bool do_cwd = false;
> +  bool do_exe = false;
> +  bool do_mappings = false;
> +  bool do_status = false;
> +  bool do_stat = false;
> +  int pid;
> +
> +  switch (what)
> +    {
> +    case IP_MINIMAL:
> +      do_cmdline = true;
> +      do_cwd = true;
> +      do_exe = true;
> +      break;
> +    case IP_MAPPINGS:
> +      do_mappings = true;
> +      break;
> +    case IP_STATUS:
> +      do_status = true;
> +      break;
> +    case IP_STAT:
> +      do_stat = true;
> +      break;
> +    case IP_CMDLINE:
> +      do_cmdline = true;
> +      break;
> +    case IP_EXE:
> +      do_exe = true;
> +      break;
> +    case IP_CWD:
> +      do_cwd = true;
> +      break;
> +    case IP_ALL:
> +      do_cmdline = true;
> +      do_cwd = true;
> +      do_exe = true;
> +      do_mappings = true;
> +      do_status = true;
> +      do_stat = true;
> +      break;
> +    default:
> +      error (_("Not supported on this architecture."));

If you mean "not supported for FreeBSD", I'm not sure architecture
is the right word, since architecture usually refers to CPU architecture.

> +    }
> +
> +  pid = bfd_core_file_pid (core_bfd);
> +  if (pid != 0)
> +    printf_filtered (_("process %d\n"), pid);
> +
> +  if (do_cmdline)
> +    {
> +      const char *cmdline;
> +
> +      cmdline = bfd_core_file_failing_command (core_bfd);
> +      if (cmdline)
> +	printf_filtered ("cmdline = '%s'\n", cmdline);
> +      else
> +	warning (_("Command line unavailable"));
> +    }
> +  if (do_cwd)
> +    {
> +      gdb::unique_xmalloc_ptr<char> cwd =
> +	fbsd_core_vnode_path (gdbarch, KINFO_FILE_FD_TYPE_CWD);
> +      if (cwd)
> +	printf_filtered ("cwd = '%s'\n", cwd.get ());
> +      else
> +	warning (_("unable to read current working directory"));
> +    }
> +  if (do_exe)
> +    {
> +      gdb::unique_xmalloc_ptr<char> exe =
> +	fbsd_core_vnode_path (gdbarch, KINFO_FILE_FD_TYPE_TEXT);
> +      if (exe)
> +	printf_filtered ("exe = '%s'\n", exe.get ());
> +      else
> +	warning (_("unable to read executable path name"));
> +    }
> +  if (do_mappings)
> +    fbsd_core_info_proc_mappings (gdbarch);
> +  if (do_status)
> +    fbsd_core_info_proc_status (gdbarch);
> +  if (do_stat)
> +    fbsd_core_info_proc_stat (gdbarch);
> +}
> +
>  /* Print descriptions of FreeBSD-specific AUXV entries to FILE.  */
>  
>  static void
> @@ -519,6 +1216,7 @@ fbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>    set_gdbarch_core_thread_name (gdbarch, fbsd_core_thread_name);
>    set_gdbarch_core_xfer_siginfo (gdbarch, fbsd_core_xfer_siginfo);
>    set_gdbarch_make_corefile_notes (gdbarch, fbsd_make_corefile_notes);
> +  set_gdbarch_core_info_proc (gdbarch, fbsd_core_info_proc);
>    set_gdbarch_print_auxv_entry (gdbarch, fbsd_print_auxv_entry);
>    set_gdbarch_get_siginfo_type (gdbarch, fbsd_get_siginfo_type);
>  
> diff --git a/gdb/fbsd-tdep.h b/gdb/fbsd-tdep.h
> index ff2e207aae..0029e03d41 100644
> --- a/gdb/fbsd-tdep.h
> +++ b/gdb/fbsd-tdep.h
> @@ -21,5 +21,6 @@
>  #define FBSD_TDEP_H
>  
>  extern void fbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch);
> +extern const char *fbsd_vm_map_entry_flags (int kve_flags, int kve_protection);

Can you please add doc for this new function?

>  
>  #endif /* fbsd-tdep.h */
> 


Thanks,

Simon

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/4] Support 'info proc' for native FreeBSD processes.
  2017-12-22 22:05 ` [PATCH 3/4] Support 'info proc' for native FreeBSD processes John Baldwin
@ 2017-12-27  2:23   ` Simon Marchi
  2018-01-03 19:05     ` John Baldwin
  0 siblings, 1 reply; 18+ messages in thread
From: Simon Marchi @ 2017-12-27  2:23 UTC (permalink / raw)
  To: John Baldwin, gdb-patches, binutils

On 2017-12-22 05:05 PM, John Baldwin wrote:
> - Command line arguments are fetched via the kern.proc.args.<pid>
>   sysctl.
> - The 'cwd' and 'exe' values are obtained from the per-process
>   file descriptor table returned by kinfo_getfile() from libutil.
> - 'mappings' is implemented by walking the array of VM map entries
>   returned by kinfo_getvmmap() from libutil.
> - 'stat' and 'status' output is generated by outputting fields from
>   the structure returned by the kern.proc.pid.<pid> sysctl.
> 
> gdb/ChangeLog:
> 
> 	* configure.ac: Check for kinfo_getfile in libutil.
> 	* configure: Regenerate.
> 	* config.in: Regenerate.
> 	* fbsd-nat.c: Include "fbsd-tdep.h".
> 	(fbsd_fetch_cmdline): New.
> 	(fbsd_fetch_kinfo_proc): Move earlier and change to return a bool
> 	rather than calling error.
> 	(fbsd_info_proc): New.
> 	(fbsd_thread_name): Report error if fbsd_fetch_kinfo_proc fails.
> 	(fbsd_wait): Report warning if fbsd_fetch_kinfo_proc fails.
> 	(fbsd_nat_add_target): Set "to_info_proc" to "fbsd_info_proc".
> ---
>  gdb/ChangeLog    |  14 ++
>  gdb/config.in    |   3 +
>  gdb/configure    |  60 +++++++++
>  gdb/configure.ac |   5 +
>  gdb/fbsd-nat.c   | 404 +++++++++++++++++++++++++++++++++++++++++++++++++++----
>  5 files changed, 462 insertions(+), 24 deletions(-)
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 2311749de7..3d580c1c9e 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,17 @@
> +2017-12-22  John Baldwin  <jhb@FreeBSD.org>
> +
> +	* configure.ac: Check for kinfo_getfile in libutil.
> +	* configure: Regenerate.
> +	* config.in: Regenerate.
> +	* fbsd-nat.c: Include "fbsd-tdep.h".
> +	(fbsd_fetch_cmdline): New.
> +	(fbsd_fetch_kinfo_proc): Move earlier and change to return a bool
> +	rather than calling error.
> +	(fbsd_info_proc): New.
> +	(fbsd_thread_name): Report error if fbsd_fetch_kinfo_proc fails.
> +	(fbsd_wait): Report warning if fbsd_fetch_kinfo_proc fails.
> +	(fbsd_nat_add_target): Set "to_info_proc" to "fbsd_info_proc".
> +
>  2017-12-22  John Baldwin  <jhb@FreeBSD.org>
>  
>  	* fbsd-tdep.c (KVE_STRUCTSIZE, KVE_START, KVE_END, KVE_OFFSET)
> diff --git a/gdb/config.in b/gdb/config.in
> index 1d11a97080..ad2cc1754e 100644
> --- a/gdb/config.in
> +++ b/gdb/config.in
> @@ -219,6 +219,9 @@
>  /* Define to 1 if you have the <inttypes.h> header file. */
>  #undef HAVE_INTTYPES_H
>  
> +/* Define to 1 if your system has the kinfo_getfile function. */
> +#undef HAVE_KINFO_GETFILE
> +
>  /* Define to 1 if your system has the kinfo_getvmmap function. */
>  #undef HAVE_KINFO_GETVMMAP
>  
> diff --git a/gdb/configure b/gdb/configure
> index 7b250079de..a0baa7a53a 100755
> --- a/gdb/configure
> +++ b/gdb/configure
> @@ -7927,6 +7927,66 @@ $as_echo "#define HAVE_KINFO_GETVMMAP 1" >>confdefs.h
>  fi
>  
>  
> +# fbsd-nat.c can also use kinfo_getfile.
> +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for library containing kinfo_getfile" >&5
> +$as_echo_n "checking for library containing kinfo_getfile... " >&6; }
> +if test "${ac_cv_search_kinfo_getfile+set}" = set; then :
> +  $as_echo_n "(cached) " >&6
> +else
> +  ac_func_search_save_LIBS=$LIBS
> +cat confdefs.h - <<_ACEOF >conftest.$ac_ext
> +/* end confdefs.h.  */
> +
> +/* Override any GCC internal prototype to avoid an error.
> +   Use char because int might match the return type of a GCC
> +   builtin and then its argument prototype would still apply.  */
> +#ifdef __cplusplus
> +extern "C"
> +#endif
> +char kinfo_getfile ();
> +int
> +main ()
> +{
> +return kinfo_getfile ();
> +  ;
> +  return 0;
> +}
> +_ACEOF
> +for ac_lib in '' util util-freebsd; do
> +  if test -z "$ac_lib"; then
> +    ac_res="none required"
> +  else
> +    ac_res=-l$ac_lib
> +    LIBS="-l$ac_lib  $ac_func_search_save_LIBS"
> +  fi
> +  if ac_fn_c_try_link "$LINENO"; then :
> +  ac_cv_search_kinfo_getfile=$ac_res
> +fi
> +rm -f core conftest.err conftest.$ac_objext \
> +    conftest$ac_exeext
> +  if test "${ac_cv_search_kinfo_getfile+set}" = set; then :
> +  break
> +fi
> +done
> +if test "${ac_cv_search_kinfo_getfile+set}" = set; then :
> +
> +else
> +  ac_cv_search_kinfo_getfile=no
> +fi
> +rm conftest.$ac_ext
> +LIBS=$ac_func_search_save_LIBS
> +fi
> +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_search_kinfo_getfile" >&5
> +$as_echo "$ac_cv_search_kinfo_getfile" >&6; }
> +ac_res=$ac_cv_search_kinfo_getfile
> +if test "$ac_res" != no; then :
> +  test "$ac_res" = "none required" || LIBS="$ac_res $LIBS"
> +
> +$as_echo "#define HAVE_KINFO_GETFILE 1" >>confdefs.h
> +
> +fi
> +
> +
>  
>        if test "X$prefix" = "XNONE"; then
>      acl_final_prefix="$ac_default_prefix"
> diff --git a/gdb/configure.ac b/gdb/configure.ac
> index 8e706b6e27..96ec77e0af 100644
> --- a/gdb/configure.ac
> +++ b/gdb/configure.ac
> @@ -523,6 +523,11 @@ AC_SEARCH_LIBS(kinfo_getvmmap, util util-freebsd,
>    [AC_DEFINE(HAVE_KINFO_GETVMMAP, 1,
>              [Define to 1 if your system has the kinfo_getvmmap function. ])])
>  
> +# fbsd-nat.c can also use kinfo_getfile.
> +AC_SEARCH_LIBS(kinfo_getfile, util util-freebsd,
> +  [AC_DEFINE(HAVE_KINFO_GETFILE, 1,
> +            [Define to 1 if your system has the kinfo_getfile function. ])])
> +
>  AM_ICONV
>  
>  # GDB may fork/exec the iconv program to get the list of supported character
> diff --git a/gdb/fbsd-nat.c b/gdb/fbsd-nat.c
> index 29b7ee5e33..e460c23d95 100644
> --- a/gdb/fbsd-nat.c
> +++ b/gdb/fbsd-nat.c
> @@ -32,14 +32,16 @@
>  #include <sys/signal.h>
>  #include <sys/sysctl.h>
>  #include <sys/user.h>
> -#ifdef HAVE_KINFO_GETVMMAP
> +#if defined(HAVE_KINFO_GETFILE) || defined(HAVE_KINFO_GETVMMAP)
>  #include <libutil.h>
> -#else
> +#endif
> +#if !defined(HAVE_KINFO_GETVMMAP)
>  #include "filestuff.h"
>  #endif
>  
>  #include "elf-bfd.h"
>  #include "fbsd-nat.h"
> +#include "fbsd-tdep.h"
>  
>  #include <list>
>  
> @@ -77,7 +79,7 @@ fbsd_pid_to_exec_file (struct target_ops *self, int pid)
>    return NULL;
>  }
>  
> -#ifdef HAVE_KINFO_GETVMMAP
> +#if defined(HAVE_KINFO_GETFILE) || defined(HAVE_KINFO_GETVMMAP)
>  /* Deleter for std::unique_ptr that invokes free.  */
>  
>  template <typename T>
> @@ -85,7 +87,9 @@ struct free_deleter
>  {
>    void operator() (T *ptr) const { free (ptr); }
>  };
> +#endif
>  
> +#ifdef HAVE_KINFO_GETVMMAP
>  /* Iterate over all the memory regions in the current inferior,
>     calling FUNC for each memory region.  OBFD is passed as the last
>     argument to FUNC.  */
> @@ -210,6 +214,369 @@ fbsd_find_memory_regions (struct target_ops *self,
>  }
>  #endif
>  
> +/* Fetch the command line for a running process.  */
> +
> +static gdb::unique_xmalloc_ptr<char>
> +fbsd_fetch_cmdline (pid_t pid)
> +{
> +  size_t len;
> +  int mib[4];
> +
> +  len = 0;
> +  mib[0] = CTL_KERN;
> +  mib[1] = KERN_PROC;
> +  mib[2] = KERN_PROC_ARGS;
> +  mib[3] = pid;
> +  if (sysctl (mib, 4, NULL, &len, NULL, 0) == -1)
> +    return nullptr;
> +
> +  gdb::unique_xmalloc_ptr<char> cmdline ((char *) xmalloc (len));
> +  if (sysctl (mib, 4, cmdline.get (), &len, NULL, 0) == -1)
> +    return nullptr;
> +
> +  return cmdline;
> +}
> +
> +/* Fetch the external variant of the kernel's internal process
> +   structure for the process PID into KP.  */
> +
> +static bool
> +fbsd_fetch_kinfo_proc (pid_t pid, struct kinfo_proc *kp)
> +{
> +  size_t len;
> +  int mib[4];
> +
> +  len = sizeof *kp;
> +  mib[0] = CTL_KERN;
> +  mib[1] = KERN_PROC;
> +  mib[2] = KERN_PROC_PID;
> +  mib[3] = pid;
> +  return (sysctl (mib, 4, kp, &len, NULL, 0) == 0);
> +}
> +
> +/* Implement the "to_info_proc target_ops" method.  */
> +
> +static void
> +fbsd_info_proc (struct target_ops *ops, const char *args,
> +		enum info_proc_what what)
> +{
> +#ifdef HAVE_KINFO_GETFILE
> +  std::unique_ptr<struct kinfo_file, free_deleter<struct kinfo_file>> fdtbl;
> +  int nfd = 0;
> +#endif
> +  struct kinfo_proc kp;
> +  char *tmp;
> +  pid_t pid;
> +  bool do_cmdline = false;
> +  bool do_cwd = false;
> +  bool do_exe = false;
> +#ifdef HAVE_KINFO_GETVMMAP
> +  bool do_mappings = false;
> +#endif
> +  bool do_status = false;
> +  bool do_stat = false;
> +  bool kp_valid = false;
> +
> +  switch (what)
> +    {
> +    case IP_MINIMAL:
> +      do_cmdline = true;
> +      do_cwd = true;
> +      do_exe = true;
> +      break;
> +#ifdef HAVE_KINFO_GETVMMAP
> +    case IP_MAPPINGS:
> +      do_mappings = true;
> +      break;
> +#endif
> +    case IP_STATUS:
> +      do_status = true;
> +      break;
> +    case IP_STAT:
> +      do_stat = true;
> +      break;
> +    case IP_CMDLINE:
> +      do_cmdline = true;
> +      break;
> +    case IP_EXE:
> +      do_exe = true;
> +      break;
> +    case IP_CWD:
> +      do_cwd = true;
> +      break;
> +    case IP_ALL:
> +      do_cmdline = true;
> +      do_cwd = true;
> +      do_exe = true;
> +#ifdef HAVE_KINFO_GETVMMAP
> +      do_mappings = true;
> +#endif
> +      do_status = true;
> +      do_stat = true;
> +      break;
> +    default:
> +      error (_("Not supported on this target."));
> +    }
> +
> +  gdb_argv built_argv (args);
> +  if (built_argv.count () == 0)
> +    {
> +      pid = ptid_get_pid (inferior_ptid);
> +      if (pid == 0)
> +	error (_("No current process: you must name one."));
> +    }
> +  else
> +    {
> +      pid = strtol (built_argv[0], NULL, 10);
> +    }

Unnecessary curly braces.

> +
> +  printf_filtered (_("process %d\n"), pid);
> +#ifdef HAVE_KINFO_GETFILE
> +  if (do_cwd || do_exe)
> +    fdtbl.reset (kinfo_getfile (pid, &nfd));
> +#endif
> +  if (do_stat || do_status)
> +    {
> +      kp_valid = fbsd_fetch_kinfo_proc(pid, &kp);
> +      if (!kp_valid)
> +	warning (_("Failed to fetch process information"));
> +    }
> +
> +  if (do_cmdline)
> +    {
> +      gdb::unique_xmalloc_ptr<char> cmdline = fbsd_fetch_cmdline (pid);
> +      if (cmdline)
> +	printf_filtered ("cmdline = '%s'\n", cmdline.get ());
> +      else
> +	warning (_("unable to fetch command line"));
> +    }
> +  if (do_cwd)
> +    {
> +      const char *cwd = NULL;
> +#ifdef HAVE_KINFO_GETFILE
> +      struct kinfo_file *kf = fdtbl.get ();
> +      for (int i = 0; i < nfd; i++, kf++)
> +	{
> +	  if (kf->kf_type == KF_TYPE_VNODE && kf->kf_fd == KF_FD_TYPE_CWD)
> +	    {
> +	      cwd = kf->kf_path;
> +	      break;
> +	    }
> +	}
> +#endif
> +      if (cwd)
> +	printf_filtered ("cwd = '%s'\n", cwd);
> +      else
> +	warning (_("unable to fetch current working directory"));
> +    }
> +  if (do_exe)
> +    {
> +      const char *exe = NULL;
> +#ifdef HAVE_KINFO_GETFILE
> +      struct kinfo_file *kf = fdtbl.get ();
> +      for (int i = 0; i < nfd; i++, kf++)
> +	{
> +	  if (kf->kf_type == KF_TYPE_VNODE && kf->kf_fd == KF_FD_TYPE_TEXT)
> +	    {
> +	      exe = kf->kf_path;
> +	      break;
> +	    }
> +	}
> +#endif
> +      if (exe == NULL)
> +	exe = fbsd_pid_to_exec_file (ops, pid);
> +      if (exe)
> +	printf_filtered ("exe = '%s'\n", exe);
> +      else
> +	warning (_("unable to fetch executable path name"));
> +    }
> +#ifdef HAVE_KINFO_GETVMMAP
> +  if (do_mappings)
> +    {
> +      int nvment;
> +      std::unique_ptr<struct kinfo_vmentry, free_deleter<struct kinfo_vmentry>>

Is there a reason to have and use free_deleter rather than gdb::unique_xmalloc_ptr?

> +	vmentl (kinfo_getvmmap (pid, &nvment));
> +
> +      if (vmentl)

vmentl != NULL

There are a few other instances of if (ptr) that should be changed to if (ptr != NULL).

> +	{
> +	  printf_filtered (_("Mapped address spaces:\n\n"));
> +#ifdef __LP64__
> +	  printf_filtered ("  %18s %18s %10s %10s %9s %s\n",
> +			   "Start Addr",
> +			   "  End Addr",
> +			   "      Size", "    Offset", "Flags  ", "File");
> +#else
> +	  printf_filtered ("\t%10s %10s %10s %10s %9s %s\n",
> +			   "Start Addr",
> +			   "  End Addr",
> +			   "      Size", "    Offset", "Flags  ", "File");
> +#endif
> +
> +	  struct kinfo_vmentry *kve = vmentl.get ();
> +	  for (int i = 0; i < nvment; i++, kve++)
> +	    {
> +	      ULONGEST start, end;
> +
> +	      start = kve->kve_start;
> +	      end = kve->kve_end;
> +#ifdef __LP64__
> +	      printf_filtered ("  %18s %18s %10s %10s %9s %s\n",
> +			       hex_string (start),
> +			       hex_string (end),
> +			       hex_string (end - start),
> +			       hex_string (kve->kve_offset),
> +			       fbsd_vm_map_entry_flags (kve->kve_flags,
> +							kve->kve_protection),
> +			       kve->kve_path);
> +#else
> +	      printf_filtered ("\t%10s %10s %10s %10s %9s %s\n",
> +			       hex_string (start),
> +			       hex_string (end),
> +			       hex_string (end - start),
> +			       hex_string (kve->kve_offset),
> +			       fbsd_vm_map_entry_flags (kve->kve_flags,
> +							kve->kve_protection),
> +			       kve->kve_path);
> +#endif
> +	    }
> +	}
> +      else
> +	warning (_("unable to fetch virtual memory map"));
> +    }
> +#endif
> +  if (do_status && kp_valid)
> +    {
> +      const char *state;
> +      int pgtok;
> +
> +      printf_filtered ("Name:\t%s\n", kp.ki_comm);
> +      switch (kp.ki_stat)
> +	{
> +	case SIDL:
> +	  state = "I (idle)";
> +	  break;
> +	case SRUN:
> +	  state = "R (running)";
> +	  break;
> +	case SSTOP:
> +	  state = "T (stopped)";
> +	  break;
> +	case SZOMB:
> +	  state = "Z (zombie)";
> +	  break;
> +	case SSLEEP:
> +	  state = "S (sleeping)";
> +	  break;
> +	case SWAIT:
> +	  state = "W (interrupt wait)";
> +	  break;
> +	case SLOCK:
> +	  state = "L (blocked on lock)";
> +	  break;
> +	default:
> +	  state = "? (unknown)";
> +	  break;
> +	}
> +      printf_filtered ("State:\t%s\n", state);
> +      printf_filtered ("Pid:\t%d\n", kp.ki_pid);
> +      printf_filtered ("PPid:\t%d\n", kp.ki_ppid);
> +      printf_filtered ("Uid:\t%d %d %d\n", kp.ki_ruid, kp.ki_uid, kp.ki_svuid);
> +      printf_filtered ("Gid:\t%d %d %d\n", kp.ki_rgid, kp.ki_groups[0],
> +		       kp.ki_svgid);
> +      printf_filtered ("Groups:\t");
> +      for (int i = 0; i < kp.ki_ngroups; i++)
> +	printf_filtered ("%d ", kp.ki_groups[i]);
> +      printf_filtered ("\n");
> +      pgtok = getpagesize () / 1024;
> +      printf_filtered ("VmSize:\t%8ju kB\n", (uintmax_t) kp.ki_size / 1024);
> +      printf_filtered ("VmRSS:\t%8ju kB\n", (uintmax_t) kp.ki_rssize * pgtok);
> +      printf_filtered ("VmData:\t%8ju kB\n", (uintmax_t) kp.ki_dsize * pgtok);
> +      printf_filtered ("VmStk:\t%8ju kB\n", (uintmax_t) kp.ki_ssize * pgtok);
> +      printf_filtered ("VmExe:\t%8ju kB\n", (uintmax_t) kp.ki_tsize * pgtok);
> +      printf_filtered ("SigPnd:\t");
> +      for (int i = 0; i < _SIG_WORDS; i++)
> +	printf_filtered ("%08x ", kp.ki_siglist.__bits[i]);
> +      printf_filtered ("\n");
> +      printf_filtered ("SigIgn:\t");
> +      for (int i = 0; i < _SIG_WORDS; i++)
> +	printf_filtered ("%08x ", kp.ki_sigignore.__bits[i]);
> +      printf_filtered ("\n");
> +      printf_filtered ("SigCgt:\t");
> +      for (int i = 0; i < _SIG_WORDS; i++)
> +	printf_filtered ("%08x ", kp.ki_sigcatch.__bits[i]);
> +      printf_filtered ("\n");
> +    }
> +  if (do_stat && kp_valid)
> +    {
> +      char state;
> +      int pgtok;
> +
> +      printf_filtered ("Exec file: %s\n", kp.ki_comm);
> +      switch (kp.ki_stat)
> +	{
> +	case SIDL:
> +	  state = 'I';
> +	  break;
> +	case SRUN:
> +	  state = 'R';
> +	  break;
> +	case SSTOP:
> +	  state = 'T';
> +	  break;
> +	case SZOMB:
> +	  state = 'Z';
> +	  break;
> +	case SSLEEP:
> +	  state = 'S';
> +	  break;
> +	case SWAIT:
> +	  state = 'W';
> +	  break;
> +	case SLOCK:
> +	  state = 'L';
> +	  break;
> +	default:
> +	  state = '?';
> +	  break;
> +	}
> +      printf_filtered ("State: %c\n", state);
> +      printf_filtered ("Parent process: %d\n", kp.ki_ppid);
> +      printf_filtered ("Process group: %d\n", kp.ki_pgid);
> +      printf_filtered ("Session id: %d\n", kp.ki_sid);
> +      printf_filtered ("TTY: %ju\n", (uintmax_t) kp.ki_tdev);
> +      printf_filtered ("TTY owner process group: %d\n", kp.ki_tpgid);
> +      printf_filtered ("Minor faults (no memory page): %ld\n",
> +		       kp.ki_rusage.ru_minflt);
> +      printf_filtered ("Minor faults, children: %ld\n",
> +		       kp.ki_rusage_ch.ru_minflt);
> +      printf_filtered ("Major faults (memory page faults): %ld\n",
> +		       kp.ki_rusage.ru_majflt);
> +      printf_filtered ("Major faults, children: %ld\n",
> +		       kp.ki_rusage_ch.ru_majflt);
> +      printf_filtered ("utime: %jd.%06ld\n",
> +		       (intmax_t) kp.ki_rusage.ru_utime.tv_sec,
> +		       kp.ki_rusage.ru_utime.tv_usec);
> +      printf_filtered ("stime: %jd.%06ld\n",
> +		       (intmax_t) kp.ki_rusage.ru_stime.tv_sec,
> +		       kp.ki_rusage.ru_stime.tv_usec);
> +      printf_filtered ("utime, children: %jd.%06ld\n",
> +		       (intmax_t) kp.ki_rusage_ch.ru_utime.tv_sec,
> +		       kp.ki_rusage_ch.ru_utime.tv_usec);
> +      printf_filtered ("stime, children: %jd.%06ld\n",
> +		       (intmax_t) kp.ki_rusage_ch.ru_stime.tv_sec,
> +		       kp.ki_rusage_ch.ru_stime.tv_usec);
> +      printf_filtered ("'nice' value: %d\n", kp.ki_nice);
> +      printf_filtered ("start time: %jd.%06ld\n", kp.ki_start.tv_sec,
> +		       kp.ki_start.tv_usec);
> +      pgtok = getpagesize () / 1024;
> +      printf_filtered ("Virtual memory size: %ju kB\n",
> +		       (uintmax_t) kp.ki_size / 1024);
> +      printf_filtered ("Resident set size: %ju kB\n",
> +		       (uintmax_t) kp.ki_rssize * pgtok);
> +      printf_filtered ("rlim: %ju kB\n", (uintmax_t) kp.ki_rusage.ru_maxrss);
> +    }
> +}
> +
>  #ifdef KERN_PROC_AUXV
>  static enum target_xfer_status (*super_xfer_partial) (struct target_ops *ops,
>  						      enum target_object object,
> @@ -461,23 +828,6 @@ show_fbsd_lwp_debug (struct ui_file *file, int from_tty,
>  }
>  
>  #if defined(TDP_RFPPWAIT) || defined(HAVE_STRUCT_PTRACE_LWPINFO_PL_TDNAME)
> -/* Fetch the external variant of the kernel's internal process
> -   structure for the process PID into KP.  */
> -
> -static void
> -fbsd_fetch_kinfo_proc (pid_t pid, struct kinfo_proc *kp)
> -{
> -  size_t len;
> -  int mib[4];
> -
> -  len = sizeof *kp;
> -  mib[0] = CTL_KERN;
> -  mib[1] = KERN_PROC;
> -  mib[2] = KERN_PROC_PID;
> -  mib[3] = pid;
> -  if (sysctl (mib, 4, kp, &len, NULL, 0) == -1)
> -    perror_with_name (("sysctl"));
> -}
>  #endif

This leaves an empty #if/#endif.  Should it be removed or moved with the function?

>  
>  /*
> @@ -565,7 +915,8 @@ fbsd_thread_name (struct target_ops *self, struct thread_info *thr)
>    /* Note that ptrace_lwpinfo returns the process command in pl_tdname
>       if a name has not been set explicitly.  Return a NULL name in
>       that case.  */
> -  fbsd_fetch_kinfo_proc (pid, &kp);
> +  if (!fbsd_fetch_kinfo_proc (pid, &kp))
> +    perror_with_name (_("Failed to fetch process information"));
>    if (ptrace (PT_LWPINFO, lwp, (caddr_t) &pl, sizeof pl) == -1)
>      perror_with_name (("ptrace"));
>    if (strcmp (kp.ki_comm, pl.pl_tdname) == 0)
> @@ -975,9 +1326,13 @@ fbsd_wait (struct target_ops *ops,
>  #ifndef PTRACE_VFORK
>  	      /* For vfork, the child process will have the P_PPWAIT
>  		 flag set.  */
> -	      fbsd_fetch_kinfo_proc (child, &kp);
> -	      if (kp.ki_flag & P_PPWAIT)
> -		ourstatus->kind = TARGET_WAITKIND_VFORKED;
> +	      if (fbsd_fetch_kinfo_proc (child, &kp))
> +		{
> +		  if (kp.ki_flag & P_PPWAIT)
> +		    ourstatus->kind = TARGET_WAITKIND_VFORKED;
> +		}
> +	      else
> +		warning (_("Failed to fetch process information"));
>  #endif
>  	      ourstatus->value.related_pid = child_ptid;
>  
> @@ -1181,6 +1536,7 @@ fbsd_nat_add_target (struct target_ops *t)
>  {
>    t->to_pid_to_exec_file = fbsd_pid_to_exec_file;
>    t->to_find_memory_regions = fbsd_find_memory_regions;
> +  t->to_info_proc = fbsd_info_proc;
>  #ifdef KERN_PROC_AUXV
>    super_xfer_partial = t->to_xfer_partial;
>    t->to_xfer_partial = fbsd_xfer_partial;
> 

Thanks,

Simon

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/4] Create psuedo sections for FreeBSD NT_PROCSTAT_(PROC|FILES|VMMAP) notes.
  2017-12-22 22:05 ` [PATCH 1/4] Create psuedo sections for FreeBSD NT_PROCSTAT_(PROC|FILES|VMMAP) notes John Baldwin
  2017-12-27  1:18   ` Simon Marchi
@ 2018-01-02 11:49   ` Nick Clifton
  1 sibling, 0 replies; 18+ messages in thread
From: Nick Clifton @ 2018-01-02 11:49 UTC (permalink / raw)
  To: John Baldwin, gdb-patches, binutils

Hi John,

> bfd/ChangeLog:
> 
> 	* elf.c (elfcore_grok_freebsd_note): Handle
> 	NT_FREEBSD_PROCSTAT_PROC, NT_FREEBSD_PROCSTAT_FILES, and
> 	NT_FREEBSD_PROCSTAT_VMMAP.

Approved - please apply.

Cheers
  Nick

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/4] Support 'info proc' for FreeBSD process core dumps.
  2017-12-27  1:56   ` Simon Marchi
@ 2018-01-03 19:05     ` John Baldwin
  0 siblings, 0 replies; 18+ messages in thread
From: John Baldwin @ 2018-01-03 19:05 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches, binutils

On Tuesday, December 26, 2017 08:56:51 PM Simon Marchi wrote:
> On 2017-12-22 05:05 PM, John Baldwin wrote:
> > - Command line arguments are obtained from the pr_psargs[] array
> >   saved in the NT_PRPSINFO note.
> > - The 'cwd' and 'exe' values are obtained from the per-process file
> >   descriptor table stored in the NT_PROCSTAT_FILES core note.
> > - 'mappings' is implemented by walking the array of VM map entries
> >   stored in the NT_PROCSTAT_VMMAP core note.
> > - 'stat' and 'status' output is generated by outputting fields from
> >   the first structure stored in the NT_PROCSTAT_PROC core note.
> > 
> > diff --git a/gdb/fbsd-tdep.c b/gdb/fbsd-tdep.c
> > index f89b520c5f..454036dcac 100644
> > --- a/gdb/fbsd-tdep.c
> > +++ b/gdb/fbsd-tdep.c
> > +   is not available from a core dump.  Instead, the per-thread data
> > +   structures contain the value of these fields for individual
> > +   threads.  */
> > +
> > +struct kinfo_proc_layout
> 
> struct definitions should not be indented, they should look like:
> 
> struct kinfo_proc_layout
> {
>   int ki_layout;
>   ...
> }

Fixed.

> > +
> > +struct kinfo_proc_layout kinfo_proc_layout_32 =
> 
> I would suggest making these const.

Fixed.

> > +static void
> > +fbsd_print_sigset (const char *descr, unsigned char *sigset)
> > +{
> > +  printf_filtered ("%s:\t", descr);
> > +  for (int i = 0; i < _SIG_WORDS; i++)
> 
> _SIG_WORDS seems to be FreeBSD-specific, so shouldn't be used in the tdep file,
> unless we redefine it.

Oops, yes.  I added a local constant.

> > +  printf_filtered ("Name:\t%.19s\n", descdata + kp->ki_comm);
> > +  printf_filtered ("Pid:\t%s\n",
> > +		   pulongest(bfd_get_32 (core_bfd, descdata + kp->ki_pid)));
> 
> Missing a few spaces before parentheses here and there.

Fixed.
 
> > +    default:
> > +      error (_("Not supported on this architecture."));
> 
> If you mean "not supported for FreeBSD", I'm not sure architecture
> is the right word, since architecture usually refers to CPU architecture.

Mmm, yes.  I think I was trying to say "not supported on this gdbarch"
in effect.  However, the core target doesn't output any error if there
is no valid core_info_proc method, so perhaps it would be best to just
not output any error at all.  This also matches linux-tdep.c which doesn't
output anything for an unsupported enum value.

> > diff --git a/gdb/fbsd-tdep.h b/gdb/fbsd-tdep.h
> > index ff2e207aae..0029e03d41 100644
> > --- a/gdb/fbsd-tdep.h
> > +++ b/gdb/fbsd-tdep.h
> > @@ -21,5 +21,6 @@
> >  #define FBSD_TDEP_H
> >  
> >  extern void fbsd_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch);
> > +extern const char *fbsd_vm_map_entry_flags (int kve_flags, int kve_protection);
> 
> Can you please add doc for this new function?

Fixed.

-- 
John Baldwin

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/4] Support for 'info proc' on FreeBSD cores and native
  2017-12-27  1:53 ` [PATCH 0/4] Support for 'info proc' on FreeBSD cores and native Simon Marchi
@ 2018-01-03 19:05   ` John Baldwin
  2018-01-03 19:15     ` Simon Marchi
  0 siblings, 1 reply; 18+ messages in thread
From: John Baldwin @ 2018-01-03 19:05 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches, binutils

On Tuesday, December 26, 2017 08:53:08 PM Simon Marchi wrote:
> On 2017-12-22 05:05 PM, John Baldwin wrote:
> > This series adds initial support for the 'info proc' command on
> > FreeBSD native processes and process cores.  FreeBSD generally does
> > not use the /proc filesystem, but instead exports data structures
> > containing process information either via kernel system control nodes
> > (for live processes), or in core dump notes.
> > 
> > My assumption is that the format of 'info proc' is expected to be
> > somewhat OS-specific though probably not gratuitously so.
> > 
> > For 'info proc mappings' I choose to include both mapping attributes
> > (such as permissions) along with the object file name.
> > 
> > I did choose to implement versions of 'info proc stat' and 'info proc
> > status' that are similar to the output on Linux for now.  However,
> > given that the output on FreeBSD is not tied to the output of files in
> > /proc and that having both 'stat' and 'status' with overlapping
> > content seems ambiguous, I do wonder if it wouldn't be better to just
> > have a single command that includes one copy of the information (and
> > perhaps treat 'stat' as an alias of 'status' on FreeBSD)?  I also
> > noticed in the document that there are older commands such as 'info
> > proc id' and 'info proc time' that if implemented would contain a
> > subset of the info in the 'stat' commands.  I would possibly prefer to
> > resurrect these commands on FreeBSD as subsets of 'stat/status'?  What
> > do you all think?
> > 
> > I do eventually plan on adding a 'info proc files' that outputs a
> > table of open file descriptors.
> > 
> > For the documentation I made minimal changes to the existing
> > documentation for 'info proc' to not state that it requires /proc, but
> > the wording could probably use improvement.  I have also not yet
> > documented that FreeBSD supports 'proc stat' and 'proc status' due to
> > the question above.
> 
> Hi John,
> 
> From reading the documentation, "info proc" seems to have been introduced
> specifically to print things from /proc.  I find it too bad however that
> the command line interface is based so closely on the /proc interface,
> since it brings all of its quirks with it (e.g. stat vs status).  Also,
> the important thing to the user is the information, regardless of where
> it comes from.
> 
> With your patch, it moves "info proc" a little bit from "printing /proc"
> to "print things about a process", which I think is totally fine.  I think
> you could change the doc to put even less emphasis on the fact that the info
> comes from /proc.

Ok, I'll try to update the documentation a bit more towards that vein.

> I'm fine with what you suggested above.

To be clear, which of these suggestions are you fine with?

1) Having a merged 'info proc stat/status' for FreeBSD.

2) Resurrecting 'info proc id' and 'info proc time'.

-- 
John Baldwin

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/4] Support 'info proc' for native FreeBSD processes.
  2017-12-27  2:23   ` Simon Marchi
@ 2018-01-03 19:05     ` John Baldwin
  2018-01-03 19:13       ` Simon Marchi
  0 siblings, 1 reply; 18+ messages in thread
From: John Baldwin @ 2018-01-03 19:05 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches, binutils

On Tuesday, December 26, 2017 09:22:52 PM Simon Marchi wrote:
> On 2017-12-22 05:05 PM, John Baldwin wrote:
> > +  else
> > +    {
> > +      pid = strtol (built_argv[0], NULL, 10);
> > +    }
> 
> Unnecessary curly braces.

Fixed.

> > +#ifdef HAVE_KINFO_GETVMMAP
> > +  if (do_mappings)
> > +    {
> > +      int nvment;
> > +      std::unique_ptr<struct kinfo_vmentry, free_deleter<struct kinfo_vmentry>>
> 
> Is there a reason to have and use free_deleter rather than gdb::unique_xmalloc_ptr?
> 
> > +	vmentl (kinfo_getvmmap (pid, &nvment));

This function (kinfo_getvmmap) which is defined in the libutil library included in
FreeBSD's base system calls malloc() internally, so the memory returned must be
freed with free() rather than xfree().  This deleter is already used earlier in
fbsd_find_memory_regions() for another call to kinfo_getvmmap() for the same reason.

> > +
> > +      if (vmentl)
> 
> vmentl != NULL
> 
> There are a few other instances of if (ptr) that should be changed to if (ptr != NULL).

Ok.  The style in GDB doesn't seem consistent in this regard (I largely based this
code on the 'info proc' implementation in linux-tdep.c which doesn't explicitly
compare against NULL/nullptr (though I prefer the explicit comparison myself)).
Also, I feel like we should use nullptr rather than NULL when working with "smart"
pointer types like unique_ptr<> at least?

> > @@ -461,23 +828,6 @@ show_fbsd_lwp_debug (struct ui_file *file, int from_tty,
> >  }
> >  
> >  #if defined(TDP_RFPPWAIT) || defined(HAVE_STRUCT_PTRACE_LWPINFO_PL_TDNAME)
> > -/* Fetch the external variant of the kernel's internal process
> > -   structure for the process PID into KP.  */
> > -
> > -static void
> > -fbsd_fetch_kinfo_proc (pid_t pid, struct kinfo_proc *kp)
> > -{
> > -  size_t len;
> > -  int mib[4];
> > -
> > -  len = sizeof *kp;
> > -  mib[0] = CTL_KERN;
> > -  mib[1] = KERN_PROC;
> > -  mib[2] = KERN_PROC_PID;
> > -  mib[3] = pid;
> > -  if (sysctl (mib, 4, kp, &len, NULL, 0) == -1)
> > -    perror_with_name (("sysctl"));
> > -}
> >  #endif
> 
> This leaves an empty #if/#endif.  Should it be removed or moved with the function?

Removed, good catch.

-- 
John Baldwin

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/4] Support 'info proc' for native FreeBSD processes.
  2018-01-03 19:05     ` John Baldwin
@ 2018-01-03 19:13       ` Simon Marchi
  2018-01-03 21:56         ` John Baldwin
  0 siblings, 1 reply; 18+ messages in thread
From: Simon Marchi @ 2018-01-03 19:13 UTC (permalink / raw)
  To: John Baldwin; +Cc: gdb-patches, binutils

Hi John,

>> > +#ifdef HAVE_KINFO_GETVMMAP
>> > +  if (do_mappings)
>> > +    {
>> > +      int nvment;
>> > +      std::unique_ptr<struct kinfo_vmentry, free_deleter<struct kinfo_vmentry>>
>> 
>> Is there a reason to have and use free_deleter rather than 
>> gdb::unique_xmalloc_ptr?
>> 
>> > +	vmentl (kinfo_getvmmap (pid, &nvment));
> 
> This function (kinfo_getvmmap) which is defined in the libutil library
> included in
> FreeBSD's base system calls malloc() internally, so the memory returned 
> must be
> freed with free() rather than xfree().  This deleter is already used 
> earlier in
> fbsd_find_memory_regions() for another call to kinfo_getvmmap() for
> the same reason.

But isn't xfree just a wrapper around free?

>> > +
>> > +      if (vmentl)
>> 
>> vmentl != NULL
>> 
>> There are a few other instances of if (ptr) that should be changed to 
>> if (ptr != NULL).
> 
> Ok.  The style in GDB doesn't seem consistent in this regard (I
> largely based this
> code on the 'info proc' implementation in linux-tdep.c which doesn't 
> explicitly
> compare against NULL/nullptr (though I prefer the explicit comparison 
> myself)).
> Also, I feel like we should use nullptr rather than NULL when working
> with "smart"
> pointer types like unique_ptr<> at least?

You are right, there is code that comes from an era where the coding 
style wasn't as strictly enforced, it seems.  I don't think we should go 
fix coding style issues just for fun, but when we modify or copy 
existing code, we should make it respect the style.

Thanks,

Simon

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/4] Support for 'info proc' on FreeBSD cores and native
  2018-01-03 19:05   ` John Baldwin
@ 2018-01-03 19:15     ` Simon Marchi
  2018-01-03 23:39       ` John Baldwin
  0 siblings, 1 reply; 18+ messages in thread
From: Simon Marchi @ 2018-01-03 19:15 UTC (permalink / raw)
  To: John Baldwin; +Cc: gdb-patches, binutils

> To be clear, which of these suggestions are you fine with?
> 
> 1) Having a merged 'info proc stat/status' for FreeBSD.

I am fine with this.

> 2) Resurrecting 'info proc id' and 'info proc time'.

I am fine with this, if you think it's useful.  I didn't quite 
understand what it would show.  If it just shows a subset of the 
information of stat/status, maybe it's not so useful, but if it's 
additional info, then sure.

Simon

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/4] Support 'info proc' for native FreeBSD processes.
  2018-01-03 19:13       ` Simon Marchi
@ 2018-01-03 21:56         ` John Baldwin
  0 siblings, 0 replies; 18+ messages in thread
From: John Baldwin @ 2018-01-03 21:56 UTC (permalink / raw)
  To: gdb-patches; +Cc: Simon Marchi, binutils

On Wednesday, January 03, 2018 02:13:28 PM Simon Marchi wrote:
> Hi John,
> 
> >> > +#ifdef HAVE_KINFO_GETVMMAP
> >> > +  if (do_mappings)
> >> > +    {
> >> > +      int nvment;
> >> > +      std::unique_ptr<struct kinfo_vmentry, free_deleter<struct kinfo_vmentry>>
> >> 
> >> Is there a reason to have and use free_deleter rather than 
> >> gdb::unique_xmalloc_ptr?
> >> 
> >> > +	vmentl (kinfo_getvmmap (pid, &nvment));
> > 
> > This function (kinfo_getvmmap) which is defined in the libutil library
> > included in
> > FreeBSD's base system calls malloc() internally, so the memory returned 
> > must be
> > freed with free() rather than xfree().  This deleter is already used 
> > earlier in
> > fbsd_find_memory_regions() for another call to kinfo_getvmmap() for
> > the same reason.
> 
> But isn't xfree just a wrapper around free?

Ah, for some reason I thought that xmalloc/xfree were a matched pair that could in
some cases refer to a separate malloc implementation, not always a wrapper around
normal malloc() / free().  I'll remove the deleter altogether (and it's one
existing use) in a future change.

-- 
John Baldwin

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/4] Support for 'info proc' on FreeBSD cores and native
  2018-01-03 19:15     ` Simon Marchi
@ 2018-01-03 23:39       ` John Baldwin
  0 siblings, 0 replies; 18+ messages in thread
From: John Baldwin @ 2018-01-03 23:39 UTC (permalink / raw)
  To: Simon Marchi; +Cc: gdb-patches, binutils

On Wednesday, January 03, 2018 02:15:50 PM Simon Marchi wrote:
> > To be clear, which of these suggestions are you fine with?
> > 
> > 1) Having a merged 'info proc stat/status' for FreeBSD.
> 
> I am fine with this.
> 
> > 2) Resurrecting 'info proc id' and 'info proc time'.
> 
> I am fine with this, if you think it's useful.  I didn't quite 
> understand what it would show.  If it just shows a subset of the 
> information of stat/status, maybe it's not so useful, but if it's 
> additional info, then sure.

Just a subset, so I'm fine with just skipping 2).

-- 
John Baldwin

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2018-01-03 23:39 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-22 22:05 [PATCH 0/4] Support for 'info proc' on FreeBSD cores and native John Baldwin
2017-12-22 22:05 ` [PATCH 2/4] Support 'info proc' for FreeBSD process core dumps John Baldwin
2017-12-27  1:56   ` Simon Marchi
2018-01-03 19:05     ` John Baldwin
2017-12-22 22:05 ` [PATCH 3/4] Support 'info proc' for native FreeBSD processes John Baldwin
2017-12-27  2:23   ` Simon Marchi
2018-01-03 19:05     ` John Baldwin
2018-01-03 19:13       ` Simon Marchi
2018-01-03 21:56         ` John Baldwin
2017-12-22 22:05 ` [PATCH 1/4] Create psuedo sections for FreeBSD NT_PROCSTAT_(PROC|FILES|VMMAP) notes John Baldwin
2017-12-27  1:18   ` Simon Marchi
2018-01-02 11:49   ` Nick Clifton
2017-12-22 22:13 ` [PATCH 4/4] Document support for 'info proc' on FreeBSD John Baldwin
2017-12-23  8:46   ` Eli Zaretskii
2017-12-27  1:53 ` [PATCH 0/4] Support for 'info proc' on FreeBSD cores and native Simon Marchi
2018-01-03 19:05   ` John Baldwin
2018-01-03 19:15     ` Simon Marchi
2018-01-03 23:39       ` John Baldwin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).