Re: [PATCH] Linux/ptrace: don't convert ptids when asking inf-ptrace layer to resume LWP

Re: [PATCH] Linux/ptrace: don't convert ptids when asking inf-ptrace layer to resume LWP

[Mark Kettenis]

> From: Pedro Alves <palves@redhat.com>
> Date: Tue, 3 Mar 2015 13:33:44 +0000
> 
> Tested on x86-64 Fedora 20, -m32.
> 
> gdb/ChangeLog:
> 2015-03-03  Pedro Alves  <palves@redhat.com>
> 
> 	* i386-linux-nat.c (i386_linux_resume): Get the ptrace PID out of
> 	the lwp field of ptid.  Pass the full ptid to get_thread_regcache.
> 	* inf-ptrace.c (get_ptrace_pid): New function.
> 	(inf_ptrace_resume): Use it.
> 	* linux-nat.c (linux_resume_one_lwp): Pass the LWP's ptid ummodified
> 	to the lower layer.
> ---
>  gdb/ChangeLog        |  9 +++++++++
>  gdb/i386-linux-nat.c |  5 ++---
>  gdb/inf-ptrace.c     | 25 ++++++++++++++++++++++---
>  gdb/linux-nat.c      |  6 +-----
>  4 files changed, 34 insertions(+), 11 deletions(-)

I have some fear this is going to break non-Linux targets.

