Re: bsd-kvm target, always a thread

[Pedro Alves <pedro@codesourcery.com>]

[-- Attachment #1: Type: text/plain, Size: 2670 bytes --]

On Saturday 09 August 2008 13:11:16, Mark Kettenis wrote:
> > From: Pedro Alves <pedro@codesourcery.com>
> > Date: Sat, 9 Aug 2008 12:27:34 +0100
> >
> > On Saturday 09 August 2008 09:32:57, Mark Kettenis wrote:
> > > > From: Pedro Alves <pedro@codesourcery.com>
> > >
> > > Hmm, it is unfortunate that a process ID of 0 is "verboten", since
> > > that's what you are really looking at with "target kvm".  And it
> > > should be possible for me to actually make all the running processes
> > > visible as kernel "threads".
> > >
> > >
> > > I guess your diff is right, although I'd prefer a less arbitrary ptid
> > > to be used.  Would something like ptid_build(0, 1, 0) work?
> >
> > I'd prefer to get away without pid == 0.  I'm going to
> > introduce later a "struct inferior" which holds an "int pid", and
> > we will match a ptid to a struct inferior by its ptid.pid.
> > I'd rather avoid having an inferior with pid == 0.
> >
> > Does something like this work for you?
>
> Something like that'd work fine for the OpenBSD kernel.

> Sure, I'd just think you should use something that's a bit less
> arbitrary than 42000 (which could be confused with a real process ID)
> here.  

Eh, 42 carries some history.  :-)  It was used in several targets already,
even before I started changing then to use ptid(pid,0,tid), and always
registering a thread.  monitor used 42000, remote used 42000, remote-sim
used 42, go32-nat.c uses 42.  I just thought that carrying it around
would make it easier to spot what it is.

> I see that remote.c uses negative numbers for special cases. 
> Would using -1 or -2 work for you?

Those are in the tid field, which I just carried around when I
made the remote target use ptid(pid,0,tid) for threads instead
of ptid(tid,0,0).  The magic is in using lwp != 0.  The
special -1,-2 numbers have has some binding to the remote
stub current thread.

Let's not use -1, as that conflicts a bit with the special
ptid(-1,0,0) (aka, minus_one_ptid).

I actually have a patchlet in my series to bring back the 42000:

 remote.c:
   /* Take advantage of the fact that the LWP field is not used, to tag
      special ptids with it set to != 0.  */
 -  magic_null_ptid = ptid_build (0, 1, -1);
 -  not_sent_ptid = ptid_build (0, 1, -2);
 -  any_thread_ptid = ptid_build (0, 1, 0);
 +  magic_null_ptid = ptid_build (42000, 1, -1);
 +  not_sent_ptid = ptid_build (42000, 1, -2);
 +  any_thread_ptid = ptid_build (42000, 1, 0);

I guess we're numerically converging :-)

How about the attached?

With your fix for the %eip in, I now get,

 (gdb) tar kvm
 #0  0xd034ee05 in ?? ()
 (gdb) info threads
 * 1 <kvm>  0xd034ee05 in ?? ()

OK?

-- 
Pedro Alves

