public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [RFA 1/3] New option 'set exec-file-mismatch (reload|warn|off)'.  Fixes PR gdb/17626.
@ 2019-12-21 14:36 Philippe Waroquiers
  2019-12-21 14:36 ` [RFA 2/3] Test 'set exec-file-mismatch reload|warn|off' Philippe Waroquiers
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Philippe Waroquiers @ 2019-12-21 14:36 UTC (permalink / raw)
  To: gdb-patches

New option 'set exec-file-mismatch (reload|warn|off)'.  Fixes PR gdb/17626.

This option allows to tell GDB to detect and possibly handle mismatched
exec-files.

A recurrent problem with GDB is that GDB uses the wrong exec-file
when using the attach/detach commands successively.
Also, in case the user specifies a file on the command line but attaches
to the wrong PID, this error is not made visible and gives a not user
understandable behaviour.

RFA 1/3 Implement 'set/show exec-file-mismatch'.
   implements the option.
RFA 2/2 modifies gdb.base/attach.exp to test the option.
RFA 3/3 documents the option in NEWS and in gdb.texinfo

Compared to the RFC, this version handles the comments of Tom.

Some feedback related to these comments:
  * I have styled the warning/error messages mentionning files in
    exec.c, but styling in such messages only works with the patch:
      [RFA] Ensure GDB warnings are styled.
      https://sourceware.org/ml/gdb-patches/2019-12/msg00691.html

  * Related to new lines in the warning message: many other warning
    messages have new lines.  I have kept them, as it looks to me that
    the message is more clear like that.  Of course, I can remove them
    if deemed better without.

  * What happens if the inferior execs another program ?
    This option only controls the behaviour when attaching to a running
    program.  The behaviour of 'exec' is not changed: GDB already properly
    determines the executable to use after exec.



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [RFA 3/3] Document 'set|show exec-file-mismatch (reload|warn|off)'
  2019-12-21 14:36 [RFA 1/3] New option 'set exec-file-mismatch (reload|warn|off)'. Fixes PR gdb/17626 Philippe Waroquiers
  2019-12-21 14:36 ` [RFA 2/3] Test 'set exec-file-mismatch reload|warn|off' Philippe Waroquiers
@ 2019-12-21 14:36 ` Philippe Waroquiers
  2019-12-21 17:44   ` Eli Zaretskii
  2019-12-22  2:46   ` Christian Biesinger via gdb-patches
  2019-12-21 14:36 ` [RFA 1/3] Implement 'set/show exec-file-mismatch' Philippe Waroquiers
  2 siblings, 2 replies; 11+ messages in thread
From: Philippe Waroquiers @ 2019-12-21 14:36 UTC (permalink / raw)
  To: gdb-patches; +Cc: Philippe Waroquiers

Mention in NEWS the new option and the set/show commands.

Document in gdb.texinfo the new option and the set/show commands.

gdb/ChangeLog
YYYY-MM-DD  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* NEWS: Mention the new option and the set/show commands.

gdb/doc/ChangeLog
YYYY-MM-DD  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* gdb.texinfo (Attach): Document the new option and the
	set/show commands.
	(Connecting): Reference the exec-file-mismatch option.
---
 gdb/NEWS            | 12 ++++++++++++
 gdb/doc/gdb.texinfo | 31 +++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+)

diff --git a/gdb/NEWS b/gdb/NEWS
index ee10914fd8..1a90862b0d 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -3,6 +3,18 @@
 
 *** Changes since GDB 9
 
+* New commands
+
+set exec-file-mismatch -- Set exec-file-mismatch handling (reload|warn|off).
+show exec-file-mismatch -- Show exec-file-mismatch handling (reload|warn|off).
+  Set or show the option 'exec-file-mismatch'.  When GDB attaches to
+  a running program and can determine the running program, this new option
+  indicates how to handle a mismatch between the current exec-file and
+  the automatically detected file.
+  'reload' is the default value: in case of mismatch, GDB will warn the user
+  and reload the automatically determined file after user confirmation.
+
+
 *** Changes in GDB 9
 
 * 'thread-exited' event is now available in the annotations interface.
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 4d25f755d7..a246ba0bf5 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -2904,6 +2904,31 @@ the program is not found) by using the source file search path
 the @code{file} command to load the program.  @xref{Files, ,Commands to
 Specify Files}.
 
+@anchor{set exec-file-mismatch}
+If the debugger can determine the program running in the process
+and this program does not match the current exec-file, the option
+@code{exec-file-mismatch} specifies how to handle the mismatch.
+
+@table @code
+@kindex exec-file-mismatch
+@cindex set exec-file-mismatch
+@item set exec-file-mismatch @samp{reload|warn|off}
+In case of mismatch between the current exec-file and the automatically
+determined exec-file of the PID the debugger is attaching to,
+@samp{reload} indicates to give a warning to the user and reload
+the automatically determined exec-file.  The user will be asked to
+confirm the loading of the automatically determined file.
+With @samp{warn}, @value{GDBN} just gives a warning to the user to
+signal the mismatch.  @samp{off} indicates to not check for mismatch.
+The default value is @samp{reload}.
+
+
+@cindex show exec-file-mismatch
+@item show exec-file-mismatch
+Show the current value of @code{exec-file-mismatch}.
+
+@end table
+
 The first thing @value{GDBN} does after arranging to debug the specified
 process is to stop it.  You can examine and modify an attached process
 with all the @value{GDBN} commands that are ordinarily available when
@@ -21780,6 +21805,12 @@ established.  If you are using @code{gdbserver}, you may also invoke
 @code{gdbserver} using the @option{--attach} option
 (@pxref{Running gdbserver}).
 
+Some remote targets allow @value{GDBN} to determine the program running
+in the process the debugger is attaching to.  In such a case, @value{GDBN}
+uses the value of @code{exec-file-mismatch} to handle a possible mismatch
+between the program running in the process and the current exec-file.
+(@pxref{set exec-file-mismatch}).
+
 @end table
 
 @anchor{Host and target files}
-- 
2.20.1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [RFA 1/3] Implement 'set/show exec-file-mismatch'.
  2019-12-21 14:36 [RFA 1/3] New option 'set exec-file-mismatch (reload|warn|off)'. Fixes PR gdb/17626 Philippe Waroquiers
  2019-12-21 14:36 ` [RFA 2/3] Test 'set exec-file-mismatch reload|warn|off' Philippe Waroquiers
  2019-12-21 14:36 ` [RFA 3/3] Document 'set|show exec-file-mismatch (reload|warn|off)' Philippe Waroquiers
@ 2019-12-21 14:36 ` Philippe Waroquiers
  2020-01-08  0:58   ` Tom Tromey
  2 siblings, 1 reply; 11+ messages in thread
From: Philippe Waroquiers @ 2019-12-21 14:36 UTC (permalink / raw)
  To: gdb-patches; +Cc: Philippe Waroquiers

This option allows to tell GDB to detect and possibly handle mismatched exec-files.

A recurrent problem with GDB is that GDB uses the wrong exec-file
when using the attach/detach commands successively.
Also, in case the user specifies a file on the command line but attaches
to the wrong PID, this error is not made visible and gives a not user
understandable behaviour.

