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

[Pedro Alves <palves@redhat.com>]

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