qXfer:exec-file:read and non multiprocess target

qXfer:exec-file:read and non multiprocess target

[Philippe Waroquiers]

I am busy adding qXfer:exec-file:read  to the Valgrind gdbserver.

Even if Valgrind reports it supports qXfer:exec-file, 
GDB does not query it.
This is due to the fact that GDB does not query the exec-file when 
the pid is a fake pid, which is the case for Valgrind, as the target
command to use is:
   target remote | vgdb

The following change in remote.c ensures GDB queries the
remote exec-file:
1561,1562c1561,1562
<   if (try_open_exec && !fake_pid_p && get_exec_file (0) == NULL)
<     exec_file_locate_attach (pid, 1);
---
>   if (try_open_exec && get_exec_file (0) == NULL)
>     exec_file_locate_attach (fake_pid_p ? 0 : pid, 1);

With that change, GDB can use a Valgrind target without having
to specify the exec file.
The idea is that when the stub gets a pid 0 in this request, it
replies with the exec file of the current process.

I have not yet investigated a remaining problem:

if the same GDB does first a
  target remote|vgdb 
and gets as exec-file   firstexecfile,
then after the first target has terminated,
a second target remote|vgdb for another
process does not re-query the exec file : GDB uses the
first exec file, even if the second target has another file.

Feedback about allowing the exec-file of a fake pid to be queried ?

Philippe

Re: qXfer:exec-file:read and non multiprocess target

[Gary Benson]

Philippe Waroquiers wrote:
> I am busy adding qXfer:exec-file:read  to the Valgrind gdbserver.
> 
> Even if Valgrind reports it supports qXfer:exec-file, GDB does
> not query it.  This is due to the fact that GDB does not query
> the exec-file when the pid is a fake pid, which is the case for
> Valgrind, as the target command to use is:
>    target remote | vgdb
> 
> The following change in remote.c ensures GDB queries the
> remote exec-file:
> 1561,1562c1561,1562
> <   if (try_open_exec && !fake_pid_p && get_exec_file (0) == NULL)
> <     exec_file_locate_attach (pid, 1);
> ---
> >   if (try_open_exec && get_exec_file (0) == NULL)
> >     exec_file_locate_attach (fake_pid_p ? 0 : pid, 1);
> 
> With that change, GDB can use a Valgrind target without having
> to specify the exec file.  The idea is that when the stub gets
> a pid 0 in this request, it replies with the exec file of the
> current process.

The PID is fake because vgdb does not support multiprocess extensions.
I don't like sending a fake/zero PID over the wire, but how about I
change qXfer:exec-file:read to send a NULL annex if the remote does
not have multiprocess extensions?  Can you make your side work with
the patch inlined below?  If so I'll tidy and document it and submit
it for review.

Thanks,
Gary