For example:
  $ gdb
  ...
  (gdb) atta 2682  ############################################  PID running 'sleepers' executable
  Attaching to process 2682
  [New LWP 2683]
  [New LWP 2684]
  [New LWP 2685]
  [Thread debugging using libthread_db enabled]
  Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
  0x00007f5ff829f603 in select () at ../sysdeps/unix/syscall-template.S:84
  84    ../sysdeps/unix/syscall-template.S: No such file or directory.
  (gdb) det
  Detaching from program: /home/philippe/valgrind/git/trunk_untouched/gdbserver_tests/sleepers, process 2682
  [Inferior 1 (process 2682) detached]
  (gdb) atta 31069 ############################################  PID running 'gdb' executable
  Attaching to program: /home/philippe/valgrind/git/trunk_untouched/gdbserver_tests/sleepers, process 31069
  Reading symbols from /lib64/ld-linux-x86-64.so.2...
  Reading symbols from /usr/lib/debug/.build-id/60/6df9c355103e82140d513bc7a25a635591c153.debug...
  0x00007f43c23478a0 in ?? ()
  (gdb) bt
  #0  0x00007f43c23478a0 in ?? ()
  #1  0x0000558909e3ad91 in ?? ()
  #2  0x0000202962646700 in ?? ()
  #3  0x00007ffc69c74e70 in ?? ()
  #4  0x000055890c1d2350 in ?? ()
  #5  0x0000000000000000 in ?? ()
  (gdb)

The second attach has kept the executable of the first attach.
(in this case, 31069 is the PID of a GDB, that has nothing to do
with the first determined 'sleepers' executable).

Similarly, if specifying an executable, but attaching to a wrong pid,
we get:

  gdb /home/philippe/valgrind/git/trunk_untouched/gdbserver_tests/sleepers
  ...
  Reading symbols from /home/philippe/valgrind/git/trunk_untouched/gdbserver_tests/sleepers...
  (gdb) atta 31069 ############################################  PID running 'gdb' executable
  Attaching to program: /home/philippe/valgrind/git/trunk_untouched/gdbserver_tests/sleepers, process 31069
  Reading symbols from /lib64/ld-linux-x86-64.so.2...
  Reading symbols from /usr/lib/debug/.build-id/60/6df9c355103e82140d513bc7a25a635591c153.debug...
  0x00007f43c23478a0 in ?? ()
  (gdb) bt
  #0  0x00007f43c23478a0 in ?? ()
  #1  0x0000558909e3ad91 in ?? ()
  #2  0x0000202962646700 in ?? ()
  #3  0x00007ffc69c74e70 in ?? ()
  #4  0x000055890c1d2350 in ?? ()
  #5  0x0000000000000000 in ?? ()
  (gdb)

And it is unclear to the user what has happened/what is going wrong.

This patch series implements a new option:
  (gdb) apropos exec-file-mismatch
  set exec-file-mismatch -- Set exec-file-mismatch handling (reload|warn|off).
  show exec-file-mismatch -- Show exec-file-mismatch handling (reload|warn|off).
  (gdb) help set exec-file-mismatch
  Set exec-file-mismatch handling (reload|warn|off).
  Specifies how to handle a mismatch between the current exec-file
  and the exec-file automatically determined when attaching to a process:

   reload - warn the user, reload the automatically determined exec-file.
   warn   - warn the user, but do not change the exec-file.
   off    - do not check for mismatch.
  (gdb)

"reload" means: in case of mismatch between the current exec-file
and the automatically determined exec-file of the PID we are attaching to,
give a warning to the user and reload the automatically determined exec-file.

"warn" means: in case of mismatch, just give a warning to the user.

"off" means: do not check for mismatch.

This fixes PR gdb/17626.
There was a previous trial to fix this PR.
See https://sourceware.org/ml/gdb-patches/2015-07/msg00118.html
This trial was however only fixing the problem for the automatically
determined executable files when doing attach.
It was differentiating the 'user specified executable files' ("sticky")
from the executable files automatically found by GDB.
But such user specified sticky executables are in most cases due
to a wrong manipulation by the user, giving unexpected results
such as backtrace showing no function like in the above example.

This patch ensures that whenever a process executable can be
determined, that the user is warned if there is a mismatch.

The same tests as above then give:

  (gdb) atta 2682
  Attaching to process 2682
  [New LWP 2683]
  [New LWP 2684]
  [New LWP 2685]
  [Thread debugging using libthread_db enabled]
  Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
  0x00007f5ff829f603 in select () at ../sysdeps/unix/syscall-template.S:84
  84    ../sysdeps/unix/syscall-template.S: No such file or directory.
  (gdb) det
  Detaching from program: /home/philippe/valgrind/git/trunk_untouched/gdbserver_tests/sleepers, process 2682
  [Inferior 1 (process 2682) detached]
  (gdb) atta 31069
  Attaching to program: /home/philippe/valgrind/git/trunk_untouched/gdbserver_tests/sleepers, process 31069
  warning: Mismatch between current exec-file /home/philippe/valgrind/git/trunk_untouched/gdbserver_tests/sleepers
  and automatically determined exec-file /bd/home/philippe/gdb/git/build_fixes/gdb/gdb
  exec-file-mismatch handling is currently "reload"
  Load new symbol table from "/bd/home/philippe/gdb/git/build_fixes/gdb/gdb"? (y or n) y
  Reading symbols from /bd/home/philippe/gdb/git/build_fixes/gdb/gdb...
  Setting up the environment for debugging gdb.
  ...
  Reading symbols from /usr/lib/debug/.build-id/60/6df9c355103e82140d513bc7a25a635591c153.debug...
  0x00007f43c23478a0 in __poll_nocancel () at ../sysdeps/unix/syscall-template.S:84
  84    ../sysdeps/unix/syscall-template.S: No such file or directory.
  (top-gdb) bt
  During symbol reading: incomplete CFI data; unspecified registers (e.g., rax) at 0x7f43c23478ad
  During symbol reading: unsupported tag: 'DW_TAG_unspecified_type'
  During symbol reading: cannot get low and high bounds for subprogram DIE at 0x12282a7
  During symbol reading: Child DIE 0x12288ba and its abstract origin 0x1228b26 have different parents
  During symbol reading: DW_AT_call_target target DIE has invalid low pc, for referencing DIE 0x1229540 [in module /bd/home/philippe/gdb/git/build_fixes/gdb/gdb]
  #0  0x00007f43c23478a0 in __poll_nocancel () at ../sysdeps/unix/syscall-template.S:84
  #1  0x0000558909e3ad91 in poll (__timeout=-1, __nfds=<optimized out>, __fds=<optimized out>) at /usr/include/x86_64-linux-gnu/bits/poll2.h:46
  #2  gdb_wait_for_event (block=block@entry=1) at ../../fixes/gdb/event-loop.c:772
  #3  0x0000558909e3aef4 in gdb_do_one_event () at ../../fixes/gdb/event-loop.c:347
  #4  0x0000558909e3b085 in gdb_do_one_event () at ../../fixes/gdb/common/common-exceptions.h:219
  #5  start_event_loop () at ../../fixes/gdb/event-loop.c:371
  During symbol reading: Member function "~_Sp_counted_base" (offset 0x1c69bf7) is virtual but the vtable offset is not specified
  During symbol reading: Multiple children of DIE 0x1c8f5a0 refer to DIE 0x1c8f0ee as their abstract origin
  #6  0x0000558909ed3b78 in captured_command_loop () at ../../fixes/gdb/main.c:331
  #7  0x0000558909ed4b6d in captured_main (data=<optimized out>) at ../../fixes/gdb/main.c:1174
  #8  gdb_main (args=<optimized out>) at ../../fixes/gdb/main.c:1190
  #9  0x0000558909c1e9a8 in main (argc=<optimized out>, argv=<optimized out>) at ../../fixes/gdb/gdb.c:32
  (top-gdb)

  gdb /home/philippe/valgrind/git/trunk_untouched/gdbserver_tests/sleepers
  ...
  Reading symbols from /home/philippe/valgrind/git/trunk_untouched/gdbserver_tests/sleepers...
  (gdb) atta 31069
  Attaching to program: /home/philippe/valgrind/git/trunk_untouched/gdbserver_tests/sleepers, process 31069
  warning: Mismatch between current exec-file /home/philippe/valgrind/git/trunk_untouched/gdbserver_tests/sleepers
  and automatically determined exec-file /bd/home/philippe/gdb/git/build_fixes/gdb/gdb
  exec-file-mismatch handling is currently "reload"
  Load new symbol table from "/bd/home/philippe/gdb/git/build_fixes/gdb/gdb"? (y or n) y
  Reading symbols from /bd/home/philippe/gdb/git/build_fixes/gdb/gdb...
  Setting up the environment for debugging gdb.
  ....

In other words, it now works as intuitively expected by the user.
If ever the user gave the correct executable on the command line,
then attached to the wrong pid, then confirmed loading the wrong executable,
the user can simply fix this by detaching, and attaching to the correct pid,
GDB will then tell again to the user that the exec-file might better
be reloaded.

The default value of "reload" is chosen instead of e.g. "warn" as in most
cases, switching of executable will be the correct action,
and in any case, the user can decide to not load the executable,
as GDB asks a confirmation to the user to load the new executable.

For settings "reload" and "warn", the new function validate_exec_file ()
tries to get the inferior pid exec file and compares it with the current
exec file.  In case of mismatch, it warns the user and optionally reload
the executable.
This function is called in the attach_command implementation to cover
most cases of attaching to a running process.
It must also be called in remote.c, as the attach command is not supported
for all types of remote gdbserver.

gdb/ChangeLog
YYYY-MM-DD  Philippe Waroquiers  <philippe.waroquiers@skynet.be>

	* exec.c (xec_file_mismatch_names, exec_file_mismatch_mode)
	(show_exec_file_mismatch_command, set_exec_file_mismatch_command)
	(validate_exec_file): New variables, enums, functions.
	(exec_file_locate_attach, print_section_info): Style the filenames.
	(_initialize_exec): Install show_exec_file_mismatch_command and
	 set_exec_file_mismatch_command.
	* gdbcore.h (validate_exec_file): Declare.
	* infcmd.c (attach_command): Call validate_exec_file.
	* remote.c ( remote_target::remote_add_inferior): Likewise.
---
 gdb/exec.c    | 144 ++++++++++++++++++++++++++++++++++++++++++++++----
 gdb/gdbcore.h |   5 ++
 gdb/infcmd.c  |   3 ++
 gdb/remote.c  |   3 ++
 4 files changed, 145 insertions(+), 10 deletions(-)

diff --git a/gdb/exec.c b/gdb/exec.c
index efc504eca4..3d36a0ee61 100644
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -46,6 +46,7 @@
 #include "solist.h"
 #include <algorithm>
 #include "gdbsupport/pathstuff.h"
+#include "cli/cli-style.h"
 
 void (*deprecated_file_changed_hook) (const char *);
 
@@ -82,6 +83,50 @@ struct exec_target final : public target_ops
 
 static exec_target exec_ops;
 
+/* How to handle a mismatch between the current exec file and the exec
+   file determined from target.  */
+
+static const char *const exec_file_mismatch_names[]
+  = {"reload", "warn", "off", NULL };
+enum exec_file_mismatch_mode
+  {
+    exec_file_mismatch_reload, exec_file_mismatch_warn, exec_file_mismatch_off
+  };
+static const char *exec_file_mismatch = exec_file_mismatch_names[0];
+static enum exec_file_mismatch_mode exec_file_mismatch_mode
+  = exec_file_mismatch_reload;
+
+/* Show command.  */
+static void
+show_exec_file_mismatch_command (struct ui_file *file, int from_tty,
+				 struct cmd_list_element *c, const char *value)
+{
+  fprintf_filtered (gdb_stdout,
+		    _("exec-file-mismatch handling is currently \"%s\".\n"),
+		    exec_file_mismatch_names[exec_file_mismatch_mode]);
+}
+
+/* Set command.  Change the setting for range checking.  */
+static void
+set_exec_file_mismatch_command (const char *ignore,
+				int from_tty, struct cmd_list_element *c)
+{
+  for (enum exec_file_mismatch_mode mode = exec_file_mismatch_reload;
+       ;
+       mode = static_cast<enum exec_file_mismatch_mode>(1 + (int) mode))
+    {
+      if (strcmp (exec_file_mismatch, exec_file_mismatch_names[mode]) == 0)
+	{
+	  exec_file_mismatch_mode = mode;
+	  return;
+	}
+      if (mode == exec_file_mismatch_off)
+	internal_error (__FILE__, __LINE__,
+			_("Unrecognized exec-file-mismatch setting: \"%s\""),
+			exec_file_mismatch);
+    }
+}
+
 /* Whether to open exec and core files read-only or read-write.  */
 
 bool write_files = false;
