Make only user-specified executable filenames sticky

  Hi all,

In GDB some executable files are supplied by the user (e.g. using a
"file" command) and some are determined by GDB (e.g. while processing
an "attach" command).  GDB will not attempt to determine a filename if
one has been set.  This causes problems if you attach to one process
and then attach to another: GDB will not attempt to discover the main
executable on the second attach.  If the two processes have different
main executable files then the symbols will now be wrong.

This commit updates GDB to keep track of which executable filenames
were supplied by the user.  When GDB might attempt to determine an
executable filename and one is already set, filenames determined by
GDB may be overridden but user-supplied filenames will not.

Built and regtested on RHEL6.6 x86_64.

Is this ok to commit?

Cheers,
Gary


gdb/ChangeLog:

	* progspace.h (struct program_space)
	<pspace_user_supplied_exec_file_p>: New field.
	* exec.h (user_supplied_exec_file_p): New macro.
	* exec.c (exec_close): Clear user_supplied_exec_file_p.
	(exec_file_locate_attach): Remove get_exec_file check.
	(exec_file_command): Set user_supplied_exec_file_p.
	* inferior.c (add_inferior_command): Likewise.
	* main.c (captured_main): Likewise.
	* infcmd.c (attach_command_post_wait): Always try and
	locate main executable if !user_supplied_exec_file_p.
	* remote.c (remote_add_inferior): Likewise.
---
 gdb/ChangeLog   |   14 ++++++++++++++
 gdb/exec.c      |    7 ++-----
 gdb/exec.h      |    2 ++
 gdb/infcmd.c    |   12 +++++++-----
 gdb/inferior.c  |    1 +
 gdb/main.c      |   16 +++++++++++-----
 gdb/progspace.h |    7 +++++++
 gdb/remote.c    |    7 ++++---
 8 files changed, 48 insertions(+), 18 deletions(-)
  

On 05/06/2015 11:26 AM, Gary Benson wrote:
> Hi all,
> 
> In GDB some executable files are supplied by the user (e.g. using a
> "file" command) and some are determined by GDB (e.g. while processing
> an "attach" command).  GDB will not attempt to determine a filename if
> one has been set.  This causes problems if you attach to one process
> and then attach to another: GDB will not attempt to discover the main
> executable on the second attach.  If the two processes have different
> main executable files then the symbols will now be wrong.
> 
> This commit updates GDB to keep track of which executable filenames
> were supplied by the user.  When GDB might attempt to determine an
> executable filename and one is already set, filenames determined by
> GDB may be overridden but user-supplied filenames will not.

I have a feeling this would be simpler if the flag's sense was
reversed?  That is, mark the exec as auto-discovered instead of
marking it user-loaded.

How does this interact with "symbol-file FILE" ?

> Built and regtested on RHEL6.6 x86_64.
> 
> Is this ok to commit?

This fixes PR 17626 (so please add that to the ChangeLog), which is
marked as duplicate of PR 16266 currently, but in a different way
than 16266 suggests.

 https://sourceware.org/bugzilla/show_bug.cgi?id=16266
 https://sourceware.org/bugzilla/show_bug.cgi?id=17626

I think this needs a NEWS entry, and probably a tweak to the
manual somewhere.

> --- a/gdb/exec.h
> +++ b/gdb/exec.h
> @@ -32,6 +32,8 @@ struct objfile;
>  #define exec_bfd current_program_space->ebfd
>  #define exec_bfd_mtime current_program_space->ebfd_mtime
>  #define exec_filename current_program_space->pspace_exec_filename
> +#define user_supplied_exec_file_p \
> +  current_program_space->pspace_user_supplied_exec_file_p

Nit, but I'd suggest 'exec_file_is_user_supplied', which would
fit the pattern of vars related to the exec being prefixed exec_.

>  
> --- a/gdb/progspace.h
> +++ b/gdb/progspace.h
> @@ -154,6 +154,13 @@ struct program_space
>         It needs to be freed by xfree.  It is not NULL iff EBFD is not NULL.  */
>      char *pspace_exec_filename;
>  
> +    /* Nonzero if pspace_exec_filename was supplied by the user,
> +       either at startup (on the command-line) or via a "file"
> +       an "add-inferior -exec" command.  Zero if

Sounds like an "or" is missing between the commands.

Thanks,
Pedro Alves
  

On 05/06/2015 01:19 PM, Pedro Alves wrote:

> I think this needs a NEWS entry, and probably a tweak to the
> manual somewhere.

Oh, I forgot to say, could you add a test too?

Thanks,
Pedro Alves
  

On Wed, 2015-05-06 at 11:26 +0100, Gary Benson wrote:
> Hi all,
> 
> In GDB some executable files are supplied by the user (e.g. using a
> "file" command) and some are determined by GDB (e.g. while processing
> an "attach" command).  GDB will not attempt to determine a filename if
> one has been set.  This causes problems if you attach to one process
> and then attach to another: GDB will not attempt to discover the main
> executable on the second attach.  If the two processes have different
> main executable files then the symbols will now be wrong.
> 
> This commit updates GDB to keep track of which executable filenames
> were supplied by the user.  When GDB might attempt to determine an
> executable filename and one is already set, filenames determined by
> GDB may be overridden but user-supplied filenames will not.
If not overriding the file set by the user, maybe GDB could/should give
a warning when the exec-file reported by the target does not match the
file as set by the user ?

Philippe
  

Pedro Alves wrote:
> On 05/06/2015 11:26 AM, Gary Benson wrote:
> > In GDB some executable files are supplied by the user (e.g. using
> > a "file" command) and some are determined by GDB (e.g. while
> > processing an "attach" command).  GDB will not attempt to
> > determine a filename if one has been set.  This causes problems if
> > you attach to one process and then attach to another: GDB will not
> > attempt to discover the main executable on the second attach.  If
> > the two processes have different main executable files then the
> > symbols will now be wrong.
> > 
> > This commit updates GDB to keep track of which executable
> > filenames were supplied by the user.  When GDB might attempt to
> > determine an executable filename and one is already set, filenames
> > determined by GDB may be overridden but user-supplied filenames
> > will not.
> 
> I have a feeling this would be simpler if the flag's sense was
> reversed?  That is, mark the exec as auto-discovered instead of
> marking it user-loaded.

I'm easy either way.  I spent about four hours trying to name the
flag (and thinking about making it an enum) so right now I'm about
ready to be told what to do :)

I think having the sense the other way around would make the checks
more complex, you'd have to check for exec_file being empty as well
as being auto-discovered.  If the user set it it isn't empty.

> How does this interact with "symbol-file FILE" ?

I'm not sure... badly? :)

exec_file_locate_attach (the bit that does the auto-discovery) does
both exec_file_attach and symbol_file_add_main.  file_command also
does both, albeit indirectly, and add_inferior_command does both
too.  But, on startup you can specify separate symbol file, and of
course you can use the symbol-file command.

I don't really know in what circumstances you would use a separate
symbol file.  Should they both be protected individually do you
think?  I'm leaning that way.

> This fixes PR 17626 (so please add that to the ChangeLog), which is
> marked as duplicate of PR 16266 currently, but in a different way
> than 16266 suggests.
> 
>  https://sourceware.org/bugzilla/show_bug.cgi?id=16266
>  https://sourceware.org/bugzilla/show_bug.cgi?id=17626

Ok.

> I think this needs a NEWS entry, and probably a tweak to the
> manual somewhere.

Ok.

> > --- a/gdb/exec.h
> > +++ b/gdb/exec.h
> > @@ -32,6 +32,8 @@ struct objfile;
> >  #define exec_bfd current_program_space->ebfd
> >  #define exec_bfd_mtime current_program_space->ebfd_mtime
> >  #define exec_filename current_program_space->pspace_exec_filename
> > +#define user_supplied_exec_file_p \
> > +  current_program_space->pspace_user_supplied_exec_file_p
> 
> Nit, but I'd suggest 'exec_file_is_user_supplied', which would
> fit the pattern of vars related to the exec being prefixed exec_.

Ok.  Or exec_file_is_sticky (and symfile_is_sticky)?

> > --- a/gdb/progspace.h
> > +++ b/gdb/progspace.h
> > @@ -154,6 +154,13 @@ struct program_space
> >         It needs to be freed by xfree.  It is not NULL iff EBFD is not NULL.  */
> >      char *pspace_exec_filename;
> >  
> > +    /* Nonzero if pspace_exec_filename was supplied by the user,
> > +       either at startup (on the command-line) or via a "file"
> > +       an "add-inferior -exec" command.  Zero if
> 
> Sounds like an "or" is missing between the commands.

Got it.

Thanks,
Gary
  

Philippe Waroquiers wrote:
> On Wed, 2015-05-06 at 11:26 +0100, Gary Benson wrote:
> > In GDB some executable files are supplied by the user (e.g. using
> > a "file" command) and some are determined by GDB (e.g. while
> > processing an "attach" command).  GDB will not attempt to
> > determine a filename if one has been set.  This causes problems if
> > you attach to one process and then attach to another: GDB will not
> > attempt to discover the main executable on the second attach.  If
> > the two processes have different main executable files then the
> > symbols will now be wrong.
> > 
> > This commit updates GDB to keep track of which executable
> > filenames were supplied by the user.  When GDB might attempt to
> > determine an executable filename and one is already set, filenames
> > determined by GDB may be overridden but user-supplied filenames
> > will not.
> 
> If not overriding the file set by the user, maybe GDB could/should
> give a warning when the exec-file reported by the target does not
> match the file as set by the user ?

I'm wondering whether we should always override the executable file,
and treat the symbol file as the special one.  Pedro?

Cheers,
Gary
  

On 05/06/2015 04:20 PM, Gary Benson wrote:
> Pedro Alves wrote:
>> On 05/06/2015 11:26 AM, Gary Benson wrote:
>>> In GDB some executable files are supplied by the user (e.g. using
>>> a "file" command) and some are determined by GDB (e.g. while
>>> processing an "attach" command).  GDB will not attempt to
>>> determine a filename if one has been set.  This causes problems if
>>> you attach to one process and then attach to another: GDB will not
>>> attempt to discover the main executable on the second attach.  If
>>> the two processes have different main executable files then the
>>> symbols will now be wrong.
>>>
>>> This commit updates GDB to keep track of which executable
>>> filenames were supplied by the user.  When GDB might attempt to
>>> determine an executable filename and one is already set, filenames
>>> determined by GDB may be overridden but user-supplied filenames
>>> will not.
>>
>> I have a feeling this would be simpler if the flag's sense was
>> reversed?  That is, mark the exec as auto-discovered instead of
>> marking it user-loaded.
> 
> I'm easy either way.  I spent about four hours trying to name the
> flag (and thinking about making it an enum) so right now I'm about
> ready to be told what to do :)
> 
> I think having the sense the other way around would make the checks
> more complex, you'd have to check for exec_file being empty as well
> as being auto-discovered.  If the user set it it isn't empty.

It's not that complex, and, both the checks and the sets of the
flags would only appear in places that relate to auto-discovery,
instead of setting the flag in the several user-specified code paths
(which seem to be more than the auto-discover paths), and then checking
them in the auto-discover paths; seems more centralized to keep most
things in the auto-discover paths.

> 
>> How does this interact with "symbol-file FILE" ?
> 
> I'm not sure... badly? :)
> 

:-)

> exec_file_locate_attach (the bit that does the auto-discovery) does
> both exec_file_attach and symbol_file_add_main.  file_command also
> does both, albeit indirectly, and add_inferior_command does both
> too.  But, on startup you can specify separate symbol file, and of
> course you can use the symbol-file command.
> 
> I don't really know in what circumstances you would use a separate
> symbol file.

Yeah, I don't think it's a very common thing to do.  At least
not on systems where the same file format holds both
the executable and the debug info.  Not all systems are like
that: I'm thinking of Symbian, with PE dll files for exec
file, and ELF .sym files for symbols
(https://sourceware.org/ml/gdb-patches/2010-03/msg00237.html),
or even Windows' pdb files (which we don't support, but alas).
Maybe there are more systems like that.

> Should they both be protected individually do you
> think?  I'm leaning that way.

Yeah, that might be the simplest/best option, as opposed to
trying to come up with rules/policy related to how symbol-file
and exec-file influence each other.

> 
>>> --- a/gdb/exec.h
>>> +++ b/gdb/exec.h
>>> @@ -32,6 +32,8 @@ struct objfile;
>>>  #define exec_bfd current_program_space->ebfd
>>>  #define exec_bfd_mtime current_program_space->ebfd_mtime
>>>  #define exec_filename current_program_space->pspace_exec_filename
>>> +#define user_supplied_exec_file_p \
>>> +  current_program_space->pspace_user_supplied_exec_file_p
>>
>> Nit, but I'd suggest 'exec_file_is_user_supplied', which would
>> fit the pattern of vars related to the exec being prefixed exec_.
> 
> Ok.  Or exec_file_is_sticky (and symfile_is_sticky)?

Sounds fine.

Thanks,
Pedro Alves
  

On 05/06/2015 04:41 PM, Gary Benson wrote:
> Philippe Waroquiers wrote:
>> On Wed, 2015-05-06 at 11:26 +0100, Gary Benson wrote:
>>> In GDB some executable files are supplied by the user (e.g. using
>>> a "file" command) and some are determined by GDB (e.g. while
>>> processing an "attach" command).  GDB will not attempt to
>>> determine a filename if one has been set.  This causes problems if
>>> you attach to one process and then attach to another: GDB will not
>>> attempt to discover the main executable on the second attach.  If
>>> the two processes have different main executable files then the
>>> symbols will now be wrong.
>>>
>>> This commit updates GDB to keep track of which executable
>>> filenames were supplied by the user.  When GDB might attempt to
>>> determine an executable filename and one is already set, filenames
>>> determined by GDB may be overridden but user-supplied filenames
>>> will not.
>>
>> If not overriding the file set by the user, maybe GDB could/should
>> give a warning when the exec-file reported by the target does not
>> match the file as set by the user ?

Giving a warning may be be good.  Note sure whether basing it
on file name alone would be noisy.  Basing the warning on GNU build-id
as suggested on PR 16266 would be bullet proof.

> 
> I'm wondering whether we should always override the executable file,
> and treat the symbol file as the special one.  Pedro?

Not sure about that.  Sounds like "file program1" +
"attach program2" would end up with the symbol file pointing
to program1?  Not seeing how that would be useful, but
maybe if you detail the idea it gets clearer.

Thanks,
Pedro Alves
  

On 5/6/2015 3:26 AM, Gary Benson wrote:
> Hi all,
> 
> In GDB some executable files are supplied by the user (e.g. using a
> "file" command) and some are determined by GDB (e.g. while processing
> an "attach" command).  GDB will not attempt to determine a filename if
> one has been set.  This causes problems if you attach to one process
> and then attach to another: GDB will not attempt to discover the main
> executable on the second attach.  If the two processes have different
> main executable files then the symbols will now be wrong.
> 
> This commit updates GDB to keep track of which executable filenames
> were supplied by the user.  When GDB might attempt to determine an
> executable filename and one is already set, filenames determined by
> GDB may be overridden but user-supplied filenames will not.
> 
> Built and regtested on RHEL6.6 x86_64.
> 
> Is this ok to commit?
> 
> Cheers,
> Gary
> 
> 
> gdb/ChangeLog:
> 
> 	* progspace.h (struct program_space)
> 	<pspace_user_supplied_exec_file_p>: New field.
> 	* exec.h (user_supplied_exec_file_p): New macro.
> 	* exec.c (exec_close): Clear user_supplied_exec_file_p.
> 	(exec_file_locate_attach): Remove get_exec_file check.
> 	(exec_file_command): Set user_supplied_exec_file_p.
> 	* inferior.c (add_inferior_command): Likewise.
> 	* main.c (captured_main): Likewise.
> 	* infcmd.c (attach_command_post_wait): Always try and
> 	locate main executable if !user_supplied_exec_file_p.
> 	* remote.c (remote_add_inferior): Likewise.
> ---
>  gdb/ChangeLog   |   14 ++++++++++++++
>  gdb/exec.c      |    7 ++-----
>  gdb/exec.h      |    2 ++
>  gdb/infcmd.c    |   12 +++++++-----
>  gdb/inferior.c  |    1 +
>  gdb/main.c      |   16 +++++++++++-----
>  gdb/progspace.h |    7 +++++++
>  gdb/remote.c    |    7 ++++---
>  8 files changed, 48 insertions(+), 18 deletions(-)
> 
> diff --git a/gdb/exec.c b/gdb/exec.c
> index 8a4ab6f..278df83 100644
> --- a/gdb/exec.c
> +++ b/gdb/exec.c
> @@ -102,6 +102,7 @@ exec_close (void)
>  
>        xfree (exec_filename);
>        exec_filename = NULL;
> +      user_supplied_exec_file_p = 0;
>      }
>  }
>  
> @@ -142,11 +143,6 @@ exec_file_locate_attach (int pid, int from_tty)
>  {
>    char *exec_file, *full_exec_path = NULL;
>  
> -  /* Do nothing if we already have an executable filename.  */
> -  exec_file = (char *) get_exec_file (0);
> -  if (exec_file != NULL)
> -    return;
> -
>    /* Try to determine a filename from the process itself.  */
>    exec_file = target_pid_to_exec_file (pid);
>    if (exec_file == NULL)
> @@ -376,6 +372,7 @@ exec_file_command (char *args, int from_tty)
>        filename = tilde_expand (*argv);
>        make_cleanup (xfree, filename);
>        exec_file_attach (filename, from_tty);
> +      user_supplied_exec_file_p = 1;
>  
>        do_cleanups (cleanups);
>      }
> diff --git a/gdb/exec.h b/gdb/exec.h
> index c7f3b56..3794aba 100644
> --- a/gdb/exec.h
> +++ b/gdb/exec.h
> @@ -32,6 +32,8 @@ struct objfile;
>  #define exec_bfd current_program_space->ebfd
>  #define exec_bfd_mtime current_program_space->ebfd_mtime
>  #define exec_filename current_program_space->pspace_exec_filename
> +#define user_supplied_exec_file_p \
> +  current_program_space->pspace_user_supplied_exec_file_p
>  
>  /* Builds a section table, given args BFD, SECTABLE_PTR, SECEND_PTR.
>     Returns 0 if OK, 1 on error.  */
> diff --git a/gdb/infcmd.c b/gdb/infcmd.c
> index 7e2484b..491cbb6 100644
> --- a/gdb/infcmd.c
> +++ b/gdb/infcmd.c
> @@ -2467,15 +2467,17 @@ attach_command_post_wait (char *args, int from_tty, int async_exec)
>    inferior = current_inferior ();
>    inferior->control.stop_soon = NO_STOP_QUIETLY;
>  
> -  /* If no exec file is yet known, try to determine it from the
> -     process itself.  */
> -  if (get_exec_file (0) == NULL)
> -    exec_file_locate_attach (ptid_get_pid (inferior_ptid), from_tty);
> -  else
> +  if (user_supplied_exec_file_p)
>      {
>        reopen_exec_file ();
>        reread_symbols ();
>      }
> +  else
> +    {
> +      /* Attempt to open the file that was executed to create this
> +	 inferior.  */
> +      exec_file_locate_attach (ptid_get_pid (inferior_ptid), from_tty);
> +    }
>  
>    /* Take any necessary post-attaching actions for this platform.  */
>    target_post_attach (ptid_get_pid (inferior_ptid));
> diff --git a/gdb/inferior.c b/gdb/inferior.c
> index ba320b5..87b2133 100644
> --- a/gdb/inferior.c
> +++ b/gdb/inferior.c
> @@ -889,6 +889,7 @@ add_inferior_command (char *args, int from_tty)
>  
>  	  exec_file_attach (exec, from_tty);
>  	  symbol_file_add_main (exec, from_tty);
> +	  user_supplied_exec_file_p = 1;
>  	}
>      }
>  
> diff --git a/gdb/main.c b/gdb/main.c
> index 477fd68..9d8a196 100644
> --- a/gdb/main.c
> +++ b/gdb/main.c
> @@ -1051,14 +1051,20 @@ captured_main (void *data)
>           catch_command_errors returns non-zero on success!  */
>        if (catch_command_errors_const (exec_file_attach, execarg,
>  				      !batch_flag))
> -	catch_command_errors_const (symbol_file_add_main, symarg,
> -				    !batch_flag);
> +	{
> +	  user_supplied_exec_file_p = 1;
> +
> +	  catch_command_errors_const (symbol_file_add_main, symarg,
> +				      !batch_flag);
> +	}
>      }
>    else
>      {
> -      if (execarg != NULL)
> -	catch_command_errors_const (exec_file_attach, execarg,
> -				    !batch_flag);
> +      if (execarg != NULL
> +	  && catch_command_errors_const (exec_file_attach, execarg,
> +					 !batch_flag))
> +	user_supplied_exec_file_p = 1;
> +
>        if (symarg != NULL)
>  	catch_command_errors_const (symbol_file_add_main, symarg,
>  				    !batch_flag);
> diff --git a/gdb/progspace.h b/gdb/progspace.h
> index f960093..f8c39b6 100644
> --- a/gdb/progspace.h
> +++ b/gdb/progspace.h
> @@ -154,6 +154,13 @@ struct program_space
>         It needs to be freed by xfree.  It is not NULL iff EBFD is not NULL.  */
>      char *pspace_exec_filename;
>  
> +    /* Nonzero if pspace_exec_filename was supplied by the user,
> +       either at startup (on the command-line) or via a "file"
> +       an "add-inferior -exec" command.  Zero if
> +       pspace_exec_filename is unset or was discovered by GDB
> +       somehow.  */
> +    int pspace_user_supplied_exec_file_p;
> +
>      /* The address space attached to this program space.  More than one
>         program space may be bound to the same address space.  In the
>         traditional unix-like debugging scenario, this will usually
> diff --git a/gdb/remote.c b/gdb/remote.c
> index 099ddbb..b2f1bba 100644
> --- a/gdb/remote.c
> +++ b/gdb/remote.c
> @@ -1556,9 +1556,10 @@ remote_add_inferior (int fake_pid_p, int pid, int attached,
>    inf->attach_flag = attached;
>    inf->fake_pid_p = fake_pid_p;
>  
> -  /* If no main executable is currently open then attempt to
> -     open the file that was executed to create this inferior.  */
> -  if (try_open_exec && !fake_pid_p && get_exec_file (0) == NULL)
> +  /* If the user did not explicitly specify an executable file
> +     then attempt to open the file that was executed to create
> +     this inferior.  */
> +  if (try_open_exec && !fake_pid_p && !user_supplied_exec_file_p)
>      exec_file_locate_attach (pid, 1);
>  
>    return inf;
> 

I realize that I am coming late to this discussion, sorry about that.

How does this interact with follow-exec-mode?  If follow-exec-mode is
'new' and the program execs, then 'run' will use the original executable
file.  But if follow-exec-mode is 'same' and the program execs, then
'run' will use the executable file that was active after the exec call.

In the follow-exec-mode == 'same' instance, is the assumption that the
exec'd executable file takes on the same 'user-supplied' attribute as
the initial executable, since it is using the original inferior?

If so, is there a scenario where:
 * the user supplies the exec file name
 * the program execs, so the exec file name is now different
 * then the user tries to do an attach (without an exec file name) to a
process running the original exec file, and gets the wrong exec file name?

Thanks
-Don
  

On Wed, May 6, 2015 at 3:26 AM, Gary Benson <gbenson@redhat.com> wrote:
> Hi all,
>
> In GDB some executable files are supplied by the user (e.g. using a
> "file" command) and some are determined by GDB (e.g. while processing
> an "attach" command).  GDB will not attempt to determine a filename if
> one has been set.  This causes problems if you attach to one process
> and then attach to another: GDB will not attempt to discover the main
> executable on the second attach.  If the two processes have different
> main executable files then the symbols will now be wrong.
>
> This commit updates GDB to keep track of which executable filenames
> were supplied by the user.  When GDB might attempt to determine an
> executable filename and one is already set, filenames determined by
> GDB may be overridden but user-supplied filenames will not.

Hi.

I can imagine sometimes wanting either behaviour, depending on
the situation.
E.g., if I supply a file name do some stuff, and then change
my mind or wish to investigate a difference process I may
wish gdb to automagically pick up the file name of the new process.
OTOH other times I may wish to override what gdb would
automagically choose and supply my own file name.

This suggests having an option to the command to choose
the behaviour one wants (I'd hate a global switch for this).
This doesn't have to be added today of course.
I only bring this up in case a behavioural change
is being introduced (it doesn't appear so, but I could be wrong),
in which case now is a good time to discuss it.

My main reason for sending this message, though, is that
I think a notification to the user is warranted here.
E.g., something like
"Using xyz for the symbol file name, since that's
what you specified. If this is wrong, please do [...]."
or whatever. Just something so that the user knows
gdb is not automagically picking the file.
  

On Wed, May 6, 2015 at 7:46 AM, Philippe Waroquiers
<philippe.waroquiers@skynet.be> wrote:
> On Wed, 2015-05-06 at 11:26 +0100, Gary Benson wrote:
>> Hi all,
>>
>> In GDB some executable files are supplied by the user (e.g. using a
>> "file" command) and some are determined by GDB (e.g. while processing
>> an "attach" command).  GDB will not attempt to determine a filename if
>> one has been set.  This causes problems if you attach to one process
>> and then attach to another: GDB will not attempt to discover the main
>> executable on the second attach.  If the two processes have different
>> main executable files then the symbols will now be wrong.
>>
>> This commit updates GDB to keep track of which executable filenames
>> were supplied by the user.  When GDB might attempt to determine an
>> executable filename and one is already set, filenames determined by
>> GDB may be overridden but user-supplied filenames will not.
> If not overriding the file set by the user, maybe GDB could/should give
> a warning when the exec-file reported by the target does not match the
> file as set by the user ?

Heh.  +1

[I don't have a strong opinion on how to perform the file matching test,
just that some notification should be given, especially if the files
in fact don't match.]
  

On 05/11/2015 09:23 PM, Doug Evans wrote:
> On Wed, May 6, 2015 at 3:26 AM, Gary Benson <gbenson@redhat.com> wrote:

>> This commit updates GDB to keep track of which executable filenames
>> were supplied by the user.  When GDB might attempt to determine an
>> executable filename and one is already set, filenames determined by
>> GDB may be overridden but user-supplied filenames will not.
> 
> I can imagine sometimes wanting either behaviour, depending on
> the situation.

Yeah, AFAICS, both examples you gave work the same before
and after Gary's patch.

> E.g., if I supply a file name do some stuff, and then change
> my mind or wish to investigate a difference process I may
> wish gdb to automagically pick up the file name of the new process.

In that case, one can use "file; attach PID".

That is, you can just unload the previous program, so that GDB picks
up the new one automatically on next attach.

Another way to handle that would be to leave the file loaded
in inferior 1, and switch to a new inferior to investigate
the other process.

> OTOH other times I may wish to override what gdb would
> automagically choose and supply my own file name.

That'd be "file PROGRAM; attach PID".

The difference is that with Gary's patch, "attach PID1; attach PID2"
just works, while today we don't even get a warning.
If for some odd reason, after Gary's patch, the user wants to force
the executable of PID1 on PID2, he can still do e.g.,
"info inferiors; file paste_program_name" before the second attach.


We don't have a command to get out of:

  "file wrong_program; attach PID"

that is, a command to issue after the attach to re-fetch the program
name from the running process, without detaching first.
Though if we had a warning that printed the expected program name,
the user could copy/paste from that.

Thanks,
Pedro Alves
  

Pedro Alves wrote:
> We don't have a command to get out of:
> 
>   "file wrong_program; attach PID"

file wrong_program; file; attach PID

Cheers,
Gary
  

On 05/12/2015 12:13 PM, Gary Benson wrote:
> Pedro Alves wrote:
>> We don't have a command to get out of:
>>
>>   "file wrong_program; attach PID"
> 
> file wrong_program; file; attach PID

That's different from what I was saying.  That is, a command
to fix things up _after_ the attach.

Thanks,
Pedro Alves
  

Pedro Alves wrote:
> On 05/12/2015 12:13 PM, Gary Benson wrote:
> > Pedro Alves wrote:
> > > We don't have a command to get out of:
> > >
> > >   "file wrong_program; attach PID"
> > 
> > file wrong_program; file; attach PID
> 
> That's different from what I was saying.  That is, a command
> to fix things up _after_ the attach.

Ah, ok, sorry.

A different question: is there any reason that we should not
*always* set the executable file from whatever the running
program is?

Cheers,
Gary
  

On 05/12/2015 02:48 PM, Gary Benson wrote:
> Pedro Alves wrote:
>> On 05/12/2015 12:13 PM, Gary Benson wrote:
>>> Pedro Alves wrote:
>>>> We don't have a command to get out of:
>>>>
>>>>   "file wrong_program; attach PID"
>>>
>>> file wrong_program; file; attach PID
>>
>> That's different from what I was saying.  That is, a command
>> to fix things up _after_ the attach.
> 
> Ah, ok, sorry.
> 
> A different question: is there any reason that we should not
> *always* set the executable file from whatever the running
> program is?

We need at least the section info, which may be missing in the
running file.

Thanks,
Pedro Alves
  

On Tue, May 12, 2015 at 3:36 AM, Pedro Alves <palves@redhat.com> wrote:
> On 05/11/2015 09:23 PM, Doug Evans wrote:
>> On Wed, May 6, 2015 at 3:26 AM, Gary Benson <gbenson@redhat.com> wrote:
>
>>> This commit updates GDB to keep track of which executable filenames
>>> were supplied by the user.  When GDB might attempt to determine an
>>> executable filename and one is already set, filenames determined by
>>> GDB may be overridden but user-supplied filenames will not.
>>
>> I can imagine sometimes wanting either behaviour, depending on
>> the situation.
>
> Yeah, AFAICS, both examples you gave work the same before
> and after Gary's patch.
>
>> E.g., if I supply a file name do some stuff, and then change
>> my mind or wish to investigate a difference process I may
>> wish gdb to automagically pick up the file name of the new process.
>
> In that case, one can use "file; attach PID".
>
> That is, you can just unload the previous program, so that GDB picks
> up the new one automatically on next attach.

I realize one *could* do that.
Thing is, someone's muscle memory may make them expect
"attach PID" to Just Work.
After all, "bash$ gdb" + "(gdb) attach PID" Just Works.

Plus that's two steps.
Why do I *have* to first type "file" with no arguments?
(Joe User may be thinking)
The difference in the two scenarios is explainable, but there's
still an incongruity here.

We go to lengths to reduce typing in the CLI session.
IWBN if one could type, say,
"attach -f PID" (f for "force gdb to use the binary of the attached process",
or whatever).
  

On Tue, May 12, 2015 at 3:36 AM, Pedro Alves <palves@redhat.com> wrote:
> Another way to handle that would be to leave the file loaded
> in inferior 1, and switch to a new inferior to investigate
> the other process.

Sorry, these things don't always occur to me before I hit Send.

Switching to a new inferior is three steps.
(gdb) add-inf
(gdb) infer 2
(gdb) attach PID

IWBN to have something like the following

(gdb) attach -n PID  # "n" for "in new inferior"

I kinda would rather do it differently, because it's more than
just "attach" where one would like to do something in a new inferior,
and IWBN to solve them all with one new command (or extension
of existing command).  E.g., "new-inferior <command>", as in
"new-inferior attach PID". Or some such.

OTOH, "new-inferior pwd" doesn't make much sense,
and "attach -n PID" is simple (we'd want to enumerate
all such commands and make sure the same option letter is
available for use in all of them).
  

Doug Evans wrote:
> On Tue, May 12, 2015 at 3:36 AM, Pedro Alves <palves@redhat.com> wrote:
> > On 05/11/2015 09:23 PM, Doug Evans wrote:
> > > On Wed, May 6, 2015 at 3:26 AM, Gary Benson <gbenson@redhat.com> wrote:
> > > > This commit updates GDB to keep track of which executable
> > > > filenames were supplied by the user.  When GDB might attempt
> > > > to determine an executable filename and one is already set,
> > > > filenames determined by GDB may be overridden but
> > > > user-supplied filenames will not.
> > >
> > > I can imagine sometimes wanting either behaviour, depending on
> > > the situation.
> >
> > Yeah, AFAICS, both examples you gave work the same before
> > and after Gary's patch.
> >
> > > E.g., if I supply a file name do some stuff, and then change
> > > my mind or wish to investigate a difference process I may
> > > wish gdb to automagically pick up the file name of the new
> > > process.
> >
> > In that case, one can use "file; attach PID".
> >
> > That is, you can just unload the previous program, so that GDB
> > picks up the new one automatically on next attach.
> 
> I realize one *could* do that.
> Thing is, someone's muscle memory may make them expect
> "attach PID" to Just Work.
> After all, "bash$ gdb" + "(gdb) attach PID" Just Works.
> 
> Plus that's two steps.
> Why do I *have* to first type "file" with no arguments?
> (Joe User may be thinking)
> The difference in the two scenarios is explainable, but there's
> still an incongruity here.
> 
> We go to lengths to reduce typing in the CLI session.
> IWBN if one could type, say,
> "attach -f PID" (f for "force gdb to use the binary of the attached
> process", or whatever).

I asked already, but nobody answered, so...

If you say "attach PID", and GDB can see that PID's executable is
/foo/bar, and the current exec-file is not /foo/bar/, under what
circumstances should GDB *not* automatically reload the new exec-
file?  i.e. why could this "attach -f" behavior not be the default?

Cheers,
Gary
  

On 05/12/2015 04:49 PM, Doug Evans wrote:
> On Tue, May 12, 2015 at 3:36 AM, Pedro Alves <palves@redhat.com> wrote:
>> On 05/11/2015 09:23 PM, Doug Evans wrote:
>>> On Wed, May 6, 2015 at 3:26 AM, Gary Benson <gbenson@redhat.com> wrote:
>>
>>>> This commit updates GDB to keep track of which executable filenames
>>>> were supplied by the user.  When GDB might attempt to determine an
>>>> executable filename and one is already set, filenames determined by
>>>> GDB may be overridden but user-supplied filenames will not.
>>>
>>> I can imagine sometimes wanting either behaviour, depending on
>>> the situation.
>>
>> Yeah, AFAICS, both examples you gave work the same before
>> and after Gary's patch.
>>
>>> E.g., if I supply a file name do some stuff, and then change
>>> my mind or wish to investigate a difference process I may
>>> wish gdb to automagically pick up the file name of the new process.
>>
>> In that case, one can use "file; attach PID".
>>
>> That is, you can just unload the previous program, so that GDB picks
>> up the new one automatically on next attach.
> 
> I realize one *could* do that.
> Thing is, someone's muscle memory may make them expect
> "attach PID" to Just Work.
> After all, "bash$ gdb" + "(gdb) attach PID" Just Works.
> 
> Plus that's two steps.
> Why do I *have* to first type "file" with no arguments?
> (Joe User may be thinking)
> The difference in the two scenarios is explainable, but there's
> still an incongruity here.
> 
> We go to lengths to reduce typing in the CLI session.
> IWBN if one could type, say,
> "attach -f PID" (f for "force gdb to use the binary of the attached process",
> or whatever).

We're kind of going on a tangent now.  While I agree
that streamlining the sequence of commands is desirable,
I don't think it fixes the issue with muscle memory you raise.
For the very same reason, you'll forget to use "attach -f PID"
instead of "attach PID".  A warning (or query but that may be
annoying) may be the best bet for that.

Thanks,
Pedro Alves
  

On 05/12/2015 05:03 PM, Doug Evans wrote:
> On Tue, May 12, 2015 at 3:36 AM, Pedro Alves <palves@redhat.com> wrote:
>> Another way to handle that would be to leave the file loaded
>> in inferior 1, and switch to a new inferior to investigate
>> the other process.
> 
> Sorry, these things don't always occur to me before I hit Send.
> 
> Switching to a new inferior is three steps.
> (gdb) add-inf
> (gdb) infer 2
> (gdb) attach PID
> 
> IWBN to have something like the following
> 
> (gdb) attach -n PID  # "n" for "in new inferior"

I agree here.  It's actually one thing that has been crossing
my mind recently.  I've been experimenting a lot with gdb's CLI around
multi-thread and multi-inferior things, in context of
i/t sets (see [1]), and making it possible to have "attach" create
new inferiors would help.  E.g., attaching to more than one process
would be nice, like "attach PID1 PID2" and "attach --all-children PID".
Maybe even make "attach PID" automatically decide to reuse the
same inferior, so that gdb; attach" reuses inferior 1, but if you've
already used inferior 1, "attach" creates another one.
We do something like that when "target extended-remote" finds multiple
processes already running on the server side.

For preparing a new inferior for "run", we
have "add-inferior -exec FILE", though maybe "file -n" would
be nicer.

[1] https://github.com/palves/gdb/commits/palves/itsets-wip-width
    (note: very experimental, not ready for general feedback yet)
> 
> I kinda would rather do it differently, because it's more than
> just "attach" where one would like to do something in a new inferior,
> and IWBN to solve them all with one new command (or extension
> of existing command).  E.g., "new-inferior <command>", as in
> "new-inferior attach PID". Or some such.

Guess that could be the existing "add-inferior", as in
"add-inferior -exec PROG attach PID".  Not really sure I
like that direction though.  Seems less convenient and more
prone to have users forget/miss this usage than teaching
"attach" etc. directly to me.

In an i/t sets world, another option would be for commands
that create new inferiors to set the current focus to the just
created inferior(s).  Then the following commands would simply
apply to the new inferiors.

> 
> OTOH, "new-inferior pwd" doesn't make much sense,
> and "attach -n PID" is simple (we'd want to enumerate
> all such commands and make sure the same option letter is
> available for use in all of them).

Yeah.

Thanks,
Pedro Alves
  

On 05/13/2015 08:54 AM, Gary Benson wrote:

> I asked already, but nobody answered, so...

I did reply :-)  Here:

 https://sourceware.org/ml/gdb-patches/2015-05/msg00273.html

> 
> If you say "attach PID", and GDB can see that PID's executable is
> /foo/bar, and the current exec-file is not /foo/bar/, under what
> circumstances should GDB *not* automatically reload the new exec-
> file?  

For the symbol-file part, it's clear: we need the debug info
that might well be missing on the running copy of the binary
(I know that's not what you're asking).

When the running image does not have all the info we need out
of the executable (in which case you'll have passed a non-stripped
binary to gdb).  I pointed out section info, as that's what I thought
of off hand.  There may be other bits needed.  Grepping around for
exec_bfd may find more.  Of course, determining whether the
file has the necessary bits requires downloading/opening
the file, which is something the user may want to avoid.

> i.e. why could this "attach -f" behavior not be the default?

If we come up with means to force usage of the current
exec, maybe.  Though maybe we're trying to make gdb
too smart, as in, some obscure cases things may go wrong,
depending on program/binary and target you connect to,
which is confusing.  The "user-specified==sticky" idea seems
simpler to explain to users to me.  If GDB warned on exec-file vs
running image mismatch, I think the default would matter less.

Thanks,
Pedro Alves
  

> Though maybe we're trying to make gdb
> too smart, as in, some obscure cases things may go wrong,
> depending on program/binary and target you connect to,
> which is confusing.

After quickly going through the discussion, I tend to agree.
How about:
  (1) Provide a way to recover the situation _after_ the "attach";
  (2) Print a warning if the attach notices that the executable name
      does not match the one given by the user, and include the
      procedure for fixing it if the user made an error.
?

We could do this in two steps:

  a. Push Gary's patch today, after having enhanced it to print
     a warning when discrepancies are discovered;

  b. Work on a way to achieve (1), and then enhance the warning
     to mention how to resolve this issue.

That way, we could have (a) in time for 7.10, and work for (b)
without delaying 7.10. Whether (b) makes it to 7.10 will then
depend on how quickly we find a solution, but it's OK IMO for it
to provide that in a later release.
  

Don Breazeal wrote:
> On 5/6/2015 3:26 AM, Gary Benson wrote:
> > In GDB some executable files are supplied by the user (e.g. using
> > a "file" command) and some are determined by GDB (e.g. while
> > processing an "attach" command).  GDB will not attempt to
> > determine a filename if one has been set.  This causes problems if
> > you attach to one process and then attach to another: GDB will not
> > attempt to discover the main executable on the second attach.  If
> > the two processes have different main executable files then the
> > symbols will now be wrong.
> > 
> > This commit updates GDB to keep track of which executable
> > filenames were supplied by the user.  When GDB might attempt to
> > determine an executable filename and one is already set, filenames
> > determined by GDB may be overridden but user-supplied filenames
> > will not.
> 
> I realize that I am coming late to this discussion, sorry about
> that.

Likewise, sorry for taking so long to reply :)

> How does this interact with follow-exec-mode?  If follow-exec-mode
> is 'new' and the program execs, then 'run' will use the original
> executable file.  But if follow-exec-mode is 'same' and the program
> execs, then 'run' will use the executable file that was active after
> the exec call.
> 
> In the follow-exec-mode == 'same' instance, is the assumption that
> the exec'd executable file takes on the same 'user-supplied'
> attribute as the initial executable, since it is using the original
> inferior?
> 
> If so, is there a scenario where:
>  * the user supplies the exec file name
>  * the program execs, so the exec file name is now different
>  * then the user tries to do an attach (without an exec file name) to a
> process running the original exec file, and gets the wrong exec file name?

I'm not sure.  Where would I need to look to check this out?
(Where is the bit that updates the filename after exec?)

Cheers,
Gary
  

On 6/5/2015 2:37 AM, Gary Benson wrote:
> Don Breazeal wrote:
>> On 5/6/2015 3:26 AM, Gary Benson wrote:
>>> In GDB some executable files are supplied by the user (e.g. using
>>> a "file" command) and some are determined by GDB (e.g. while
>>> processing an "attach" command).  GDB will not attempt to
>>> determine a filename if one has been set.  This causes problems if
>>> you attach to one process and then attach to another: GDB will not
>>> attempt to discover the main executable on the second attach.  If
>>> the two processes have different main executable files then the
>>> symbols will now be wrong.
>>>
>>> This commit updates GDB to keep track of which executable
>>> filenames were supplied by the user.  When GDB might attempt to
>>> determine an executable filename and one is already set, filenames
>>> determined by GDB may be overridden but user-supplied filenames
>>> will not.
>>
>> I realize that I am coming late to this discussion, sorry about
>> that.
> 
> Likewise, sorry for taking so long to reply :)
> 
>> How does this interact with follow-exec-mode?  If follow-exec-mode
>> is 'new' and the program execs, then 'run' will use the original
>> executable file.  But if follow-exec-mode is 'same' and the program
>> execs, then 'run' will use the executable file that was active after
>> the exec call.
>>
>> In the follow-exec-mode == 'same' instance, is the assumption that
>> the exec'd executable file takes on the same 'user-supplied'
>> attribute as the initial executable, since it is using the original
>> inferior?
>>
>> If so, is there a scenario where:
>>  * the user supplies the exec file name
>>  * the program execs, so the exec file name is now different
>>  * then the user tries to do an attach (without an exec file name) to a
>> process running the original exec file, and gets the wrong exec file name?
> 
> I'm not sure.  Where would I need to look to check this out?
> (Where is the bit that updates the filename after exec?)
> 
Hi Gary
I think it goes like this: infrun.c:follow_exec calls
exec.c:exec_file_attach, which updates the name of the executable.

Sorry I haven't taken the time to review your patch to see how
it affects this.  Hopefully it will be obvious to you.
Thanks
--Don
  

Don Breazeal wrote:
> On 6/5/2015 2:37 AM, Gary Benson wrote:
> > Don Breazeal wrote:
> > > On 5/6/2015 3:26 AM, Gary Benson wrote:
> > > > In GDB some executable files are supplied by the user
> > > > (e.g. using a "file" command) and some are determined by GDB
> > > > (e.g. while processing an "attach" command).  GDB will not
> > > > attempt to determine a filename if one has been set.  This
> > > > causes problems if you attach to one process and then attach
> > > > to another: GDB will not attempt to discover the main
> > > > executable on the second attach.  If the two processes have
> > > > different main executable files then the symbols will now be
> > > > wrong.
> > > >
> > > > This commit updates GDB to keep track of which executable
> > > > filenames were supplied by the user.  When GDB might attempt
> > > > to determine an executable filename and one is already set,
> > > > filenames determined by GDB may be overridden but
> > > > user-supplied filenames will not.
> > > 
> > > How does this interact with follow-exec-mode?  If
> > > follow-exec-mode is 'new' and the program execs, then 'run' will
> > > use the original executable file.  But if follow-exec-mode is
> > > 'same' and the program execs, then 'run' will use the executable
> > > file that was active after the exec call.
> > >
> > > In the follow-exec-mode == 'same' instance, is the assumption
> > > that the exec'd executable file takes on the same
> > > 'user-supplied' attribute as the initial executable, since it is
> > > using the original inferior?
> > >
> > > If so, is there a scenario where:
> > >  * the user supplies the exec file name
> > >  * the program execs, so the exec file name is now different
> > >  * then the user tries to do an attach (without an exec file name)
> > >    to a process running the original exec file, and gets the wrong
> > >    exec file name?
> > 
> > I'm not sure.  Where would I need to look to check this out?
> > (Where is the bit that updates the filename after exec?)
> 
> I think it goes like this: infrun.c:follow_exec calls
> exec.c:exec_file_attach, which updates the name of the executable.

Ah, there is exactly the scenario you describe Don, good call.
Joel, I can fix v3 of this patch to zero exec_file_is_user_supplied
before the exec_file_attach in follow_exec.

I'm not convinced this patch will not introduce new bugs, the whole
handling of how/where executable and/or symbol files get changed
seems... haphazard :)  That's mainly why I'd put this patch on the
back burner.

I'm on the fence as to whether this should be committed, so I'll
defer to you Joel.  If you say commit I'll re-test and push.

Cheers,
Gary
  

> > I think it goes like this: infrun.c:follow_exec calls
> > exec.c:exec_file_attach, which updates the name of the executable.
> 
> Ah, there is exactly the scenario you describe Don, good call.
> Joel, I can fix v3 of this patch to zero exec_file_is_user_supplied
> before the exec_file_attach in follow_exec.
> 
> I'm not convinced this patch will not introduce new bugs, the whole
> handling of how/where executable and/or symbol files get changed
> seems... haphazard :)  That's mainly why I'd put this patch on the
> back burner.
> 
> I'm on the fence as to whether this should be committed, so I'll
> defer to you Joel.  If you say commit I'll re-test and push.

Thanks, Gary.

Let's not commit a risky patch just before branching, especially
for something that's not a regression. So, let's branch first and
fix on master afterwards. Later on, if the fix proves solid, and
backporting is an option, then we can backport for 7.10 or 7.10.1.
  

On Fri, Jul 03, 2015 at 12:14:18PM +0100, Gary Benson wrote:
> Don Breazeal wrote:
> > On 6/5/2015 2:37 AM, Gary Benson wrote:
> > > Don Breazeal wrote:
> > > > On 5/6/2015 3:26 AM, Gary Benson wrote:
> > > > > In GDB some executable files are supplied by the user
> > > > > (e.g. using a "file" command) and some are determined by GDB
> > > > > (e.g. while processing an "attach" command).  GDB will not
> > > > > attempt to determine a filename if one has been set.  This
> > > > > causes problems if you attach to one process and then attach
> > > > > to another: GDB will not attempt to discover the main
> > > > > executable on the second attach.  If the two processes have
> > > > > different main executable files then the symbols will now be
> > > > > wrong.
> > > > >
> > > > > This commit updates GDB to keep track of which executable
> > > > > filenames were supplied by the user.  When GDB might attempt
> > > > > to determine an executable filename and one is already set,
> > > > > filenames determined by GDB may be overridden but
> > > > > user-supplied filenames will not.
> > > > 
> > > > How does this interact with follow-exec-mode?  If
> > > > follow-exec-mode is 'new' and the program execs, then 'run' will
> > > > use the original executable file.  But if follow-exec-mode is
> > > > 'same' and the program execs, then 'run' will use the executable
> > > > file that was active after the exec call.
Hi Gary,

> > > > In the follow-exec-mode == 'same' instance, is the assumption
> > > > that the exec'd executable file takes on the same
> > > > 'user-supplied' attribute as the initial executable, since it is
> > > > using the original inferior?
> > > >
> > > > If so, is there a scenario where:
> > > >  * the user supplies the exec file name
> > > >  * the program execs, so the exec file name is now different
> > > >  * then the user tries to do an attach (without an exec file name)
> > > >    to a process running the original exec file, and gets the wrong
> > > >    exec file name?
> > > 
> > > I'm not sure.  Where would I need to look to check this out?
> > > (Where is the bit that updates the filename after exec?)
> > 
> > I think it goes like this: infrun.c:follow_exec calls
> > exec.c:exec_file_attach, which updates the name of the executable.
> 
> Ah, there is exactly the scenario you describe Don, good call.
> Joel, I can fix v3 of this patch to zero exec_file_is_user_supplied
> before the exec_file_attach in follow_exec.
> 
> I'm not convinced this patch will not introduce new bugs, the whole
> handling of how/where executable and/or symbol files get changed
> seems... haphazard :)  That's mainly why I'd put this patch on the
> back burner.
> 
> I'm on the fence as to whether this should be committed, so I'll
> defer to you Joel.  If you say commit I'll re-test and push.

Now that the branch has been cut, I'm thinking it would be a good
to start getting the ball rolling again. WDYT?