-- 
diff --git a/gdb/remote.c b/gdb/remote.c
index 099ddbb..42d990a 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1558,7 +1558,7 @@
 
   /* 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 (try_open_exec && get_exec_file (0) == NULL)
     exec_file_locate_attach (pid, 1);
 
   return inf;
@@ -11710,7 +11710,7 @@
 remote_pid_to_exec_file (struct target_ops *self, int pid)
 {
   static char *filename = NULL;
-  char annex[9];
+  char *annex = NULL;
 
   if (packet_support (PACKET_qXfer_exec_file) != PACKET_ENABLE)
     return NULL;
@@ -11718,7 +11718,14 @@
   if (filename != NULL)
     xfree (filename);
 
-  xsnprintf (annex, sizeof (annex), "%x", pid);
+  if (remote_multi_process_p (get_remote_state ()))
+    {
+      const int annex_size = 9;
+
+      annex = alloca (annex_size);
+      xsnprintf (annex, annex_size, "%x", pid);
+    }
+
   filename = target_read_stralloc (&current_target,
 				   TARGET_OBJECT_EXEC_FILE, annex);
 

Re: qXfer:exec-file:read and non multiprocess target

[Gary Benson]

Philippe Waroquiers wrote:
> I have not yet investigated a remaining problem:
> 
> if the same GDB does first a
>   target remote|vgdb 
> and gets as exec-file   firstexecfile,
> then after the first target has terminated,
> a second target remote|vgdb for another
> process does not re-query the exec file : GDB uses the
> first exec file, even if the second target has another file.

I'm working on a patch for this, will Cc you when I mail it.

Thanks,
Gary

-- 
http://gbenson.net/

Re: qXfer:exec-file:read and non multiprocess target

[Philippe Waroquiers]

On Tue, 2015-05-05 at 12:02 +0100, Gary Benson wrote:
> The PID is fake because vgdb does not support multiprocess extensions.
> I don't like sending a fake/zero PID over the wire, but how about I
> change qXfer:exec-file:read to send a NULL annex if the remote does
> not have multiprocess extensions?  Can you make your side work with
> the patch inlined below?  If so I'll tidy and document it and submit
> it for review.
It looks effectively better to not send the fake or 0 pid
(e.g. similar to the qAttached packet).

The valgrind gdbserver side works ok with the patch to send an empty
annex.
This is the resulting exchange, traced at Valgrind side:
  ...
  --4980:3:  gdbsrv     getpkt ("qAttached");  [no ack] 
  --4980:3:  gdbsrv     putpkt ("$1#31"); [no ack]
  --4980:3:  gdbsrv     getpkt ("qXfer:exec-file:read::0,fff");  [no ack] 
  --4980:3:  gdbsrv     putpkt ("$l/home/philippe/valgrind/better_stats/memcheck/tests/trivialleak#2c"); [no ack]
  --4980:3:  gdbsrv     getpkt ("vFile:open:2f686f6d652f7068696c697070652f76616c6772696e642f6265747465725f73746174732f6d656d636865636b2f74657374732f7472697669616c6c65616b,0,0");  [no ack] 
  --4980:3:  gdbsrv     putpkt ("$#00"); [no ack]
  ...

and then GDB could properly use the returned exec-file
(even if vFile is not supported by Valgrind gdbserver).

Thanks

Philippe

[PATCH] Make only user-specified executable filenames sticky

[Gary Benson]

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

Re: qXfer:exec-file:read and non multiprocess target

[Gary Benson]

Philippe Waroquiers wrote:
> On Tue, 2015-05-05 at 12:02 +0100, Gary Benson wrote:
> > The PID is fake because vgdb does not support multiprocess
> > extensions.  I don't like sending a fake/zero PID over the wire,
> > but how about I change qXfer:exec-file:read to send a NULL annex
> > if the remote does not have multiprocess extensions?  Can you make
> > your side work with the patch inlined below?  If so I'll tidy and
> > document it and submit it for review.
> 
> It looks effectively better to not send the fake or 0 pid
> (e.g. similar to the qAttached packet).
> 
> The valgrind gdbserver side works ok with the patch to send an empty
> annex.  This is the resulting exchange, traced at Valgrind side:
>   ...
>   --4980:3:  gdbsrv     getpkt ("qAttached");  [no ack] 
>   --4980:3:  gdbsrv     putpkt ("$1#31"); [no ack]
>   --4980:3:  gdbsrv     getpkt ("qXfer:exec-file:read::0,fff");  [no ack] 
>   --4980:3:  gdbsrv     putpkt ("$l/home/philippe/valgrind/better_stats/memcheck/tests/trivialleak#2c"); [no ack]
>   --4980:3:  gdbsrv     getpkt ("vFile:open:2f686f6d652f7068696c697070652f76616c6772696e642f6265747465725f73746174732f6d656d636865636b2f74657374732f7472697669616c6c65616b,0,0");  [no ack] 
>   --4980:3:  gdbsrv     putpkt ("$#00"); [no ack]
>   ...

Great, I will get that turned into a proper patch today.

> and then GDB could properly use the returned exec-file
> (even if vFile is not supported by Valgrind gdbserver).

Yes, there is a special workaround just for vgdb ;)

Cheers,
Gary

-- 
http://gbenson.net/

Re: [PATCH] Make only user-specified executable filenames sticky

[Pedro Alves]

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

Re: [PATCH] Make only user-specified executable filenames sticky

[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

Re: [PATCH] Make only user-specified executable filenames sticky

[Philippe Waroquiers]

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

Re: [PATCH] Make only user-specified executable filenames sticky

[Gary Benson]

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

-- 
http://gbenson.net/

Re: [PATCH] Make only user-specified executable filenames sticky

[Gary Benson]

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

-- 
http://gbenson.net/

[PATCH] Locate executables on remote stubs without multiprocess extensions

[Gary Benson]

Hi all,

This commit allows GDB to determine filenames of main executables
when debugging using remote stubs without multiprocess extensions.
The qXfer:exec-file:read packet is extended to allow an empty
annex, with the meaning that the remote stub should supply the
filename of whatever it thinks is the current process.

Built and regtested on RHEL6.6 x86_64.

Is this ok to commit?

Cheers,
Gary


gdb/ChangeLog:

	* remote.c (remote_add_inferior): Call exec_file_locate_attach
	for fake PIDs as well as real ones.
	(remote_pid_to_exec_file): Send empty annex if PID is fake.

gdb/doc/ChangeLog:

	* gdb.texinfo (General Query Packets): Document
	qXfer:exec-file:read with empty annex.

gdb/gdbserver/ChangeLog:

	* server.c (handle_qxfer_exec_file): Use current process
	if annex is empty.
---
 gdb/ChangeLog           |    6 ++++++
 gdb/doc/ChangeLog       |    5 +++++
 gdb/doc/gdb.texinfo     |    3 ++-
 gdb/gdbserver/ChangeLog |    5 +++++
 gdb/gdbserver/server.c  |   25 ++++++++++++++++++++-----
 gdb/remote.c            |   15 ++++++++++++---
 6 files changed, 50 insertions(+), 9 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 9e2787d..63e063a 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -36558,7 +36558,8 @@ by supplying an appropriate @samp{qSupported} response (@pxref{qSupported}).
 Return the full absolute name of the file that was executed to create
 a process running on the remote system.  The annex specifies the
 numeric process ID of the process to query, encoded as a hexadecimal
-number.
+number.  If the annex part is empty the remote stub should return the
+filename corresponding to the currently executing process.
 
 This packet is not probed by default; the remote stub must request it,
 by supplying an appropriate @samp{qSupported} response (@pxref{qSupported}).
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index d2e20d9..516a311 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -1144,17 +1144,32 @@ handle_qxfer_exec_file (const char *const_annex,
 			gdb_byte *readbuf, const gdb_byte *writebuf,
 			ULONGEST offset, LONGEST len)
 {
-  char *annex, *file;
+  char *file;
   ULONGEST pid;
   int total_len;
 
   if (the_target->pid_to_exec_file == NULL || writebuf != NULL)
     return -2;
 
-  annex = alloca (strlen (const_annex) + 1);
-  strcpy (annex, const_annex);
-  annex = unpack_varlen_hex (annex, &pid);
-  if (annex[0] != '\0' || pid == 0)
+  if (const_annex[0] == '\0')
+    {
+      if (current_thread == NULL)
+	return -1;
+
+      pid = pid_of (current_thread);
+    }
+  else
+    {
+      char *annex = alloca (strlen (const_annex) + 1);
+
+      strcpy (annex, const_annex);
+      annex = unpack_varlen_hex (annex, &pid);
+
+      if (annex[0] != '\0')
+	return -1;
+    }
+
+  if (pid < 0)
     return -1;
 
   file = (*the_target->pid_to_exec_file) (pid);
diff --git a/gdb/remote.c b/gdb/remote.c
index 099ddbb..6b18960 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1558,7 +1558,7 @@ remote_add_inferior (int fake_pid_p, int pid, int attached,
 
   /* 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 (try_open_exec && get_exec_file (0) == NULL)
     exec_file_locate_attach (pid, 1);
 
   return inf;
@@ -11710,7 +11710,8 @@ static char *
 remote_pid_to_exec_file (struct target_ops *self, int pid)
 {
   static char *filename = NULL;
-  char annex[9];
+  struct inferior *inf;
+  char *annex = NULL;
 
   if (packet_support (PACKET_qXfer_exec_file) != PACKET_ENABLE)
     return NULL;
@@ -11718,7 +11719,15 @@ remote_pid_to_exec_file (struct target_ops *self, int pid)
   if (filename != NULL)
     xfree (filename);
 
-  xsnprintf (annex, sizeof (annex), "%x", pid);
+  inf = find_inferior_pid (pid);
+  if (inf != NULL && !inf->fake_pid_p)
+    {
+      const int annex_size = 9;
+
+      annex = alloca (annex_size);
+      xsnprintf (annex, annex_size, "%x", pid);
+    }
+
   filename = target_read_stralloc (&current_target,
 				   TARGET_OBJECT_EXEC_FILE, annex);
 
-- 
1.7.1

Re: [PATCH] Locate executables on remote stubs without multiprocess extensions

[Eli Zaretskii]

> From: Gary Benson <gbenson@redhat.com>
> Cc: Philippe Waroquiers <philippe.waroquiers@skynet.be>
> Date: Wed,  6 May 2015 18:10:30 +0100
> 
> This commit allows GDB to determine filenames of main executables
> when debugging using remote stubs without multiprocess extensions.
> The qXfer:exec-file:read packet is extended to allow an empty
> annex, with the meaning that the remote stub should supply the
> filename of whatever it thinks is the current process.
> 
> Built and regtested on RHEL6.6 x86_64.
> 
> Is this ok to commit?
> 
> Cheers,
> Gary
> 
> 
> gdb/ChangeLog:
> 
> 	* remote.c (remote_add_inferior): Call exec_file_locate_attach
> 	for fake PIDs as well as real ones.
> 	(remote_pid_to_exec_file): Send empty annex if PID is fake.
> 
> gdb/doc/ChangeLog:
> 
> 	* gdb.texinfo (General Query Packets): Document
> 	qXfer:exec-file:read with empty annex.
> 
> gdb/gdbserver/ChangeLog:
> 
> 	* server.c (handle_qxfer_exec_file): Use current process
> 	if annex is empty.

The documentation part is OK.

Thanks.

Re: [PATCH] Locate executables on remote stubs without multiprocess extensions

[Gary Benson]

Gary Benson wrote:
> diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
> index d2e20d9..516a311 100644
> --- a/gdb/gdbserver/server.c
> +++ b/gdb/gdbserver/server.c
> @@ -1144,17 +1144,32 @@ handle_qxfer_exec_file (const char *const_annex,
>  			gdb_byte *readbuf, const gdb_byte *writebuf,
>  			ULONGEST offset, LONGEST len)
>  {
> -  char *annex, *file;
> +  char *file;
>    ULONGEST pid;
>    int total_len;
>  
>    if (the_target->pid_to_exec_file == NULL || writebuf != NULL)
>      return -2;
>  
> -  annex = alloca (strlen (const_annex) + 1);
> -  strcpy (annex, const_annex);
> -  annex = unpack_varlen_hex (annex, &pid);
> -  if (annex[0] != '\0' || pid == 0)
> +  if (const_annex[0] == '\0')
> +    {
> +      if (current_thread == NULL)
> +	return -1;
> +
> +      pid = pid_of (current_thread);
> +    }
> +  else
> +    {
> +      char *annex = alloca (strlen (const_annex) + 1);
> +
> +      strcpy (annex, const_annex);
> +      annex = unpack_varlen_hex (annex, &pid);
> +
> +      if (annex[0] != '\0')
> +	return -1;
> +    }
> +
> +  if (pid < 0)
>      return -1;

Oops, this should be "<=".

Cheers,
Gary

--
http://gbenson.net/

Re: [PATCH] Make only user-specified executable filenames sticky

[Pedro Alves]

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

Re: [PATCH] Make only user-specified executable filenames sticky

[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

Re: [PATCH] Locate executables on remote stubs without multiprocess extensions

[Pedro Alves]

On 05/06/2015 06:16 PM, Gary Benson wrote:
> Gary Benson wrote:

> @@ -11718,7 +11719,15 @@ remote_pid_to_exec_file (struct target_ops *self, int pid)
>    if (filename != NULL)
>      xfree (filename);
>
> -  xsnprintf (annex, sizeof (annex), "%x", pid);
> +  inf = find_inferior_pid (pid);
> +  if (inf != NULL && !inf->fake_pid_p)

This will silently do the wrong thing (retrieve the exec file
of the server's current thread/process) if this method is ever
used to try to fetch the exec out of a process that we're
_not_ currently attached to.  Maybe this should be:

  if (inf == NULL)
    internal_error (__FILE__, __LINE__,
                    "attempt to retrieve exec-file of not-debugged process");
  if (!inf->fake_pid_p)

>> diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
>> index d2e20d9..516a311 100644
>> --- a/gdb/gdbserver/server.c
>> +++ b/gdb/gdbserver/server.c
>> @@ -1144,17 +1144,32 @@ handle_qxfer_exec_file (const char *const_annex,
>>  			gdb_byte *readbuf, const gdb_byte *writebuf,
>>  			ULONGEST offset, LONGEST len)
>>  {
>> -  char *annex, *file;
>> +  char *file;
>>    ULONGEST pid;
>>    int total_len;
>>  
>>    if (the_target->pid_to_exec_file == NULL || writebuf != NULL)
>>      return -2;
>>  
>> -  annex = alloca (strlen (const_annex) + 1);
>> -  strcpy (annex, const_annex);
>> -  annex = unpack_varlen_hex (annex, &pid);
>> -  if (annex[0] != '\0' || pid == 0)
>> +  if (const_annex[0] == '\0')
>> +    {
>> +      if (current_thread == NULL)
>> +	return -1;
>> +
>> +      pid = pid_of (current_thread);
>> +    }
>> +  else
>> +    {
>> +      char *annex = alloca (strlen (const_annex) + 1);
>> +
>> +      strcpy (annex, const_annex);
>> +      annex = unpack_varlen_hex (annex, &pid);
>> +
>> +      if (annex[0] != '\0')
>> +	return -1;
>> +    }
>> +
>> +  if (pid < 0)
>>      return -1;
> 
> Oops, this should be "<=".

This is OK with that change and the point above addressed.

Thanks,
Pedro Alves

Re: [PATCH] Make only user-specified executable filenames sticky

[Don Breazeal]

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

Re: [PATCH] Make only user-specified executable filenames sticky

[Doug Evans]

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.

Re: [PATCH] Make only user-specified executable filenames sticky

[Doug Evans]

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

Re: [PATCH] Make only user-specified executable filenames sticky

[Pedro Alves]

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

Re: [PATCH] Locate executables on remote stubs without multiprocess extensions

[Gary Benson]

Pedro Alves wrote:
> On 05/06/2015 06:16 PM, Gary Benson wrote:
> > Gary Benson wrote:
> > @@ -11718,7 +11719,15 @@ remote_pid_to_exec_file
> >    if (filename != NULL)
> >      xfree (filename);
> >
> > -  xsnprintf (annex, sizeof (annex), "%x", pid);
> > +  inf = find_inferior_pid (pid);
> > +  if (inf != NULL && !inf->fake_pid_p)
> 
> This will silently do the wrong thing (retrieve the exec file of
> the server's current thread/process) if this method is ever used
> to try to fetch the exec out of a process that we're _not_ currently
> attached to.  Maybe this should be:
> 
>   if (inf == NULL)
>     internal_error (__FILE__, __LINE__,
>                     "attempt to retrieve exec-file of not-debugged process");
>   if (!inf->fake_pid_p)

Good catch, thanks!

> > > @@ -1144,17 +1144,32 @@ handle_qxfer_exec_file
[snip]
> > > +
> > > +  if (pid < 0)
> > >      return -1;
> > 
> > Oops, this should be "<=".
> 
> This is OK with that change and the point above addressed.

Ok, I've pushed this, thanks Pedro and Eli for the reviews.

Cheers,
Gary

-- 
http://gbenson.net/

Re: [PATCH] Make only user-specified executable filenames sticky

[Gary Benson]

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

-- 
http://gbenson.net/

Re: [PATCH] Make only user-specified executable filenames sticky

[Pedro Alves]

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

Re: [PATCH] Make only user-specified executable filenames sticky

[Gary Benson]

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

-- 
http://gbenson.net/

Re: [PATCH] Make only user-specified executable filenames sticky

[Pedro Alves]

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

Re: [PATCH] Make only user-specified executable filenames sticky

[Doug Evans]

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

Re: [PATCH] Make only user-specified executable filenames sticky

[Doug Evans]

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

Re: [PATCH] Make only user-specified executable filenames sticky

[Gary Benson]

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

-- 
http://gbenson.net/

Re: [PATCH] Make only user-specified executable filenames sticky

[Pedro Alves]

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

Re: [PATCH] Make only user-specified executable filenames sticky

[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

Re: [PATCH] Make only user-specified executable filenames sticky

[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

Re: [PATCH] Make only user-specified executable filenames sticky

[Joel Brobecker]

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

-- 
Joel

Re: [PATCH] Make only user-specified executable filenames sticky

[Gary Benson]

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

-- 
http://gbenson.net/

[PATCH v2] Make only user-specified executable and symbol filenames sticky

[Gary Benson]

Joel Brobecker wrote:
> 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.

Ok, in the interests of getting this into 7.10, here's an updated
patch.  I'm not sure I've addressed everything but I think it's
what you're after right now.  Main changes from v1 are:

 - I renamed user_supplied_exec_file_p as exec_file_is_user_supplied
   so it has the same "exec_file_" prefix as other exec-file-related
   values.

 - I introduced another field, symfile_objfile_is_user_supplied,
   which does much the same as exec_file_is_user_supplied but for
   the symbol file.

 - I added warnings if GDB detects that the wrong filename is being
   used for either the executable file or the symbol file.

Don's query about follow-exec-mode is still outstanding (I don't know
where to look to check this).

Built and regtested on RHEL6.6 x86_64.

Should I commit?

Thanks,
Gary

--
On attach, GDB will only attempt to determine the main executable's
filename if one is not already 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 is PR gdb/17626.

In GDB some filenames are supplied by the user (e.g. using the "file"
or "symbol-file" commands) and some are determined by GDB (e.g. while
processing an "attach" command).  This commit updates GDB to track
which 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.

gdb/ChangeLog:

	PR gdb/17626
	* progspace.h (struct program_space)
	<pspace_exec_file_is_user_supplied>: New field.
	<symfile_object_file_is_user_supplied>: Likewise.
	(symfile_objfile_is_user_supplied): New macro.
	* exec.h (exec_file_is_user_supplied): Likewise.
	* exec.c (exec_close): Clear exec_file_is_user_supplied.
	(exec_file_locate_attach): Remove get_exec_file check.
	Do not replace user-supplied executable or symbol files.
	Warn if user-supplied executable or symbol files do not
	match discovered file.
	(exec_file_command): Set exec_file_is_user_supplied.
	* symfile.c (symbol_file_clear): Clear
	symfile_objfile_is_user_supplied.
	(symbol_file_command): Set symfile_objfile_is_user_supplied.
	* inferior.c (add_inferior_command): Set
	exec_file_is_user_supplied and symfile_objfile_is_user_supplied.
	* main.c (captured_main): Likewise.
	* infcmd.c (attach_command_post_wait): Always call
	exec_file_locate_attach.  Only call reopen_exec_file and
	reread_symbols if exec_file_is_user_supplied.
	* remote.c (remote_add_inferior): Remove get_exec_file check
	around exec_file_locate_attach.
---
 gdb/ChangeLog   |   26 ++++++++++++++++++++++++++
 gdb/exec.c      |   30 +++++++++++++++++++++++-------
 gdb/exec.h      |    2 ++
 gdb/infcmd.c    |   13 ++++++++-----
 gdb/inferior.c  |    3 +++
 gdb/main.c      |   24 ++++++++++++++++--------
 gdb/progspace.h |   17 +++++++++++++++++
 gdb/remote.c    |    9 ++++++---
 gdb/symfile.c   |    3 +++
 9 files changed, 104 insertions(+), 23 deletions(-)

diff --git a/gdb/exec.c b/gdb/exec.c
index 8a4ab6f..11e5db0 100644
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -102,6 +102,7 @@ exec_close (void)
 
       xfree (exec_filename);
       exec_filename = NULL;
+      exec_file_is_user_supplied = 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)
@@ -171,8 +167,27 @@ exec_file_locate_attach (int pid, int from_tty)
 	full_exec_path = xstrdup (exec_file);
     }
 
-  exec_file_attach (full_exec_path, from_tty);
-  symbol_file_add_main (full_exec_path, from_tty);
+  if (exec_file_is_user_supplied)
+    {
+      if (strcmp (full_exec_path, exec_filename) != 0)
+	warning (_("Process %d has executable file %s,"
+		   " but executable file is currently set to %s"),
+		 pid, full_exec_path, exec_filename);
+    }
+  else
+    exec_file_attach (full_exec_path, from_tty);
+
+  if (symfile_objfile_is_user_supplied)
+    {
+      const char *symbol_filename = objfile_filename (symfile_objfile);
+
+      if (strcmp (full_exec_path, symbol_filename) != 0)
+	warning (_("Process %d has symbol file %s,"
+		   " but symbol file is currently set to %s"),
+		 pid, full_exec_path, symbol_filename);
+    }
+  else
+    symbol_file_add_main (full_exec_path, from_tty);
 }
 
 /* Set FILENAME as the new exec file.
@@ -376,6 +391,7 @@ exec_file_command (char *args, int from_tty)
       filename = tilde_expand (*argv);
       make_cleanup (xfree, filename);
       exec_file_attach (filename, from_tty);
+      exec_file_is_user_supplied = 1;
 
       do_cleanups (cleanups);
     }
diff --git a/gdb/exec.h b/gdb/exec.h
index c7f3b56..d984e97 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 exec_file_is_user_supplied \
+  current_program_space->pspace_exec_file_is_user_supplied
 
 /* 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..440871f 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -2467,11 +2467,14 @@ 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
+  /* Attempt to open the file that was executed to create this
+     inferior.  If the user has explicitly specified executable
+     and/or symbol files then warn the user if their choices do
+     not match.  Otherwise, set exec_file and symfile_objfile to
+     the new file.  */
+  exec_file_locate_attach (ptid_get_pid (inferior_ptid), from_tty);
+
+  if (exec_file_is_user_supplied)
     {
       reopen_exec_file ();
       reread_symbols ();
diff --git a/gdb/inferior.c b/gdb/inferior.c
index ba320b5..30e1036 100644
--- a/gdb/inferior.c
+++ b/gdb/inferior.c
@@ -888,7 +888,10 @@ add_inferior_command (char *args, int from_tty)
 	  switch_to_thread (null_ptid);
 
 	  exec_file_attach (exec, from_tty);
+	  exec_file_is_user_supplied = 1;
+
 	  symbol_file_add_main (exec, from_tty);
+	  symfile_objfile_is_user_supplied = 1;
 	}
     }
 
