[PATCH 0/1] Fix internal warning when "gdb -p xxx"

[PATCH 0/1] Fix internal warning when "gdb -p xxx"

[Hui Zhu]

ps -e | grep a.out
28886 pts/12   00:00:00 a.out
gdb -p 28886
Loaded symbols for /lib64/ld-linux-x86-64.so.2
0x0000003b0ccbc970 in __nanosleep_nocancel () from /lib64/libc.so.6
../../binutils-gdb/gdb/cleanups.c:265: internal-warning: restore_my_cleanups has found a stale cleanup
A problem internal to GDB has been detected,
further debugging may prove unreliable.
Quit this debugging session? (y or n)

The backtrace of this issue:
(gdb) bt
#0  0x0000003b0cc35c39 in raise () from /lib64/libc.so.6
#1  0x0000003b0cc37348 in abort () from /lib64/libc.so.6
#2  0x00000000006fdd38 in dump_core () at ../../binutils-gdb/gdb/utils.c:589
#3  0x00000000006fe039 in internal_vproblem (problem=0xcbdc90 <internal_warning_problem>,
     file=0x8b0c10 "s' failed.", line=265, fmt=0x8b0c38 "nutils-gdb/gdb/cleanups.c",
     ap=0x7fff803e3ed8) at ../../binutils-gdb/gdb/utils.c:748
#4  0x00000000006fe19c in internal_vwarning (file=0x8b0c10 "s' failed.", line=265,
     fmt=0x8b0c38 "nutils-gdb/gdb/cleanups.c", ap=0x7fff803e3ed8)
     at ../../binutils-gdb/gdb/utils.c:799
#5  0x00000000006fe246 in internal_warning (file=0x8b0c10 "s' failed.", line=265,
     string=0x8b0c38 "nutils-gdb/gdb/cleanups.c") at ../../binutils-gdb/gdb/utils.c:809
#6  0x000000000057da3d in restore_my_cleanups (pmy_chain=0xcba518 <cleanup_chain>, chain=0x14ec030)
     at ../../binutils-gdb/gdb/cleanups.c:265
#7  0x000000000057da67 in restore_cleanups (chain=0x14ec030)
     at ../../binutils-gdb/gdb/cleanups.c:276
#8  0x00000000005f0dc7 in catcher_pop () at ../../binutils-gdb/gdb/exceptions.c:116
#9  0x00000000005f0e62 in exceptions_state_mc (action=CATCH_ITER)
     at ../../binutils-gdb/gdb/exceptions.c:142
#10 0x00000000005f0fe6 in exceptions_state_mc (action=CATCH_ITER)
     at ../../binutils-gdb/gdb/exceptions.c:203
#11 0x00000000005f18ed in catch_command_errors (
     command=0x5d5fb8 <attach_command_continuation_free_args+18>, arg=0x7fff803e525b "2914",
     from_tty=1, mask=RETURN_MASK_ALL) at ../../binutils-gdb/gdb/exceptions.c:549
---Type <return> to continue, or q <return> to quit---
#12 0x00000000005f5ed7 in captured_main (data=0x7fff803e4280) at ../../binutils-gdb/gdb/main.c:968
#13 0x00000000005f180b in catch_errors (func=0x5f501c <VEC_cmdarg_s_safe_push+43>,
     func_args=0x7fff803e4280, errstring=0x8cf2e4 "/local/bin", mask=RETURN_MASK_ALL)
     at ../../binutils-gdb/gdb/exceptions.c:522
#14 0x00000000005f6180 in gdb_main (args=0x7fff803e4280) at ../../binutils-gdb/gdb/main.c:1061
#15 0x000000000045d087 in main (argc=3, argv=0x7fff803e4388) at ../../binutils-gdb/gdb/gdb.c:33

This is a new issue.  It is introduced by commit https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commit;h=8bc2fe488957946d2cdccda3ce8d4f39e4003ea0
It removed the discard_cleanups (back_to) inside attach_command.
Then restore_my_cleanups will throw a internal_warning.

The most of the pid_to_exec_file target_ops method for some platforms will
allocate memory for exec_file and add them to cleanup.
So I add a null cleanup to attach_command_post_wait to fix this issue.

It is tested and and passed regression test in x86_64-linux.

Thanks,
Hui

2014-03-17  Hui Zhu  <hui@codesourcery.com>

	* infcmd.c (attach_command_post_wait): Add null_cleanup.

--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -2337,6 +2337,7 @@ attach_command_post_wait (char *args, in
    char *exec_file;
    char *full_exec_path = NULL;
    struct inferior *inferior;
+  struct cleanup *back_to = make_cleanup (null_cleanup, NULL);
  
    inferior = current_inferior ();
    inferior->control.stop_soon = NO_STOP_QUIETLY;
@@ -2421,6 +2422,12 @@ attach_command_post_wait (char *args, in
        if (deprecated_attach_hook)
  	deprecated_attach_hook ();
      }
+
+  /* The pid_to_exec_file target_ops method for some platforms will
+     allocate memory for exec_file and add them to cleanup.
+     Release them in here because function attach_command will be call
+     by function captured_main too.  */
+  do_cleanups (back_to);
  }
  
  struct attach_command_continuation_args

[PATCH 1/1] Fix memleak of the pid_to_exec_file target_ops method for some platforms

[Hui Zhu]

I said most of the pid_to_exec_file target_ops method for some platforms will
allocate memory for exec_file and add them to cleanup.
But some of them didn't do that.
So I make a patch to fix this memleak.

Thanks,
Hui

2014-03-17  Hui Zhu  <hui@codesourcery.com>

	* fbsd-nat.c (fbsd_pid_to_exec_file): Add make_cleanup.
	* nbsd-nat.c (nbsd_pid_to_exec_file): Ditto.

