Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* bsd-kvm target, always a thread
@ 2008-08-08  3:20 Pedro Alves
  2008-08-09  8:17 ` Mark Kettenis
  2008-08-09  8:34 ` Mark Kettenis
  0 siblings, 2 replies; 8+ messages in thread
From: Pedro Alves @ 2008-08-08  3:20 UTC (permalink / raw)
  To: gdb-patches

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

Hi,

This patches makes the bsd-kvm target register a main thread.

I've "tested" this on a x86 OpenBSD-4.3 VM, but I'm not qualified
to do much more openbsd kernel debugging other than:

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

B.T.W, with GDB 6.3, which came with the distro I always get:

 (gdb) tar kvm
 #0  0xd034ee05 in ?? ()

With HEAD I always get 0x00000006.

Is this difference expected?  Related to the recent change to
build on 4.3?

OK?

-- 
Pedro Alves

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

2008-08-08  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.

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

Index: src/gdb/bsd-kvm.c
===================================================================
--- src.orig/gdb/bsd-kvm.c	2008-08-08 04:04:34.000000000 +0100
+++ src/gdb/bsd-kvm.c	2008-08-08 04:10:58.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,12 @@ 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.  Its value is
+   arbitrary, as the kvm target don't have a notion or processes or
+   thread ids, but we need something non-null to place in
+   inferior_ptid.  */
+static ptid_t bsd_kvm_ptid;
+
 static void
 bsd_kvm_open (char *filename, int from_tty)
 {
@@ -89,6 +96,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 +114,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 +310,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 +343,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 +364,8 @@ 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);
+
+  /* Yes, 42000 is arbitrary.  The only sense out of it, is that it
+     isn't 0.  */
+  bsd_kvm_ptid = ptid_build (42000, 0, 42000);
 }

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

* Re: bsd-kvm target, always a thread
  2008-08-08  3:20 bsd-kvm target, always a thread Pedro Alves
@ 2008-08-09  8:17 ` Mark Kettenis
  2008-08-09  8:34 ` Mark Kettenis
  1 sibling, 0 replies; 8+ messages in thread
From: Mark Kettenis @ 2008-08-09  8:17 UTC (permalink / raw)
  To: pedro; +Cc: gdb-patches

> From: Pedro Alves <pedro@codesourcery.com>
> Date: Fri, 8 Aug 2008 04:20:04 +0100
> 
> Hi,
> 
> This patches makes the bsd-kvm target register a main thread.
> 
> I've "tested" this on a x86 OpenBSD-4.3 VM, but I'm not qualified
> to do much more openbsd kernel debugging other than:
> 
>  (gdb) tar kvm
>  #0  0x00000006 in ?? ()
>  (gdb) info threads
>  * 1 <kvm>  0x00000006 in ?? ()
> 
> B.T.W, with GDB 6.3, which came with the distro I always get:
> 
>  (gdb) tar kvm
>  #0  0xd034ee05 in ?? ()
> 
> With HEAD I always get 0x00000006.
> 
> Is this difference expected?  Related to the recent change to
> build on 4.3?

Hmm, for some reason the .sf_eip got lost when I committed that code.
Fixed by the attached diff, which I committed.