diff --git a/gdb/main.c b/gdb/main.c
index 477fd68..3a68b0b 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -1051,17 +1051,25 @@ 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);
+	{
+	  exec_file_is_user_supplied = 1;
+
+	  if (catch_command_errors_const (symbol_file_add_main, symarg,
+					  !batch_flag))
+	    symfile_objfile_is_user_supplied = 1;
+	}
     }
   else
     {
-      if (execarg != NULL)
-	catch_command_errors_const (exec_file_attach, execarg,
-				    !batch_flag);
-      if (symarg != NULL)
-	catch_command_errors_const (symbol_file_add_main, symarg,
-				    !batch_flag);
+      if (execarg != NULL
+	  && catch_command_errors_const (exec_file_attach, execarg,
+					 !batch_flag))
+	exec_file_is_user_supplied = 1;
+
+      if (symarg != NULL
+	  && catch_command_errors_const (symbol_file_add_main, symarg,
+					 !batch_flag))
+	symfile_objfile_is_user_supplied = 1;
     }
 
   if (corearg && pidarg)
diff --git a/gdb/progspace.h b/gdb/progspace.h
index f960093..561e093 100644
--- a/gdb/progspace.h
+++ b/gdb/progspace.h
@@ -154,6 +154,12 @@ 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 the "file"
+       or "add-inferior -exec" commands.  Zero if
+       pspace_exec_filename is unset or was discovered by GDB.  */
+    int pspace_exec_file_is_user_supplied;
+
     /* 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
@@ -183,6 +189,12 @@ struct program_space
        (e.g. the argument to the "symbol-file" or "file" command).  */
     struct objfile *symfile_object_file;
 
+    /* Nonzero if symfile_object_file was supplied by the user,
+       either at startup (on the command-line) or via the "file",
+       "symbol-file" or "add-inferior -exec" commands.  Zero if
+       symfile_object_file is unset or was discovered by GDB.  */
+    int symfile_object_file_is_user_supplied;
+
     /* All known objfiles are kept in a linked list.  This points to
        the head of this list.  */
     struct objfile *objfiles;
@@ -215,6 +227,11 @@ struct program_space
 
 #define symfile_objfile current_program_space->symfile_object_file
 
+/* See program_space->symfile_object_file_is_user_supplied.  */
+
+#define symfile_objfile_is_user_supplied \
+  current_program_space->symfile_object_file_is_user_supplied
+
 /* All known objfiles are kept in a linked list.  This points to the
    root of this list.  */
 #define object_files current_program_space->objfiles
diff --git a/gdb/remote.c b/gdb/remote.c
index 099ddbb..af6b8d8 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1556,9 +1556,12 @@ 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)
+  /* Attempt to open the file that was executed to create this
+     inferior.  If the user has explicitly specified executable
+     and/or symbol files then warn the user if their choices do
+     not match.  Otherwise, set exec_file and symfile_objfile to
+     the new file.  */
+  if (try_open_exec && !fake_pid_p)
     exec_file_locate_attach (pid, 1);
 
   return inf;
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 0c35ffa..d44cb7d 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -1347,6 +1347,8 @@ symbol_file_clear (int from_tty)
   gdb_assert (symfile_objfile == NULL);
   if (from_tty)
     printf_unfiltered (_("No symbol file now.\n"));
+
+  symfile_objfile_is_user_supplied = 0;
 }
 
 static int
@@ -1664,6 +1666,7 @@ symbol_file_command (char *args, int from_tty)
 	  else
 	    {
 	      symbol_file_add_main_1 (*argv, from_tty, flags);
+	      symfile_objfile_is_user_supplied = 1;
 	      name = *argv;
 	    }
 
-- 
1.7.1

Re: [PATCH] Make only user-specified executable filenames sticky

[Don Breazeal]

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

Re: [PATCH v2] Make only user-specified executable and symbol filenames sticky

[Philippe Waroquiers]

On Fri, 2015-06-05 at 12:22 +0100, Gary Benson wrote:
> Built and regtested on RHEL6.6 x86_64.
I tested with the last SVN version of the Valgrind gdbserver (that
supports qXfer:exec-file:read+).

The patch introduces a regression:
with the patch, GDB does not anymore automatically load the
exec-file.

I bypassed this problem by ignoring fake_pid_p in remote.c:
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1624,9 +1624,13 @@ 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 && get_exec_file (0) == NULL)
+  /* Attempt to open the file that was executed to create this
+     inferior.  If the user has explicitly specified executable
+     and/or symbol files then warn the user if their choices do
+     not match.  Otherwise, set exec_file and symfile_objfile to
+     the new file.  */
+  printf("fake_pid_p %d\n", fake_pid_p);
+  if (try_open_exec)// && !fake_pid_p)
     exec_file_locate_attach (pid, 1);

Effectively, the printf shows that with Valgrind gdbsrv,
fake_pid_p value is 1.

When ignoring fake_pid_p, GDB can properly attach
to different Valgrind gdbsrv, and changes of executable
as expected.

Philippe

Re: [PATCH v2] Make only user-specified executable and symbol filenames sticky

[Philippe Waroquiers]

On Fri, 2015-06-05 at 12:22 +0100, Gary Benson wrote:
> Built and regtested on RHEL6.6 x86_64.
I also tested the patch (unmodified, i.e. keeping the !fake_pid_p test) 
with 'native' attach/detach.
I have encountered some problems.

Here is the test I am doing:
In one terminal, I launch
  /bin/sleep 10000
In another terminal, I am launching
 ./gdbserver_tests/sleepers 1000000 1000000 1000000 BSBSBSBS
(sleepers is a program used for testing Valgrind gdbsrv).

Then I used the patched GDB to attach first to sleep, then detach,
then attach to sleepers, then detach, then attach again to sleep.
The 3 attach does not work properly: it believes it has to use
the sleepers executable, asks to switch to this symbol file,
and then GDB locks, and I have to kill it.

Philippe


(gdb) atta 27434
Attaching to process 27434
Reading symbols from /bin/sleep...(no debugging symbols found)...done.
warning: Cannot call inferior functions on this system - Linux kernel
with broken i386 NX (non-executable pages) support detected!
Reading symbols from /lib/libc.so.6...(no debugging symbols
found)...done.
Reading symbols from /lib/ld-linux.so.2...(no debugging symbols
found)...done.
0x00dab416 in __kernel_vsyscall ()
(gdb) detach
Detaching from program: /bin/sleep, process 27434
(gdb) attach 27393
Attaching to program: /bin/sleep, process 27393
Reading symbols
from /home/philippe/valgrind/trunk_untouched/gdbserver_tests/sleepers...done.
Reading symbols from /lib/libpthread.so.0...(no debugging symbols
found)...done.
[New LWP 27396]
[New LWP 27395]
[New LWP 27394]
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/libthread_db.so.1".
Reading symbols from /lib/libc.so.6...(no debugging symbols
found)...done.
Reading symbols from /lib/ld-linux.so.2...(no debugging symbols
found)...done.
0x00843416 in __kernel_vsyscall ()
(gdb) detach
Detaching from
program: /home/philippe/valgrind/trunk_untouched/gdbserver_tests/sleepers, process 27393
(gdb) atta 27434
Attaching to
program: /home/philippe/valgrind/trunk_untouched/gdbserver_tests/sleepers, process 27434
Load new symbol table from "/bin/sleep"? (y or n) 
//////////// here GDB does not respond anymore.

Re: [PATCH v2] Make only user-specified executable and symbol filenames sticky

[Philippe Waroquiers]

On Fri, 2015-06-05 at 12:22 +0100, Gary Benson wrote:
> Built and regtested on RHEL6.6 x86_64.
Finally, here are some tests with GDB gdbserver.
There is also something strange.

I launch 2 gdbserver:
  gdbserver :1234 /bin/sleep 10000
and
  gdbserver :1235 ./gdbserver_tests/sleepers 1000000 1000000 1000000 BSBSBSBS

