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 2/3] attach-fail-reasons: Say more than ptrace: Operation not permitted.


On Tue, 13 Mar 2012 11:33:19 +0100, Pedro Alves wrote:
> On 03/13/2012 09:43 AM, Jan Kratochvil wrote:
> > I agree, this is affecting all the /proc operations.  

OK, so not completely all.


> Yeah, although if you're already attached to the process, then the risk of /proc/..
> hitting the wrong pid is much lower, though it does exist.

How?  It remains as 'Z (zombie)' with non-zero 'TracerPid' and even root
cannot reuse that PID without killing the tracer.

The problem is if ptrace fails, then there are no lifetime guarantees.


> There's common/buffer.[h|c] for that.

I did not know.  So GDB uses two different implementations of dynamic strings
(with mem_fileopen).


> >   (2b) If introducing a new framework to gdbserver code we should bring in STL
> >        and not the libiberty/gdb code.  But we cannot yet use STL.
> 
> Although I'm a C++ supporter, I really don't think such C++ arguments hold
> much weight for the current C based code base.

Offtopic here but I think this code was another argument for C++, wasn't it?

No regressions on {x86_64,x86_64-m32,i686}-fedora17-linux-gnu and in
non-extended gdbserver mode.


Thanks,
Jan


gdb/
2012-03-13  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* common/linux-procfs.c (linux_proc_get_int): New, from
	linux_proc_get_tgid, change its LWPID type to pid_t, add parameter
	field.
	(linux_proc_get_tgid): Only call linux_proc_get_int.
	(linux_proc_get_tracerpid): New.
	(linux_proc_pid_has_state): New, from linux_proc_pid_is_zombie.
	(linux_proc_pid_is_stopped, linux_proc_pid_is_zombie): Only call
	linux_proc_pid_has_state.
	* common/linux-procfs.h (linux_proc_get_tracerpid): New declaration.
	* common/linux-ptrace.c: Include linux-procfs.h and buffer.h.
	(linux_ptrace_attach_warnings): New.
	* common/linux-ptrace.h (struct buffer, linux_ptrace_attach_warnings):
	New declaration.
	* linux-nat.c: Include exceptions.h, linux-ptrace.h and buffer.h.
	(linux_nat_attach): New variables ex, buffer, message and message_s.
	Wrap to_attach by TRY_CATCH and call linux_ptrace_attach_warnings.

gdb/gdbserver/
2012-03-13  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* linux-low.c (linux_attach_lwp_1): New variable buffer.  Call
	linux_ptrace_attach_warnings.

gdb/testsuite/
2012-03-06  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.base/attach-twice.c: New files.
	* gdb.base/attach-twice.exp: New files.

--- a/gdb/common/linux-procfs.c
+++ b/gdb/common/linux-procfs.c
@@ -28,67 +28,54 @@
 /* Return the TGID of LWPID from /proc/pid/status.  Returns -1 if not
    found.  */
 
-int
-linux_proc_get_tgid (int lwpid)
+static int
+linux_proc_get_int (pid_t lwpid, const char *field)
 {
+  size_t field_len = strlen (field);
   FILE *status_file;
   char buf[100];
-  int tgid = -1;
+  int retval = -1;
 
   snprintf (buf, sizeof (buf), "/proc/%d/status", (int) lwpid);
   status_file = fopen (buf, "r");
-  if (status_file != NULL)
+  if (status_file == NULL)
     {
-      while (fgets (buf, sizeof (buf), status_file))
-	{
-	  if (strncmp (buf, "Tgid:", 5) == 0)
-	    {
-	      tgid = strtoul (buf + strlen ("Tgid:"), NULL, 10);
-	      break;
-	    }
-	}
-
-      fclose (status_file);
+      warning (_("unable to open /proc file '%s'"), buf);
+      return -1;
     }
 
-  return tgid;
+  while (fgets (buf, sizeof (buf), status_file))
+    if (strncmp (buf, field, field_len) == 0 && buf[field_len] == ':')
+      {
+	retval = strtol (&buf[field_len + 1], NULL, 10);
+	break;
+      }
+
+  fclose (status_file);
+  return retval;
 }
 
-/* Detect `T (stopped)' in `/proc/PID/status'.
-   Other states including `T (tracing stop)' are reported as false.  */
+/* Return the TGID of LWPID from /proc/pid/status.  Returns -1 if not
+   found.  */
 
 int
-linux_proc_pid_is_stopped (pid_t pid)
+linux_proc_get_tgid (pid_t lwpid)
 {
-  FILE *status_file;
-  char buf[100];
-  int retval = 0;
+  return linux_proc_get_int (lwpid, "Tgid");
+}
 