--- a/gdb/fbsd-nat.c
+++ b/gdb/fbsd-nat.c
@@ -43,6 +43,8 @@ fbsd_pid_to_exec_file (struct target_ops
    char *buf = xcalloc (len, sizeof (char));
    char *path;
  
+  make_cleanup (xfree, buf);
+
  #ifdef KERN_PROC_PATHNAME
    int mib[4];
  
--- a/gdb/nbsd-nat.c
+++ b/gdb/nbsd-nat.c
@@ -31,6 +31,8 @@ nbsd_pid_to_exec_file (struct target_ops
    char *buf = xcalloc (len, sizeof (char));
    char *path;
  
+  make_cleanup (xfree, buf);
+
    path = xstrprintf ("/proc/%d/exe", pid);
    if (readlink (path, buf, PATH_MAX - 1) == -1)
      {

Re: [PATCH 1/1] Fix memleak of the pid_to_exec_file target_ops method for some platforms

[Hui Zhu]

I just posted a new version for [patch 0/1] that include this patch.
So please ignore it.

Thanks,
Hui

On 03/18/14 00:02, Hui Zhu wrote:
> I said most of the pid_to_exec_file target_ops method for some platforms will
> allocate memory for exec_file and add them to cleanup.
> But some of them didn't do that.
> So I make a patch to fix this memleak.
>
> Thanks,
> Hui
>
> 2014-03-17  Hui Zhu  <hui@codesourcery.com>
>
>      * fbsd-nat.c (fbsd_pid_to_exec_file): Add make_cleanup.
>      * nbsd-nat.c (nbsd_pid_to_exec_file): Ditto.
>
> --- a/gdb/fbsd-nat.c
> +++ b/gdb/fbsd-nat.c
> @@ -43,6 +43,8 @@ fbsd_pid_to_exec_file (struct target_ops
>     char *buf = xcalloc (len, sizeof (char));
>     char *path;
>
> +  make_cleanup (xfree, buf);
> +
>   #ifdef KERN_PROC_PATHNAME
>     int mib[4];
>
> --- a/gdb/nbsd-nat.c
> +++ b/gdb/nbsd-nat.c
> @@ -31,6 +31,8 @@ nbsd_pid_to_exec_file (struct target_ops
>     char *buf = xcalloc (len, sizeof (char));
>     char *path;
>
> +  make_cleanup (xfree, buf);
> +
>     path = xstrprintf ("/proc/%d/exe", pid);
>     if (readlink (path, buf, PATH_MAX - 1) == -1)
>       {
>

Re: [PATCH 0/1] Fix internal warning when "gdb -p xxx"

[Pedro Alves]

On 03/17/2014 04:00 PM, Hui Zhu wrote:

> The most of the pid_to_exec_file target_ops method for some platforms will
> allocate memory for exec_file and add them to cleanup.

Which platforms?

-- 
Pedro Alves

Re: [PATCH 0/1] Fix internal warning when "gdb -p xxx"

[Pedro Alves]

On 03/17/2014 04:12 PM, Pedro Alves wrote:
> On 03/17/2014 04:00 PM, Hui Zhu wrote:
> 
>> The most of the pid_to_exec_file target_ops method for some platforms will
>> allocate memory for exec_file and add them to cleanup.
> 
> Which platforms?

OK, I see several do that, including linux-nat.c.

IMO, that ends up being a silly interface.  The current
interface is documented as:

/* Attempts to find the pathname of the executable file
   that was run to create a specified process.

   The process PID must be stopped when this operation is used.

   If the executable file cannot be determined, NULL is returned.

   Else, a pointer to a character string containing the pathname
   is returned.  This string should be copied into a buffer by
   the client if the string will not be immediately used, or if
   it must persist.  */

#define target_pid_to_exec_file(pid) \
     (current_target.to_pid_to_exec_file) (&current_target, pid)

The "This string should be copied into a buffer by the client if
the string will not be immediately used, or if it must persist."
part hints that the implementation is supposed to return a pointer
to a static buffer, like e.g., target_pid_to_str, paddress, and
friends, etc.

Either we make target_pid_to_exec_file return a pointer to
a malloc buffer that the caller is responsible for xfree'ing (and
adjust the interface comments in target.h) or we make targets
indeed return a pointer to a static buffer, as the current
method's description hints at.  Returning a malloced buffer, and
installing a cleanup like that is a silly interface, IMO.  Note
that GDB used to have more random memory-release cleanups installed
like this, but we've removed most, I believe.  Although it's really
not harmful to install a cleanup that just releases memory
later at any random time, OTOH, it potentially makes debugging
nasty cleanup issues harder, so we've moved away from doing that,
and we now have that warning.


Bummer that we don't have a test that caught this.  :-(

-- 
Pedro Alves

Re: [PATCH 0/1] Fix internal warning when "gdb -p xxx"

[Hui Zhu]

On 03/18/14 00:56, Pedro Alves wrote:
> On 03/17/2014 04:12 PM, Pedro Alves wrote:
>> On 03/17/2014 04:00 PM, Hui Zhu wrote:
>>
>>> The most of the pid_to_exec_file target_ops method for some platforms will
>>> allocate memory for exec_file and add them to cleanup.
>>
>> Which platforms?
>
> OK, I see several do that, including linux-nat.c.

I am sorry that I should not use "most".
Just darwin and linux add buf to cleanup.

fbsd and nbsd allocate memory but don't add them to cleanup.

windows use the static buf.

>
> IMO, that ends up being a silly interface.  The current
> interface is documented as:
>
> /* Attempts to find the pathname of the executable file
>     that was run to create a specified process.
>
>     The process PID must be stopped when this operation is used.
>
>     If the executable file cannot be determined, NULL is returned.
>
>     Else, a pointer to a character string containing the pathname
>     is returned.  This string should be copied into a buffer by
>     the client if the string will not be immediately used, or if
>     it must persist.  */
>
> #define target_pid_to_exec_file(pid) \
>       (current_target.to_pid_to_exec_file) (&current_target, pid)
>
> The "This string should be copied into a buffer by the client if
> the string will not be immediately used, or if it must persist."
> part hints that the implementation is supposed to return a pointer
> to a static buffer, like e.g., target_pid_to_str, paddress, and
> friends, etc.
>
> Either we make target_pid_to_exec_file return a pointer to
> a malloc buffer that the caller is responsible for xfree'ing (and
> adjust the interface comments in target.h) or we make targets
> indeed return a pointer to a static buffer, as the current
> method's description hints at.  Returning a malloced buffer, and
> installing a cleanup like that is a silly interface, IMO.  Note
> that GDB used to have more random memory-release cleanups installed
> like this, but we've removed most, I believe.  Although it's really
> not harmful to install a cleanup that just releases memory
> later at any random time, OTOH, it potentially makes debugging
> nasty cleanup issues harder, so we've moved away from doing that,
> and we now have that warning.

According to your comments,  I make a new patch that change all
methods return a static buffer.

Please help me review it.

>
>
> Bummer that we don't have a test that caught this.  :-(
>

Yes, I found it when I did regression test.
Does testsuite have some example to test a GDB feature like "gdb -p"?

Thanks,
Hui

2014-03-18  Hui Zhu  <hui@codesourcery.com>

	* darwin-nat.c (darwin_pid_to_exec_file): Change xmalloc to
	static buffer.
	* fbsd-nat.c (fbsd_pid_to_exec_file): Ditto.
	* linux-nat.c (linux_child_pid_to_exec_file): Ditto.
	* nbsd-nat.c (nbsd_pid_to_exec_file): Ditto.

--- a/gdb/darwin-nat.c
+++ b/gdb/darwin-nat.c
@@ -1991,12 +1991,9 @@ set_enable_mach_exceptions (char *args,
  static char *
  darwin_pid_to_exec_file (struct target_ops *self, int pid)
  {
-  char *path;
+  static char path[PATH_MAX];
    int res;
  
-  path = xmalloc (PATH_MAX);
-  make_cleanup (xfree, path);
-
    res = proc_pidinfo (pid, PROC_PIDPATHINFO, 0, path, PATH_MAX);
    if (res >= 0)
      return path;
--- a/gdb/fbsd-nat.c
+++ b/gdb/fbsd-nat.c
@@ -40,8 +40,8 @@ char *
  fbsd_pid_to_exec_file (struct target_ops *self, int pid)
  {
    size_t len = PATH_MAX;
-  char *buf = xcalloc (len, sizeof (char));
-  char *path;
+  static char buf[PATH_MAX];
+  char *path, *ret;
  
  #ifdef KERN_PROC_PATHNAME
    int mib[4];
@@ -56,13 +56,12 @@ fbsd_pid_to_exec_file (struct target_ops
  
    path = xstrprintf ("/proc/%d/file", pid);
    if (readlink (path, buf, PATH_MAX - 1) == -1)
-    {
-      xfree (buf);
-      buf = NULL;
-    }
+    ret = NULL;
+  else
+    ret = buf;
  
    xfree (path);
-  return buf;
+  return ret;
  }
  
  static int
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -4011,19 +4011,18 @@ linux_nat_thread_name (struct target_ops
  static char *
  linux_child_pid_to_exec_file (struct target_ops *self, int pid)
  {
-  char *name1, *name2;
+  static char buf[PATH_MAX];
+  char name1[PATH_MAX], name2[PATH_MAX];
  
-  name1 = xmalloc (PATH_MAX);
-  name2 = xmalloc (PATH_MAX);
-  make_cleanup (xfree, name1);
-  make_cleanup (xfree, name2);
    memset (name2, 0, PATH_MAX);
  
    xsnprintf (name1, PATH_MAX, "/proc/%d/exe", pid);
    if (readlink (name1, name2, PATH_MAX - 1) > 0)
-    return name2;
+    strcpy (buf, name2);
    else
-    return name1;
+    strcpy (buf, name1);
+
+  return buf;
  }
  
  /* Records the thread's register state for the corefile note
--- a/gdb/nbsd-nat.c
+++ b/gdb/nbsd-nat.c
@@ -28,16 +28,15 @@ char *
  nbsd_pid_to_exec_file (struct target_ops *self, int pid)
  {
    size_t len = PATH_MAX;
-  char *buf = xcalloc (len, sizeof (char));
-  char *path;
+  static char buf[PATH_MAX];
+  char *path, *ret;
  
    path = xstrprintf ("/proc/%d/exe", pid);
    if (readlink (path, buf, PATH_MAX - 1) == -1)
-    {
-      xfree (buf);
-      buf = NULL;
-    }
+    ret = NULL;
+  else
+    ret = buf;
  
    xfree (path);
-  return buf;
+  return ret;
  }

Re: [PATCH 0/1] Fix internal warning when "gdb -p xxx"

[Pedro Alves]

On 03/18/2014 07:37 AM, Hui Zhu wrote:
> On 03/18/14 00:56, Pedro Alves wrote:

> According to your comments,  I make a new patch that change all
> methods return a static buffer.

Thank you.

>> Bummer that we don't have a test that caught this.  :-(
>>
> 
> Yes, I found it when I did regression test.

WDYM?  What test failed then?

> Does testsuite have some example to test a GDB feature like "gdb -p"?

I guess we'd refactor attach.exp a little to test that in addition
to "attach", on native targets.  And I supposed we could pass down the
-p to gdb by tweaking GDBFLAGS, like several tests do.  gdb.base/corefile.exp
sounds like the model to follow, as that spawns "gdb -core", which is very
similar to "gdb -p".

> 2014-03-18  Hui Zhu  <hui@codesourcery.com>
> 
> 	* darwin-nat.c (darwin_pid_to_exec_file): Change xmalloc to
> 	static buffer.
> 	* fbsd-nat.c (fbsd_pid_to_exec_file): Ditto.
> 	* linux-nat.c (linux_child_pid_to_exec_file): Ditto.
> 	* nbsd-nat.c (nbsd_pid_to_exec_file): Ditto.
> 
> --- a/gdb/darwin-nat.c
> +++ b/gdb/darwin-nat.c
> @@ -1991,12 +1991,9 @@ set_enable_mach_exceptions (char *args,
>   static char *
>   darwin_pid_to_exec_file (struct target_ops *self, int pid)
>   {
> -  char *path;
> +  static char path[PATH_MAX];
>     int res;
>   
> -  path = xmalloc (PATH_MAX);
> -  make_cleanup (xfree, path);
> -
>     res = proc_pidinfo (pid, PROC_PIDPATHINFO, 0, path, PATH_MAX);
>     if (res >= 0)
>       return path;
> --- a/gdb/fbsd-nat.c
> +++ b/gdb/fbsd-nat.c
> @@ -40,8 +40,8 @@ char *
>   fbsd_pid_to_exec_file (struct target_ops *self, int pid)
>   {
>     size_t len = PATH_MAX;
> -  char *buf = xcalloc (len, sizeof (char));
> -  char *path;
> +  static char buf[PATH_MAX];
> +  char *path, *ret;
>   
>   #ifdef KERN_PROC_PATHNAME
>     int mib[4];
> @@ -56,13 +56,12 @@ fbsd_pid_to_exec_file (struct target_ops
>   
>     path = xstrprintf ("/proc/%d/file", pid);
>     if (readlink (path, buf, PATH_MAX - 1) == -1)

readlink does not '\0' terminate, we need to do that ourselves.
See below.

> -    {
> -      xfree (buf);
> -      buf = NULL;
> -    }
> +    ret = NULL;
> +  else
> +    ret = buf;
>   
>     xfree (path);
> -  return buf;
> +  return ret;
>   }
>   
>   static int
> --- a/gdb/linux-nat.c
> +++ b/gdb/linux-nat.c
> @@ -4011,19 +4011,18 @@ linux_nat_thread_name (struct target_ops
>   static char *
>   linux_child_pid_to_exec_file (struct target_ops *self, int pid)
>   {
> -  char *name1, *name2;
> +  static char buf[PATH_MAX];
> +  char name1[PATH_MAX], name2[PATH_MAX];
>   
> -  name1 = xmalloc (PATH_MAX);
> -  name2 = xmalloc (PATH_MAX);
> -  make_cleanup (xfree, name1);
> -  make_cleanup (xfree, name2);
>     memset (name2, 0, PATH_MAX);
>   
>     xsnprintf (name1, PATH_MAX, "/proc/%d/exe", pid);
>     if (readlink (name1, name2, PATH_MAX - 1) > 0)
> -    return name2;
> +    strcpy (buf, name2);
>     else
> -    return name1;
> +    strcpy (buf, name1);
> +
> +  return buf;

No need for three buffers.  AFAICS, this should suffice:

  static char buf[PATH_MAX];
  char name[PATH_MAX];

  xsnprintf (name, PATH_MAX, "/proc/%d/exe", pid);
  memset (buf, 0, PATH_MAX);
  if (readlink (name, buf, PATH_MAX - 1) <= 0)
    strcpy (buf, name);
  return buf;

>   }
>   
>   /* Records the thread's register state for the corefile note
> --- a/gdb/nbsd-nat.c
> +++ b/gdb/nbsd-nat.c
> @@ -28,16 +28,15 @@ char *
>   nbsd_pid_to_exec_file (struct target_ops *self, int pid)
>   {
>     size_t len = PATH_MAX;
> -  char *buf = xcalloc (len, sizeof (char));
> -  char *path;
> +  static char buf[PATH_MAX];
> +  char *path, *ret;
>   
>     path = xstrprintf ("/proc/%d/exe", pid);
>     if (readlink (path, buf, PATH_MAX - 1) == -1)
> -    {
> -      xfree (buf);
> -      buf = NULL;
> -    }
> +    ret = NULL;
> +  else
> +    ret = buf;
>   
>     xfree (path);
> -  return buf;
> +  return ret;
>   }

Same with the nul termination.  The most standard solution is:

  static char buf[PATH_MAX];
  char name[PATH_MAX];
  ssize_t len;

  xsnprintf (name, PATH_MAX, "/proc/%d/exe", pid);
  len = readlink (name, buf, PATH_MAX - 1);
  if (len != -1)
    {
      buf[len] = '\0';
      return buf;
    }
  return NULL;

-- 
Pedro Alves

Re: [PATCH 0/1] Fix internal warning when "gdb -p xxx"

[Hui Zhu]

On 03/18/14 18:14, Pedro Alves wrote:
> On 03/18/2014 07:37 AM, Hui Zhu wrote:
>> On 03/18/14 00:56, Pedro Alves wrote:
>
>> According to your comments,  I make a new patch that change all
>> methods return a static buffer.
>
> Thank you.
>
>>> Bummer that we don't have a test that caught this.  :-(
>>>
>>
>> Yes, I found it when I did regression test.
>
> WDYM?  What test failed then?

Sorry, my mean is I found the issue cannot be found by regression test.

>
>> Does testsuite have some example to test a GDB feature like "gdb -p"?
>
> I guess we'd refactor attach.exp a little to test that in addition
> to "attach", on native targets.  And I supposed we could pass down the
> -p to gdb by tweaking GDBFLAGS, like several tests do.  gdb.base/corefile.exp
> sounds like the model to follow, as that spawns "gdb -core", which is very
> similar to "gdb -p".

I make a patch for that.  I will post it in a separate mail.

>
>> 2014-03-18  Hui Zhu  <hui@codesourcery.com>
>>
>> 	* darwin-nat.c (darwin_pid_to_exec_file): Change xmalloc to
>> 	static buffer.
>> 	* fbsd-nat.c (fbsd_pid_to_exec_file): Ditto.
>> 	* linux-nat.c (linux_child_pid_to_exec_file): Ditto.
>> 	* nbsd-nat.c (nbsd_pid_to_exec_file): Ditto.
>>
>> --- a/gdb/darwin-nat.c
>> +++ b/gdb/darwin-nat.c
>> @@ -1991,12 +1991,9 @@ set_enable_mach_exceptions (char *args,
>>    static char *
>>    darwin_pid_to_exec_file (struct target_ops *self, int pid)
>>    {
>> -  char *path;
>> +  static char path[PATH_MAX];
>>      int res;
>>
>> -  path = xmalloc (PATH_MAX);
>> -  make_cleanup (xfree, path);
>> -
>>      res = proc_pidinfo (pid, PROC_PIDPATHINFO, 0, path, PATH_MAX);
>>      if (res >= 0)
>>        return path;
>> --- a/gdb/fbsd-nat.c
>> +++ b/gdb/fbsd-nat.c
>> @@ -40,8 +40,8 @@ char *
>>    fbsd_pid_to_exec_file (struct target_ops *self, int pid)
>>    {
>>      size_t len = PATH_MAX;
>> -  char *buf = xcalloc (len, sizeof (char));
>> -  char *path;
>> +  static char buf[PATH_MAX];
>> +  char *path, *ret;
>>
>>    #ifdef KERN_PROC_PATHNAME
>>      int mib[4];
>> @@ -56,13 +56,12 @@ fbsd_pid_to_exec_file (struct target_ops
>>
>>      path = xstrprintf ("/proc/%d/file", pid);
>>      if (readlink (path, buf, PATH_MAX - 1) == -1)
>
> readlink does not '\0' terminate, we need to do that ourselves.
> See below.
>
>> -    {
>> -      xfree (buf);
>> -      buf = NULL;
>> -    }
>> +    ret = NULL;
>> +  else
>> +    ret = buf;
>>
>>      xfree (path);
>> -  return buf;
>> +  return ret;
>>    }
>>
>>    static int
>> --- a/gdb/linux-nat.c
>> +++ b/gdb/linux-nat.c
>> @@ -4011,19 +4011,18 @@ linux_nat_thread_name (struct target_ops
>>    static char *
>>    linux_child_pid_to_exec_file (struct target_ops *self, int pid)
>>    {
>> -  char *name1, *name2;
>> +  static char buf[PATH_MAX];
>> +  char name1[PATH_MAX], name2[PATH_MAX];
>>
>> -  name1 = xmalloc (PATH_MAX);
>> -  name2 = xmalloc (PATH_MAX);
>> -  make_cleanup (xfree, name1);
>> -  make_cleanup (xfree, name2);
>>      memset (name2, 0, PATH_MAX);
>>
>>      xsnprintf (name1, PATH_MAX, "/proc/%d/exe", pid);
>>      if (readlink (name1, name2, PATH_MAX - 1) > 0)
>> -    return name2;
>> +    strcpy (buf, name2);
>>      else
>> -    return name1;
>> +    strcpy (buf, name1);
>> +
>> +  return buf;
>
> No need for three buffers.  AFAICS, this should suffice:
>
>    static char buf[PATH_MAX];
>    char name[PATH_MAX];
>
>    xsnprintf (name, PATH_MAX, "/proc/%d/exe", pid);
>    memset (buf, 0, PATH_MAX);
>    if (readlink (name, buf, PATH_MAX - 1) <= 0)
>      strcpy (buf, name);
>    return buf;
>
>>    }
>>
>>    /* Records the thread's register state for the corefile note
>> --- a/gdb/nbsd-nat.c
>> +++ b/gdb/nbsd-nat.c
>> @@ -28,16 +28,15 @@ char *
>>    nbsd_pid_to_exec_file (struct target_ops *self, int pid)
>>    {
>>      size_t len = PATH_MAX;
>> -  char *buf = xcalloc (len, sizeof (char));
>> -  char *path;
>> +  static char buf[PATH_MAX];
>> +  char *path, *ret;
>>
>>      path = xstrprintf ("/proc/%d/exe", pid);
>>      if (readlink (path, buf, PATH_MAX - 1) == -1)
>> -    {
>> -      xfree (buf);
>> -      buf = NULL;
>> -    }
>> +    ret = NULL;
>> +  else
>> +    ret = buf;
>>
>>      xfree (path);
>> -  return buf;
>> +  return ret;
>>    }
>
> Same with the nul termination.  The most standard solution is:
>
>    static char buf[PATH_MAX];
>    char name[PATH_MAX];
>    ssize_t len;
>
>    xsnprintf (name, PATH_MAX, "/proc/%d/exe", pid);
>    len = readlink (name, buf, PATH_MAX - 1);
>    if (len != -1)
>      {
>        buf[len] = '\0';
>        return buf;
>      }
>    return NULL;
>

I make a new patch according to your comments.
Please help me review it.

Thanks,
Hui

2014-03-19  Hui Zhu  <hui@codesourcery.com>
	    Pedro Alves  <palves@redhat.com>

	* darwin-nat.c (darwin_pid_to_exec_file): Change xmalloc to
	static buffer.
	* fbsd-nat.c (fbsd_pid_to_exec_file): Ditto.
	* linux-nat.c (linux_child_pid_to_exec_file): Ditto.
	* nbsd-nat.c (nbsd_pid_to_exec_file): Ditto.

--- a/gdb/darwin-nat.c
+++ b/gdb/darwin-nat.c
@@ -1991,12 +1991,9 @@ set_enable_mach_exceptions (char *args,
  static char *
  darwin_pid_to_exec_file (struct target_ops *self, int pid)
  {
-  char *path;
+  static char path[PATH_MAX];
    int res;
  
-  path = xmalloc (PATH_MAX);
-  make_cleanup (xfree, path);
-
    res = proc_pidinfo (pid, PROC_PIDPATHINFO, 0, path, PATH_MAX);
    if (res >= 0)
      return path;
--- a/gdb/fbsd-nat.c
+++ b/gdb/fbsd-nat.c
@@ -40,8 +40,8 @@ char *
  fbsd_pid_to_exec_file (struct target_ops *self, int pid)
  {
    size_t len = PATH_MAX;
-  char *buf = xcalloc (len, sizeof (char));
-  char *path;
+  static char buf[PATH_MAX];
+  char name[PATH_MAX];
  
  #ifdef KERN_PROC_PATHNAME
    int mib[4];
@@ -54,14 +54,11 @@ fbsd_pid_to_exec_file (struct target_ops
      return buf;
  #endif
  
-  path = xstrprintf ("/proc/%d/file", pid);
-  if (readlink (path, buf, PATH_MAX - 1) == -1)
-    {
-      xfree (buf);
-      buf = NULL;
-    }
+  xsnprintf (name, PATH_MAX, "/proc/%d/exe", pid);
+  memset (buf, 0, PATH_MAX);
+  if (readlink (name, buf, PATH_MAX - 1) <= 0)
+    strcpy (buf, name);
  
-  xfree (path);
    return buf;
  }
  
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -4011,19 +4011,15 @@ linux_nat_thread_name (struct target_ops
  static char *
  linux_child_pid_to_exec_file (struct target_ops *self, int pid)
  {
-  char *name1, *name2;
+  static char buf[PATH_MAX];
+  char name[PATH_MAX];
  
-  name1 = xmalloc (PATH_MAX);
-  name2 = xmalloc (PATH_MAX);
-  make_cleanup (xfree, name1);
-  make_cleanup (xfree, name2);
-  memset (name2, 0, PATH_MAX);
+  xsnprintf (name, PATH_MAX, "/proc/%d/exe", pid);
+  memset (buf, 0, PATH_MAX);
+  if (readlink (name, buf, PATH_MAX - 1) <= 0)
+    strcpy (buf, name);
  
-  xsnprintf (name1, PATH_MAX, "/proc/%d/exe", pid);
-  if (readlink (name1, name2, PATH_MAX - 1) > 0)
-    return name2;
-  else
-    return name1;
+  return buf;
  }
  
  /* Records the thread's register state for the corefile note
--- a/gdb/nbsd-nat.c
+++ b/gdb/nbsd-nat.c
@@ -27,17 +27,13 @@
  char *
  nbsd_pid_to_exec_file (struct target_ops *self, int pid)
  {
-  size_t len = PATH_MAX;
-  char *buf = xcalloc (len, sizeof (char));
-  char *path;
+  static char buf[PATH_MAX];
+  char name[PATH_MAX];
  
-  path = xstrprintf ("/proc/%d/exe", pid);
-  if (readlink (path, buf, PATH_MAX - 1) == -1)
-    {
-      xfree (buf);
-      buf = NULL;
-    }
+  xsnprintf (name, PATH_MAX, "/proc/%d/exe", pid);
+  memset (buf, 0, PATH_MAX);
+  if (readlink (name, buf, PATH_MAX - 1) <= 0)
+    strcpy (buf, name);
  
-  xfree (path);
    return buf;
  }

Re: [PATCH 0/1] Fix internal warning when "gdb -p xxx"

[Pedro Alves]

On 03/19/2014 03:57 AM, Hui Zhu wrote:
>> >
>> > Same with the nul termination.  The most standard solution is:
>> >
>> >    static char buf[PATH_MAX];
>> >    char name[PATH_MAX];
>> >    ssize_t len;
>> >
>> >    xsnprintf (name, PATH_MAX, "/proc/%d/exe", pid);
>> >    len = readlink (name, buf, PATH_MAX - 1);
>> >    if (len != -1)
>> >      {
>> >        buf[len] = '\0';
>> >        return buf;
>> >      }
>> >    return NULL;
>> >
> I make a new patch according to your comments.
> Please help me review it.

The patch changes the bsd implementations's behavior, because
you made them return the /proc path when readlink fails (like
the Linux version does), instead of what the current code does
or what I suggested above.

-- 
Pedro Alves

Re: [PATCH 0/1] Fix internal warning when "gdb -p xxx"

[Hui Zhu]

On 03/19/14 18:16, Pedro Alves wrote:
> On 03/19/2014 03:57 AM, Hui Zhu wrote:
>>>>
>>>> Same with the nul termination.  The most standard solution is:
>>>>
>>>>     static char buf[PATH_MAX];
>>>>     char name[PATH_MAX];
>>>>     ssize_t len;
>>>>
>>>>     xsnprintf (name, PATH_MAX, "/proc/%d/exe", pid);
>>>>     len = readlink (name, buf, PATH_MAX - 1);
>>>>     if (len != -1)
>>>>       {
>>>>         buf[len] = '\0';
>>>>         return buf;
>>>>       }
>>>>     return NULL;
>>>>
>> I make a new patch according to your comments.
>> Please help me review it.
>
> The patch changes the bsd implementations's behavior, because
> you made them return the /proc path when readlink fails (like
> the Linux version does), instead of what the current code does
> or what I suggested above.
>

I made a new version that change fbsd_pid_to_exec_file and nbsd_pid_to_exec_file
to your suggested above.

Please help me review it.

Thanks,
Hui

2014-03-20  Hui Zhu  <hui@codesourcery.com>
	    Pedro Alves  <palves@redhat.com>

	* darwin-nat.c (darwin_pid_to_exec_file): Change xmalloc to
	static buffer.
	* fbsd-nat.c (fbsd_pid_to_exec_file): Ditto.
	* linux-nat.c (linux_child_pid_to_exec_file): Ditto.
	* nbsd-nat.c (nbsd_pid_to_exec_file): Ditto.

--- a/gdb/darwin-nat.c
+++ b/gdb/darwin-nat.c
@@ -1991,12 +1991,9 @@ set_enable_mach_exceptions (char *args,
  static char *
  darwin_pid_to_exec_file (struct target_ops *self, int pid)
  {
-  char *path;
+  static char path[PATH_MAX];
    int res;
  
-  path = xmalloc (PATH_MAX);
-  make_cleanup (xfree, path);
-
    res = proc_pidinfo (pid, PROC_PIDPATHINFO, 0, path, PATH_MAX);
    if (res >= 0)
      return path;
--- a/gdb/fbsd-nat.c
+++ b/gdb/fbsd-nat.c
@@ -40,8 +40,8 @@ char *
  fbsd_pid_to_exec_file (struct target_ops *self, int pid)
  {
    size_t len = PATH_MAX;
-  char *buf = xcalloc (len, sizeof (char));
-  char *path;
+  static char buf[PATH_MAX];
+  char name[PATH_MAX];
  
  #ifdef KERN_PROC_PATHNAME
    int mib[4];
@@ -54,15 +54,15 @@ fbsd_pid_to_exec_file (struct target_ops
      return buf;
  #endif
  
-  path = xstrprintf ("/proc/%d/file", pid);
-  if (readlink (path, buf, PATH_MAX - 1) == -1)
+  xsnprintf (name, PATH_MAX, "/proc/%d/exe", pid);
+  len = readlink (name, buf, PATH_MAX - 1);
+  if (len != -1)
      {
-      xfree (buf);
-      buf = NULL;
+      buf[len] = '\0';
+      return buf;
      }
  
-  xfree (path);
-  return buf;
+  return NULL;
  }
  
  static int
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -4011,19 +4011,15 @@ linux_nat_thread_name (struct target_ops
  static char *
  linux_child_pid_to_exec_file (struct target_ops *self, int pid)
  {
-  char *name1, *name2;
+  static char buf[PATH_MAX];
+  char name[PATH_MAX];
  
-  name1 = xmalloc (PATH_MAX);
-  name2 = xmalloc (PATH_MAX);
-  make_cleanup (xfree, name1);
-  make_cleanup (xfree, name2);
-  memset (name2, 0, PATH_MAX);
+  xsnprintf (name, PATH_MAX, "/proc/%d/exe", pid);
+  memset (buf, 0, PATH_MAX);
+  if (readlink (name, buf, PATH_MAX - 1) <= 0)
+    strcpy (buf, name);
  
-  xsnprintf (name1, PATH_MAX, "/proc/%d/exe", pid);
-  if (readlink (name1, name2, PATH_MAX - 1) > 0)
-    return name2;
-  else
-    return name1;
+  return buf;
  }
  
  /* Records the thread's register state for the corefile note
--- a/gdb/nbsd-nat.c
+++ b/gdb/nbsd-nat.c
@@ -27,17 +27,17 @@
  char *
  nbsd_pid_to_exec_file (struct target_ops *self, int pid)
  {
-  size_t len = PATH_MAX;
-  char *buf = xcalloc (len, sizeof (char));
-  char *path;
+  size_t len;
+  static char buf[PATH_MAX];
+  char name[PATH_MAX];
  
-  path = xstrprintf ("/proc/%d/exe", pid);
-  if (readlink (path, buf, PATH_MAX - 1) == -1)
+  xsnprintf (name, PATH_MAX, "/proc/%d/exe", pid);
+  len = readlink (name, buf, PATH_MAX - 1);
+  if (len != -1)
      {
-      xfree (buf);
-      buf = NULL;
+      buf[len] = '\0';
+      return buf;
      }
  
-  xfree (path);
-  return buf;
+  return NULL;
  }

Re: [PATCH 0/1] Fix internal warning when "gdb -p xxx"

[Hui Zhu]

On 03/20/14 10:58, Hui Zhu wrote:
> On 03/19/14 18:16, Pedro Alves wrote:
>> On 03/19/2014 03:57 AM, Hui Zhu wrote:
>>>>>
>>>>> Same with the nul termination.  The most standard solution is:
>>>>>
>>>>>     static char buf[PATH_MAX];
>>>>>     char name[PATH_MAX];
>>>>>     ssize_t len;
>>>>>
>>>>>     xsnprintf (name, PATH_MAX, "/proc/%d/exe", pid);
>>>>>     len = readlink (name, buf, PATH_MAX - 1);
>>>>>     if (len != -1)
>>>>>       {
>>>>>         buf[len] = '\0';
>>>>>         return buf;
>>>>>       }
>>>>>     return NULL;
>>>>>
>>> I make a new patch according to your comments.
>>> Please help me review it.
>>
>> The patch changes the bsd implementations's behavior, because
>> you made them return the /proc path when readlink fails (like
>> the Linux version does), instead of what the current code does
>> or what I suggested above.
>>
>
> I made a new version that change fbsd_pid_to_exec_file and nbsd_pid_to_exec_file
> to your suggested above.
>
> Please help me review it.
>
> Thanks,
> Hui
>

I am sorry that I post a wrong version patch that use "size_t len".
I post a new version that fixed this issue.

Thanks,
Hui

2014-03-20  Hui Zhu  <hui@codesourcery.com>
	    Pedro Alves  <palves@redhat.com>

	* darwin-nat.c (darwin_pid_to_exec_file): Change xmalloc to
	static buffer.
	* fbsd-nat.c (fbsd_pid_to_exec_file): Ditto.
	* linux-nat.c (linux_child_pid_to_exec_file): Ditto.
	* nbsd-nat.c (nbsd_pid_to_exec_file): Ditto.

--- a/gdb/darwin-nat.c
+++ b/gdb/darwin-nat.c
@@ -1991,12 +1991,9 @@ set_enable_mach_exceptions (char *args,
  static char *
  darwin_pid_to_exec_file (struct target_ops *self, int pid)
  {
-  char *path;
+  static char path[PATH_MAX];
    int res;
  
-  path = xmalloc (PATH_MAX);
-  make_cleanup (xfree, path);
-
    res = proc_pidinfo (pid, PROC_PIDPATHINFO, 0, path, PATH_MAX);
    if (res >= 0)
      return path;
--- a/gdb/fbsd-nat.c
+++ b/gdb/fbsd-nat.c
@@ -39,9 +39,9 @@
  char *
  fbsd_pid_to_exec_file (struct target_ops *self, int pid)
  {
-  size_t len = PATH_MAX;
-  char *buf = xcalloc (len, sizeof (char));
-  char *path;
+  ssize_t len = PATH_MAX;
+  static char buf[PATH_MAX];
+  char name[PATH_MAX];
  
  #ifdef KERN_PROC_PATHNAME
    int mib[4];
@@ -54,15 +54,15 @@ fbsd_pid_to_exec_file (struct target_ops
      return buf;
  #endif
  
-  path = xstrprintf ("/proc/%d/file", pid);
-  if (readlink (path, buf, PATH_MAX - 1) == -1)
+  xsnprintf (name, PATH_MAX, "/proc/%d/exe", pid);
+  len = readlink (name, buf, PATH_MAX - 1);
+  if (len != -1)
      {
-      xfree (buf);
-      buf = NULL;
+      buf[len] = '\0';
+      return buf;
      }
  
-  xfree (path);
-  return buf;
+  return NULL;
  }
  
  static int
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -4011,19 +4011,15 @@ linux_nat_thread_name (struct target_ops
  static char *
  linux_child_pid_to_exec_file (struct target_ops *self, int pid)
  {
-  char *name1, *name2;
+  static char buf[PATH_MAX];
+  char name[PATH_MAX];
  
-  name1 = xmalloc (PATH_MAX);
-  name2 = xmalloc (PATH_MAX);
-  make_cleanup (xfree, name1);
-  make_cleanup (xfree, name2);
-  memset (name2, 0, PATH_MAX);
+  xsnprintf (name, PATH_MAX, "/proc/%d/exe", pid);
+  memset (buf, 0, PATH_MAX);
+  if (readlink (name, buf, PATH_MAX - 1) <= 0)
+    strcpy (buf, name);
  
-  xsnprintf (name1, PATH_MAX, "/proc/%d/exe", pid);
-  if (readlink (name1, name2, PATH_MAX - 1) > 0)
-    return name2;
-  else
-    return name1;
+  return buf;
  }
  
  /* Records the thread's register state for the corefile note
--- a/gdb/nbsd-nat.c
+++ b/gdb/nbsd-nat.c
@@ -27,17 +27,17 @@
  char *
  nbsd_pid_to_exec_file (struct target_ops *self, int pid)
  {
-  size_t len = PATH_MAX;
-  char *buf = xcalloc (len, sizeof (char));
-  char *path;
+  ssize_t len;
+  static char buf[PATH_MAX];
+  char name[PATH_MAX];
  
-  path = xstrprintf ("/proc/%d/exe", pid);
-  if (readlink (path, buf, PATH_MAX - 1) == -1)
+  xsnprintf (name, PATH_MAX, "/proc/%d/exe", pid);
+  len = readlink (name, buf, PATH_MAX - 1);
+  if (len != -1)
      {
-      xfree (buf);
-      buf = NULL;
+      buf[len] = '\0';
+      return buf;
      }
  
-  xfree (path);
-  return buf;
+  return NULL;
  }

Re: [PATCH 0/1] Fix internal warning when "gdb -p xxx"

[Pedro Alves]

On 03/20/2014 03:27 AM, Hui Zhu wrote:

> 2014-03-20  Hui Zhu  <hui@codesourcery.com>
> 	    Pedro Alves  <palves@redhat.com>
> 
> 	* darwin-nat.c (darwin_pid_to_exec_file): Change xmalloc to
> 	static buffer.
> 	* fbsd-nat.c (fbsd_pid_to_exec_file): Ditto.
> 	* linux-nat.c (linux_child_pid_to_exec_file): Ditto.
> 	* nbsd-nat.c (nbsd_pid_to_exec_file): Ditto.

OK, thanks.

-- 
Pedro Alves

Re: [PATCH 0/1] Fix internal warning when "gdb -p xxx"

[Hui Zhu]

On 03/20/14 19:00, Pedro Alves wrote:
> On 03/20/2014 03:27 AM, Hui Zhu wrote:
>
>> 2014-03-20  Hui Zhu  <hui@codesourcery.com>
>> 	    Pedro Alves  <palves@redhat.com>
>>
>> 	* darwin-nat.c (darwin_pid_to_exec_file): Change xmalloc to
>> 	static buffer.
>> 	* fbsd-nat.c (fbsd_pid_to_exec_file): Ditto.
>> 	* linux-nat.c (linux_child_pid_to_exec_file): Ditto.
>> 	* nbsd-nat.c (nbsd_pid_to_exec_file): Ditto.
>
> OK, thanks.
>

Thanks for your help.
Committed to https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commit;h=b4ab256ded5020a82ff7ce8dc485e7882fc5b6a7

Best,
Hui

[PATCH 1/1] Fix internal warning when "gdb -p xxx" (test)

[Hui Zhu]

We found that the issue can be found by regression test.
So make a patch to make attach.exp can test "gdb -p xxx".

Thanks,
Hui

2014-03-19  Hui Zhu  <hui@codesourcery.com>

	* gdb.base/attach.exp (do_command_attach_tests): New.

--- a/gdb/testsuite/gdb.base/attach.exp
+++ b/gdb/testsuite/gdb.base/attach.exp
@@ -384,6 +384,48 @@ proc do_call_attach_tests {} {
      remote_exec build "kill -9 ${testpid}"
  }
  
+proc do_command_attach_tests {} {
+    global gdb_prompt
+    global binfile
+    global verbose
+    global GDB
+    global INTERNAL_GDBFLAGS
+    global GDBFLAGS
+
+    if ![isnative] then {
+	unsupported "command attach test"
+	return 0
+    }
+
+    # Start the program running and then wait for a bit, to be sure
+    # that it can be attached to.
+
+    set testpid [eval exec $binfile &]
+    exec sleep 2
+    if { [istarget "*-*-cygwin*"] } {
+	# testpid is the Cygwin PID, GDB uses the Windows PID, which might be
+	# different due to the way fork/exec works.
+	set testpid [ exec ps -e | gawk "{ if (\$1 == $testpid) print \$4; }" ]
+    }
+
+    gdb_exit
+    if $verbose>1 then {
+        send_user "Spawning $GDB $INTERNAL_GDBFLAGS $GDBFLAGS --pid=$testpid\n"
+    }
+
+    eval "spawn $GDB $INTERNAL_GDBFLAGS $GDBFLAGS --pid=$testpid"
+    expect {
+	-re "Reading symbols from.*$gdb_prompt $" {
+            pass "starting with --pid=$testpid"
+	}
+	timeout 	{ fail "(timeout) starting with --pid=$testpid" }
+    }
+
+    # Get rid of the process
+
+    remote_exec build "kill -9 ${testpid}"
+}
+
  
  # Start with a fresh gdb
  
@@ -404,4 +446,8 @@ gdb_start
  gdb_reinitialize_dir $srcdir/$subdir
  do_call_attach_tests
  
+# Test "gdb --pid"
+
+do_command_attach_tests
+
  return 0

Re: [PATCH 1/1] Fix internal warning when "gdb -p xxx" (test)

[Pedro Alves]

On 03/19/2014 04:03 AM, Hui Zhu wrote:

> +    eval "spawn $GDB $INTERNAL_GDBFLAGS $GDBFLAGS --pid=$testpid"
> +    expect {
> +	-re "Reading symbols from.*$gdb_prompt $" {
> +            pass "starting with --pid=$testpid"

Don't add the $testpid to the PASS/FAIL line, as it'll change between
testsuite runs, making gdb.sum diffs harder.

> +	}
> +	timeout 	{ fail "(timeout) starting with --pid=$testpid" }

Instead of repeating the text, write:

    set test "starting with --pid"
    expect {
	-re "Reading symbols from.*$gdb_prompt $" {
            pass "$test"
	}
	timeout {
            fail "$test (timeout)"
	}
    }

Otherwise OK.

Thanks!

-- 
Pedro Alves

Re: [PATCH 1/1] Fix internal warning when "gdb -p xxx" (test)

[Hui Zhu]

On 03/19/14 18:28, Pedro Alves wrote:
> On 03/19/2014 04:03 AM, Hui Zhu wrote:
>
>> +    eval "spawn $GDB $INTERNAL_GDBFLAGS $GDBFLAGS --pid=$testpid"
>> +    expect {
>> +	-re "Reading symbols from.*$gdb_prompt $" {
>> +            pass "starting with --pid=$testpid"
>
> Don't add the $testpid to the PASS/FAIL line, as it'll change between
> testsuite runs, making gdb.sum diffs harder.
>
>> +	}
>> +	timeout 	{ fail "(timeout) starting with --pid=$testpid" }
>
> Instead of repeating the text, write:
>
>      set test "starting with --pid"
>      expect {
> 	-re "Reading symbols from.*$gdb_prompt $" {
>              pass "$test"
> 	}
> 	timeout {
>              fail "$test (timeout)"
> 	}
>      }
>
> Otherwise OK.
>
> Thanks!
>

Make a new version according to your comments.
Please help me review it.

Thanks,
Hui

2014-03-20  Hui Zhu  <hui@codesourcery.com>

	* gdb.base/attach.exp (do_command_attach_tests): New.

--- a/gdb/testsuite/gdb.base/attach.exp
+++ b/gdb/testsuite/gdb.base/attach.exp
@@ -384,6 +384,51 @@ proc do_call_attach_tests {} {
      remote_exec build "kill -9 ${testpid}"
  }
  
+proc do_command_attach_tests {} {
+    global gdb_prompt
+    global binfile
+    global verbose
+    global GDB
+    global INTERNAL_GDBFLAGS
+    global GDBFLAGS
+
+    if ![isnative] then {
+	unsupported "command attach test"
+	return 0
+    }
+
+    # Start the program running and then wait for a bit, to be sure
+    # that it can be attached to.
+
+    set testpid [eval exec $binfile &]
+    exec sleep 2
+    if { [istarget "*-*-cygwin*"] } {
+	# testpid is the Cygwin PID, GDB uses the Windows PID, which might be
+	# different due to the way fork/exec works.
+	set testpid [ exec ps -e | gawk "{ if (\$1 == $testpid) print \$4; }" ]
+    }
+
+    gdb_exit
+    if $verbose>1 then {
+        send_user "Spawning $GDB $INTERNAL_GDBFLAGS $GDBFLAGS --pid=$testpid\n"
+    }
+
+    eval "spawn $GDB $INTERNAL_GDBFLAGS $GDBFLAGS --pid=$testpid"
+    set test "starting with --pid"
+    expect {
+	-re "Reading symbols from.*$gdb_prompt $" {
+            pass "$test"
+	}
+	timeout {
+	    fail "$test (timeout)"
+	}
+    }
+
+    # Get rid of the process
+
+    remote_exec build "kill -9 ${testpid}"
+}
+
  
  # Start with a fresh gdb
  
@@ -404,4 +449,8 @@ gdb_start
  gdb_reinitialize_dir $srcdir/$subdir
  do_call_attach_tests
  
+# Test "gdb --pid"
+
+do_command_attach_tests
+
  return 0

Re: [PATCH 1/1] Fix internal warning when "gdb -p xxx" (test)

[Pedro Alves]

On 03/20/2014 03:16 AM, Hui Zhu wrote:

> +    expect {
> +	-re "Reading symbols from.*$gdb_prompt $" {
> +            pass "$test"
> +	}
> +	timeout {
> +	    fail "$test (timeout)"

Something odd with indentation here.

Otherwise OK.  Please push.

Thanks,
-- 
Pedro Alves

Re: [PATCH 1/1] Fix internal warning when "gdb -p xxx" (test)

[Hui Zhu]

On 03/20/14 19:03, Pedro Alves wrote:
> On 03/20/2014 03:16 AM, Hui Zhu wrote:
>
>> +    expect {
>> +	-re "Reading symbols from.*$gdb_prompt $" {
>> +            pass "$test"
>> +	}
>> +	timeout {
>> +	    fail "$test (timeout)"
>
> Something odd with indentation here.

Change 8 blank to a tab.

>
> Otherwise OK.  Please push.
>
> Thanks,
>

Thanks for your help.

Committed to
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=ccdd1909ad5299cf0753aaa113928a41f8f27391

Best,
Hui