(I'll send a seperate reply to discuss your diff).

Index: ChangeLog
from  Mark Kettenis  <kettenis@gnu.org>

	* i386obsd-nat.c (i386obsd_supply_pcb): Supply the right bytes for
	the %eip register.

Index: i386obsd-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/i386obsd-nat.c,v
retrieving revision 1.12
diff -u -p -r1.12 i386obsd-nat.c
--- i386obsd-nat.c 6 Aug 2008 19:56:20 -0000 1.12
+++ i386obsd-nat.c 9 Aug 2008 07:58:54 -0000
@@ -78,7 +78,7 @@ i386obsd_supply_pcb (struct regcache *re
       pcb->pcb_esp = pcb->pcb_ebp;
       pcb->pcb_ebp = read_memory_integer(pcb->pcb_esp, 4);
       sf.sf_eip = read_memory_integer(pcb->pcb_esp + 4, 4);
-      regcache_raw_supply (regcache, I386_EIP_REGNUM, &sf);
+      regcache_raw_supply (regcache, I386_EIP_REGNUM, &sf.sf_eip);
     }
 
   regcache_raw_supply (regcache, I386_EBP_REGNUM, &pcb->pcb_ebp);


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

* Re: bsd-kvm target, always a thread
  2008-08-08  3:20 bsd-kvm target, always a thread Pedro Alves
  2008-08-09  8:17 ` Mark Kettenis
@ 2008-08-09  8:34 ` Mark Kettenis
  2008-08-09 11:28   ` Pedro Alves
  1 sibling, 1 reply; 8+ messages in thread
From: Mark Kettenis @ 2008-08-09  8:34 UTC (permalink / raw)
  To: pedro; +Cc: gdb-patches

> From: Pedro Alves <pedro@codesourcery.com>
> Date: Fri, 8 Aug 2008 04:20:04 +0100
> 
> Hi,
> 
> This patches makes the bsd-kvm target register a main thread.
> 
> OK?

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?

> 2008-08-08  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.


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

* Re: bsd-kvm target, always a thread
  2008-08-09  8:34 ` Mark Kettenis
@ 2008-08-09 11:28   ` Pedro Alves
  2008-08-09 12:13     ` Mark Kettenis
  0 siblings, 1 reply; 8+ messages in thread
From: Pedro Alves @ 2008-08-09 11:28 UTC (permalink / raw)
  To: Mark Kettenis; +Cc: gdb-patches

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?

    ptid(42000, 0, 0)    ->  for use when we pass around a      
                             ptid representing the whole inferior.

    ptid(42000, 1, 0)    ->  in kernel

    ptid(42000, 1, 1)  ->  process 1.
    ptid(42000, 1, 2)  ->  process 2
    ptid(42000, 1, 3)  ->  process 3
    ...

Or, does it make sense to have one or more threads for the kernel,
distinct from user visible processes?

    ptid(42000, 0, 0)    ->  for use when we pass around a      
                             ptid representing the whole inferior.

    ptid(42000, 1, 1)    ->  kernel thread/context 1
    ptid(42000, 1, 2)    ->  kernel thread/context 2
    ...
    ptid(42000, 0, 1)  ->  user process 1.
    ptid(42000, 0, 2)  ->  user process 2
    ptid(42000, 0, 3)  ->  user process 3
    ...

These are internal ids, of course.  We only show them what we
want in target_pid_to_str and target_extra_thread_info.  The user
doesn't need to know anything about these ids.

-- 
Pedro Alves


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

* Re: bsd-kvm target, always a thread
  2008-08-09 11:28   ` Pedro Alves
@ 2008-08-09 12:13     ` Mark Kettenis
  2008-08-09 14:31       ` Pedro Alves
  0 siblings, 1 reply; 8+ messages in thread
From: Mark Kettenis @ 2008-08-09 12:13 UTC (permalink / raw)
  To: pedro; +Cc: gdb-patches

> 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?
> 
>     ptid(42000, 0, 0)    ->  for use when we pass around a      
>                              ptid representing the whole inferior.
> 
>     ptid(42000, 1, 0)    ->  in kernel
> 
>     ptid(42000, 1, 1)  ->  process 1.
>     ptid(42000, 1, 2)  ->  process 2
>     ptid(42000, 1, 3)  ->  process 3
>     ...

Something like that'd work fine for the OpenBSD kernel.

> These are internal ids, of course.  We only show them what we
> want in target_pid_to_str and target_extra_thread_info.  The user
> doesn't need to know anything about these ids.

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.  I see that remote.c uses negative numbers for special cases.
Would using -1 or -2 work for you?


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

* Re: bsd-kvm target, always a thread
  2008-08-09 12:13     ` Mark Kettenis
@ 2008-08-09 14:31       ` Pedro Alves
  2008-08-10 15:56         ` Mark Kettenis
  0 siblings, 1 reply; 8+ messages in thread
From: Pedro Alves @ 2008-08-09 14:31 UTC (permalink / raw)
  To: Mark Kettenis; +Cc: gdb-patches

[-- 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);
 }

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

* Re: bsd-kvm target, always a thread
  2008-08-09 14:31       ` Pedro Alves
@ 2008-08-10 15:56         ` Mark Kettenis
  2008-08-10 17:34           ` Pedro Alves
  0 siblings, 1 reply; 8+ messages in thread
From: Mark Kettenis @ 2008-08-10 15:56 UTC (permalink / raw)
  To: pedro; +Cc: gdb-patches

> From: Pedro Alves <pedro@codesourcery.com>
> Date: Sat, 9 Aug 2008 15:29:08 +0100
> 
> 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 can see where that 42 is coming from.  So 42000 can defenitely not
be the answer! ;).

Seriously though, the fact the it was changed for 42 to 42000 in some
places is a hint that there is a problem here.  There must have been a
collision between 42 and a real process ID somehwre.  And I guess it
was changed to 42000 since many Unix systems process IDs are 16-bit.
But there may be modern systems around that use larger values.

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

Not if you set the lwpid or tid to something non-zero.

> 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 :-)

Well, that diff would be simply wrong!  What if you're debugging
process ID 42000 remotely?  

> 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?

I guess this will do for now.

> 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.


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

* Re: bsd-kvm target, always a thread
  2008-08-10 15:56         ` Mark Kettenis
