public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Kevin Buettner <kevinb@redhat.com>
To: gdb-patches@sourceware.org
Cc: Pedro Alves <pedro@palves.net>, Kevin Buettner <kevinb@redhat.com>
Subject: [PATCH v4 3/4] Print only process ptids from linux-fork.c
Date: Tue, 25 Jun 2024 18:55:54 -0700	[thread overview]
Message-ID: <20240626020148.68109-4-kevinb@redhat.com> (raw)
In-Reply-To: <20240626020148.68109-1-kevinb@redhat.com>

This commit causes a "process ptid" to be passed to all calls
of target_pid_to_str in linux-fork.c.  A "process ptid" is one
in which only the pid component is set to a non-zero value;
both the lwp  and tid components are zero.

The reason for doing this is that pids associated with checkpoints can
never be a thread due to the fact that checkpoints (which are
implemented by forking a process) can only (reasonably) work with
single-threaded processes.

Without this commit, many of the "info checkpoints" commands
in gdb.multi/checkpoint-multi.exp will incorrectly show some
of the checkpoints as threads.  E.g...

*  1.0 A  Thread 0x7ffff7cd3740 (LWP 128534) at 0x401199, file hello.c, line 51
   1.2    process 128546 at 0x401199, file hello.c, line 51
   1.3    process 128547 at 0x401199, file hello.c, line 51
   2.1    process 128538 at 0x401258, file goodbye.c, line 62
   2.2 A  Thread 0x7ffff7cd3740 (LWP 128542) at 0x401258, file goodbye.c, line 62
   3.0 A  Thread 0x7ffff7cd3740 (LWP 128543) at 0x40115c, file hangout.c, line 31
   3.2    process 128545 at 0x40115c, file hangout.c, line 31

With this commit in place, the output looks like this instead:

*  1.0 A  process 129961 at 0x401199, file hello.c, line 51
   1.2    process 129974 at 0x401199, file hello.c, line 51
   1.3    process 129975 at 0x401199, file hello.c, line 51
   2.1    process 129965 at 0x401258, file goodbye.c, line 62
   2.2 A  process 129969 at 0x401258, file goodbye.c, line 62
   3.0 A  process 129970 at 0x40115c, file hangout.c, line 31
   3.2    process 129972 at 0x40115c, file hangout.c, line 31

(For brevity, I've removed the directory elements in each of the paths
above.)

I tried fixing this problem in another way, by not initializing the
LWP component in the fork_info constructor.  This leaves the LWP
component as 0, making the ptid in question a process ptid.  However,
when I tried this, I saw asserts in add_initial_lwp, which is called
by add_lwp, which, in turn, is called by linux_nat_switch_fork(),
which is clearly needed by the linux-fork code.  So... trying to
avoid setting the LWP component to a non-zero value doesn't work.

The testcase, gdb.multi/checkpoint-multi.exp, has been updated to
reflect the fact that only "process" should now appear in output
from "info checkpoints".
---
 gdb/linux-fork.c                             | 37 ++++++++++++++------
 gdb/testsuite/gdb.multi/checkpoint-multi.exp |  2 +-
 2 files changed, 27 insertions(+), 12 deletions(-)

diff --git a/gdb/linux-fork.c b/gdb/linux-fork.c
index 4905d4ffd88..501e8c332cb 100644
--- a/gdb/linux-fork.c
+++ b/gdb/linux-fork.c
@@ -445,6 +445,17 @@ fork_save_infrun_state (struct fork_info *fp)
     }
 }
 
+/* Given a ptid, return a "process ptid" in which only the pid member
+   is present.  This is used in calls to target_pid_to_str() to ensure
+   that only process ptids are printed by this file.  */
+
+static inline ptid_t
+proc_ptid (ptid_t ptid)
+{
+  ptid_t process_ptid (ptid.pid ());
+  return process_ptid;
+}
+
 /* Kill 'em all, let God sort 'em out...  */
 
 void
@@ -508,7 +519,7 @@ linux_fork_mourn_inferior ()
   last = find_last_fork (inf);
   fork_load_infrun_state (last);
   gdb_printf (_("[Switching to %s]\n"),
-	      target_pid_to_str (inferior_ptid).c_str ());
+	      target_pid_to_str (proc_ptid (inferior_ptid)).c_str ());
 
   /* If there's only one fork, switch back to non-fork mode.  */
   if (one_fork_p (inf))
@@ -536,7 +547,7 @@ linux_fork_detach (int from_tty, lwp_info *lp, inferior *inf)
     {
       if (ptrace (PTRACE_DETACH, inferior_ptid.pid (), 0, 0))
 	error (_("Unable to detach %s"),
-	       target_pid_to_str (inferior_ptid).c_str ());
+	       target_pid_to_str (proc_ptid (inferior_ptid)).c_str ());
     }
 
   delete_fork (inferior_ptid, inf);
@@ -551,7 +562,7 @@ linux_fork_detach (int from_tty, lwp_info *lp, inferior *inf)
 
   if (from_tty)
     gdb_printf (_("[Switching to %s]\n"),
-		target_pid_to_str (inferior_ptid).c_str ());
+		target_pid_to_str (proc_ptid (inferior_ptid)).c_str ());
 
   /* If there's only one fork, switch back to non-fork mode.  */
   if (one_fork_p (inf))
