From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 31347 invoked by alias); 13 Mar 2012 13:55:24 -0000 Received: (qmail 31336 invoked by uid 22791); 13 Mar 2012 13:55:21 -0000 X-SWARE-Spam-Status: No, hits=-6.6 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,TW_TP,TW_XF,T_FRT_PROFILE2,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 13 Mar 2012 13:55:07 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q2DDt6HH022968 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Tue, 13 Mar 2012 09:55:07 -0400 Received: from host2.jankratochvil.net (ovpn-116-16.ams2.redhat.com [10.36.116.16]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q2DDswXB030539 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Tue, 13 Mar 2012 09:55:00 -0400 Date: Tue, 13 Mar 2012 13:55:00 -0000 From: Jan Kratochvil To: Pedro Alves Cc: gdb-patches@sourceware.org Subject: Re: [patch 2/3] attach-fail-reasons: Say more than ptrace: Operation not permitted. Message-ID: <20120313135457.GA8363@host2.jankratochvil.net> References: <20120306061710.GB24004@host2.jankratochvil.net> <4F58BA4A.9090903@redhat.com> <20120313094307.GA25939@host2.jankratochvil.net> <4F5F226F.5090304@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4F5F226F.5090304@redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2012-03/txt/msg00432.txt.bz2 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 * 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 * linux-low.c (linux_attach_lwp_1): New variable buffer. Call linux_ptrace_attach_warnings. gdb/testsuite/ 2012-03-06 Jan Kratochvil * 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 #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 . */ + +#include +#include +#include +#include + +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 . + +# 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