Then :
(gdb) tar rem :1234
Remote debugging using :1234
Reading symbols from target:/bin/sleep...(no debugging symbols found)...done.
Reading symbols from target:/lib/ld-linux.so.2...(no debugging symbols found)...done.
0x001f2850 in _start () from target:/lib/ld-linux.so.2
(gdb) detach
Detaching from program: target:/bin/sleep, process 27572
Ending remote debugging.
(gdb) tar rem :1235
`target:/bin/sleep' has disappeared; keeping its symbols.
Remote debugging using :1235
Reading symbols from target:/home/philippe/valgrind/trunk_untouched/gdbserver_tests/sleepers...Can't read symbols from target:/home/philippe/valgrind/trunk_untouched/gdbserver_tests/sleepers: Invalid argument
(gdb) 


So, as you can see, GDB cannot read the symbols for the second attach
(while it has properly discovered the exec-file).

Philippe

[PATCH v3] Make only user-specified executable and symbol filenames sticky

[Gary Benson]

Philippe Waroquiers wrote:
> On Fri, 2015-06-05 at 12:22 +0100, Gary Benson wrote:
> > Built and regtested on RHEL6.6 x86_64.
> I tested with the last SVN version of the Valgrind gdbserver (that
> supports qXfer:exec-file:read+).
> 
> The patch introduces a regression:
> with the patch, GDB does not anymore automatically load the
> exec-file.
> 
> I bypassed this problem by ignoring fake_pid_p in remote.c:
> --- a/gdb/remote.c
> +++ b/gdb/remote.c
> @@ -1624,9 +1624,13 @@ 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 && get_exec_file (0) == NULL)
> +  /* Attempt to open the file that was executed to create this
> +     inferior.  If the user has explicitly specified executable
> +     and/or symbol files then warn the user if their choices do
> +     not match.  Otherwise, set exec_file and symfile_objfile to
> +     the new file.  */
> +  printf("fake_pid_p %d\n", fake_pid_p);
> +  if (try_open_exec)// && !fake_pid_p)
>      exec_file_locate_attach (pid, 1);
> 
> Effectively, the printf shows that with Valgrind gdbsrv,
> fake_pid_p value is 1.
> 
> When ignoring fake_pid_p, GDB can properly attach
> to different Valgrind gdbsrv, and changes of executable
> as expected.

Ah, it seems I mailed a bad patch, my apologies!  I was working on
two fixes that touched the same line, and it looks like I rebased
them in the wrong order.

This updated patch has been created against the latest gdb/master
(80fb91378c91a8239817a5ab2b1c3e346109db25).  Could you please try
your tests again?

Thanks,
Gary


---
On attach, GDB will only attempt to determine the main executable's
filename if one is not already 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 is PR gdb/17626.

In GDB some filenames are supplied by the user (e.g. using the "file"
or "symbol-file" commands) and some are determined by GDB (e.g. while
processing an "attach" command).  This commit updates GDB to track
which 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.

gdb/ChangeLog:

	PR gdb/17626
	* progspace.h (struct program_space)
	<pspace_exec_file_is_user_supplied>: New field.
	<symfile_object_file_is_user_supplied>: Likewise.
	(symfile_objfile_is_user_supplied): New macro.
	* exec.h (exec_file_is_user_supplied): Likewise.
	* exec.c (exec_close): Clear exec_file_is_user_supplied.
	(exec_file_locate_attach): Remove get_exec_file check.
	Do not replace user-supplied executable or symbol files.
	Warn if user-supplied executable or symbol files do not
	match discovered file.
	(exec_file_command): Set exec_file_is_user_supplied.
	* symfile.c (symbol_file_clear): Clear
	symfile_objfile_is_user_supplied.
	(symbol_file_command): Set symfile_objfile_is_user_supplied.
	* inferior.c (add_inferior_command): Set
	exec_file_is_user_supplied and symfile_objfile_is_user_supplied.
	* main.c (captured_main): Likewise.
	* infcmd.c (attach_command_post_wait): Always call
	exec_file_locate_attach.  Only call reopen_exec_file and
	reread_symbols if exec_file_is_user_supplied.
	* remote.c (remote_add_inferior): Remove get_exec_file check
	around exec_file_locate_attach.
---
 gdb/ChangeLog   |   26 ++++++++++++++++++++++++++
 gdb/exec.c      |   30 +++++++++++++++++++++++-------
 gdb/exec.h      |    2 ++
 gdb/infcmd.c    |   13 ++++++++-----
 gdb/inferior.c  |    3 +++
 gdb/main.c      |   24 ++++++++++++++++--------
 gdb/progspace.h |   17 +++++++++++++++++
 gdb/remote.c    |    9 ++++++---
 gdb/symfile.c   |    3 +++
 9 files changed, 104 insertions(+), 23 deletions(-)

diff --git a/gdb/exec.c b/gdb/exec.c
index 8a4ab6f..11e5db0 100644
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -102,6 +102,7 @@ exec_close (void)
 
       xfree (exec_filename);
       exec_filename = NULL;
+      exec_file_is_user_supplied = 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)
@@ -171,8 +167,27 @@ exec_file_locate_attach (int pid, int from_tty)
 	full_exec_path = xstrdup (exec_file);
     }
 
-  exec_file_attach (full_exec_path, from_tty);
-  symbol_file_add_main (full_exec_path, from_tty);
+  if (exec_file_is_user_supplied)
+    {
+      if (strcmp (full_exec_path, exec_filename) != 0)
+	warning (_("Process %d has executable file %s,"
+		   " but executable file is currently set to %s"),
+		 pid, full_exec_path, exec_filename);
+    }
+  else
+    exec_file_attach (full_exec_path, from_tty);
+
+  if (symfile_objfile_is_user_supplied)
+    {
+      const char *symbol_filename = objfile_filename (symfile_objfile);
+
+      if (strcmp (full_exec_path, symbol_filename) != 0)
+	warning (_("Process %d has symbol file %s,"
+		   " but symbol file is currently set to %s"),
+		 pid, full_exec_path, symbol_filename);
+    }
+  else
+    symbol_file_add_main (full_exec_path, from_tty);
 }
 
 /* Set FILENAME as the new exec file.
@@ -376,6 +391,7 @@ exec_file_command (char *args, int from_tty)
       filename = tilde_expand (*argv);
       make_cleanup (xfree, filename);
       exec_file_attach (filename, from_tty);
+      exec_file_is_user_supplied = 1;
 
       do_cleanups (cleanups);
     }
diff --git a/gdb/exec.h b/gdb/exec.h
index c7f3b56..d984e97 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 exec_file_is_user_supplied \
+  current_program_space->pspace_exec_file_is_user_supplied
 
 /* 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 03282a7..e43b299 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -2490,11 +2490,14 @@ 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
+  /* Attempt to open the file that was executed to create this
+     inferior.  If the user has explicitly specified executable
+     and/or symbol files then warn the user if their choices do
+     not match.  Otherwise, set exec_file and symfile_objfile to
+     the new file.  */
+  exec_file_locate_attach (ptid_get_pid (inferior_ptid), from_tty);
+
+  if (exec_file_is_user_supplied)
     {
       reopen_exec_file ();
       reread_symbols ();
diff --git a/gdb/inferior.c b/gdb/inferior.c
index d0783d3..ea00121 100644
--- a/gdb/inferior.c
+++ b/gdb/inferior.c
@@ -888,7 +888,10 @@ add_inferior_command (char *args, int from_tty)
 	  switch_to_thread (null_ptid);
 
 	  exec_file_attach (exec, from_tty);
+	  exec_file_is_user_supplied = 1;
+
 	  symbol_file_add_main (exec, from_tty);
+	  symfile_objfile_is_user_supplied = 1;
 	}
     }
 
diff --git a/gdb/main.c b/gdb/main.c
index aecd60a..ae2803c 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -1050,17 +1050,25 @@ 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);
+	{
+	  exec_file_is_user_supplied = 1;
+
+	  if (catch_command_errors_const (symbol_file_add_main, symarg,
+					  !batch_flag))
+	    symfile_objfile_is_user_supplied = 1;
+	}
     }
   else
     {
-      if (execarg != NULL)
-	catch_command_errors_const (exec_file_attach, execarg,
-				    !batch_flag);
-      if (symarg != NULL)
-	catch_command_errors_const (symbol_file_add_main, symarg,
-				    !batch_flag);
+      if (execarg != NULL
+	  && catch_command_errors_const (exec_file_attach, execarg,
+					 !batch_flag))
+	exec_file_is_user_supplied = 1;
+
+      if (symarg != NULL
+	  && catch_command_errors_const (symbol_file_add_main, symarg,
+					 !batch_flag))
+	symfile_objfile_is_user_supplied = 1;
     }
 
   if (corearg && pidarg)
diff --git a/gdb/progspace.h b/gdb/progspace.h
index f960093..561e093 100644
--- a/gdb/progspace.h
+++ b/gdb/progspace.h
@@ -154,6 +154,12 @@ 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 the "file"
+       or "add-inferior -exec" commands.  Zero if
+       pspace_exec_filename is unset or was discovered by GDB.  */
+    int pspace_exec_file_is_user_supplied;
+
     /* 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
@@ -183,6 +189,12 @@ struct program_space
        (e.g. the argument to the "symbol-file" or "file" command).  */
     struct objfile *symfile_object_file;
 
+    /* Nonzero if symfile_object_file was supplied by the user,
+       either at startup (on the command-line) or via the "file",
+       "symbol-file" or "add-inferior -exec" commands.  Zero if
+       symfile_object_file is unset or was discovered by GDB.  */
+    int symfile_object_file_is_user_supplied;
+
     /* All known objfiles are kept in a linked list.  This points to
        the head of this list.  */
     struct objfile *objfiles;
@@ -215,6 +227,11 @@ struct program_space
 
 #define symfile_objfile current_program_space->symfile_object_file
 
+/* See program_space->symfile_object_file_is_user_supplied.  */
+
+#define symfile_objfile_is_user_supplied \
+  current_program_space->symfile_object_file_is_user_supplied
+
 /* All known objfiles are kept in a linked list.  This points to the
    root of this list.  */
 #define object_files current_program_space->objfiles
diff --git a/gdb/remote.c b/gdb/remote.c
index 53918ca..955ea86 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1624,9 +1624,12 @@ 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 && get_exec_file (0) == NULL)
+  /* Attempt to open the file that was executed to create this
+     inferior.  If the user has explicitly specified executable
+     and/or symbol files then warn the user if their choices do
+     not match.  Otherwise, set exec_file and symfile_objfile to
+     the new file.  */
+  if (try_open_exec)
     exec_file_locate_attach (pid, 1);
 
   return inf;
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 0c35ffa..d44cb7d 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -1347,6 +1347,8 @@ symbol_file_clear (int from_tty)
   gdb_assert (symfile_objfile == NULL);
   if (from_tty)
     printf_unfiltered (_("No symbol file now.\n"));
+
+  symfile_objfile_is_user_supplied = 0;
 }
 
 static int
@@ -1664,6 +1666,7 @@ symbol_file_command (char *args, int from_tty)
 	  else
 	    {
 	      symbol_file_add_main_1 (*argv, from_tty, flags);
+	      symfile_objfile_is_user_supplied = 1;
 	      name = *argv;
 	    }
 
-- 
1.7.1

Re: [PATCH v3] Make only user-specified executable and symbol filenames sticky

[Philippe Waroquiers]

On Mon, 2015-06-08 at 10:01 +0100, Gary Benson wrote:

> This updated patch has been created against the latest gdb/master
> (80fb91378c91a8239817a5ab2b1c3e346109db25).  Could you please try
> your tests again?
First test with 'native' attach/detach/attach/detach/attach is
working ok.
However, the behaviour of the 3rd attach differs: a question
is asked, that is answered automatically as yes (for EOF).
So that is strange.
        GNU gdb (GDB) 7.9.50.20150608-cvs
        ...
        Type "apropos word" to search for commands related to "word".
        (gdb) atta 13286
        Attaching to process 13286
        Reading symbols from /bin/sleep...(no debugging symbols found)...done.
        Reading symbols from /lib/x86_64-linux-gnu/libc.so.6...(no debugging symbols found)...done.
        Reading symbols from /lib64/ld-linux-x86-64.so.2...(no debugging symbols found)...done.
        0x00007f3c5bb06f20 in nanosleep () from /lib/x86_64-linux-gnu/libc.so.6
        (gdb) detach
        Detaching from program: /bin/sleep, process 13286
        (gdb) atta 13320
        Attaching to program: /bin/sleep, process 13320
        Reading symbols from /home/philippe/valgrind/trunk_untouched/gdbserver_tests/sleepers...done.
        Reading symbols from /lib/x86_64-linux-gnu/libpthread.so.0...(no debugging symbols found)...done.
        [New LWP 13323]
        [New LWP 13322]
        [New LWP 13321]
        [Thread debugging using libthread_db enabled]
        Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
        Reading symbols from /lib/x86_64-linux-gnu/libc.so.6...(no debugging symbols found)...done.
        Reading symbols from /lib64/ld-linux-x86-64.so.2...(no debugging symbols found)...done.
        0x00007f5f538e1da3 in select () from /lib/x86_64-linux-gnu/libc.so.6
        (gdb) detach
        Detaching from program: /home/philippe/valgrind/trunk_untouched/gdbserver_tests/sleepers, process 13320
        (gdb) atta 13286
        Attaching to program: /home/philippe/valgrind/trunk_untouched/gdbserver_tests/sleepers, process 13286
        Load new symbol table from "/bin/sleep"? (y or n) EOF [assumed Y]
        Reading symbols from /bin/sleep...(no debugging symbols found)...done.
        Reading symbols from /lib/x86_64-linux-gnu/libc.so.6...(no debugging symbols found)...done.
        Reading symbols from /lib64/ld-linux-x86-64.so.2...(no debugging symbols found)...done.
        0x00007f3c5bb06f20 in nanosleep () from /lib/x86_64-linux-gnu/libc.so.6
        (gdb) 

2nd test is with Valgrind gdbsrv. Also working ok. However, here
we have a question (no EOF answer) asking to change or not the symbol file.
        GNU gdb (GDB) 7.9.50.20150608-cvs
        ...
        Type "apropos word" to search for commands related to "word".
        (gdb) tar rem | lvgdb
        Remote debugging using | lvgdb
        relaying data between gdb and process 13394
        warning: remote target does not support file transfer, attempting to access files from local filesystem.
        Reading symbols from /home/philippe/valgrind/trunk_untouched/gdbserver_tests/sleepers...done.
        Reading symbols from /lib64/ld-linux-x86-64.so.2...(no debugging symbols found)...done.
        0x00000000040012d0 in ?? () from /lib64/ld-linux-x86-64.so.2
        (gdb) detach
        Detaching from program: /home/philippe/valgrind/trunk_untouched/gdbserver_tests/sleepers, Remote target
        Ending remote debugging.
        (gdb) tar rem | lvgdb --pid=13413
        Remote debugging using | lvgdb --pid=13413
        relaying data between gdb and process 13413
        Load new symbol table from "/home/philippe/valgrind/trunk_untouched/memcheck/tests/trivialleak"? (y or n) y
        Reading symbols from /home/philippe/valgrind/trunk_untouched/memcheck/tests/trivialleak...done.
        Reading symbols from /lib64/ld-linux-x86-64.so.2...(no debugging symbols found)...done.
        0x00000000040012d0 in ?? () from /lib64/ld-linux-x86-64.so.2
        (gdb) detach
        Detaching from program: /home/philippe/valgrind/trunk_untouched/memcheck/tests/trivialleak, Remote target
        Ending remote debugging.
        (gdb) tar rem | lvgdb --pid=13394
        Remote debugging using | lvgdb --pid=13394
        relaying data between gdb and process 13394
        Load new symbol table from "/home/philippe/valgrind/trunk_untouched/gdbserver_tests/sleepers"? (y or n) y
        Reading symbols from /home/philippe/valgrind/trunk_untouched/gdbserver_tests/sleepers...done.
        Reading symbols from /home/philippe/valgrind/trunk_untouched/./.in_place/vgpreload_core-amd64-linux.so...done.
        Reading symbols from /home/philippe/valgrind/trunk_untouched/./.in_place/vgpreload_memcheck-amd64-linux.so...done.
        Reading symbols from /lib/x86_64-linux-gnu/libpthread.so.0...(no debugging symbols found)...done.
        Reading symbols from /lib/x86_64-linux-gnu/libc.so.6...(no debugging symbols found)...done.
        Reading symbols from /lib64/ld-linux-x86-64.so.2...(no debugging symbols found)...done.
        0x0000000005145da3 in select () from /lib/x86_64-linux-gnu/libc.so.6
        (gdb) tar rem | lvgdb --pid=13427
        A program is being debugged already.  Kill it? (y or n) n
        Program not killed.
        (gdb) detach
        Detaching from program: /home/philippe/valgrind/trunk_untouched/gdbserver_tests/sleepers, Remote target
        Ending remote debugging.
        (gdb) tar rem | lvgdb --pid=13427
        Remote debugging using | lvgdb --pid=13427
        relaying data between gdb and process 13427
        Load new symbol table from "/home/philippe/valgrind/trunk_untouched/memcheck/tests/trivialleak"? (y or n) y
        Reading symbols from /home/philippe/valgrind/trunk_untouched/memcheck/tests/trivialleak...done.
        Reading symbols from /lib64/ld-linux-x86-64.so.2...(no debugging symbols found)...done.
        0x00000000040012d0 in ?? () from /lib64/ld-linux-x86-64.so.2
        (gdb) 

3rd test with GDB gdbserver: there is still the problem of invalid
target.
        GNU gdb (GDB) 7.9.50.20150608-cvs
        ...
        Type "apropos word" to search for commands related to "word".
        (gdb) tar rem :1234
        Remote debugging using :1234
        Reading symbols from target:/home/philippe/valgrind/trunk_untouched/memcheck/tests/trivialleak...done.
        Reading symbols from target:/lib64/ld-linux-x86-64.so.2...(no debugging symbols found)...done.
        0x00007ffff7ddb2d0 in ?? () from target:/lib64/ld-linux-x86-64.so.2
        (gdb) detach
        Detaching from program: target:/home/philippe/valgrind/trunk_untouched/memcheck/tests/trivialleak, process 13447
        Ending remote debugging.
        (gdb) tar rem :1235
        `target:/home/philippe/valgrind/trunk_untouched/memcheck/tests/trivialleak' has disappeared; keeping its symbols.
        Remote debugging using :1235
        Load new symbol table from "target:/home/philippe/valgrind/trunk_untouched/gdbserver_tests/sleepers"? (y or n) y
        Reading symbols from target:/home/philippe/valgrind/trunk_untouched/gdbserver_tests/sleepers...Can't read symbols from target:/home/philippe/valgrind/trunk_untouched/gdbserver_tests/sleepers: Invalid argument
        (gdb) 