@@ -626,7 +637,7 @@ class scoped_switch_fork_info
 	catch (const gdb_exception &ex)
 	  {
 	    warning (_("Couldn't restore checkpoint state in %s: %s"),
-		     target_pid_to_str (m_oldfp->ptid).c_str (),
+		     target_pid_to_str (proc_ptid (m_oldfp->ptid)).c_str (),
 		     ex.what ());
 	  }
       }
@@ -702,10 +713,12 @@ delete_checkpoint_command (const char *args, int from_tty)
     error (_("Cannot delete active checkpoint"));
 
   if (ptrace (PTRACE_KILL, ptid.pid (), 0, 0))
-    error (_("Unable to kill pid %s"), target_pid_to_str (ptid).c_str ());
+    error (_("Unable to kill pid %s"),
+	   target_pid_to_str (proc_ptid (ptid)).c_str ());
 
   if (from_tty)
-    gdb_printf (_("Killed %s\n"), target_pid_to_str (ptid).c_str ());
+    gdb_printf (_("Killed %s\n"),
+		target_pid_to_str (proc_ptid (ptid)).c_str ());
 
   delete_fork (ptid, inf);
 
@@ -730,7 +743,7 @@ delete_checkpoint_command (const char *args, int from_tty)
     {
       if (inferior_call_waitpid (pptid, ptid.pid ()))
 	warning (_("Unable to wait pid %s"),
-		 target_pid_to_str (ptid).c_str ());
+		 target_pid_to_str (proc_ptid (ptid)).c_str ());
     }
 }
 
@@ -750,10 +763,12 @@ detach_checkpoint_command (const char *args, int from_tty)
 Please switch to another checkpoint before detaching the current one"));
 
   if (ptrace (PTRACE_DETACH, ptid.pid (), 0, 0))
-    error (_("Unable to detach %s"), target_pid_to_str (ptid).c_str ());
+    error (_("Unable to detach %s"),
+	   target_pid_to_str (proc_ptid (ptid)).c_str ());
 
   if (from_tty)
-    gdb_printf (_("Detached %s\n"), target_pid_to_str (ptid).c_str ());
+    gdb_printf (_("Detached %s\n"),
+	        target_pid_to_str (proc_ptid (ptid)).c_str ());
 
   delete_fork (ptid, current_inferior ());
 }
@@ -816,7 +831,7 @@ info_checkpoints_command (const char *arg, int from_tty)
 	  else
 	    gdb_printf ("   ");
 
-	  gdb_printf ("%s", target_pid_to_str (fi.ptid).c_str ());
+	  gdb_printf ("%s", target_pid_to_str (proc_ptid (fi.ptid)).c_str ());
 
 	  gdb_printf (_(" at "));
 	  ULONGEST pc
@@ -983,7 +998,7 @@ linux_fork_context (struct fork_info *newfp, int from_tty, inferior *newinf)
       insert_breakpoints ();
       if (!inferior_changed)
 	gdb_printf (_("Switching to %s\n"),
-		    target_pid_to_str (inferior_ptid).c_str ());
+		    target_pid_to_str (proc_ptid (inferior_ptid)).c_str ());
     }
 
   notify_user_selected_context_changed
diff --git a/gdb/testsuite/gdb.multi/checkpoint-multi.exp b/gdb/testsuite/gdb.multi/checkpoint-multi.exp
index a8f0190747b..6d4b70e1adb 100644
--- a/gdb/testsuite/gdb.multi/checkpoint-multi.exp
+++ b/gdb/testsuite/gdb.multi/checkpoint-multi.exp
@@ -22,7 +22,7 @@ require {istarget "*-*-linux*"}
 # Checkpoint support is implemented for the (Linux) native target.
 require gdb_protocol_is_native
 
-set proc_re "(?:process $::decimal|Thread $::hex \\(LWP $::decimal\\))"
+set proc_re "(?:process $::decimal)"
 set ckpt_re "Checkpoint"
 set main_proc "\\(main process\\)"
 set hello_c "hello\\.c"
-- 
2.45.2


  parent reply	other threads:[~2024-06-26  2:02 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-26  1:55 [PATCH v4 0/4] Make linux checkpoints work with multiple inferiors Kevin Buettner
2024-06-26  1:55 ` [PATCH v4 1/4] " Kevin Buettner
2024-06-26  1:55 ` [PATCH v4 2/4] Capitalize output of successful checkpoint command Kevin Buettner
2024-06-26  1:55 ` Kevin Buettner [this message]
2024-06-26  1:55 ` [PATCH v4 4/4] Linux checkpoints: Update NEWS and gdb.texinfo regarding multiple inferiors Kevin Buettner
2024-06-26 12:25   ` Eli Zaretskii

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240626020148.68109-4-kevinb@redhat.com \
    --to=kevinb@redhat.com \
    --cc=gdb-patches@sourceware.org \
    --cc=pedro@palves.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).