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

Message ID 53271C09.5000709@mentor.com
State Superseded
Headers

Commit Message

Hui Zhu March 17, 2014, 4 p.m. UTC
  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.
  

Comments

Pedro Alves March 17, 2014, 4:12 p.m. UTC | #1
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 March 17, 2014, 4:56 p.m. UTC | #2
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.  :-(
  

Patch

--- 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