> --- a/gdb/inf-ptrace.c
> +++ b/gdb/inf-ptrace.c
> @@ -289,6 +289,22 @@ inf_ptrace_stop (struct target_ops *self, ptid_t ptid)
>    kill (-inferior_process_group (), SIGINT);
>  }
>  
> +/* Return which PID to pass to ptrace in order to observe/control the
> +   tracee identified by PTID.  */
> +
> +static pid_t
> +get_ptrace_pid (ptid_t ptid)
> +{
> +  pid_t pid;
> +
> +  /* If we have an LWPID to work with, use it.  Otherwise, we're
> +     dealing with a non-threaded program/target.  */
> +  pid = ptid_get_lwp (ptid);
> +  if (pid == 0)
> +    pid = ptid_get_pid (ptid);
> +  return pid;
> +}
> +
>  /* Resume execution of thread PTID, or all threads if PTID is -1.  If
>     STEP is nonzero, single-step it.  If SIGNAL is nonzero, give it
>     that signal.  */
> @@ -297,13 +313,16 @@ static void
>  inf_ptrace_resume (struct target_ops *ops,
>  		   ptid_t ptid, int step, enum gdb_signal signal)
>  {
> -  pid_t pid = ptid_get_pid (ptid);
> +  pid_t pid;
> +
>    int request;

Please don't introduce a blank line here.

> -  if (pid == -1)
> +  if (ptid_equal (minus_one_ptid, ptid))
>      /* Resume all threads.  Traditionally ptrace() only supports
>         single-threaded processes, so simply resume the inferior.  */
> -    pid = ptid_get_pid (inferior_ptid);
> +    pid = get_ptrace_pid (inferior_ptid);

This defenitely should remain ptid_get_pid(); you want to resume the
entire process and not an individual thread.

> +  else
> +    pid = get_ptrace_pid (ptid);

This should work for OpenBSD, and probably works for FreeBSD.  It
won't work for NetBSD, but they will probably need their own
implementation of this function, so it's probably fine.

Re: [PATCH] Linux/ptrace: don't convert ptids when asking inf-ptrace layer to resume LWP

[Pedro Alves]

On 03/03/2015 02:39 PM, Mark Kettenis wrote:
>> From: Pedro Alves <palves@redhat.com>
>> Date: Tue, 3 Mar 2015 13:33:44 +0000
>>
>> Tested on x86-64 Fedora 20, -m32.
>>
>> gdb/ChangeLog:
>> 2015-03-03  Pedro Alves  <palves@redhat.com>
>>
>> 	* i386-linux-nat.c (i386_linux_resume): Get the ptrace PID out of
>> 	the lwp field of ptid.  Pass the full ptid to get_thread_regcache.
>> 	* inf-ptrace.c (get_ptrace_pid): New function.
>> 	(inf_ptrace_resume): Use it.
>> 	* linux-nat.c (linux_resume_one_lwp): Pass the LWP's ptid ummodified
>> 	to the lower layer.
>> ---
>>  gdb/ChangeLog        |  9 +++++++++
>>  gdb/i386-linux-nat.c |  5 ++---
>>  gdb/inf-ptrace.c     | 25 ++++++++++++++++++++++---
>>  gdb/linux-nat.c      |  6 +-----
>>  4 files changed, 34 insertions(+), 11 deletions(-)
> 
> I have some fear this is going to break non-Linux targets.
> 
>> --- a/gdb/inf-ptrace.c
>> +++ b/gdb/inf-ptrace.c
>> @@ -289,6 +289,22 @@ inf_ptrace_stop (struct target_ops *self, ptid_t ptid)
>>    kill (-inferior_process_group (), SIGINT);
>>  }
>>  
>> +/* Return which PID to pass to ptrace in order to observe/control the
>> +   tracee identified by PTID.  */
>> +
>> +static pid_t
>> +get_ptrace_pid (ptid_t ptid)
>> +{
>> +  pid_t pid;
>> +
>> +  /* If we have an LWPID to work with, use it.  Otherwise, we're
>> +     dealing with a non-threaded program/target.  */
>> +  pid = ptid_get_lwp (ptid);
>> +  if (pid == 0)
>> +    pid = ptid_get_pid (ptid);
>> +  return pid;
>> +}
>> +
>>  /* Resume execution of thread PTID, or all threads if PTID is -1.  If
>>     STEP is nonzero, single-step it.  If SIGNAL is nonzero, give it
>>     that signal.  */
>> @@ -297,13 +313,16 @@ static void
>>  inf_ptrace_resume (struct target_ops *ops,
>>  		   ptid_t ptid, int step, enum gdb_signal signal)
>>  {
>> -  pid_t pid = ptid_get_pid (ptid);
>> +  pid_t pid;
>> +
>>    int request;
> 
> Please don't introduce a blank line here.

Darn, LOL, I removed one, added another...:

 i386_linux_resume (struct target_ops *ops,
                   ptid_t ptid, int step, enum gdb_signal signal)
 {
-  int pid = ptid_get_pid (ptid);
-
+  int pid = ptid_get_lwp (ptid);
   int request;

I'll remove the one I added...

> 
>> -  if (pid == -1)
>> +  if (ptid_equal (minus_one_ptid, ptid))
>>      /* Resume all threads.  Traditionally ptrace() only supports
>>         single-threaded processes, so simply resume the inferior.  */
>> -    pid = ptid_get_pid (inferior_ptid);
>> +    pid = get_ptrace_pid (inferior_ptid);
> 
> This defenitely should remain ptid_get_pid(); you want to resume the
> entire process and not an individual thread.

My thinking is that it doesn't matter in practice.

Or is it that if there are multiple kernel threads in the
process, ptrace(PTRACE_CONT, PID) on bsd actually resumes
them all?  Then other things must be broken anyway.

I was assuming that on BSD targets that use this method,
there would only be one thread in the core thread list, and
it would either have LWPID==0, or have PID==LWPID, thus it didn't
matter if get_ptrace_pid returned the PID or the LWPID.

If there anything that actually creates other threads with
a different LWPID on these targets?

> 
>> +  else
>> +    pid = get_ptrace_pid (ptid);
> 
> This should work for OpenBSD, and probably works for FreeBSD.  It
> won't work for NetBSD, but they will probably need their own
> implementation of this function, so it's probably fine.
> 

Thanks,
Pedro Alves

Re: [PATCH] Linux/ptrace: don't convert ptids when asking inf-ptrace layer to resume LWP

[Pedro Alves]

On 03/03/2015 03:12 PM, Pedro Alves wrote:
> On 03/03/2015 02:39 PM, Mark Kettenis wrote:

>>> -  if (pid == -1)
>>> +  if (ptid_equal (minus_one_ptid, ptid))
>>>      /* Resume all threads.  Traditionally ptrace() only supports
>>>         single-threaded processes, so simply resume the inferior.  */
>>> -    pid = ptid_get_pid (inferior_ptid);
>>> +    pid = get_ptrace_pid (inferior_ptid);
>>
>> This defenitely should remain ptid_get_pid(); you want to resume the
>> entire process and not an individual thread.
> 
> My thinking is that it doesn't matter in practice.
> 
> Or is it that if there are multiple kernel threads in the
> process, ptrace(PTRACE_CONT, PID) on bsd actually resumes
> them all?  Then other things must be broken anyway.
> 
> I was assuming that on BSD targets that use this method,
> there would only be one thread in the core thread list, and
> it would either have LWPID==0, or have PID==LWPID, thus it didn't
> matter if get_ptrace_pid returned the PID or the LWPID.
> 
> If there anything that actually creates other threads with
> a different LWPID on these targets?
> 
>>
>>> +  else
>>> +    pid = get_ptrace_pid (ptid);
>>
>> This should work for OpenBSD, and probably works for FreeBSD.  It
>> won't work for NetBSD, but they will probably need their own
>> implementation of this function, so it's probably fine.
>>
> 

In any case, I agree it's at least clearer.  I'm testing this.

Thanks.

From b8480e9cfcc2107b81751084c01a262eb1d52c74 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Tue, 3 Mar 2015 15:27:54 +0000
Subject: [PATCH] inf-ptrace.c: use ptid_get_pid when resuming all threads

In this case, we want to resume the entire process and not an
individual thread.  Using the overall process id is clearer and more
correct.

gdb/
2015-03-03  Pedro Alves  <palves@redhat.com>

	* inf-ptrace.c (inf_ptrace_resume): Remove spurious whitespace.
	Use ptid_get_pid to get the overall process id when resuming all
	threads.
---
 gdb/ChangeLog    | 6 ++++++
 gdb/inf-ptrace.c | 3 +--
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 31a672e..20250e4 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,11 @@
 2015-03-03  Pedro Alves  <palves@redhat.com>
 
+	* inf-ptrace.c (inf_ptrace_resume): Remove spurious whitespace.
+	Use ptid_get_pid to get the overall process id when resuming all
+	threads.
+
+2015-03-03  Pedro Alves  <palves@redhat.com>
+
 	* i386-linux-nat.c (i386_linux_resume): Get the ptrace PID out of
 	the lwp field of ptid.  Pass the full ptid to get_thread_regcache.
 	* inf-ptrace.c (get_ptrace_pid): New function.
diff --git a/gdb/inf-ptrace.c b/gdb/inf-ptrace.c
index 606bdb4..3502a12 100644
--- a/gdb/inf-ptrace.c
+++ b/gdb/inf-ptrace.c
@@ -314,13 +314,12 @@ inf_ptrace_resume (struct target_ops *ops,
 		   ptid_t ptid, int step, enum gdb_signal signal)
 {
   pid_t pid;
-
   int request;
 
   if (ptid_equal (minus_one_ptid, ptid))
     /* Resume all threads.  Traditionally ptrace() only supports
        single-threaded processes, so simply resume the inferior.  */
-    pid = get_ptrace_pid (inferior_ptid);
+    pid = ptid_get_pid (inferior_ptid);
   else
     pid = get_ptrace_pid (ptid);
 
-- 
1.9.3

Re: [PATCH] Linux/ptrace: don't convert ptids when asking inf-ptrace layer to resume LWP

[Pedro Alves]

On 03/03/2015 03:34 PM, Pedro Alves wrote:
> On 03/03/2015 03:12 PM, Pedro Alves wrote:
>> On 03/03/2015 02:39 PM, Mark Kettenis wrote:
> 
>>>> -  if (pid == -1)
>>>> +  if (ptid_equal (minus_one_ptid, ptid))
>>>>      /* Resume all threads.  Traditionally ptrace() only supports
>>>>         single-threaded processes, so simply resume the inferior.  */
>>>> -    pid = ptid_get_pid (inferior_ptid);
>>>> +    pid = get_ptrace_pid (inferior_ptid);
>>>
>>> This defenitely should remain ptid_get_pid(); you want to resume the
>>> entire process and not an individual thread.


> In any case, I agree it's at least clearer.  I'm testing this.

Alright, pushed as below.  I dropped the "clearer" comment, as as
discussed, it's a correctness issue.

Thanks!

---
From c1593e4fa9901c65a32e85c3c5d3ec41598be887 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Tue, 3 Mar 2015 16:28:15 +0000
Subject: [PATCH] inf-ptrace.c: use ptid_get_pid when resuming all threads

In this case, we want to resume the entire process and not an
individual thread.

gdb/
2015-03-03  Pedro Alves  <palves@redhat.com>

	* inf-ptrace.c (inf_ptrace_resume): Remove spurious whitespace.
	Use ptid_get_pid to get the overall process id when resuming all
	threads.
---
 gdb/ChangeLog    | 6 ++++++
 gdb/inf-ptrace.c | 3 +--
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 31a672e..20250e4 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,11 @@
 2015-03-03  Pedro Alves  <palves@redhat.com>
 
+	* inf-ptrace.c (inf_ptrace_resume): Remove spurious whitespace.
+	Use ptid_get_pid to get the overall process id when resuming all
+	threads.
+
+2015-03-03  Pedro Alves  <palves@redhat.com>
+
 	* i386-linux-nat.c (i386_linux_resume): Get the ptrace PID out of
 	the lwp field of ptid.  Pass the full ptid to get_thread_regcache.
 	* inf-ptrace.c (get_ptrace_pid): New function.
diff --git a/gdb/inf-ptrace.c b/gdb/inf-ptrace.c
index 606bdb4..3502a12 100644
--- a/gdb/inf-ptrace.c
+++ b/gdb/inf-ptrace.c
@@ -314,13 +314,12 @@ inf_ptrace_resume (struct target_ops *ops,
 		   ptid_t ptid, int step, enum gdb_signal signal)
 {
   pid_t pid;
-
   int request;
 
   if (ptid_equal (minus_one_ptid, ptid))
     /* Resume all threads.  Traditionally ptrace() only supports
        single-threaded processes, so simply resume the inferior.  */
-    pid = get_ptrace_pid (inferior_ptid);
+    pid = ptid_get_pid (inferior_ptid);
   else
     pid = get_ptrace_pid (ptid);
 
-- 
1.9.3

Re: [PATCH] Linux/ptrace: don't convert ptids when asking inf-ptrace layer to resume LWP

[Mark Kettenis]

> Date: Tue, 03 Mar 2015 15:12:48 +0000
> From: Pedro Alves <palves@redhat.com>
> 
> On 03/03/2015 02:39 PM, Mark Kettenis wrote:
> >> From: Pedro Alves <palves@redhat.com>
> >> Date: Tue, 3 Mar 2015 13:33:44 +0000
> >>
> >> Tested on x86-64 Fedora 20, -m32.
> >>
> >> gdb/ChangeLog:
> >> 2015-03-03  Pedro Alves  <palves@redhat.com>
> >>
> >> 	* i386-linux-nat.c (i386_linux_resume): Get the ptrace PID out of
> >> 	the lwp field of ptid.  Pass the full ptid to get_thread_regcache.
> >> 	* inf-ptrace.c (get_ptrace_pid): New function.
> >> 	(inf_ptrace_resume): Use it.
> >> 	* linux-nat.c (linux_resume_one_lwp): Pass the LWP's ptid ummodified
> >> 	to the lower layer.
> >> ---
> >>  gdb/ChangeLog        |  9 +++++++++
> >>  gdb/i386-linux-nat.c |  5 ++---
> >>  gdb/inf-ptrace.c     | 25 ++++++++++++++++++++++---
> >>  gdb/linux-nat.c      |  6 +-----
> >>  4 files changed, 34 insertions(+), 11 deletions(-)
> > 
> > I have some fear this is going to break non-Linux targets.
> > 
> >> --- a/gdb/inf-ptrace.c
> >> +++ b/gdb/inf-ptrace.c
> >> -  if (pid == -1)
> >> +  if (ptid_equal (minus_one_ptid, ptid))
> >>      /* Resume all threads.  Traditionally ptrace() only supports
> >>         single-threaded processes, so simply resume the inferior.  */
> >> -    pid = ptid_get_pid (inferior_ptid);
> >> +    pid = get_ptrace_pid (inferior_ptid);
> > 
> > This defenitely should remain ptid_get_pid(); you want to resume the
> > entire process and not an individual thread.
> 
> My thinking is that it doesn't matter in practice.
> 
> Or is it that if there are multiple kernel threads in the
> process, ptrace(PTRACE_CONT, PID) on bsd actually resumes
> them all?  Then other things must be broken anyway.

I can only speak for OpenBSD here, but yes, on OpenBSD, if you pass
"PID" here, all threads within a process are resumed.  If you want to
resume an individual thread, you need to pass its thread ID.  Thread
IDs are also integers, but start at THREAD_PID_OFFSET, which is
currently defined as 1000000.

Are things broken?  Perhaps.  GDB used to properly support an
all-stop/all-go model, and things still seem to work mostly ok.
Perhaps this diff will actually make things better if there are places
where GDB wants to resume or step a single thread that isn't the
thread that stopped the process in the first place.

I've always considered it a serious flaw that Linux doesn't have a way
to resume the entire process and that we need almost 5000 lines of
code to deal with the consequences.

> I was assuming that on BSD targets that use this method,
> there would only be one thread in the core thread list, and
> it would either have LWPID==0, or have PID==LWPID, thus it didn't
> matter if get_ptrace_pid returned the PID or the LWPID.

That assumption is incorrect.  I see that this assumption has made its
way into infrun.c:

          inferior_ptid = ptid_build (child_pid, child_pid, 0);

That's wrong.  The OS-specific code should fill in the LWPID part with
the appropriate value by using thread_change_ptid().  AFAICT, the
linux-nat.c code does that properly.

> If there anything that actually creates other threads with
> a different LWPID on these targets?

The initial thread ID of an OpenBSD process will be PID + THREAD_PID_OFFSET.

Re: [PATCH] Linux/ptrace: don't convert ptids when asking inf-ptrace layer to resume LWP

[Pedro Alves]

On 03/03/2015 04:04 PM, Mark Kettenis wrote:
>> > Date: Tue, 03 Mar 2015 15:12:48 +0000
>> > From: Pedro Alves <palves@redhat.com>
>> > 


>> > Or is it that if there are multiple kernel threads in the
>> > process, ptrace(PTRACE_CONT, PID) on bsd actually resumes
>> > them all?  Then other things must be broken anyway.
> I can only speak for OpenBSD here, but yes, on OpenBSD, if you pass
> "PID" here, all threads within a process are resumed.  If you want to
> resume an individual thread, you need to pass its thread ID.  Thread
> IDs are also integers, but start at THREAD_PID_OFFSET, which is
> currently defined as 1000000.
> 
> Are things broken?  Perhaps.  GDB used to properly support an
> all-stop/all-go model, and things still seem to work mostly ok.
> Perhaps this diff will actually make things better if there are places
> where GDB wants to resume or step a single thread that isn't the
> thread that stopped the process in the first place.

Yes, I assume so.  Things like schedlock and stepping over a
breakpoint must have been subtly broken thus far, if they have
been letting all threads run while core gdb wants only one
thread to run.

> 
> I've always considered it a serious flaw that Linux doesn't have a way
> to resume the entire process and that we need almost 5000 lines of
> code to deal with the consequences.

:-)

> 
>> > I was assuming that on BSD targets that use this method,
>> > there would only be one thread in the core thread list, and
>> > it would either have LWPID==0, or have PID==LWPID, thus it didn't
>> > matter if get_ptrace_pid returned the PID or the LWPID.
> That assumption is incorrect.  

OK.

> I see that this assumption has made its way into infrun.c:
> 
>           inferior_ptid = ptid_build (child_pid, child_pid, 0);
> 
> That's wrong.  The OS-specific code should fill in the LWPID part with
> the appropriate value by using thread_change_ptid().  AFAICT, the
> linux-nat.c code does that properly.

I think you're pointing at the follow fork code.  That was born out
of a generalization of code that used to live in linux-nat.c, inf-ptrace.c,
etc. (d83ad864).  The old inf-ptrace.c code used to do:

      /* Switch inferior_ptid out of the parent's way.  */
      inferior_ptid = pid_to_ptid (fpid);

      /* Delete the parent.  */
      detach_inferior (pid);

      add_thread_silent (inferior_ptid);


'child_pid' is set at the top of follow_fork_inferior, like:

  child_pid
    = ptid_get_pid (inferior_thread ()->pending_follow.value.related_pid);

I think I even remember that long ago "related_pid" used to be
a single int, not a ptid.

This is again another case of losing information.  I think we could
easily make that:

  child_pid = inferior_thread ()->pending_follow.value.related_pid;

and drop that ptid_build, and things should work everywhere.

I have no idea if "follow fork" actually works on BSD targets correctly.
A buildbot slave would catch breakages like these.  ( hint :-) )

> 
>> > If there anything that actually creates other threads with
>> > a different LWPID on these targets?
> The initial thread ID of an OpenBSD process will be PID + THREAD_PID_OFFSET.
> 

Thanks, didn't know that.

Pedro Alves

Re: [PATCH] Linux/ptrace: don't convert ptids when asking inf-ptrace layer to resume LWP

[Pedro Alves]

On 03/03/2015 04:25 PM, Pedro Alves wrote:
> On 03/03/2015 04:04 PM, Mark Kettenis wrote:

>> I see that this assumption has made its way into infrun.c:
>>
>>           inferior_ptid = ptid_build (child_pid, child_pid, 0);
>>
>> That's wrong.  The OS-specific code should fill in the LWPID part with
>> the appropriate value by using thread_change_ptid().  AFAICT, the
>> linux-nat.c code does that properly.
> 
> I think you're pointing at the follow fork code.  That was born out
> of a generalization of code that used to live in linux-nat.c, inf-ptrace.c,
> etc. (d83ad864).  The old inf-ptrace.c code used to do:
> 
>       /* Switch inferior_ptid out of the parent's way.  */
>       inferior_ptid = pid_to_ptid (fpid);
> 
>       /* Delete the parent.  */
>       detach_inferior (pid);
> 
>       add_thread_silent (inferior_ptid);
> 
> 
> 'child_pid' is set at the top of follow_fork_inferior, like:
> 
>   child_pid
>     = ptid_get_pid (inferior_thread ()->pending_follow.value.related_pid);
> 
> I think I even remember that long ago "related_pid" used to be
> a single int, not a ptid.
> 
> This is again another case of losing information.  I think we could
> easily make that:
> 
>   child_pid = inferior_thread ()->pending_follow.value.related_pid;
> 
> and drop that ptid_build, and things should work everywhere.
> 
> I have no idea if "follow fork" actually works on BSD targets correctly.
> A buildbot slave would catch breakages like these.  ( hint :-) )

Like this.  Let me know what you think.

----
From c557c13f6bfe0126dd9ab74576951b127fd39531 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Tue, 3 Mar 2015 16:36:28 +0000
Subject: [PATCH] follow-fork: don't lose the ptids as set by the target

This Linuxism has made its way into infrun.c, in the follow-fork code:

       inferior_ptid = ptid_build (child_pid, child_pid, 0);

The OS-specific code should fill in the LWPID, TID parts with the
appropriate values, if any, and the core code should not be peeking at
the components of the ptids.

gdb/
2015-03-03  Pedro Alves  <palves@redhat.com>

	* infrun.c (follow_fork_inferior): Use the whole of the
	inferior_ptid and pending_follow.related_pid ptids instead of
	building ptids from the process components.  Adjust verbose output
	to use target_pid_to_str.
	* linux-nat.c (linux_child_follow_fork): Use the whole of the
	inferior_ptid and pending_follow.related_pid ptids instead of
	building ptids from the process components.
---
 gdb/infrun.c    | 33 ++++++++++++++-------------------
 gdb/linux-nat.c | 15 +++++++--------
 2 files changed, 21 insertions(+), 27 deletions(-)

diff --git a/gdb/infrun.c b/gdb/infrun.c
index f87ed4c..abfeeee 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -408,15 +408,12 @@ static int
 follow_fork_inferior (int follow_child, int detach_fork)
 {
   int has_vforked;
-  int parent_pid, child_pid;
+  ptid_t parent_ptid, child_ptid;
 
   has_vforked = (inferior_thread ()->pending_follow.kind
 		 == TARGET_WAITKIND_VFORKED);
-  parent_pid = ptid_get_lwp (inferior_ptid);
-  if (parent_pid == 0)
-    parent_pid = ptid_get_pid (inferior_ptid);
-  child_pid
-    = ptid_get_pid (inferior_thread ()->pending_follow.value.related_pid);
+  parent_ptid = inferior_ptid;
+  child_ptid = inferior_thread ()->pending_follow.value.related_pid;
 
   if (has_vforked
       && !non_stop /* Non-stop always resumes both branches.  */
@@ -460,10 +457,9 @@ holding the child stopped.  Try \"set detach-on-fork\" or \
 	    {
 	      target_terminal_ours_for_output ();
 	      fprintf_filtered (gdb_stdlog,
-				_("Detaching after %s from "
-				  "child process %d.\n"),
+				_("Detaching after %s from child %s.\n"),
 				has_vforked ? "vfork" : "fork",
-				child_pid);
+				target_pid_to_str (child_ptid));
 	    }
 	}
       else
@@ -472,7 +468,7 @@ holding the child stopped.  Try \"set detach-on-fork\" or \
 	  struct cleanup *old_chain;
 
 	  /* Add process to GDB's tables.  */
-	  child_inf = add_inferior (child_pid);
+	  child_inf = add_inferior (ptid_get_pid (child_ptid));
 
 	  parent_inf = current_inferior ();
 	  child_inf->attach_flag = parent_inf->attach_flag;
@@ -483,7 +479,7 @@ holding the child stopped.  Try \"set detach-on-fork\" or \
 	  old_chain = save_inferior_ptid ();
 	  save_current_program_space ();
 
-	  inferior_ptid = ptid_build (child_pid, child_pid, 0);
+	  inferior_ptid = child_ptid;
 	  add_thread (inferior_ptid);
 	  child_inf->symfile_flags = SYMFILE_NO_READ;
 
@@ -549,17 +545,16 @@ holding the child stopped.  Try \"set detach-on-fork\" or \
 	{
 	  target_terminal_ours_for_output ();
 	  fprintf_filtered (gdb_stdlog,
-			    _("Attaching after process %d "
-			      "%s to child process %d.\n"),
-			    parent_pid,
+			    _("Attaching after %s %s to child %s.\n"),
+			    target_pid_to_str (parent_ptid),
 			    has_vforked ? "vfork" : "fork",
-			    child_pid);
+			    target_pid_to_str (child_ptid));
 	}
 
       /* Add the new inferior first, so that the target_detach below
 	 doesn't unpush the target.  */
 
-      child_inf = add_inferior (child_pid);
+      child_inf = add_inferior (ptid_get_pid (child_ptid));
 
       parent_inf = current_inferior ();
       child_inf->attach_flag = parent_inf->attach_flag;
@@ -596,8 +591,8 @@ holding the child stopped.  Try \"set detach-on-fork\" or \
 	      target_terminal_ours_for_output ();
 	      fprintf_filtered (gdb_stdlog,
 				_("Detaching after fork from "
-				  "child process %d.\n"),
-				child_pid);
+				  "child %s.\n"),
+				target_pid_to_str (child_ptid));
 	    }
 
 	  target_detach (NULL, 0);
@@ -609,7 +604,7 @@ holding the child stopped.  Try \"set detach-on-fork\" or \
 	 this new thread, before cloning the program space, and
 	 informing the solib layer about this new process.  */
 
-      inferior_ptid = ptid_build (child_pid, child_pid, 0);
+      inferior_ptid = child_ptid;
       add_thread (inferior_ptid);
 
       /* If this is a vfork child, then the address-space is shared
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index cb10e2c..57bd1e7 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -387,20 +387,19 @@ linux_child_follow_fork (struct target_ops *ops, int follow_child,
       int status = W_STOPCODE (0);
       struct cleanup *old_chain;
       int has_vforked;
+      ptid_t parent_ptid, child_ptid;
       int parent_pid, child_pid;
 
       has_vforked = (inferior_thread ()->pending_follow.kind
 		     == TARGET_WAITKIND_VFORKED);
-      parent_pid = ptid_get_lwp (inferior_ptid);
-      if (parent_pid == 0)
-	parent_pid = ptid_get_pid (inferior_ptid);
-      child_pid
-	= ptid_get_pid (inferior_thread ()->pending_follow.value.related_pid);
-
+      parent_ptid = inferior_ptid;
+      child_ptid = inferior_thread ()->pending_follow.value.related_pid;
+      parent_pid = ptid_get_lwp (parent_ptid);
+      child_pid = ptid_get_lwp (child_ptid);
 
       /* We're already attached to the parent, by default.  */
       old_chain = save_inferior_ptid ();
-      inferior_ptid = ptid_build (child_pid, child_pid, 0);
+      inferior_ptid = child_ptid;
       child_lp = add_lwp (inferior_ptid);
       child_lp->stopped = 1;
       child_lp->last_resume_kind = resume_stop;
@@ -457,7 +456,7 @@ linux_child_follow_fork (struct target_ops *ops, int follow_child,
 	{
 	  struct lwp_info *parent_lp;
 
-	  parent_lp = find_lwp_pid (pid_to_ptid (parent_pid));
+	  parent_lp = find_lwp_pid (parent_ptid);
 	  gdb_assert (linux_supports_tracefork () >= 0);
 
 	  if (linux_supports_tracevforkdone ())
-- 
1.9.3