Test 4 is the same as test 1, but with gdb started with a filename as argument.
GDB gives a warning that the exec+sym file does not match the target one.
        gdb ./gdbserver_tests/sleepers 
        ...
        Type "apropos word" to search for commands related to "word"...
        Reading symbols from ./gdbserver_tests/sleepers...done.
        (gdb) atta 13474
        Attaching to program: /home/philippe/valgrind/trunk_untouched/gdbserver_tests/sleepers, process 13474
        Reading symbols from /lib/x86_64-linux-gnu/libpthread.so.0...(no debugging symbols found)...done.
        [New LWP 13477]
        [New LWP 13476]
        [New LWP 13475]
        [Thread debugging using libthread_db enabled]
        Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
        Reading symbols from /lib/x86_64-linux-gnu/libc.so.6...(no debugging symbols found)...done.
        Reading symbols from /lib64/ld-linux-x86-64.so.2...(no debugging symbols found)...done.
        0x00007fec5b318da3 in select () from /lib/x86_64-linux-gnu/libc.so.6
        (gdb) detach
        Detaching from program: /home/philippe/valgrind/trunk_untouched/gdbserver_tests/sleepers, process 13474
        (gdb) atta 13484
        Attaching to program: /home/philippe/valgrind/trunk_untouched/gdbserver_tests/sleepers, process 13484
        warning: Process 13484 has executable file /bin/sleep, but executable file is currently set to /home/philippe/valgrind/trunk_untouched/gdbserver_tests/sleepers
        warning: Process 13484 has symbol file /bin/sleep, but symbol file is currently set to /home/philippe/valgrind/trunk_untouched/gdbserver_tests/sleepers
        Reading symbols from /lib64/ld-linux-x86-64.so.2...(no debugging symbols found)...done.
        0x00007f1d71fdef20 in ?? ()
        (gdb) detach
        Detaching from program: /home/philippe/valgrind/trunk_untouched/gdbserver_tests/sleepers, process 13484
        (gdb) atta 13474
        Attaching to program: /home/philippe/valgrind/trunk_untouched/gdbserver_tests/sleepers, process 13474
        Reading symbols from /lib/x86_64-linux-gnu/libpthread.so.0...(no debugging symbols found)...done.
        [New LWP 13477]
        [New LWP 13476]
        [New LWP 13475]
        [Thread debugging using libthread_db enabled]
        Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
        Reading symbols from /lib/x86_64-linux-gnu/libc.so.6...(no debugging symbols found)...done.
        Reading symbols from /lib64/ld-linux-x86-64.so.2...(no debugging symbols found)...done.
        0x00007fec5b318da3 in select () from /lib/x86_64-linux-gnu/libc.so.6
        (gdb) detach
        Detaching from program: /home/philippe/valgrind/trunk_untouched/gdbserver_tests/sleepers, process 13474
        (gdb) atta 13484
        Attaching to program: /home/philippe/valgrind/trunk_untouched/gdbserver_tests/sleepers, process 13484
        warning: Process 13484 has executable file /bin/sleep, but executable file is currently set to /home/philippe/valgrind/trunk_untouched/gdbserver_tests/sleepers
        warning: Process 13484 has symbol file /bin/sleep, but symbol file is currently set to /home/philippe/valgrind/trunk_untouched/gdbserver_tests/sleepers
        Reading symbols from /lib64/ld-linux-x86-64.so.2...(no debugging symbols found)...done.
        0x00007f1d71fdef20 in ?? ()
        (gdb) file /bin/sleep
        A program is being debugged already.
        Are you sure you want to change the file? (y or n) y
        Load new symbol table from "/bin/sleep"? (y or n) y
        Reading symbols from /bin/sleep...(no debugging symbols found)...done.
        (gdb) detach
        Detaching from program: /bin/sleep, process 13484
        (gdb) atta 13474     
        Attaching to program: /bin/sleep, process 13474
        warning: Process 13474 has executable file /home/philippe/valgrind/trunk_untouched/gdbserver_tests/sleepers, but executable file is currently set to /bin/sleep
        warning: Process 13474 has symbol file /home/philippe/valgrind/trunk_untouched/gdbserver_tests/sleepers, but symbol file is currently set to /bin/sleep
        Reading symbols from /lib64/ld-linux-x86-64.so.2...(no debugging symbols found)...done.
        0x00007fec5b318da3 in ?? ()
        (gdb) 
        


So, in summary, this patch version is better but there are still some
strange behaviours.

In terms of behaviour, it is somewhat surprising to sometimes have a question,
sometimes not. IMO, it would be better to always ask a question when the currently loaded
file does not match the target file (native attach or remote target), even when
the file was supplied by the user.

Philippe

Re: [PATCH v3] Make only user-specified executable and symbol filenames sticky

[Gary Benson]

