This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]