@ 2008-08-10 17:34           ` Pedro Alves
  0 siblings, 0 replies; 8+ messages in thread
From: Pedro Alves @ 2008-08-10 17:34 UTC (permalink / raw)
  To: Mark Kettenis; +Cc: gdb-patches

On Sunday 10 August 2008 16:54:52, Mark Kettenis wrote:
> > From: Pedro Alves <pedro@codesourcery.com>

> > > 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 can see where that 42 is coming from.  So 42000 can defenitely not
> be the answer! ;).

This is used in targets that either have no notion of pids
whatsoever, (monitor, go32-nat, remote-sim fall in this category) or
the protocol/stub doesn't report those (remote-m32c I guess).  remote.c
falls a bit on both sides, and so has a somewhat special handling of
this.  See below.

> Seriously though, the fact the it was changed for 42 to 42000 in some
> places is a hint that there is a problem here.  There must have been a
> collision between 42 and a real process ID somehwre.  And I guess it
> was changed to 42000 since many Unix systems process IDs are 16-bit.
> But there may be modern systems around that use larger values.

See below.

> > Let's not use -1, as that conflicts a bit with the special
> > ptid(-1,0,0) (aka, minus_one_ptid).
>
> Not if you set the lwpid or tid to something non-zero.
>

It does, if you want to use (pid,0,0) to refer to the whole inferior,
vs (-1,0,0) to mean all threads of all inferiors.

> > 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 :-)
>
> Well, that diff would be simply wrong!  What if you're debugging
> process ID 42000 remotely?

No, notice that the lwp member is != 0.

There is currently no problem whatsoever in this case.  This id is only
used when the remote side does *not* support or report any pid/thread
id, or is supported, hasn't reported to GDB yet.  It is never passed
back to the remote side, e.g,

  if (ptid_equal (ptid, magic_null_ptid))
    {
      /* MAGIC_NULL_PTID means that we don't have any active threads,
	 so we don't have any TID numbers the inferior will
	 understand.  Make sure to only send forms that do not specify
	 a TID.  */
      if (step && siggnal != TARGET_SIGNAL_0)
	outbuf = xstrprintf ("vCont;S%02x", siggnal);
      else if (step)
	outbuf = xstrprintf ("vCont;s");
      else if (siggnal != TARGET_SIGNAL_0)
	outbuf = xstrprintf ("vCont;C%02x", siggnal);
      else
	outbuf = xstrprintf ("vCont;c");
    }

ptid(42000,1,-1) can never be mistaken with a real pid, since the
lwp member is != 0, and that is never used for a ptid representing a process
or thread id reported by the remote side.

...
Check out remote.c:set_thread:

/* If PTID is MAGIC_NULL_PTID, don't set any thread.  If PTID is
   MINUS_ONE_PTID, set the thread to -1, so the stub returns the
   thread.  If GEN is set, set the general thread, if not, then set
   the step/continue thread.  */

Also remote.c:record_currthread, where we detect that the stub reported
a pid/tid, which means we can stop using magic_null_ptid at that point.

Also see remote.c:remote_start_remote, where we override magic_null_ptid
current thread with reported by qC, if the stub suports it.

(yes, remote.c:extended_remote_create_inferior_1 could be checking
if the remote reported a pid/tid in the stop reply to vRun, and
querying the remote for the current thread with qC, if qC
is supported --- I've come across this when working on the
multi-process remote work)

> I guess this will do for now.

Thanks, I'll go check it in.  Indeed, we can always revisit this when
we need it.  I'm seriously *not* trying to get in your way.  :-)

-- 
Pedro Alves


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

end of thread, other threads:[~2008-08-10 17:34 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-08-08  3:20 bsd-kvm target, always a thread Pedro Alves
2008-08-09  8:17 ` Mark Kettenis
2008-08-09  8:34 ` Mark Kettenis
2008-08-09 11:28   ` Pedro Alves
2008-08-09 12:13     ` Mark Kettenis
2008-08-09 14:31       ` Pedro Alves
2008-08-10 15:56         ` Mark Kettenis
2008-08-10 17:34           ` Pedro Alves

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox