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

[Gary Benson <gbenson at redhat dot com>]

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