@@ -191,6 +236,61 @@ try_open_exec_file (const char *exec_file_host, struct inferior *inf,
 
 /* See gdbcore.h.  */
 
+void
+validate_exec_file (int from_tty)
+{
+  /* If user asked to ignore the mismatch, do nothing.  */
+  if (exec_file_mismatch_mode == exec_file_mismatch_off)
+    return;
+
+  const char *current_exec_file = get_exec_file (0);
+  struct inferior *inf = current_inferior ();
+  /* Try to determine a filename from the process itself.  */
+  const char *pid_exec_file = target_pid_to_exec_file (inf->pid);
+
+  /* If wee cannot validate the exec file, return.  */
+  if (current_exec_file == NULL || pid_exec_file == NULL)
+    return;
+
+  std::string exec_file_target (pid_exec_file);
+
+  /* In case the exec file is not local, exec_file_target has to point at
+     the target file system.  */
+  if (is_target_filename (current_exec_file) && !target_filesystem_is_local ())
+    exec_file_target = TARGET_SYSROOT_PREFIX + exec_file_target;
+
+  if (exec_file_target != current_exec_file)
+    {
+      warning
+	(_("Mismatch between current exec-file %ps\n"
+	   "and automatically determined exec-file %ps\n"
+	   "exec-file-mismatch handling is currently \"%s\""),
+	 styled_string (file_name_style.style (), current_exec_file),
+	 styled_string (file_name_style.style (), exec_file_target.c_str ()),
+	 exec_file_mismatch_names[exec_file_mismatch_mode]);
+      if (exec_file_mismatch_mode == exec_file_mismatch_reload)
+	{
+	  symfile_add_flags add_flags = SYMFILE_MAINLINE;
+	  if (from_tty)
+	    add_flags |= SYMFILE_VERBOSE;
+	  try
+	    {
+	      symbol_file_add_main (exec_file_target.c_str (), add_flags);
+	      exec_file_attach (exec_file_target.c_str (), from_tty);
+	    }
+	  catch (gdb_exception_error &err)
+	    {
+	      warning (_("reloading %ps %s"),
+		       styled_string (file_name_style.style (),
+				      exec_file_target.c_str ()),
+		       err.message != NULL ? err.what () : "error");
+	    }
+	}
+    }
+}
+
+/* See gdbcore.h.  */
+
 void
 exec_file_locate_attach (int pid, int defer_bp_reset, int from_tty)
 {
@@ -331,8 +431,9 @@ exec_file_attach (const char *filename, int from_tty)
 
       if (!exec_bfd)
 	{
-	  error (_("\"%s\": could not open as an executable file: %s."),
-		 scratch_pathname, bfd_errmsg (bfd_get_error ()));
+	  error (_("\"%ps\": could not open as an executable file: %s."),
+		 styled_string (file_name_style.style (), scratch_pathname),
+		 bfd_errmsg (bfd_get_error ()));
 	}
 
       /* gdb_realpath_keepfile resolves symlinks on the local
@@ -348,8 +449,8 @@ exec_file_attach (const char *filename, int from_tty)
 	  /* Make sure to close exec_bfd, or else "run" might try to use
 	     it.  */
 	  exec_close ();
-	  error (_("\"%s\": not in executable format: %s"),
-		 scratch_pathname,
+	  error (_("\"%ps\": not in executable format: %s"),
+		 styled_string (file_name_style.style (), scratch_pathname),
 		 gdb_bfd_errmsg (bfd_get_error (), matching).c_str ());
 	}
 
@@ -358,8 +459,9 @@ exec_file_attach (const char *filename, int from_tty)
 	  /* Make sure to close exec_bfd, or else "run" might try to use
 	     it.  */
 	  exec_close ();
-	  error (_("\"%s\": can't find the file sections: %s"),
-		 scratch_pathname, bfd_errmsg (bfd_get_error ()));
+	  error (_("\"%ps\": can't find the file sections: %s"),
+		 styled_string (file_name_style.style (), scratch_pathname),
+		 bfd_errmsg (bfd_get_error ()));
 	}
 
       exec_bfd_mtime = bfd_get_mtime (exec_bfd);
@@ -879,7 +981,9 @@ print_section_info (struct target_section_table *t, bfd *abfd)
   /* FIXME: 16 is not wide enough when gdbarch_addr_bit > 64.  */
   int wid = gdbarch_addr_bit (gdbarch) <= 32 ? 8 : 16;
 
-  printf_filtered ("\t`%s', ", bfd_get_filename (abfd));
+  printf_filtered ("\t`%ps', ",
+		   styled_string (file_name_style.style (),
+				  bfd_get_filename (abfd)));
   wrap_here ("        ");
   printf_filtered (_("file type %s.\n"), bfd_get_target (abfd));
   if (abfd == exec_bfd)
@@ -906,8 +1010,9 @@ print_section_info (struct target_section_table *t, bfd *abfd)
 	    }
 	}
       if (p == t->sections_end)
-	warning (_("Cannot find section for the entry point of %s."),
-		 bfd_get_filename (abfd));
+	warning (_("Cannot find section for the entry point of %ps."),
+		 styled_string (file_name_style.style (),
+				bfd_get_filename (abfd)));
 
       entry_point = gdbarch_addr_bits_remove (gdbarch, 
 					      bfd_get_start_address (abfd) 
@@ -934,7 +1039,9 @@ print_section_info (struct target_section_table *t, bfd *abfd)
 			 hex_string_custom (psect->filepos, 8));
       printf_filtered (" is %s", bfd_section_name (psect));
       if (pbfd != abfd)
-	printf_filtered (" in %s", bfd_get_filename (pbfd));
+	printf_filtered (" in %ps",
+			 styled_string (file_name_style.style (),
+					bfd_get_filename (pbfd)));
       printf_filtered ("\n");
     }
 }
@@ -1070,5 +1177,22 @@ Show writing into executable and core files."), NULL,
 			   show_write_files,
 			   &setlist, &showlist);
 
+  add_setshow_enum_cmd ("exec-file-mismatch", class_support,
+			exec_file_mismatch_names,
+			&exec_file_mismatch,
+			_("\
+Set exec-file-mismatch handling (reload|warn|off)."),
+			_("\
+Show exec-file-mismatch handling (reload|warn|off)."),
+			_("\
+Specifies how to handle a mismatch between the current exec-file\n\
+and the exec-file automatically determined when attaching to a process:\n\n\
+ reload - warn the user, reload the automatically determined exec-file.\n\
+ warn   - warn the user, but do not change the exec-file.\n\
+ off    - do not check for mismatch."),
+			set_exec_file_mismatch_command,
+			show_exec_file_mismatch_command,
+			&setlist, &showlist);
+
   add_target (exec_target_info, exec_target_open, filename_completer);
 }
diff --git a/gdb/gdbcore.h b/gdb/gdbcore.h
index cac91c097b..5ff5e9e5ab 100644
--- a/gdb/gdbcore.h
+++ b/gdb/gdbcore.h
@@ -157,6 +157,11 @@ extern void exec_file_locate_attach (int pid, int defer_bp_reset, int from_tty);
 
 extern void validate_files (void);
 
+/* Give the user a message if the current exec file does not match the exec
+   file determined from the target.  In case of mismatch, ask the user
+   if the exec file determined from target must be loaded.  */
+extern void validate_exec_file (int from_tty);
+
 /* The current default bfd target.  */
 
 extern char *gnutarget;
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index af66eaffd8..df4478a26a 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -2782,6 +2782,9 @@ attach_command (const char *args, int from_tty)
 	target_stop (ptid_t (inferior_ptid.pid ()));
     }
 
+  /* Check for exec file mismatch, and let the user solve it.  */
+  validate_exec_file (from_tty);
+
   mode = async_exec ? ATTACH_POST_WAIT_RESUME : ATTACH_POST_WAIT_STOP;
 
   /* Some system don't generate traps when attaching to inferior.
diff --git a/gdb/remote.c b/gdb/remote.c
index 3c544502a8..6894a2b074 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -2378,6 +2378,9 @@ remote_target::remote_add_inferior (bool fake_pid_p, int pid, int attached,
   if (try_open_exec && get_exec_file (0) == NULL)
     exec_file_locate_attach (pid, 0, 1);
 
+  /* Check for exec file mismatch, and let the user solve it.  */
+  validate_exec_file (1);
+
   return inf;
 }
 
-- 
2.20.1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [RFA 2/3] Test 'set exec-file-mismatch reload|warn|off'.
  2019-12-21 14:36 [RFA 1/3] New option 'set exec-file-mismatch (reload|warn|off)'. Fixes PR gdb/17626 Philippe Waroquiers