Philippe Waroquiers wrote:
> On Mon, 2015-06-08 at 10:01 +0100, Gary Benson wrote:
> > This updated patch has been created against the latest gdb/master
> > (80fb91378c91a8239817a5ab2b1c3e346109db25).  Could you please try
> > your tests again?
> 
> First test with 'native' attach/detach/attach/detach/attach is
> working ok.
> However, the behaviour of the 3rd attach differs: a question
> is asked, that is answered automatically as yes (for EOF).
> So that is strange.
>         GNU gdb (GDB) 7.9.50.20150608-cvs
>         ...
>         Type "apropos word" to search for commands related to "word".
>         (gdb) atta 13286
>         Attaching to process 13286
>         Reading symbols from /bin/sleep...(no debugging symbols found)...done.
>         Reading symbols from /lib/x86_64-linux-gnu/libc.so.6...(no debugging symbols found)...done.
>         Reading symbols from /lib64/ld-linux-x86-64.so.2...(no debugging symbols found)...done.
>         0x00007f3c5bb06f20 in nanosleep () from /lib/x86_64-linux-gnu/libc.so.6
>         (gdb) detach
>         Detaching from program: /bin/sleep, process 13286
>         (gdb) atta 13320
>         Attaching to program: /bin/sleep, process 13320
>         Reading symbols from /home/philippe/valgrind/trunk_untouched/gdbserver_tests/sleepers...done.
>         Reading symbols from /lib/x86_64-linux-gnu/libpthread.so.0...(no debugging symbols found)...done.
>         [New LWP 13323]
>         [New LWP 13322]
>         [New LWP 13321]
>         [Thread debugging using libthread_db enabled]
>         Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
>         Reading symbols from /lib/x86_64-linux-gnu/libc.so.6...(no debugging symbols found)...done.
>         Reading symbols from /lib64/ld-linux-x86-64.so.2...(no debugging symbols found)...done.
>         0x00007f5f538e1da3 in select () from /lib/x86_64-linux-gnu/libc.so.6
>         (gdb) detach
>         Detaching from program: /home/philippe/valgrind/trunk_untouched/gdbserver_tests/sleepers, process 13320
>         (gdb) atta 13286
>         Attaching to program: /home/philippe/valgrind/trunk_untouched/gdbserver_tests/sleepers, process 13286
>         Load new symbol table from "/bin/sleep"? (y or n) EOF [assumed Y]
>         Reading symbols from /bin/sleep...(no debugging symbols found)...done.
>         Reading symbols from /lib/x86_64-linux-gnu/libc.so.6...(no debugging symbols found)...done.
>         Reading symbols from /lib64/ld-linux-x86-64.so.2...(no debugging symbols found)...done.
>         0x00007f3c5bb06f20 in nanosleep () from /lib/x86_64-linux-gnu/libc.so.6
>         (gdb) 

I don't see the question:

  GNU gdb (GDB) 7.9.50.20150703-cvs
  Copyright (C) 2015 Free Software Foundation, Inc.
  License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
  This is free software: you are free to change and redistribute it.
  There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
  and "show warranty" for details.
  This GDB was configured as "x86_64-unknown-linux-gnu".
  Type "show configuration" for configuration details.
  For bug reporting instructions, please see:
  <http://www.gnu.org/software/gdb/bugs/>.
  Find the GDB manual and other documentation resources online at:
  <http://www.gnu.org/software/gdb/documentation/>.
  For help, type "help".
  Type "apropos word" to search for commands related to "word".
  (gdb) atta 15450
  Attaching to process 15450
  Reading symbols from /bin/sleep...(no debugging symbols found)...done.
  Reading symbols from /lib64/libc.so.6...warning: the debug information found in "/usr/lib/debug//lib64/libc-2.12.so.debug" does not match "/lib64/libc.so.6" (CRC mismatch).
  (no debugging symbols found)...done.
  Reading symbols from /lib64/ld-linux-x86-64.so.2...Reading symbols from /usr/lib/debug/lib64/ld-2.12.so.debug...warning: Skipping deprecated .gdb_index section in /usr/lib/debug/lib64/ld-2.12.so.debug.
  Do "set use-deprecated-index-sections on" before the file is read
  to use the section anyway.
  done.
  done.
  0x00000039b54aca20 in __nanosleep_nocancel () from /lib64/libc.so.6
  (gdb) detach
  Detaching from program: /bin/sleep, process 15450
  (gdb) atta 15465
  Attaching to program: /bin/sleep, process 15465
  Reading symbols from /bin/sleep...(no debugging symbols found)...done.
  Reading symbols from /lib64/libc.so.6...warning: the debug information found in "/usr/lib/debug//lib64/libc-2.12.so.debug" does not match "/lib64/libc.so.6" (CRC mismatch).
  (no debugging symbols found)...done.
  Reading symbols from /lib64/ld-linux-x86-64.so.2...Reading symbols from /usr/lib/debug/lib64/ld-2.12.so.debug...done.
  done.
  0x00000039b54aca20 in __nanosleep_nocancel () from /lib64/libc.so.6
  (gdb) detach
  Detaching from program: /bin/sleep, process 15465
  (gdb) atta 15450
  Attaching to program: /bin/sleep, process 15450
  Reading symbols from /bin/sleep...(no debugging symbols found)...done.
  Reading symbols from /lib64/libc.so.6...warning: the debug information found in "/usr/lib/debug//lib64/libc-2.12.so.debug" does not match "/lib64/libc.so.6" (CRC mismatch).
  (no debugging symbols found)...done.
  Reading symbols from /lib64/ld-linux-x86-64.so.2...Reading symbols from /usr/lib/debug/lib64/ld-2.12.so.debug...done.
  done.
  0x00000039b54aca20 in __nanosleep_nocancel () from /lib64/libc.so.6
  (gdb) 

I did notice that if you do the second or third attach without a
detach GDB asks "A program is being debugged already.  Kill it?"
That's not what I'd expect (I'd expect GDB to offer to detach).
But that's a separate issue...

Joel, should I commit this patch?  FAOD it's
https://sourceware.org/ml/gdb-patches/2015-06/msg00110.html

Thanks,
Gary

-- 
http://gbenson.net/

Re: [PATCH] Make only user-specified executable filenames sticky

[Gary Benson]

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

-- 
http://gbenson.net/

Re: [PATCH v3] Make only user-specified executable and symbol filenames sticky

[Pedro Alves]

Sorry for dropping the ball on this...  Comments below.

On 06/08/2015 10:01 AM, Gary Benson wrote:

> 
> 	PR gdb/17626
> 	* progspace.h (struct program_space)
> 	<pspace_exec_file_is_user_supplied>: New field.
> 	<symfile_object_file_is_user_supplied>: Likewise.
> 	(symfile_objfile_is_user_supplied): New macro.
> 	* exec.h (exec_file_is_user_supplied): Likewise.
> 	* exec.c (exec_close): Clear exec_file_is_user_supplied.
> 	(exec_file_locate_attach): Remove get_exec_file check.
> 	Do not replace user-supplied executable or symbol files.
> 	Warn if user-supplied executable or symbol files do not
> 	match discovered file.
> 	(exec_file_command): Set exec_file_is_user_supplied.
> 	* symfile.c (symbol_file_clear): Clear
> 	symfile_objfile_is_user_supplied.
> 	(symbol_file_command): Set symfile_objfile_is_user_supplied.
> 	* inferior.c (add_inferior_command): Set
> 	exec_file_is_user_supplied and symfile_objfile_is_user_supplied.
> 	* main.c (captured_main): Likewise.
> 	* infcmd.c (attach_command_post_wait): Always call
> 	exec_file_locate_attach.  Only call reopen_exec_file and
> 	reread_symbols if exec_file_is_user_supplied.
> 	* remote.c (remote_add_inferior): Remove get_exec_file check
> 	around exec_file_locate_attach.
> ---
>  gdb/ChangeLog   |   26 ++++++++++++++++++++++++++
>  gdb/exec.c      |   30 +++++++++++++++++++++++-------
>  gdb/exec.h      |    2 ++
>  gdb/infcmd.c    |   13 ++++++++-----
>  gdb/inferior.c  |    3 +++
>  gdb/main.c      |   24 ++++++++++++++++--------
>  gdb/progspace.h |   17 +++++++++++++++++
>  gdb/remote.c    |    9 ++++++---
>  gdb/symfile.c   |    3 +++
>  9 files changed, 104 insertions(+), 23 deletions(-)
> 
> diff --git a/gdb/exec.c b/gdb/exec.c
> index 8a4ab6f..11e5db0 100644
> --- a/gdb/exec.c
> +++ b/gdb/exec.c
> @@ -102,6 +102,7 @@ exec_close (void)
>  
>        xfree (exec_filename);
>        exec_filename = NULL;
> +      exec_file_is_user_supplied = 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)
> @@ -171,8 +167,27 @@ exec_file_locate_attach (int pid, int from_tty)
>  	full_exec_path = xstrdup (exec_file);
>      }
>  
> -  exec_file_attach (full_exec_path, from_tty);
> -  symbol_file_add_main (full_exec_path, from_tty);
> +  if (exec_file_is_user_supplied)
> +    {
> +      if (strcmp (full_exec_path, exec_filename) != 0)
> +	warning (_("Process %d has executable file %s,"
> +		   " but executable file is currently set to %s"),
> +		 pid, full_exec_path, exec_filename);
> +    }
> +  else
> +    exec_file_attach (full_exec_path, from_tty);
> +
> +  if (symfile_objfile_is_user_supplied)
> +    {
> +      const char *symbol_filename = objfile_filename (symfile_objfile);
> +
> +      if (strcmp (full_exec_path, symbol_filename) != 0)
> +	warning (_("Process %d has symbol file %s,"
> +		   " but symbol file is currently set to %s"),
> +		 pid, full_exec_path, symbol_filename);
> +    }
> +  else
> +    symbol_file_add_main (full_exec_path, from_tty);
>  }
>  


This function is called while the passed in PID is not the
current inferior.  E.g., the remote_add_inferior path.

Therefore seems to me that these symbol_file_add_main / exec_file_attach
calls can change the symbols of the wrong inferior.

(I still suspect that if we reverse the sense of the flag, then
its management ends up being more centralized, as then the
place that sets it is also the place that needs to check it,
instead of doing that in multiple places.  But, see below.)

I also notice that reread_symbols has an exec_file_attach
call which seems to me results in losing the is_user_supplied
flag if the executable's timestamp changed, at least here:

