This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch] PID cmdline argument should be whole from digits [+testcase]
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 <jan.kratochvil@redhat.com>
* 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 <jan.kratochvil@redhat.com>
* 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;