From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 10729 invoked by alias); 14 Feb 2010 18:56:25 -0000 Received: (qmail 10721 invoked by uid 22791); 14 Feb 2010 18:56:25 -0000 X-SWARE-Spam-Status: No, hits=-6.7 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,SPF_PASS 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; Sun, 14 Feb 2010 18:56:20 +0000 Received: from int-mx05.intmail.prod.int.phx2.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.18]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o1EIu8Lr010469 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Sun, 14 Feb 2010 13:56:08 -0500 Received: from host0.dyn.jankratochvil.net (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx05.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id o1EIu6iC010846 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Sun, 14 Feb 2010 13:56:07 -0500 Received: from host0.dyn.jankratochvil.net (localhost [127.0.0.1]) by host0.dyn.jankratochvil.net (8.14.3/8.14.3) with ESMTP id o1EIu64V030961; Sun, 14 Feb 2010 19:56:06 +0100 Received: (from jkratoch@localhost) by host0.dyn.jankratochvil.net (8.14.3/8.14.3/Submit) id o1EIu5Qf030954; Sun, 14 Feb 2010 19:56:05 +0100 Date: Sun, 14 Feb 2010 18:56:00 -0000 From: Jan Kratochvil To: Pedro Alves Cc: gdb-patches@sourceware.org, Eli Zaretskii Subject: Re: [patch] PID cmdline argument should be whole from digits [+testcase] Message-ID: <20100214185604.GA30756@host0.dyn.jankratochvil.net> References: <20100213152044.GA6115@host0.dyn.jankratochvil.net> <20100213210342.GC17808@host0.dyn.jankratochvil.net> <20100214143716.GA15916@host0.dyn.jankratochvil.net> <201002141454.11337.pedro@codesourcery.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <201002141454.11337.pedro@codesourcery.com> User-Agent: Mutt/1.5.20 (2009-08-17) 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: 2010-02/txt/msg00353.txt.bz2 On Sun, 14 Feb 2010 15:54:10 +0100, Pedro Alves wrote: > Certainly a remote PID has nothing to do with a local PID. OK, I am sorry. > On windows-nat.c, the PID can either be a Windows or Cygwin PID. On non-cygwin (mingw?) HAVE_GETPID should be false and in Cygwin it should be compatible. But I understand relying on HAVE_* being false is not good. (moved getpid checking out of parse_pid* + kept "process-id to attach" message intact) parse_pid_to_attach does not use pid_t return type as while it is defined-as-needed by config.h it would not be compatible with windows-nat.c expected DWORD type. No regressions on {x86_64,x86_64-m32,i686}-fedora12-linux-gnu. Thanks, Jan gdb/ 2010-02-14 Jan Kratochvil * defs.h (parse_pid_to_attach): New. * utils.c (parse_pid_to_attach): New. * darwin-nat.c (darwin_attach): Replace ARGS parsing by parse_pid. * gnu-nat.c (gnu_attach): Likewise. * nto-procfs.c (procfs_attach): Likewise. * procfs.c (procfs_attach): Likewise. * windows-nat.c (windows_attach): Likewise. * inf-ptrace.c (inf_ptrace_attach): Likewise. Remove variable dummy. * inf-ttrace.c (inf_ttrace_attach): Likewise. * remote.c (extended_remote_attach_1): Likewise. New comment on getpid check. gdb/testsuite/ 2010-02-14 Jan Kratochvil * gdb.base/attach.exp (attach to nonsense is prohibited): Make the "Illegal process-id" expect string more exact. (attach to digits-starting nonsense is prohibited): New. --- a/gdb/darwin-nat.c +++ b/gdb/darwin-nat.c @@ -1514,12 +1514,9 @@ darwin_attach (struct target_ops *ops, char *args, int from_tty) struct inferior *inf; kern_return_t kret; - if (!args) - error_no_arg (_("process-id to attach")); + pid = parse_pid_to_attach (args); - pid = atoi (args); - - if (pid == getpid ()) /* Trying to masturbate? */ + if (pid == getpid ()) /* Trying to masturbate? */ error (_("I refuse to debug myself!")); if (from_tty) --- a/gdb/defs.h +++ b/gdb/defs.h @@ -426,6 +426,8 @@ int compare_positive_ints (const void *ap, const void *bp); extern const char *gdb_bfd_errmsg (bfd_error_type error_tag, char **matching); +extern unsigned long parse_pid_to_attach (char *args); + /* From demangle.c */ extern void set_demangling_style (char *); --- a/gdb/gnu-nat.c +++ b/gdb/gnu-nat.c @@ -2139,12 +2139,9 @@ gnu_attach (struct target_ops *ops, char *args, int from_tty) struct inf *inf = cur_inf (); struct inferior *inferior; - if (!args) - error_no_arg (_("process-id to attach")); - - pid = atoi (args); + pid = parse_pid_to_attach (args); - if (pid == getpid ()) /* Trying to masturbate? */ + if (pid == getpid ()) /* Trying to masturbate? */ error (_("I refuse to debug myself!")); if (from_tty) --- a/gdb/inf-ptrace.c +++ b/gdb/inf-ptrace.c @@ -187,17 +187,9 @@ inf_ptrace_attach (struct target_ops *ops, char *args, int from_tty) { char *exec_file; pid_t pid; - char *dummy; struct inferior *inf; - if (!args) - error_no_arg (_("process-id to attach")); - - dummy = args; - pid = strtol (args, &dummy, 0); - /* Some targets don't set errno on errors, grrr! */ - if (pid == 0 && args == dummy) - error (_("Illegal process-id: %s."), args); + pid = parse_pid_to_attach (args); if (pid == getpid ()) /* Trying to masturbate? */ error (_("I refuse to debug myself!")); --- a/gdb/inf-ttrace.c +++ b/gdb/inf-ttrace.c @@ -691,17 +691,10 @@ inf_ttrace_attach (struct target_ops *ops, char *args, int from_tty) { char *exec_file; pid_t pid; - char *dummy; ttevent_t tte; struct inferior *inf; - if (!args) - error_no_arg (_("process-id to attach")); - - dummy = args; - pid = strtol (args, &dummy, 0); - if (pid == 0 && args == dummy) - error (_("Illegal process-id: %s."), args); + pid = parse_pid_to_attach (args); if (pid == getpid ()) /* Trying to masturbate? */ error (_("I refuse to debug myself!")); --- a/gdb/nto-procfs.c +++ b/gdb/nto-procfs.c @@ -619,10 +619,7 @@ procfs_attach (struct target_ops *ops, char *args, int from_tty) int pid; struct inferior *inf; - if (!args) - error_no_arg (_("process-id to attach")); - - pid = atoi (args); + pid = parse_pid_to_attach (args); if (pid == getpid ()) error (_("Attaching GDB to itself is not a good idea...")); --- a/gdb/procfs.c +++ b/gdb/procfs.c @@ -3606,10 +3606,8 @@ procfs_attach (struct target_ops *ops, char *args, int from_tty) char *exec_file; int pid; - if (!args) - error_no_arg (_("process-id to attach")); + pid = parse_pid_to_attach (args); - pid = atoi (args); if (pid == getpid ()) error (_("Attaching GDB to itself is not a good idea...")); --- a/gdb/remote.c +++ b/gdb/remote.c @@ -3857,17 +3857,12 @@ extended_remote_attach_1 (struct target_ops *target, char *args, int from_tty) { struct remote_state *rs = get_remote_state (); int pid; - char *dummy; char *wait_status = NULL; - if (!args) - error_no_arg (_("process-id to attach")); + pid = parse_pid_to_attach (args); - dummy = args; - pid = strtol (args, &dummy, 0); - /* Some targets don't set errno on errors, grrr! */ - if (pid == 0 && args == dummy) - error (_("Illegal process-id: %s."), args); + /* Remote PID can be freely equal to getpid, do not check it here the same + way as in other targets. */ if (remote_protocol_packets[PACKET_vAttach].support == PACKET_DISABLE) error (_("This target does not support attaching to a process")); --- a/gdb/testsuite/gdb.base/attach.exp +++ b/gdb/testsuite/gdb.base/attach.exp @@ -93,7 +93,28 @@ proc do_attach_tests {} { set test "attach to nonsense is prohibited" gdb_test_multiple "attach abc" "$test" { - -re "Illegal process-id: abc.*$gdb_prompt $" { + -re "Illegal process-id: abc\\.\r\n$gdb_prompt $" { + pass "$test" + } + -re "Attaching to.*, process .*couldn't open /proc file.*$gdb_prompt $" { + # Response expected from /proc-based systems. + pass "$test" + } + -re "Can't attach to process..*$gdb_prompt $" { + # Response expected on Cygwin + pass "$test" + } + -re "Attaching to.*$gdb_prompt $" { + fail "$test (bogus pid allowed)" + } + } + + # Verify that we cannot attach to nonsense even if its initial part is + # a valid PID. + + set test "attach to digits-starting nonsense is prohibited" + gdb_test_multiple "attach ${testpid}x" "$test" { + -re "Illegal process-id: ${testpid}x\\.\r\n$gdb_prompt $" { pass "$test" } -re "Attaching to.*, process .*couldn't open /proc file.*$gdb_prompt $" { --- a/gdb/utils.c +++ b/gdb/utils.c @@ -3648,6 +3648,26 @@ gdb_bfd_errmsg (bfd_error_type error_tag, char **matching) return ret; } +/* Return ARGS parsed as numeric pid. */ + +unsigned long +parse_pid_to_attach (char *args) +{ + unsigned long pid; + char *dummy; + + if (!args) + error_no_arg (_("process-id to attach")); + + dummy = args; + pid = strtoul (args, &dummy, 0); + /* Some targets don't set errno on errors, grrr! */ + if ((pid == 0 && dummy == args) || dummy != &args[strlen (args)]) + error (_("Illegal process-id: %s."), args); + + return pid; +} + /* Provide a prototype to silence -Wmissing-prototypes. */ extern initialize_file_ftype _initialize_utils; --- a/gdb/windows-nat.c +++ b/gdb/windows-nat.c @@ -1691,8 +1691,7 @@ windows_attach (struct target_ops *ops, char *args, int from_tty) BOOL ok; DWORD pid; - if (!args) - error_no_arg (_("process-id to attach")); + pid = parse_pid_to_attach (args); if (set_process_privilege (SE_DEBUG_NAME, TRUE) < 0) { @@ -1700,8 +1699,6 @@ windows_attach (struct target_ops *ops, char *args, int from_tty) printf_unfiltered ("This can cause attach to fail on Windows NT/2K/XP\n"); } - pid = strtoul (args, 0, 0); /* Windows pid */ - windows_init_thread_list (); ok = DebugActiveProcess (pid); saw_create = 0;