> +  /* Attempt to open the file that was executed to create this
> +     inferior.  If the user has explicitly specified executable
> +     and/or symbol files then warn the user if their choices do
> +     not match.  Otherwise, set exec_file and symfile_objfile to
> +     the new file.  */
> +  exec_file_locate_attach (ptid_get_pid (inferior_ptid), from_tty);
> +
> +  if (exec_file_is_user_supplied)
>      {
>        reopen_exec_file ();
>        reread_symbols ();


I also notice that the clone-inferior command ends up with
an exec_file_attach call in clone_program_space.  That one
should probably be setting the is_user_supplied flag too,
I'd think.  Or at least, copying it from the original.

At this point, I'm wondering about adding a parameter to
exec_file_attach to force considering (now and in future)
the right value to put in the flag in each case.

> @@ -2490,11 +2490,14 @@ 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
> +  /* Attempt to open the file that was executed to create this
> +     inferior.  If the user has explicitly specified executable
> +     and/or symbol files then warn the user if their choices do
> +     not match.  Otherwise, set exec_file and symfile_objfile to
> +     the new file.  */
> +  exec_file_locate_attach (ptid_get_pid (inferior_ptid), from_tty);
> +
> +  if (exec_file_is_user_supplied)
>      {
>        reopen_exec_file ();
>        reread_symbols ();

It seems to me that we should be able to move these reopen/reread
calls inside exec_file_locate_attach.

Thanks,
Pedro Alves

Re: [PATCH] Make only user-specified executable filenames sticky

[Joel Brobecker]

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

-- 
Joel

Re: [PATCH v3] Make only user-specified executable and symbol filenames sticky

[Pedro Alves]

On 07/03/2015 04:44 PM, Pedro Alves wrote:

> (I still suspect that if we reverse the sense of the flag, then
> its management ends up being more centralized, as then the
> place that sets it is also the place that needs to check it,
> instead of doing that in multiple places.  But, see below.)

It didn't seem fair to impose a subjective preference, so I tried
this in order to understand it myself, and I now agree that is really
doesn't make much difference, as then we'd have to mark
auto-discovered in a few more places that I wasn't originally
seeing.  There's at least the execd handling in infrun.c, and
also spu_symbol_file_add_from_memory.

As I was playing with this already, I poked at the other review
points I made, and came up with the variant of the patch below.

> This function is called while the passed in PID is not the
> current inferior.  E.g., the remote_add_inferior path.
> 
> Therefore seems to me that these symbol_file_add_main / exec_file_attach
> calls can change the symbols of the wrong inferior.

...

> I also notice that reread_symbols has an exec_file_attach
> call which seems to me results in losing the is_user_supplied
> flag if the executable's timestamp changed, at least here:
> 
>> +  /* Attempt to open the file that was executed to create this
>> +     inferior.  If the user has explicitly specified executable
>> +     and/or symbol files then warn the user if their choices do
>> +     not match.  Otherwise, set exec_file and symfile_objfile to
>> +     the new file.  */
>> +  exec_file_locate_attach (ptid_get_pid (inferior_ptid), from_tty);
>> +
>> +  if (exec_file_is_user_supplied)
>>      {
>>        reopen_exec_file ();
>>        reread_symbols ();
> 
> 
> I also notice that the clone-inferior command ends up with
> an exec_file_attach call in clone_program_space.  That one
> should probably be setting the is_user_supplied flag too,
> I'd think.  Or at least, copying it from the original.
> 
> At this point, I'm wondering about adding a parameter to
> exec_file_attach to force considering (now and in future)
> the right value to put in the flag in each case.

I tried this too (see patch below).  As this requires touching the user-supplied
paths anyway, it didn't really matter to tag files user-supplied or
auto-discovered, so I reverted back to user-supplied.  For symbols,
we already have the add_flags, so I added SYMFILE_USER_SUPPLIED instead
of a new boolean.

> 
>> @@ -2490,11 +2490,14 @@ 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
>> +  /* Attempt to open the file that was executed to create this
>> +     inferior.  If the user has explicitly specified executable
>> +     and/or symbol files then warn the user if their choices do
>> +     not match.  Otherwise, set exec_file and symfile_objfile to
>> +     the new file.  */
>> +  exec_file_locate_attach (ptid_get_pid (inferior_ptid), from_tty);
>> +
>> +  if (exec_file_is_user_supplied)
>>      {
>>        reopen_exec_file ();
>>        reread_symbols ();
> 
> It seems to me that we should be able to move these reopen/reread
> calls inside exec_file_locate_attach.

I did this too.

What do you think of the version below?

I also tweaked the warnings a bit, to explicitly say "mismatch".

This would still need docs/NEWS changes, and a test would be good too.

----------
From 49596904260dc9829fbe138c8a1070fff3a5cd63 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Mon, 6 Jul 2015 12:41:35 +0100
Subject: [PATCH] Make only user-specified executable and symbol filenames
 sticky

- pass user-supplied / auto-discovered as arguments to exec_file_attach and
  symbol_file_add_main

- Change exec_file_locate_attach parameter from pid to inferior
  pointer.

- Flip current program space temporarily to make sure we add/reload
  symbols to/of the right inferior/program space.

- Move reopen_exec_file and reread_symbols calls inside
  exec_file_locate_attach.

- Warning message tweaks.

- Rename exec_file_locate_attach -> exec_file_and_symbols_resync
---
 gdb/corefile.c   |  2 +-
 gdb/exec.c       | 86 ++++++++++++++++++++++++++++++++++++++------------------
 gdb/exec.h       |  2 ++
 gdb/gdbcore.h    | 25 ++++++++++------
 gdb/infcmd.c     | 15 ++++------
 gdb/inferior.c   |  6 ++--
 gdb/infrun.c     |  2 +-
 gdb/main.c       | 44 +++++++++++++++++++++++------
 gdb/progspace.c  |  5 ++--
 gdb/progspace.h  | 17 +++++++++++
 gdb/remote.c     | 11 +++++---
 gdb/solib-svr4.c |  2 +-
 gdb/symfile.c    | 25 ++++++++++------
 gdb/symfile.h    |  9 ++++--
 14 files changed, 175 insertions(+), 76 deletions(-)

diff --git a/gdb/corefile.c b/gdb/corefile.c
index 5246f71..5d861de 100644
--- a/gdb/corefile.c
+++ b/gdb/corefile.c
@@ -146,7 +146,7 @@ reopen_exec_file (void)
   res = stat (filename, &st);
 
   if (exec_bfd_mtime && exec_bfd_mtime != st.st_mtime)
-    exec_file_attach (filename, 0);
+    exec_file_attach (exec_file_was_user_supplied, filename, 0);
   else
     /* If we accessed the file since last opening it, close it now;
        this stops GDB from holding the executable open after it
diff --git a/gdb/exec.c b/gdb/exec.c
index 3dfc437..ce50037 100644
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -78,7 +78,7 @@ static void
 exec_open (const char *args, int from_tty)
 {
   target_preopen (from_tty);
-  exec_file_attach (args, from_tty);
+  exec_file_attach (1, args, from_tty);
 }
 
 /* Close and clear exec_bfd.  If we end up with no target sections to
@@ -102,6 +102,7 @@ exec_close (void)
 
       xfree (exec_filename);
       exec_filename = NULL;
+      exec_file_was_user_supplied = 0;
     }
 }
 
@@ -138,40 +139,67 @@ exec_file_clear (int from_tty)
 /* See gdbcore.h.  */
 
 void
-exec_file_locate_attach (int pid, int from_tty)
+exec_file_and_symbols_resync (struct inferior *inf, int from_tty)
 {
   char *exec_file, *full_exec_path = NULL;
+  struct cleanup *old_chain = save_current_program_space ();
 
-  /* Do nothing if we already have an executable filename.  */
-  exec_file = (char *) get_exec_file (0);
-  if (exec_file != NULL)
-    return;
+  /* Switch over temporarily, while reading executable and
+     symbols.  */
+  set_current_program_space (inf->pspace);
 
   /* Try to determine a filename from the process itself.  */
-  exec_file = target_pid_to_exec_file (pid);
-  if (exec_file == NULL)
-    return;
+  exec_file = target_pid_to_exec_file (inf->pid);
+  if (exec_file != NULL)
+    {
+      /* If gdb_sysroot is not empty and the discovered filename
+	 is absolute then prefix the filename with gdb_sysroot.  */
+      if (*gdb_sysroot != '\0' && IS_ABSOLUTE_PATH (exec_file))
+	full_exec_path = exec_file_find (exec_file, NULL);
 
-  /* If gdb_sysroot is not empty and the discovered filename
-     is absolute then prefix the filename with gdb_sysroot.  */
-  if (*gdb_sysroot != '\0' && IS_ABSOLUTE_PATH (exec_file))
-    full_exec_path = exec_file_find (exec_file, NULL);
+      if (full_exec_path == NULL)
+	{
+	  /* It's possible we don't have a full path, but rather just a
+	     filename.  Some targets, such as HP-UX, don't provide the
+	     full path, sigh.
+
+	     Attempt to qualify the filename against the source path.
+	     (If that fails, we'll just fall back on the original
+	     filename.  Not much more we can do...)  */
+	  if (!source_full_path_of (exec_file, &full_exec_path))
+	    full_exec_path = xstrdup (exec_file);
+	}
+    }
 
-  if (full_exec_path == NULL)
+  if (exec_filename != NULL && exec_file_was_user_supplied)
     {
-      /* It's possible we don't have a full path, but rather just a
-	 filename.  Some targets, such as HP-UX, don't provide the
-	 full path, sigh.
-
-	 Attempt to qualify the filename against the source path.
-	 (If that fails, we'll just fall back on the original
-	 filename.  Not much more we can do...)  */
-      if (!source_full_path_of (exec_file, &full_exec_path))
-	full_exec_path = xstrdup (exec_file);
+      if (full_exec_path != NULL && strcmp (full_exec_path, exec_filename) != 0)
+	warning (_("Detected exec-file mismatch on %s.  Running %s; Loaded %s"),
+		 target_pid_to_str (pid_to_ptid (inf->pid)),
+		 full_exec_path, exec_filename);
+      reopen_exec_file ();
     }
+  else if (full_exec_path != NULL)
+    exec_file_attach (0, full_exec_path, from_tty);
 
-  exec_file_attach (full_exec_path, from_tty);
-  symbol_file_add_main (full_exec_path, from_tty);
+  if (symfile_objfile != NULL && symfile_objfile_was_user_supplied)
+    {
+      const char *symbol_filename = objfile_filename (symfile_objfile);
+
+      if (full_exec_path != NULL
+	  && strcmp (full_exec_path, symbol_filename) != 0)
+	warning (_("Detected symbol-file mismatch on %s.  Running %s; Loaded %s"),
+		 target_pid_to_str (pid_to_ptid (inf->pid)),
+		 full_exec_path, symbol_filename);
+    }
+  else if (full_exec_path != NULL)
+    symbol_file_add_main (0, full_exec_path, from_tty);
+
+  /* Re-read symbol files that were modified since we last loaded
+     them.  */
+  reread_symbols ();
+
+  do_cleanups (old_chain);
 }
 
 /* Set FILENAME as the new exec file.
@@ -192,7 +220,7 @@ exec_file_locate_attach (int pid, int from_tty)
    we're supplying the exec pathname late for good reason.)  */
 
 void
-exec_file_attach (const char *filename, int from_tty)
+exec_file_attach (int user_supplied, const char *filename, int from_tty)
 {
   struct cleanup *cleanups;
 
@@ -333,6 +361,8 @@ exec_file_attach (const char *filename, int from_tty)
 
   do_cleanups (cleanups);
 
+  exec_file_was_user_supplied = user_supplied;
+
   bfd_cache_close_all ();
   observer_notify_executable_changed ();
 }
@@ -374,12 +404,12 @@ exec_file_command (char *args, int from_tty)
 
       filename = tilde_expand (*argv);
       make_cleanup (xfree, filename);
-      exec_file_attach (filename, from_tty);
+      exec_file_attach (1, filename, from_tty);
 
       do_cleanups (cleanups);
     }
   else
-    exec_file_attach (NULL, from_tty);
+    exec_file_attach (1, NULL, from_tty);
 }
 
 /* Set both the exec file and the symbol file, in one command.
diff --git a/gdb/exec.h b/gdb/exec.h
index c7f3b56..aef2926 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 exec_file_was_user_supplied \
+  current_program_space->pspace_exec_file_was_user_supplied
 
 /* Builds a section table, given args BFD, SECTABLE_PTR, SECEND_PTR.
    Returns 0 if OK, 1 on error.  */
diff --git a/gdb/gdbcore.h b/gdb/gdbcore.h
index 0c08b37..f34ee77 100644
--- a/gdb/gdbcore.h
+++ b/gdb/gdbcore.h
@@ -146,14 +146,23 @@ extern int write_files;
 
 extern void core_file_command (char *filename, int from_tty);
 
-extern void exec_file_attach (const char *filename, int from_tty);
-
-/* If the filename of the main executable is unknown, attempt to
-   determine it.  If a filename is determined, proceed as though
-   it was just specified with the "file" command.  Do nothing if
-   the filename of the main executable is already known.  */
-
-extern void exec_file_locate_attach (int pid, int from_tty);
+extern void exec_file_attach (int user_supplied, const char *filename,
+			      int from_tty);
+
+/* Resync executable and symbols.  Fetch the filename of the main
+   executable based on what the target is running.  If a filename can
+   be determined, and the current exec file loaded was not previously
+   user supplied, then load the discovered filename as exec file.  If
+   the executable loaded had been user supplied (with e.g., the "file"
+   command), issue a warning if there's a mismatch, and reload the
+   previously user-specified executable (in case the program was
+   recompiled since the last time we loaded it).  Likewise, if the
+   currenly load main symbol file was not user supplied, load the
+   discovered filename as symbol file; otherwise, if there's a
+   mismatch, warn.  End by reading all symbol files that have been
+   modified since the last time we loaded them.  */
+
+extern void exec_file_and_symbols_resync (struct inferior *inf, int from_tty);
 
 extern void exec_file_clear (int from_tty);
 
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 03282a7..528fd65 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -2490,15 +2490,12 @@ 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
-    {
-      reopen_exec_file ();
-      reread_symbols ();
-    }
+  /* Attempt to open the file that was executed to create this
+     inferior.  If the user has explicitly specified executable
+     and/or symbol files then warn the user if their choices do
+     not match.  Otherwise, set exec_file and symfile_objfile to
+     the new file.  */
+  exec_file_and_symbols_resync (inferior, 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 d0783d3..1e0d76e 100644
--- a/gdb/inferior.c
+++ b/gdb/inferior.c
@@ -882,13 +882,13 @@ add_inferior_command (char *args, int from_tty)
       if (exec != NULL)
 	{
 	  /* Switch over temporarily, while reading executable and
-	     symbols.q.  */
+	     symbols.  */
 	  set_current_program_space (inf->pspace);
 	  set_current_inferior (inf);
 	  switch_to_thread (null_ptid);
 
-	  exec_file_attach (exec, from_tty);
-	  symbol_file_add_main (exec, from_tty);
+	  exec_file_attach (1, exec, from_tty);
+	  symbol_file_add_main (SYMFILE_USER_SPECIFIED, exec, from_tty);
 	}
     }
 
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 445a612..7dae228 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -1181,7 +1181,7 @@ follow_exec (ptid_t ptid, char *execd_pathname)
   gdb_assert (current_program_space == inf->pspace);
 
   /* That a.out is now the one to use.  */
-  exec_file_attach (execd_pathname, 0);
+  exec_file_attach (0, execd_pathname, 0);
 
   /* SYMFILE_DEFER_BP_RESET is used as the proper displacement for PIE
      (Position Independent Executable) main symbol file will get applied by
diff --git a/gdb/main.c b/gdb/main.c
index aecd60a..d4a5671 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -405,6 +405,29 @@ catch_command_errors_const (catch_command_errors_const_ftype *command,
   return 1;
 }
 
+/* Like catch_command_errors, but specifically for symbol/exec load
+   routines.  */
+
+typedef void (catch_exec_or_symbol_errors_ftype) (int, const char *, int);
+
+static int
+catch_exec_or_symbol_errors (catch_exec_or_symbol_errors_ftype *func,
+			     int arg, const char *filename, int from_tty)
+{
+  TRY
+    {
+      func (arg, filename, from_tty);
+    }
+  CATCH (e, RETURN_MASK_ALL)
+    {
+      exception_print (gdb_stderr, e);
+      return 0;
+    }
+  END_CATCH
+
+  return 1;
+}
+
 /* Type of this option.  */
 enum cmdarg_kind
 {
@@ -1047,20 +1070,23 @@ captured_main (void *data)
     {
       /* The exec file and the symbol-file are the same.  If we can't
          open it, better only print one error message.
-         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);
+         catch_exec_or_symbol_errors returns non-zero on success.  */
+      if (catch_exec_or_symbol_errors (exec_file_attach,
+				       1, execarg, !batch_flag))
+	catch_exec_or_symbol_errors (symbol_file_add_main,
+				     SYMFILE_USER_SPECIFIED,
+				     symarg, !batch_flag);
     }
   else
     {
       if (execarg != NULL)
-	catch_command_errors_const (exec_file_attach, execarg,
-				    !batch_flag);
+	catch_exec_or_symbol_errors (exec_file_attach, 1,
+				     execarg, !batch_flag);
+
       if (symarg != NULL)
-	catch_command_errors_const (symbol_file_add_main, symarg,
-				    !batch_flag);
+	catch_exec_or_symbol_errors (symbol_file_add_main,
+				     SYMFILE_USER_SPECIFIED,
+				     symarg, !batch_flag);
     }
 
   if (corearg && pidarg)
diff --git a/gdb/progspace.c b/gdb/progspace.c
index 1c0a254..730f44a 100644
--- a/gdb/progspace.c
+++ b/gdb/progspace.c
@@ -182,10 +182,11 @@ clone_program_space (struct program_space *dest, struct program_space *src)
   set_current_program_space (dest);
 
   if (src->pspace_exec_filename != NULL)
-    exec_file_attach (src->pspace_exec_filename, 0);
+    exec_file_attach (1, src->pspace_exec_filename, 0);
 
   if (src->symfile_object_file != NULL)
-    symbol_file_add_main (objfile_name (src->symfile_object_file), 0);
+    symbol_file_add_main (SYMFILE_USER_SPECIFIED,
+			  objfile_name (src->symfile_object_file), 0);
 
   do_cleanups (old_chain);
   return dest;
diff --git a/gdb/progspace.h b/gdb/progspace.h
index f960093..956372f 100644
--- a/gdb/progspace.h
+++ b/gdb/progspace.h
@@ -154,6 +154,12 @@ 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 the "file" or
+       "add-inferior -exec" commands.  Zero if pspace_exec_filename is
+       unset or was discovered by GDB.  */
+    int pspace_exec_file_was_user_supplied;
+
     /* 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
@@ -183,6 +189,12 @@ struct program_space
        (e.g. the argument to the "symbol-file" or "file" command).  */
     struct objfile *symfile_object_file;
 
+    /* Nonzero if symfile_object_file is set and was discovered by
+       GDB.  Zero if symfile_object_file is not set or was supplied
+       by the user, either at startup (on the command-line) or via the
+       "file" or "add-inferior -exec" commands.  */
+    int symfile_object_file_was_user_supplied;
+
     /* All known objfiles are kept in a linked list.  This points to
        the head of this list.  */
     struct objfile *objfiles;
@@ -215,6 +227,11 @@ struct program_space
 
 #define symfile_objfile current_program_space->symfile_object_file
 
+/* See program_space->symfile_object_file_was_user_supplied.  */
+
+#define symfile_objfile_was_user_supplied \
+  current_program_space->symfile_object_file_was_user_supplied
+
 /* All known objfiles are kept in a linked list.  This points to the
    root of this list.  */
 #define object_files current_program_space->objfiles
diff --git a/gdb/remote.c b/gdb/remote.c
index 9d97f6b..aed0749 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1636,10 +1636,13 @@ 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 && get_exec_file (0) == NULL)
-    exec_file_locate_attach (pid, 1);
+  /* Attempt to open the file that was executed to create this
+     inferior.  If the user has explicitly specified executable
+     and/or symbol files then warn the user if their choices do
+     not match.  Otherwise, set exec_file and symfile_objfile to
+     the new file.  */
+  if (try_open_exec)
+    exec_file_and_symbols_resync (inf, 1);
 
   return inf;
 }
diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c
index 909dfb7..4a329c3 100644
--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -1039,7 +1039,7 @@ open_symbol_file_object (void *from_ttyp)
     }
 
   /* Have a pathname: read the symbol file.  */
