diff mbox

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

Message ID 20180908003659.37482-5-jhb@FreeBSD.org
State New, archived
Headers

Commit Message

John Baldwin Sept. 8, 2018, 12:36 a.m. UTC 
  This walks the list of struct kinfo_file objects returned by a call to
kinfo_getfile outputting a description of each open file descriptor.

gdb/ChangeLog:

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

Comments

Simon Marchi Sept. 8, 2018, 11 p.m. UTC | #1 
On 2018-09-08 01:36 AM, John Baldwin wrote:
> This walks the list of struct kinfo_file objects returned by a call to
> kinfo_getfile outputting a description of each open file descriptor.

LGTM.

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

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

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

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

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

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

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

Patch

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