-  snprintf (buf, sizeof (buf), "/proc/%d/status", (int) pid);
-  status_file = fopen (buf, "r");
-  if (status_file != NULL)
-    {
-      int have_state = 0;
-
-      while (fgets (buf, sizeof (buf), status_file))
-	{
-	  if (strncmp (buf, "State:", 6) == 0)
-	    {
-	      have_state = 1;
-	      break;
-	    }
-	}
-      if (have_state && strstr (buf, "T (stopped)") != NULL)
-	retval = 1;
-      fclose (status_file);
-    }
-  return retval;
+/* See linux-procfs.h.  */
+
+pid_t
+linux_proc_get_tracerpid (pid_t lwpid)
+{
+  return linux_proc_get_int (lwpid, "TracerPid");
 }
 
-/* See linux-procfs.h declaration.  */
+/* Return non-zero if 'State' of /proc/PID/status contains STATE.  */
 
-int
-linux_proc_pid_is_zombie (pid_t pid)
+static int
+linux_proc_pid_has_state (pid_t pid, const char *state)
 {
   char buffer[100];
   FILE *procfile;
@@ -110,8 +97,24 @@ linux_proc_pid_is_zombie (pid_t pid)
 	have_state = 1;
 	break;
       }
-  retval = (have_state
-	    && strcmp (buffer, "State:\tZ (zombie)\n") == 0);
+  retval = (have_state && strstr (buffer, state) != NULL);
   fclose (procfile);
   return retval;
 }
+
+/* Detect `T (stopped)' in `/proc/PID/status'.
+   Other states including `T (tracing stop)' are reported as false.  */
+
+int
+linux_proc_pid_is_stopped (pid_t pid)
+{
+  return linux_proc_pid_has_state (pid, "T (stopped)");
+}
+
+/* See linux-procfs.h declaration.  */
+
+int
+linux_proc_pid_is_zombie (pid_t pid)
+{
+  return linux_proc_pid_has_state (pid, "Z (zombie)");
+}
--- a/gdb/common/linux-procfs.h
+++ b/gdb/common/linux-procfs.h
@@ -24,7 +24,12 @@
 /* Return the TGID of LWPID from /proc/pid/status.  Returns -1 if not
    found.  */
 
-extern int linux_proc_get_tgid (int lwpid);
+extern int linux_proc_get_tgid (pid_t lwpid);
+
+/* Return the TracerPid of LWPID from /proc/pid/status.  Returns -1 if not
+   found.  */
+
+extern pid_t linux_proc_get_tracerpid (pid_t lwpid);
 
 /* Detect `T (stopped)' in `/proc/PID/status'.
    Other states including `T (tracing stop)' are reported as false.  */
--- a/gdb/common/linux-ptrace.c
+++ b/gdb/common/linux-ptrace.c
@@ -24,3 +24,26 @@
 #endif
 
 #include "linux-ptrace.h"
+#include "linux-procfs.h"
+#include "buffer.h"
+
+/* Find all possible reasons we could fail to attach PID and append these
+   newline terminated reason strings to initialized BUFFER.  '\0' termination
+   of BUFFER must be done by the caller.  */
+
+void
+linux_ptrace_attach_warnings (pid_t pid, struct buffer *buffer)
+{
+  pid_t tracerpid;
+
+  tracerpid = linux_proc_get_tracerpid (pid);
+  if (tracerpid > 0)
+    buffer_xml_printf (buffer, _("warning: process %d is already traced "
+				 "by process %d\n"),
+		       (int) pid, (int) tracerpid);
+
+  if (linux_proc_pid_is_zombie (pid))
+    buffer_xml_printf (buffer, _("warning: process %d is a zombie "
+				 "- the process has already terminated\n"),
+		       (int) pid);
+}
--- a/gdb/common/linux-ptrace.h
+++ b/gdb/common/linux-ptrace.h
@@ -18,6 +18,8 @@
 #ifndef COMMON_LINUX_PTRACE_H
 #define COMMON_LINUX_PTRACE_H
 
+struct buffer;
+
 #include <sys/ptrace.h>
 
 #ifndef PTRACE_GETSIGINFO
@@ -65,4 +67,6 @@
 #define __WALL          0x40000000 /* Wait for any child.  */
 #endif
 