@ 2019-12-21 14:36 ` Philippe Waroquiers
  2019-12-21 14:36 ` [RFA 3/3] Document 'set|show exec-file-mismatch (reload|warn|off)' Philippe Waroquiers
  2019-12-21 14:36 ` [RFA 1/3] Implement 'set/show exec-file-mismatch' Philippe Waroquiers
  2 siblings, 0 replies; 11+ messages in thread
From: Philippe Waroquiers @ 2019-12-21 14:36 UTC (permalink / raw)
  To: gdb-patches; +Cc: Philippe Waroquiers

Modify gdb.base/attach.exp to test the behaviour of the option
exec-file-mismatch.  Note that this test can also be run using/
  make check RUNTESTFLAGS="--target_board=native-extended-gdbserver" TESTS=gdb.base/attach.exp

to test the behaviour of attaching to running program using a gdb server.

Note: when running the test with a gdbserver, the tests in
test_command_line_attach_run fail because the command "run" is not supported.
I tried to extend the condition
    if ![isnative] then {
	unsupported "commandline attach run test"
	return 0
    }
but unclear to me how to best do that.  The below trials all failed
to work properly:
    if { ![isnative] || [target_is_gdbserver] } then {
    if { ![isnative] || [use_gdb_stub] } then {
    if { ![isnative] || [is_remote target] } then {
  => could never obtain a condition that was true with gdbserver.
---
 gdb/testsuite/gdb.base/attach.c   |  1 +
 gdb/testsuite/gdb.base/attach.exp | 84 ++++++++++++++++++++++++++++++-
 gdb/testsuite/gdb.base/attach2.c  |  1 +
 3 files changed, 85 insertions(+), 1 deletion(-)

diff --git a/gdb/testsuite/gdb.base/attach.c b/gdb/testsuite/gdb.base/attach.c
index 91b180c73c..2e87f9b710 100644
--- a/gdb/testsuite/gdb.base/attach.c
+++ b/gdb/testsuite/gdb.base/attach.c
@@ -7,6 +7,7 @@
 #include <stdio.h>
 #include <unistd.h>
 
+int  bidule = 0;
 int  should_exit = 0;
 
 int main ()
diff --git a/gdb/testsuite/gdb.base/attach.exp b/gdb/testsuite/gdb.base/attach.exp
index ec6e45766b..b09020ae8e 100644
--- a/gdb/testsuite/gdb.base/attach.exp
+++ b/gdb/testsuite/gdb.base/attach.exp
@@ -474,7 +474,12 @@ proc test_command_line_attach_run {} {
     global gdb_prompt
     global binfile
 
-    if ![isnative] then {
+    # Skip test if we cannot attach on the command line and use the run command.
+    # ??? Unclear what condition to use to return here when using gdbserver.
+    # ??? None of the below works.
+    #    ![isnative] || [target_is_gdbserver]
+    #    ![isnative] || [use_gdb_stub]
+    if { ![isnative] || [is_remote target] } then {
 	unsupported "commandline attach run test"
 	return 0
     }
@@ -513,14 +518,91 @@ proc test_command_line_attach_run {} {
     }
 }
 
+
+# This is a test of 'set exec-file-mismatch' handling.
+
+proc_with_prefix do_attach_exec_mismatch_handling_tests {} {
+    global gdb_prompt
+    global binfile
+    global binfile2
+
+    clean_restart $binfile
+
+    # Start two programs that can be attached to.
+    # The first program contains a 'int bidule' variable, the second a 'float bidule'.
+
+    set test_spawn_id [spawn_wait_for_attach $binfile]
+    set testpid [spawn_id_get_pid $test_spawn_id]
+    set test_spawn_id2 [spawn_wait_for_attach $binfile2]
+    set testpid2 [spawn_id_get_pid $test_spawn_id2]
+
+
+    # Test with the default value of 'set exec-file-mismatch load".
+    set test "mismatch load"
+    gdb_test "attach $testpid" "Attaching to program.*" "$test attach1"
+    # Verify that we can "see" the variable "bidule" in the
+    # program, and that it is an integer.
+    gdb_test "ptype bidule" " = int" "$test after attach1, bidule is int"
+    # Detach the process.
+    gdb_test "detach" "Detaching from program: .* detached\\\]" "$test detach1"
+    gdb_test_multiple "attach $testpid2" "$test attach2" {
+	-re "Attaching to program.*exec-file-mismatch handling is currently \"reload\".*Load new symbol table from .*attach2\".*\(y or n\)" {
+	    pass "$test attach2"
+	}
+    }
+    gdb_test "y" "Reading symbols from .*attach2.*" "$test load attach2"
+    # Verify that we can "see" the variable "bidule" in the
+    # program, and that it is a float.
+    gdb_test "ptype bidule" " = float" "$test after attach2 and load, bidule is float"
+    # Detach the process.
+    gdb_test "detach" "Detaching from program: .* detached\\\]" "$test detach attach2"
+
+
+    # Test with 'set exec-file-mismatch warn".
+    set test "mismatch warn"
+    gdb_test_no_output "set exec-file-mismatch warn"
+    gdb_test_multiple "attach $testpid" "$test attach" {
+	-re "Attaching to program.*exec-file-mismatch handling is currently \"warn\".*$gdb_prompt" {
+	    pass "$test attach"
+	}
+    }
+    # Verify that we still (wrongly) "see" the variable "bidule" as a float,
+    # as we have not loaded the correct exec-file.
+    gdb_test "ptype bidule" " = float" "$test after attach and warn, bidule is float"
+    # Detach the process.
+    gdb_test "detach" "Detaching from program: .* detached\\\]" "$test detach attach"
+
+
+    # Same test but with 'set exec-file-mismatch off".
+    set test "mismatch off"
+    gdb_test_no_output "set exec-file-mismatch off"
+    gdb_test_multiple "attach $testpid" "$test attach" {
+	-re "Attaching to program.*$gdb_prompt" {
+	    pass "$test attach"
+	}
+    }
+    # Verify that we still (wrongly) "see" the variable "bidule" as a float,
+    # as we have not warned the user and not loaded the correct exec-file
+    gdb_test "ptype bidule" " = float" "$test after attach and warn, bidule is float"
+    # Detach the process.
+    gdb_test "detach" "Detaching from program: .* detached\\\]" "$test detach attach"
+
+
+    # Don't leave a process around
+    kill_wait_spawned_process $test_spawn_id
+    kill_wait_spawned_process $test_spawn_id2
+}
+
 do_attach_tests
 do_attach_failure_tests
 do_call_attach_tests
+do_attach_exec_mismatch_handling_tests
 
 # Test "gdb --pid"
 
 do_command_attach_tests
 
+
 test_command_line_attach_run
 
 return 0
diff --git a/gdb/testsuite/gdb.base/attach2.c b/gdb/testsuite/gdb.base/attach2.c
index a78037ed38..44d37258fb 100644
--- a/gdb/testsuite/gdb.base/attach2.c
+++ b/gdb/testsuite/gdb.base/attach2.c
@@ -8,6 +8,7 @@
 #include <stdlib.h>
 #include <unistd.h>
 
+float  bidule = 0.0;
 int  should_exit = 0;
 
 int main ()
-- 
2.20.1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFA 3/3] Document 'set|show exec-file-mismatch (reload|warn|off)'
  2019-12-21 14:36 ` [RFA 3/3] Document 'set|show exec-file-mismatch (reload|warn|off)' Philippe Waroquiers
