[PATCH 3/3] Bail out of "attach" if a thread cannot be traced

Tom Tromey tromey@adacore.com
Tue Oct 3 17:56:24 GMT 2023


On Linux, threads are treated much like separate processes by the
kernel.  In particular, it's possible to ptrace just a single thread.
If gdb tries to attach to a multi-threaded inferior, where a non-main
thread is already being traced (e.g., by strace), then gdb will get
into an infinite loop attempting to attach.

This patch fixes this problem by having the attach fail if ptrace
fails to attach to any thread of the inferior.
---
 gdb/linux-nat.c                          |  49 +++++++++++----
 gdb/testsuite/gdb.base/traced-thread.c   | 105 +++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.base/traced-thread.exp |  54 ++++++++++++++++
 gdbserver/linux-low.cc                   |  15 ++++-
 4 files changed, 210 insertions(+), 13 deletions(-)

diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 7e36ced6292..5974ab95390 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -1024,8 +1024,8 @@ attach_proc_task_lwp_callback (ptid_t ptid)
 	      std::string reason
 		= linux_ptrace_attach_fail_reason_string (ptid, err);
 
-	      warning (_("Cannot attach to lwp %d: %s"),
-		       lwpid, reason.c_str ());
+	      error (_("Cannot attach to lwp %d: %s"),
+		     lwpid, reason.c_str ());
 	    }
 	}
       else
@@ -1045,13 +1045,6 @@ attach_proc_task_lwp_callback (ptid_t ptid)
 
 	  /* So that wait collects the SIGSTOP.  */
 	  lp->resumed = 1;
-
-	  /* Also add the LWP to gdb's thread list, in case a
-	     matching libthread_db is not found (or the process uses
-	     raw clone).  */
-	  add_thread (linux_target, lp->ptid);
-	  set_running (linux_target, lp->ptid, true);
-	  set_executing (linux_target, lp->ptid, true);
 	}
 
       return 1;
@@ -1146,8 +1139,42 @@ linux_nat_target::attach (const char *args, int from_tty)
      of threads/LWPs, and those structures may well be corrupted.
      Note that once thread_db is loaded, we'll still use it to list
      threads and associate pthread info with each LWP.  */
-  linux_proc_attach_tgid_threads (lp->ptid.pid (),
-				  attach_proc_task_lwp_callback);
+  try
+    {
+      linux_proc_attach_tgid_threads (lp->ptid.pid (),
+				      attach_proc_task_lwp_callback);
+    }
+  catch (const gdb_exception_error &)
+    {
+      /* Failed to attach to some LWP.  Detach any we've already
+	 attached to.  */
+      iterate_over_lwps (ptid_t (ptid.pid ()),
+			 [] (struct lwp_info *lwp) -> int
+			 {
+			   /* Ignore errors when detaching.  */
+			   ptrace (PTRACE_DETACH, lwp->ptid.lwp (), 0, 0);
+			   delete_lwp (lwp->ptid);
+			   return 0;
+			 });
+
+      target_terminal::ours ();
+      target_mourn_inferior (inferior_ptid);
+
+      throw;
+    }
+
+  /* Add all the LWPs to gdb's thread list.  */
+  iterate_over_lwps (ptid_t (ptid.pid ()),
+		     [] (struct lwp_info *lwp) -> int
+		     {
+		       if (lwp->ptid.pid () != lwp->ptid.lwp ())
+			 {
+			   add_thread (linux_target, lwp->ptid);
+			   set_running (linux_target, lwp->ptid, true);
+			   set_executing (linux_target, lwp->ptid, true);
+			 }
+		       return 0;
+		     });
 }
 
 /* Ptrace-detach the thread with pid PID.  */
diff --git a/gdb/testsuite/gdb.base/traced-thread.c b/gdb/testsuite/gdb.base/traced-thread.c
new file mode 100644
index 00000000000..f30e09b1fc5
--- /dev/null
+++ b/gdb/testsuite/gdb.base/traced-thread.c
@@ -0,0 +1,105 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2023 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#define _GNU_SOURCE
+
+#include <assert.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <sys/ptrace.h>
+#include <pthread.h>
+#include <stdlib.h>
+#include <errno.h>
+
+int fds[2];
+
+_Atomic pid_t bg_tid = 0;
+
+pthread_barrier_t barrier;
+
+#define FIVE_MINUTES (5 * 60)
+
+/* One thread of the child process.  This is traced by the parent
+   process.  */
+void *
+block (void *ignore)
+{
+  bg_tid = gettid ();
+  pthread_barrier_wait (&barrier);
+  sleep (FIVE_MINUTES);
+  return 0;
+}
+
+/* The parent process blocks in this function.  */
+void
+parent_stop (pid_t child_thread_pid)
+{
+  sleep (FIVE_MINUTES);
+}
+
+int
+main ()
+{
+  int result;
+
+  pthread_barrier_init (&barrier, NULL, 2);
+
+  result = pipe (fds);
+  assert (result != -1);
+
+  pid_t child = fork ();
+  if (child != 0)
+    {
+      /* Parent.  */
+      close (fds[1]);
+
+      pid_t the_tid;
+      result = read (fds[0], &the_tid, sizeof (the_tid));
+      assert (result == sizeof (the_tid));
+
+      /* Trace a single, non-main thread of the child.  This should
+	 prevent gdb from attaching to the child at all.  The bug here
+	 was that gdb would get into an infinite loop repeatedly
+	 trying to attach to this thread.  */
+      result = ptrace (PTRACE_SEIZE, the_tid, (void *) 0, (void *) 0);
+      if (result == -1)
+	perror ("ptrace");
+
+      parent_stop (child);
+    }
+  else
+    {
+      /* Child.  */
+
+      close (fds[0]);
+
+      pthread_t thr;
+      result = pthread_create (&thr, 0, block, 0);
+      assert (result == 0);
+
+      /* Wait until the TID has been assigned.  */
+      pthread_barrier_wait (&barrier);
+      assert (bg_tid != 0);
+
+      result = write (fds[1], &bg_tid, sizeof (bg_tid));
+      assert (result == sizeof (bg_tid));
+
+      sleep (FIVE_MINUTES);
+    }
+
+  exit (0);
+}
diff --git a/gdb/testsuite/gdb.base/traced-thread.exp b/gdb/testsuite/gdb.base/traced-thread.exp
new file mode 100644
index 00000000000..d590ab1bc5e
--- /dev/null
+++ b/gdb/testsuite/gdb.base/traced-thread.exp
@@ -0,0 +1,54 @@
+# Copyright 2023 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+# This test only works on GNU/Linux.
+require !use_gdb_stub isnative
+require {!is_remote host}
+require {istarget *-linux*}
+
+standard_testfile
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile \
+	 {debug pthreads}]} {
+    return -1
+}
+
+if {![runto "parent_stop"]} {
+  return -1
+}
+
+# We don't want to be bothered.
+gdb_test_no_output "set confirm off"
+
+set child_pid [get_integer_valueof child_thread_pid -1 "get child pid"]
+
+# We should not be able to attach to the child at all, because one
+# thread is being traced.  The bug here was that gdb would get into an
+# infinite loop trying to attach to this thread.
+gdb_test "add-inferior" "Added inferior 2.*" "add empty inferior 2"
+gdb_test "inferior 2" "Switching to inferior 2.*" "switch to inferior 2"
+# Recognize failures from either gdb or gdbserver.
+gdb_test "attach $child_pid" \
+    "(Cannot attach to|Attaching to process $decimal failed).*" \
+    "should not attach to child process"
+
+
+# Now kill the parent process, ending the trace.
+gdb_test "inferior 1" "Switching to inferior 1.*" "switch to inferior 1"
+gdb_test "kill" ".*" "kill the parent process"
+
+# Kill the child process as well.  Use the shell to avoid funny
+# business with gdbserver testing.
+remote_exec target "kill -9 $child_pid"
diff --git a/gdbserver/linux-low.cc b/gdbserver/linux-low.cc
index 40b6a907ad9..cdaf730af47 100644
--- a/gdbserver/linux-low.cc
+++ b/gdbserver/linux-low.cc
@@ -1128,7 +1128,7 @@ attach_proc_task_lwp_callback (ptid_t ptid)
 	  std::string reason
 	    = linux_ptrace_attach_fail_reason_string (ptid, err);
 
-	  warning (_("Cannot attach to lwp %d: %s"), lwpid, reason.c_str ());
+	  error (_("Cannot attach to lwp %d: %s"), lwpid, reason.c_str ());
 	}
 
       return 1;
@@ -1181,7 +1181,18 @@ linux_process_target::attach (unsigned long pid)
      threads/LWPs, and those structures may well be corrupted.  Note
      that once thread_db is loaded, we'll still use it to list threads
      and associate pthread info with each LWP.  */
-  linux_proc_attach_tgid_threads (pid, attach_proc_task_lwp_callback);
+  try
+    {
+      linux_proc_attach_tgid_threads (pid, attach_proc_task_lwp_callback);
+    }
+  catch (const gdb_exception_error &)
+    {
+      /* Make sure we do not deliver the SIGSTOP to the process.  */
+      initial_thread->last_resume_kind = resume_continue;
+
+      this->detach (proc);
+      throw;
+    }
 
   /* GDB will shortly read the xml target description for this
      process, to figure out the process' architecture.  But the target

-- 
2.40.1



More information about the Gdb-patches mailing list