[-- Attachment #2: bsd_kvm.diff --]
[-- Type: text/x-diff, Size: 3597 bytes --]

2008-08-09  Pedro Alves  <pedro@codesourcery.com>

	* bsd-kvm.c: Include "gdbthread.h".
	(bsd_kvm_ptid): New.
	(bsd_kvm_open): Add a main thread.
	(bsd_kvm_close): Delete it.
	(bsd_kvm_thread_alive): New.
	(bsd_kvm_pid_to_str): New.
	(bsd_kvm_add_target): Register bsd_kvm_thread_alive and
	bsd_kvm_pid_to_str.
	(bsd_kvm_add_target): Initialize bsd_kvm_ptid.

---
 gdb/bsd-kvm.c |   44 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

Index: src/gdb/bsd-kvm.c
===================================================================
--- src.orig/gdb/bsd-kvm.c	2008-08-09 15:04:34.000000000 +0100
+++ src/gdb/bsd-kvm.c	2008-08-09 15:28:08.000000000 +0100
@@ -25,6 +25,7 @@
 #include "target.h"
 #include "value.h"
 #include "gdbcore.h"		/* for get_exec_file */
+#include "gdbthread.h"
 
 #include "gdb_assert.h"
 #include <fcntl.h>
@@ -56,6 +57,11 @@ static int (*bsd_kvm_supply_pcb)(struct 
 /* Target ops for libkvm interface.  */
 static struct target_ops bsd_kvm_ops;
 
+/* This is the ptid we use while we're connected to kvm.  The kvm
+   target currently doesn't export any view of the running processes,
+   so this represents the kernel task.  */
+static ptid_t bsd_kvm_ptid;
+
 static void
 bsd_kvm_open (char *filename, int from_tty)
 {
@@ -89,6 +95,9 @@ bsd_kvm_open (char *filename, int from_t
   core_kd = temp_kd;
   push_target (&bsd_kvm_ops);
 
+  add_thread_silent (bsd_kvm_ptid);
+  inferior_ptid = bsd_kvm_ptid;
+
   target_fetch_registers (get_current_regcache (), -1);
 
   reinit_frame_cache ();
@@ -104,6 +113,9 @@ bsd_kvm_close (int quitting)
 	warning (("%s"), kvm_geterr(core_kd));
       core_kd = NULL;
     }
+
+  inferior_ptid = null_ptid;
+  delete_thread_silent (bsd_kvm_ptid);
 }
 
 static LONGEST
@@ -297,6 +309,20 @@ bsd_kvm_pcb_cmd (char *arg, int fromtty)
   print_stack_frame (get_selected_frame (NULL), -1, 1);
 }
 
+static int
+bsd_kvm_thread_alive (ptid_t ptid)
+{
+  return 1;
+}
+
+static char *
+bsd_kvm_pid_to_str (ptid_t ptid)
+{
+  static char buf[64];
+  xsnprintf (buf, sizeof buf, "<kvm>");
+  return buf;
+}
+
 /* Add the libkvm interface to the list of all possible targets and
    register CUPPLY_PCB as the architecture-specific process control
    block interpreter.  */
@@ -316,6 +342,8 @@ Optionally specify the filename of a cor
   bsd_kvm_ops.to_fetch_registers = bsd_kvm_fetch_registers;
   bsd_kvm_ops.to_xfer_partial = bsd_kvm_xfer_partial;
   bsd_kvm_ops.to_files_info = bsd_kvm_files_info;
+  bsd_kvm_ops.to_thread_alive = bsd_kvm_thread_alive;
+  bsd_kvm_ops.to_pid_to_str = bsd_kvm_pid_to_str;
   bsd_kvm_ops.to_stratum = process_stratum;
   bsd_kvm_ops.to_has_memory = 1;
   bsd_kvm_ops.to_has_stack = 1;
@@ -335,4 +363,20 @@ Generic command for manipulating the ker
   add_cmd ("pcb", class_obscure, bsd_kvm_pcb_cmd,
 	   /* i18n: PCB == "Process Control Block" */
 	   _("Set current context from pcb address"), &bsd_kvm_cmdlist);
+
+  /* Some notes on the ptid usage on this target.
+
+     The pid field represents the kvm inferior instance.  Currently,
+     we don't support multiple kvm inferiors, but we start at 1
+     anyway.  The lwp field is set to != 0, in case the core wants to
+     refer to the whole kvm inferior with ptid(1,0,0).
+
+     If kvm is made to export running processes as gdb threads,
+     the following form can be used:
+     ptid (1, 1, 0) -> kvm inferior 1, in kernel
+     ptid (1, 1, 1) -> kvm inferior 1, process 1
+     ptid (1, 1, 2) -> kvm inferior 1, process 2
+     ptid (1, 1, n) -> kvm inferior 1, process n
+  */
+  bsd_kvm_ptid = ptid_build (1, 1, 0);
 }