@ 2019-12-21 17:44   ` Eli Zaretskii
  2019-12-21 20:16     ` Philippe Waroquiers
  2019-12-22  2:46   ` Christian Biesinger via gdb-patches
  1 sibling, 1 reply; 11+ messages in thread
From: Eli Zaretskii @ 2019-12-21 17:44 UTC (permalink / raw)
  To: Philippe Waroquiers; +Cc: gdb-patches

> From: Philippe Waroquiers <philippe.waroquiers@skynet.be>
> Cc: Philippe Waroquiers <philippe.waroquiers@skynet.be>
> Date: Sat, 21 Dec 2019 15:36:32 +0100
> 
> +  Set or show the option 'exec-file-mismatch'.  When GDB attaches to
> +  a running program and can determine the running program, this new option

"attached to a running program and can determine the running program"
sounds strange and confusing (if GDB has attached to a program, it
should not have any trouble determining it, right?).  I guess you
meant something like

  When GDB attaches to a running process, and can determine the
  executable program file the process runs, this new option ...

> +  indicates how to handle a mismatch between the current exec-file and
> +  the automatically detected file.

Here, instead of "current exec-file" I'd use "the program file on
disk", and instead of "automatically detected" I'd use "the program
file used to start the process".  The main point is not to introduce
terminology not referenced in previous text, otherwise you risk losing
or confusing the reader.

> +  and reload the automatically determined file after user confirmation.

Well, the "automatically determined file" can no longer be reloaded,
since it was updated on disk, right?

> +@anchor{set exec-file-mismatch}
> +If the debugger can determine the program running in the process
> +and this program does not match the current exec-file, the option
> +@code{exec-file-mismatch} specifies how to handle the mismatch.

Suggest to explain how the mismatch is determined, because otherwise
this text sounds unclear and maybe confusing.

> +@table @code
> +@kindex exec-file-mismatch
> +@cindex set exec-file-mismatch
> +@item set exec-file-mismatch @samp{reload|warn|off}
> +In case of mismatch between the current exec-file and the automatically
> +determined exec-file of the PID the debugger is attaching to,
> +@samp{reload} indicates to give a warning to the user and reload
> +the automatically determined exec-file.  The user will be asked to
> +confirm the loading of the automatically determined file.
> +With @samp{warn}, @value{GDBN} just gives a warning to the user to
> +signal the mismatch.  @samp{off} indicates to not check for mismatch.
> +The default value is @samp{reload}.

I'd reword this text as follows:

  Whether to detect mismatch between the program file used to start
  the process and the current executable file of that program on disk.
  If @samp{reload}, the default, display a warning and ask the user
  whether to reload the program's file; if @samp{warn}, just display a
  warning; if @samp{off}, don't attempt to detect a mismatch.

> +Some remote targets allow @value{GDBN} to determine the program running
> +in the process the debugger is attaching to.  In such a case, @value{GDBN}
> +uses the value of @code{exec-file-mismatch} to handle a possible mismatch
> +between the program running in the process and the current exec-file.
> +(@pxref{set exec-file-mismatch}).

The period before @pxref in parentheses is redundant and should be
removed.

Thanks.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFA 3/3] Document 'set|show exec-file-mismatch (reload|warn|off)'
  2019-12-21 17:44   ` Eli Zaretskii
@ 2019-12-21 20:16     ` Philippe Waroquiers
  2019-12-22 18:20       ` Eli Zaretskii
  0 siblings, 1 reply; 11+ messages in thread
From: Philippe Waroquiers @ 2019-12-21 20:16 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

Thanks for the detailed comments, I will fix/improve
according to your suggestions.

However, your comments make me think that the words used
in the command itself might have to be improved, as the
word "reload" seems confusing.

I will try to clarify what "reload" currently does using an example.

Imagine you have 2 running processes:
  process PID 123 running program xxxxx
  process PID 345 running program yyyyy

When GDB attaches to 123, it determines that the program
is xxxxx.  When detaching, and then attaching to 345,
GDB git master version *silently keeps* xxxxx as exec-file
for PID 345.

When the below new option is reload or warn, when attaching
to 345, GDB will determine that the exec-file of 345 
is yyyyy.
It then compares the file name yyyyy with the current exec file xxxxx,
and because file differs, it will warn the user and (for reload option)
ask the user if the exec-file yyyyy must be reloaded.

So, "reload" does not mean:  "reload the same file if it was changed on disk",
it means "reload a new exec-file, because the PID we attach to
runs another exec-file".

Note that with the above 2 processes, GDB git master similarly uses the wrong
file if you do:  
   gdb xxxxx
   (gdb) attach 345
      => GDB now silently uses the (wrong) file xxxxx to debug 345.
That is equally solved (or warned) with the new option.

So, maybe "reload" in 'set exec-file-mismatch reload|warn|off' should be replaced
by a less confusing word?

Maybe 'set exec-file-mismatch ask-to-load|warn|off' ?


Feedback ? Any other suggestion ?


Thanks

Philippe


On Sat, 2019-12-21 at 19:44 +0200, Eli Zaretskii wrote:
> > From: Philippe Waroquiers <philippe.waroquiers@skynet.be>
> > Cc: Philippe Waroquiers <philippe.waroquiers@skynet.be>
> > Date: Sat, 21 Dec 2019 15:36:32 +0100
> > 
> > +  Set or show the option 'exec-file-mismatch'.  When GDB attaches to
> > +  a running program and can determine the running program, this new option
> 
> "attached to a running program and can determine the running program"
> sounds strange and confusing (if GDB has attached to a program, it
> should not have any trouble determining it, right?).  I guess you
> meant something like
> 
>   When GDB attaches to a running process, and can determine the
>   executable program file the process runs, this new option ...
> 
> > +  indicates how to handle a mismatch between the current exec-file and
> > +  the automatically detected file.
> 
> Here, instead of "current exec-file" I'd use "the program file on
> disk", and instead of "automatically detected" I'd use "the program
> file used to start the process".  The main point is not to introduce
> terminology not referenced in previous text, otherwise you risk losing
> or confusing the reader.
> 
> > +  and reload the automatically determined file after user confirmation.
> 
> Well, the "automatically determined file" can no longer be reloaded,
> since it was updated on disk, right?
> 
> > +@anchor{set exec-file-mismatch}
> > +If the debugger can determine the program running in the process
> > +and this program does not match the current exec-file, the option
> > +@code{exec-file-mismatch} specifies how to handle the mismatch.
> 
> Suggest to explain how the mismatch is determined, because otherwise
> this text sounds unclear and maybe confusing.
> 
> > +@table @code
> > +@kindex exec-file-mismatch
> > +@cindex set exec-file-mismatch
> > +@item set exec-file-mismatch @samp{reload|warn|off}
> > +In case of mismatch between the current exec-file and the automatically
> > +determined exec-file of the PID the debugger is attaching to,
> > +@samp{reload} indicates to give a warning to the user and reload
> > +the automatically determined exec-file.  The user will be asked to
> > +confirm the loading of the automatically determined file.
> > +With @samp{warn}, @value{GDBN} just gives a warning to the user to
> > +signal the mismatch.  @samp{off} indicates to not check for mismatch.
> > +The default value is @samp{reload}.
> 
> I'd reword this text as follows:
> 
>   Whether to detect mismatch between the program file used to start
>   the process and the current executable file of that program on disk.
>   If @samp{reload}, the default, display a warning and ask the user
>   whether to reload the program's file; if @samp{warn}, just display a
>   warning; if @samp{off}, don't attempt to detect a mismatch.
> 
> > +Some remote targets allow @value{GDBN} to determine the program running
> > +in the process the debugger is attaching to.  In such a case, @value{GDBN}
> > +uses the value of @code{exec-file-mismatch} to handle a possible mismatch
> > +between the program running in the process and the current exec-file.
> > +(@pxref{set exec-file-mismatch}).
> 
> The period before @pxref in parentheses is redundant and should be
> removed.
> 
> Thanks.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFA 3/3] Document 'set|show exec-file-mismatch (reload|warn|off)'
  2019-12-21 14:36 ` [RFA 3/3] Document 'set|show exec-file-mismatch (reload|warn|off)' Philippe Waroquiers
  2019-12-21 17:44   ` Eli Zaretskii