+extern void linux_ptrace_attach_warnings (pid_t pid, struct buffer *buffer);
+
 #endif /* COMMON_LINUX_PTRACE_H */
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -652,6 +652,8 @@ linux_attach_lwp_1 (unsigned long lwpid, int initial)
 
   if (ptrace (PTRACE_ATTACH, lwpid, 0, 0) != 0)
     {
+      struct buffer buffer;
+
       if (!initial)
 	{
 	  /* If we fail to attach to an LWP, just warn.  */
@@ -662,8 +664,11 @@ linux_attach_lwp_1 (unsigned long lwpid, int initial)
 	}
 
       /* If we fail to attach to a process, report an error.  */
-      error ("Cannot attach to lwp %ld: %s (%d)\n", lwpid,
-	     strerror (errno), errno);
+      buffer_init (&buffer);
+      linux_ptrace_attach_warnings (lwpid, &buffer);
+      buffer_grow_str0 (&buffer, "");
+      error ("%sCannot attach to lwp %ld: %s (%d)", buffer_finish (&buffer),
+	     lwpid, strerror (errno), errno);
     }
 
   if (initial)
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -62,6 +62,9 @@
 #include "symfile.h"
 #include "agent.h"
 #include "tracepoint.h"
+#include "exceptions.h"
+#include "linux-ptrace.h"
+#include "buffer.h"
 
 #ifndef SPUFS_MAGIC
 #define SPUFS_MAGIC 0x23c9b64e
@@ -1612,11 +1615,33 @@ linux_nat_attach (struct target_ops *ops, char *args, int from_tty)
   struct lwp_info *lp;
   int status;
   ptid_t ptid;
+  volatile struct gdb_exception ex;
 
   /* Make sure we report all signals during attach.  */
   linux_nat_pass_signals (0, NULL);
 
-  linux_ops->to_attach (ops, args, from_tty);
+  TRY_CATCH (ex, RETURN_MASK_ERROR)
+    {
+      linux_ops->to_attach (ops, args, from_tty);
+    }
+  if (ex.reason < 0)
+    {
+      pid_t pid = parse_pid_to_attach (args);
+      struct buffer buffer;
+      char *message, *buffer_s;
+
+      message = xstrdup (ex.message);
+      make_cleanup (xfree, message);
+
+      buffer_init (&buffer);
+      linux_ptrace_attach_warnings (pid, &buffer);
+
+      buffer_grow_str0 (&buffer, "");
+      buffer_s = buffer_finish (&buffer);
+      make_cleanup (xfree, buffer_s);
+
+      throw_error (ex.error, "%s%s", buffer_s, message);
+    }
 
   /* The ptrace base target adds the main thread with (pid,0,0)
      format.  Decorate it with lwp info.  */
--- /dev/null
+++ b/gdb/testsuite/gdb.base/attach-twice.c
@@ -0,0 +1,42 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2011-2012 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/>.  */
+
+#include <stdlib.h>
+#include <unistd.h>
+#include <sys/ptrace.h>
+#include <errno.h>
+
+int
+main (void)
+{
+  long l;
+
+  switch (fork ())
+  {
+    case -1:
+      perror ("fork");
+      exit (1);
+    case 0:
+      errno = 0;
+      ptrace (PTRACE_ATTACH, getppid (), NULL, NULL);
+      if (errno != 0)
+	perror ("PTRACE_ATTACH");
+      break;
+  }
+  sleep (600);
+  return 0;
+}
--- /dev/null
+++ b/gdb/testsuite/gdb.base/attach-twice.exp
@@ -0,0 +1,52 @@
+# Copyright (C) 2012 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/>.
+
+# Manipulation with PID on target is not supported.
+if [is_remote target] then {
+    return 0
+}
+
+set testfile attach-twice
+set executable ${testfile}
+set binfile ${objdir}/${subdir}/${executable}
+
+if { [prepare_for_testing ${testfile}.exp $executable] } {
+    return -1
+}
+
+set testpid [eval exec $binfile &]
+exec sleep 2
+
+set parentpid 0
+
+set test "attach"
+gdb_test_multiple "attach $testpid" $test {
+    -re "Attaching to program: \[^\r\n\]*, process $testpid\r\n.*warning: process $testpid is already traced by process (\[0-9\]+)\r\n.*ptrace: Operation not permitted\\.\r\n$gdb_prompt $" {
+	set parentpid $expect_out(1,string)
+	pass $test
+    }
+    -re "Attaching to program: \[^\r\n\]*, process $testpid\r\n.*ptrace: Operation not permitted\\.\r\n$gdb_prompt $" {
+	fail $test
+    }
+    -re "\r\n$gdb_prompt $" {
+	xfail $test
+    }
+}
+
+eval exec ps xfw
+if {$parentpid != 0} {
+  eval exec kill -9 $parentpid
+}
+eval exec kill -9 $testpid


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