-  symbol_file_add_main (filename, from_tty);
+  symbol_file_add_main (0, filename, from_tty);
 
   do_cleanups (cleanups);
   return 1;
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 0c35ffa..4b3d6c3 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -85,7 +85,8 @@ int readnow_symbol_files;	/* Read full symbols immediately.  */
 
 static void load_command (char *, int);
 
-static void symbol_file_add_main_1 (const char *args, int from_tty, int flags);
+static void symbol_file_add_main_1 (int add_flags, const char *args,
+				    int from_tty, int flags);
 
 static void add_symbol_file_command (char *, int);
 
@@ -1306,16 +1307,17 @@ symbol_file_add (const char *name, int add_flags,
    command itself.  */
 
 void
-symbol_file_add_main (const char *args, int from_tty)
+symbol_file_add_main (int add_flags, const char *args, int from_tty)
 {
-  symbol_file_add_main_1 (args, from_tty, 0);
+  symbol_file_add_main_1 (add_flags, args, from_tty, 0);
 }
 
 static void
-symbol_file_add_main_1 (const char *args, int from_tty, int flags)
+symbol_file_add_main_1 (int add_flags, const char *args, int from_tty,
+			int flags)
 {
-  const int add_flags = (current_inferior ()->symfile_flags
-			 | SYMFILE_MAINLINE | (from_tty ? SYMFILE_VERBOSE : 0));
+  add_flags |= (current_inferior ()->symfile_flags
+		| SYMFILE_MAINLINE | (from_tty ? SYMFILE_VERBOSE : 0));
 
   symbol_file_add (args, add_flags, NULL, flags);
 
@@ -1325,6 +1327,9 @@ symbol_file_add_main_1 (const char *args, int from_tty, int flags)
 
   if ((flags & SYMFILE_NO_READ) == 0)
     set_initial_language ();
+
+  symfile_objfile_was_user_supplied
+    = (add_flags & SYMFILE_USER_SPECIFIED) != 0;
 }
 
 void
@@ -1347,6 +1352,8 @@ symbol_file_clear (int from_tty)
   gdb_assert (symfile_objfile == NULL);
   if (from_tty)
     printf_unfiltered (_("No symbol file now.\n"));
+
+  symfile_objfile_was_user_supplied = 0;
 }
 
 static int
@@ -1663,7 +1670,8 @@ symbol_file_command (char *args, int from_tty)
 	    error (_("unknown option `%s'"), *argv);
 	  else
 	    {
-	      symbol_file_add_main_1 (*argv, from_tty, flags);
+	      symbol_file_add_main_1 (SYMFILE_USER_SPECIFIED,
+				      *argv, from_tty, flags);
 	      name = *argv;
 	    }
 
@@ -2536,7 +2544,8 @@ reread_symbols (void)
 	    {
 	      /* Reload EXEC_BFD without asking anything.  */
 
-	      exec_file_attach (bfd_get_filename (objfile->obfd), 0);
+	      exec_file_attach (exec_file_was_user_supplied,
+				bfd_get_filename (objfile->obfd), 0);
 	    }
 
 	  /* Keep the calls order approx. the same as in free_objfile.  */
diff --git a/gdb/symfile.h b/gdb/symfile.h
index 9ef3f0b..9ef3d19 100644
--- a/gdb/symfile.h
+++ b/gdb/symfile.h
@@ -457,7 +457,11 @@ enum symfile_add_flags
 
     /* Do not immediately read symbols for this file.  By default,
        symbols are read when the objfile is created.  */
-    SYMFILE_NO_READ = 1 << 4
+    SYMFILE_NO_READ = 1 << 4,
+
+    /* Whether the symbol file was user-supplied, as opposed to
+       auto-discovered by GDB.  */
+    SYMFILE_USER_SPECIFIED = 1 << 5,
   };
 
 extern struct objfile *symbol_file_add (const char *, int,
@@ -556,7 +560,8 @@ extern CORE_ADDR overlay_unmapped_address (CORE_ADDR, struct obj_section *);
 extern CORE_ADDR symbol_overlayed_address (CORE_ADDR, struct obj_section *);
 
 /* Load symbols from a file.  */
-extern void symbol_file_add_main (const char *args, int from_tty);
+extern void symbol_file_add_main (int add_flags,
+				  const char *args, int from_tty);
 
 /* Clear GDB symbol tables.  */
 extern void symbol_file_clear (int from_tty);
-- 
1.9.3

Re: [PATCH] Make only user-specified executable filenames sticky

[Joel Brobecker]

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?

-- 
Joel