@ 2019-12-22  2:46   ` Christian Biesinger via gdb-patches
  2019-12-22  9:09     ` Philippe Waroquiers
  2020-01-08  0:52     ` Tom Tromey
  1 sibling, 2 replies; 11+ messages in thread
From: Christian Biesinger via gdb-patches @ 2019-12-22  2:46 UTC (permalink / raw)
  To: Philippe Waroquiers; +Cc: gdb-patches

On Sat, Dec 21, 2019, 09:37 Philippe Waroquiers <
philippe.waroquiers@skynet.be> wrote:

> Mention in NEWS the new option and the set/show commands.
>
> Document in gdb.texinfo the new option and the set/show commands.
>

When would someone set this to off, or even warn? I'm wondering is it makes
sense to add this setting vs always having the reload behavior?


> gdb/ChangeLog
> YYYY-MM-DD  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
>
>         * NEWS: Mention the new option and the set/show commands.
>
> gdb/doc/ChangeLog
> YYYY-MM-DD  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
>
>         * gdb.texinfo (Attach): Document the new option and the
>         set/show commands.
>         (Connecting): Reference the exec-file-mismatch option.
> ---
>  gdb/NEWS            | 12 ++++++++++++
>  gdb/doc/gdb.texinfo | 31 +++++++++++++++++++++++++++++++
>  2 files changed, 43 insertions(+)
>
> diff --git a/gdb/NEWS b/gdb/NEWS
> index ee10914fd8..1a90862b0d 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -3,6 +3,18 @@
>
>  *** Changes since GDB 9
>
> +* New commands
> +
> +set exec-file-mismatch -- Set exec-file-mismatch handling
> (reload|warn|off).
> +show exec-file-mismatch -- Show exec-file-mismatch handling
> (reload|warn|off).
> +  Set or show the option 'exec-file-mismatch'.  When GDB attaches to
> +  a running program and can determine the running program, this new option
> +  indicates how to handle a mismatch between the current exec-file and
> +  the automatically detected file.
> +  'reload' is the default value: in case of mismatch, GDB will warn the
> user
> +  and reload the automatically determined file after user confirmation.
> +
> +
>  *** Changes in GDB 9
>
>  * 'thread-exited' event is now available in the annotations interface.
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 4d25f755d7..a246ba0bf5 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -2904,6 +2904,31 @@ the program is not found) by using the source file
> search path
>  the @code{file} command to load the program.  @xref{Files, ,Commands to
>  Specify Files}.
>
> +@anchor{set exec-file-mismatch}
> +If the debugger can determine the program running in the process
> +and this program does not match the current exec-file, the option
> +@code{exec-file-mismatch} specifies how to handle the mismatch.
> +
> +@table @code
> +@kindex exec-file-mismatch
> +@cindex set exec-file-mismatch
> +@item set exec-file-mismatch @samp{reload|warn|off}
> +In case of mismatch between the current exec-file and the automatically
> +determined exec-file of the PID the debugger is attaching to,
> +@samp{reload} indicates to give a warning to the user and reload
> +the automatically determined exec-file.  The user will be asked to
> +confirm the loading of the automatically determined file.
> +With @samp{warn}, @value{GDBN} just gives a warning to the user to
> +signal the mismatch.  @samp{off} indicates to not check for mismatch.
> +The default value is @samp{reload}.
> +
> +
> +@cindex show exec-file-mismatch
> +@item show exec-file-mismatch
> +Show the current value of @code{exec-file-mismatch}.
> +
> +@end table
> +
>  The first thing @value{GDBN} does after arranging to debug the specified
>  process is to stop it.  You can examine and modify an attached process
>  with all the @value{GDBN} commands that are ordinarily available when
> @@ -21780,6 +21805,12 @@ established.  If you are using @code{gdbserver},
> you may also invoke
>  @code{gdbserver} using the @option{--attach} option
>  (@pxref{Running gdbserver}).
>
> +Some remote targets allow @value{GDBN} to determine the program running
> +in the process the debugger is attaching to.  In such a case, @value{GDBN}
> +uses the value of @code{exec-file-mismatch} to handle a possible mismatch
> +between the program running in the process and the current exec-file.
> +(@pxref{set exec-file-mismatch}).
> +
>  @end table
>
>  @anchor{Host and target files}
> --
> 2.20.1
>
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFA 3/3] Document 'set|show exec-file-mismatch (reload|warn|off)'
  2019-12-22  2:46   ` Christian Biesinger via gdb-patches
@ 2019-12-22  9:09     ` Philippe Waroquiers
  2020-01-08  0:52     ` Tom Tromey
  1 sibling, 0 replies; 11+ messages in thread
From: Philippe Waroquiers @ 2019-12-22  9:09 UTC (permalink / raw)
  To: Christian Biesinger; +Cc: gdb-patches

On Sat, 2019-12-21 at 21:46 -0500, Christian Biesinger via gdb-patches wrote:
> On Sat, Dec 21, 2019, 09:37 Philippe Waroquiers <
> philippe.waroquiers@skynet.be> wrote:
> 
> > Mention in NEWS the new option and the set/show commands.
> > 
> > Document in gdb.texinfo the new option and the set/show commands.
> > 
> 
> When would someone set this to off, or even warn? I'm wondering is it makes
> sense to add this setting vs always having the reload behavior?
Effectively, I think that in a large majority of the cases, the reload
behavior is the correct thing to do, which is why it is the default value.

However, I have added the option for the following reasons:
* There are today already several features to choose
  flexibly the exec-file and symbol-file (see GDB manual
  "18.1 Commands to Specify Files").  The new option allows to retain
  the flexible manual control, which I assume is needed for some
  use cases.
* Also, the previous trial to fix this problem was based on the notion
  of "sticky" exec-file: if the exec-file was explicitly given
  by the user, then it was never changed automatically when
  attaching to another process having another exec-file.
  IMO, this was not the best approach, as GDB would still
  silently use the wrong file in some cases (such as typo in the PID,
  or successive attach/detach after having given a program file
  as GDB argument).
  But this fix trial also seems to indicate some desire to let the
  user choose (in unusual circumstances) to keep a specific exec file.

Note that for reload and warn, the message given to the user explicitly
mention the 'exec-file-mismatch' option, so that users that need some
control are informed of the possibility.

Once the function 'validate_exec_file' was implemented and called at
the right places, implementing the option itself was not a big deal,
apart of the difficulty of properly naming and documenting the values :).

Thanks
Philippe


> 
> 
> > gdb/ChangeLog
> > YYYY-MM-DD  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
> > 
> >         * NEWS: Mention the new option and the set/show commands.
> > 
> > gdb/doc/ChangeLog
> > YYYY-MM-DD  Philippe Waroquiers  <philippe.waroquiers@skynet.be>
> > 
> >         * gdb.texinfo (Attach): Document the new option and the
> >         set/show commands.
> >         (Connecting): Reference the exec-file-mismatch option.
> > ---
> >  gdb/NEWS            | 12 ++++++++++++
> >  gdb/doc/gdb.texinfo | 31 +++++++++++++++++++++++++++++++
> >  2 files changed, 43 insertions(+)
> > 
> > diff --git a/gdb/NEWS b/gdb/NEWS
> > index ee10914fd8..1a90862b0d 100644
> > --- a/gdb/NEWS
> > +++ b/gdb/NEWS
> > @@ -3,6 +3,18 @@
> > 
> >  *** Changes since GDB 9
> > 
> > +* New commands
> > +
> > +set exec-file-mismatch -- Set exec-file-mismatch handling
> > (reload|warn|off).
> > +show exec-file-mismatch -- Show exec-file-mismatch handling
> > (reload|warn|off).
> > +  Set or show the option 'exec-file-mismatch'.  When GDB attaches to
> > +  a running program and can determine the running program, this new option
> > +  indicates how to handle a mismatch between the current exec-file and
> > +  the automatically detected file.
> > +  'reload' is the default value: in case of mismatch, GDB will warn the
> > user
> > +  and reload the automatically determined file after user confirmation.
> > +
> > +
> >  *** Changes in GDB 9
> > 
> >  * 'thread-exited' event is now available in the annotations interface.
> > diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> > index 4d25f755d7..a246ba0bf5 100644
> > --- a/gdb/doc/gdb.texinfo
> > +++ b/gdb/doc/gdb.texinfo
> > @@ -2904,6 +2904,31 @@ the program is not found) by using the source file
> > search path
> >  the @code{file} command to load the program.  @xref{Files, ,Commands to
> >  Specify Files}.
> > 
> > +@anchor{set exec-file-mismatch}
> > +If the debugger can determine the program running in the process
> > +and this program does not match the current exec-file, the option
> > +@code{exec-file-mismatch} specifies how to handle the mismatch.
> > +
> > +@table @code
> > +@kindex exec-file-mismatch
> > +@cindex set exec-file-mismatch
> > +@item set exec-file-mismatch @samp{reload|warn|off}
> > +In case of mismatch between the current exec-file and the automatically
> > +determined exec-file of the PID the debugger is attaching to,
> > +@samp{reload} indicates to give a warning to the user and reload
> > +the automatically determined exec-file.  The user will be asked to
> > +confirm the loading of the automatically determined file.
> > +With @samp{warn}, @value{GDBN} just gives a warning to the user to
> > +signal the mismatch.  @samp{off} indicates to not check for mismatch.
> > +The default value is @samp{reload}.
> > +
> > +
> > +@cindex show exec-file-mismatch
> > +@item show exec-file-mismatch
> > +Show the current value of @code{exec-file-mismatch}.
> > +
> > +@end table
> > +
> >  The first thing @value{GDBN} does after arranging to debug the specified
> >  process is to stop it.  You can examine and modify an attached process
> >  with all the @value{GDBN} commands that are ordinarily available when
> > @@ -21780,6 +21805,12 @@ established.  If you are using @code{gdbserver},
> > you may also invoke
> >  @code{gdbserver} using the @option{--attach} option
> >  (@pxref{Running gdbserver}).
> > 
> > +Some remote targets allow @value{GDBN} to determine the program running
> > +in the process the debugger is attaching to.  In such a case, @value{GDBN}
> > +uses the value of @code{exec-file-mismatch} to handle a possible mismatch
> > +between the program running in the process and the current exec-file.
> > +(@pxref{set exec-file-mismatch}).
> > +
> >  @end table
> > 
> >  @anchor{Host and target files}
> > --
> > 2.20.1
> > 
> > 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFA 3/3] Document 'set|show exec-file-mismatch (reload|warn|off)'
  2019-12-21 20:16     ` Philippe Waroquiers
@ 2019-12-22 18:20       ` Eli Zaretskii
  0 siblings, 0 replies; 11+ messages in thread
From: Eli Zaretskii @ 2019-12-22 18:20 UTC (permalink / raw)
  To: Philippe Waroquiers; +Cc: gdb-patches

> From: Philippe Waroquiers <philippe.waroquiers@skynet.be>
> Cc: gdb-patches@sourceware.org
> Date: Sat, 21 Dec 2019 21:15:48 +0100
> 
> However, your comments make me think that the words used
> in the command itself might have to be improved, as the
> word "reload" seems confusing.

Yes, I've misunderstood what it means.  The text needs to be rewritten
to make clear what this feature tries to detect.  An example you've
shown could be a good idea.

> So, maybe "reload" in 'set exec-file-mismatch reload|warn|off' should be replaced
> by a less confusing word?
> 
> Maybe 'set exec-file-mismatch ask-to-load|warn|off' ?

How about ask|warn|off ?

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFA 3/3] Document 'set|show exec-file-mismatch (reload|warn|off)'
  2019-12-22  2:46   ` Christian Biesinger via gdb-patches
  2019-12-22  9:09     ` Philippe Waroquiers
@ 2020-01-08  0:52     ` Tom Tromey
  1 sibling, 0 replies; 11+ messages in thread
From: Tom Tromey @ 2020-01-08  0:52 UTC (permalink / raw)
  To: Christian Biesinger via gdb-patches
  Cc: Philippe Waroquiers, Christian Biesinger

Christian> When would someone set this to off, or even warn? I'm wondering is it makes
Christian> sense to add this setting vs always having the reload behavior?

Last time this stuff came up, it turned out there are users who point
gdb at the file with debug symbols while debugging a stripped version on
the target.  IIRC, at the time, build-id and separate debuginfo weren't
widely used, so this was the main way to do this kind of thing.

I don't know if this is still important.  I've only run into trouble
with this behavior, so I welcome the new default at least.

Tom

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFA 1/3] Implement 'set/show exec-file-mismatch'.
  2019-12-21 14:36 ` [RFA 1/3] Implement 'set/show exec-file-mismatch' Philippe Waroquiers
@ 2020-01-08  0:58   ` Tom Tromey
  0 siblings, 0 replies; 11+ messages in thread
From: Tom Tromey @ 2020-01-08  0:58 UTC (permalink / raw)
  To: Philippe Waroquiers; +Cc: gdb-patches

>>>>> "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:

Philippe> This option allows to tell GDB to detect and possibly handle mismatched exec-files.
Philippe> A recurrent problem with GDB is that GDB uses the wrong exec-file
Philippe> when using the attach/detach commands successively.
Philippe> Also, in case the user specifies a file on the command line but attaches
Philippe> to the wrong PID, this error is not made visible and gives a not user
Philippe> understandable behaviour.

Thanks.  This looks ok to me.
I will have to do some searching to see what is up with the test suite
patch.  I don't know the answer there either.

Tom

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2020-01-08  0:58 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-21 14:36 [RFA 1/3] New option 'set exec-file-mismatch (reload|warn|off)'. Fixes PR gdb/17626 Philippe Waroquiers
2019-12-21 14:36 ` [RFA 2/3] Test 'set exec-file-mismatch reload|warn|off' Philippe Waroquiers
2019-12-21 14:36 ` [RFA 3/3] Document 'set|show exec-file-mismatch (reload|warn|off)' Philippe Waroquiers
2019-12-21 17:44   ` Eli Zaretskii
2019-12-21 20:16     ` Philippe Waroquiers
2019-12-22 18:20       ` Eli Zaretskii
2019-12-22  2:46   ` Christian Biesinger via gdb-patches
2019-12-22  9:09     ` Philippe Waroquiers
2020-01-08  0:52     ` Tom Tromey
2019-12-21 14:36 ` [RFA 1/3] Implement 'set/show exec-file-mismatch' Philippe Waroquiers
2020-01-08  0:58   ` Tom Tromey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).