[PATCH] gdb: use current executable for 'remote exec-file' in some cases

[Andrew Burgess <aburgess@redhat.com>]

This commit allows GDB to make use of the file set with the 'file'
command when starting a new inferior on an extended-remote target.
There are however some restrictions.

If the user has used 'set remote exec-file', then this setting is
always used in preference to the file set with the 'file' command.

Similarly, if the qExecAndArgs packet has succeeded, and GDB knows
that the remote target has an executable set, then this will be used
in preference to the file set with the 'file' command; this preserves
GDB's existing behaviour.  In effect, when GDB connects to the remote
target, the remote sets the 'remote exec-file' and this prevents GDB
from using the 'file' filename.

And, GDB can only use the file set with the 'file' command if it
believes that both GDB and the remote target will both be able to
access this file.  This means that one of these is true:

  + the the remote_target::filesystem_is_local function returns
    true (see the implementation of that function for details of when
    this can happen).  This means GDB and the remote target can see
    the same file system, GDB can just use the current executable's
    filename as is, or

  + the user has set the 'file' to something with a 'target:' prefix,
    e.g. 'file target:/path/to/exec'.  In this last case, GDB will use
    the exec filename without the 'target:' prefix, this filename is,
    by definition, something the remote target can see, or

  + the sysroot has been updated by the user and no longer contains a
    'target:' prefix.  In this case, if the 'file' filename is within
    the sysroot, then it is assumed the remote will also be able to
    see a file with the same filename.  For example, if the sysroot is
    '/aa/', and the current executable is '/aa/bb/cc', then GDB will
    tell the remote to run '/bb/cc'.  One common case here is when the
    sysroot is set to the empty string, which is usually done when GDB
    and the remote target can see the same filesystem, in this case
    GDB will use the current executable's filename unmodified.

If one of these conditions is met, then GDB will use the current
executable's filename (with possible modifications as mentioned
above), when starting a new extended-remote inferior, in all other
cases, GDB will use the file name  set with 'set remote exec-file'.

This change could be useful any time a user is running a remote target
on the same machine as GDB, but I am specifically thinking of the case
where GDB is using a tool other than gdbserver, e.g. valgrind, as this
saves one additional step that a user must remember.  The current
steps to start valgrind with GDB, as given on the valgrind
website (https://valgrind.org/docs/manual/manual-core-adv.html) are:

  $ gdb prog
  (gdb) set remote exec-file prog
  (gdb) set sysroot /
  (gdb) target extended-remote | vgdb --multi --vargs -q
  (gdb) start

With this GDB work, and once support for the qExecAndArgs packet is
added to valgrind, then the 'set remote exec-file' line can be dropped
from those instructions.

This commit also extends the 'show remote exec-file' command so that
GDB will display the automatic value that it plans to use.  Here's an
example of the new output:

  $ gdb -q /tmp/hello
  Reading symbols from /tmp/hello...
  (gdb) set sysroot
  (gdb) target extended-remote | ./gdbserver/gdbserver --multi --once -
  Remote debugging using | ./gdbserver/gdbserver --multi --once -
  Remote debugging using stdio
  (gdb) show remote exec-file
  The remote exec-file is unset, using automatic value "/tmp/hello".

The last line shows the new output.
---
 gdb/NEWS                             |   7 ++
 gdb/doc/gdb.texinfo                  |   4 +-
 gdb/remote.c                         | 153 ++++++++++++++++++++++---
 gdb/testsuite/gdb.server/ext-run.exp | 160 ++++++++++++++++++++++-----
 4 files changed, 282 insertions(+), 42 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index 2c73776944f..cfea3cf95c9 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -31,6 +31,13 @@
   subsequent runs of the inferior will use the same arguments as the
   first run.
 
+* When connected to an extended-remote target which does not have an
+  executable specified, if the current executable is specified using a
+  'target:' prefix; or if the sysroot does not have a 'target:'
+  prefix, and the current executable is within the sysroot; then GDB
+  can use the current executable name in order to start inferiors on
+  the remote without having to 'set remote exec-file'.
+
 * New targets
 
 GNU/Linux/MicroBlaze (gdbserver) microblazeel-*linux*
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 1b463b167e7..efd9d80de07 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -24668,7 +24668,9 @@ Remote Configuration
 Select the file used for @code{run} with @code{target
 extended-remote}.  This should be set to a filename valid on the
 target system.  If it is not set, the target will use a default
-filename (e.g.@: the last program run).
+filename (e.g.@: the last program run, or a filename derived from the
+current executable if @value{GDBN} and the remote can see the same
+file system).
 
 When connecting to a remote system, with @kbd{target extended-remote}
 or @kbd{target remote}, if the remote server supports the
diff --git a/gdb/remote.c b/gdb/remote.c
index ce6cfdc3bfb..edefdee57e2 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -46,6 +46,7 @@
 #include "gdbsupport/rsp-low.h"
 #include "disasm.h"
 #include "location.h"
+#include "filenames.h"
 
 #include "gdbsupport/gdb_sys_time.h"
 
@@ -1555,8 +1556,42 @@ class extended_remote_target final : public remote_target
 
   void post_attach (int) override;
   bool supports_disable_randomization () override;
+
+  /* Return the file name for the executable that GDB should ask the remote
+     target to use when starting an inferior.
+
+     EXEC_FILE is the string passed from GDB core, which is the file name
+     of the current executable, which can be NULL.
+
+     If the user has done 'set remote exec-file', or the remote told GDB
+     which executable to use via the 'qExecAndArgs' packet, then this is
+     the value returned by this function, EXEC_FILE is ignored.
+
+     But if there is no remote exec-file set, then we might be able to use
+     EXEC_FILE, or a variation of EXEC_FILE, as the 'remote exec-file'
+     setting.  This will depend on the value of EXEC_FILE and/or the
+     current gdb_sysroot setting.  In this case the file name derived from
+     EXEC_FILE is returned.
+
+     If neither approach comes up with a suitable file name to run then
+     the empty string is returned.  */
+
+  std::string get_exec_file_for_create_inferior (const char *exec_file);
 };
 
+/* Get a pointer to the current remote target.  If not connected to a
+   remote target, return NULL.  The template argument allows users of this
+   function to ask for 'remote_target' or 'extended_remote_target'.  */
+
+template<typename T = remote_target,
+	 std::enable_if_t<std::is_base_of_v<remote_target, T>, bool> = true>
+static T *
+get_current_remote_target ()
+{
+  target_ops *proc_target = current_inferior ()->process_target ();
+  return dynamic_cast<T *> (proc_target);
+}
+
 struct stop_reply : public notif_event
 {
   /* The identifier of the thread about this event  */
@@ -2038,8 +2073,26 @@ show_remote_exec_file (struct ui_file *file, int from_tty,
     gdb_printf (file, _("The remote exec-file is unset, the default "
 			"remote executable will be used.\n"));
   else if (info.second == remote_exec_source::UNSET_VALUE)
-    gdb_printf (file, _("The remote exec-file is unset, the remote has "
-			"no default executable set.\n"));
+    {
+      std::string remote_exec_filename;
+      extended_remote_target *remote
+	= get_current_remote_target<extended_remote_target> ();
+      if (remote != nullptr)
+	{
+	  const char *exec_file = current_program_space->exec_filename ();
+	  remote_exec_filename
+	    = remote->get_exec_file_for_create_inferior (exec_file);
+	}
+
+      if (!remote_exec_filename.empty ())
+	gdb_printf (file, _("The remote exec-file is unset, using "
+			    "automatic value \"%ps\".\n"),
+		    styled_string (file_name_style.style (),
+				   remote_exec_filename.c_str ()));
+      else
+	gdb_printf (file, _("The remote exec-file is unset, the remote has "
+			    "no default executable set.\n"));
+    }
   else
     gdb_printf (file, _("The remote exec-file is \"%ps\".\n"),
 		styled_string (file_name_style.style (),
@@ -2145,16 +2198,6 @@ remote_arch_state::remote_arch_state (struct gdbarch *gdbarch)
     this->remote_packet_size = (this->sizeof_g_packet * 2 + 32);
 }
 
-/* Get a pointer to the current remote target.  If not connected to a
-   remote target, return NULL.  */
-
-static remote_target *
-get_current_remote_target ()
-{
-  target_ops *proc_target = current_inferior ()->process_target ();
-  return dynamic_cast<remote_target *> (proc_target);
-}
-
 /* Return the current allowed size of a remote packet.  This is
    inferred from the current architecture, and should be used to
    limit the length of outgoing packets.  */
@@ -11313,6 +11356,88 @@ directory: %s"),
     }
 }
 
+/* See class declaration above.  */
+
+std::string
+extended_remote_target::get_exec_file_for_create_inferior
+  (const char *exec_file)
+{
+  const remote_exec_file_info &info
+    = get_remote_exec_file_info (current_program_space);
+
+  /* Historically, when the remote target started a new inferior GDB would
+     ignore the filename from GDB core (EXEC_FILE) and would use whatever
+     value the user had set in 'remote exec-file'.  Now we try to be
+     smarter.
+
+     Obviously, if 'remote exec-file' has been set, then this should be
+     considered definitive.  But if 'remote exec-file' has not been set,
+     then, in some cases, we might be able to use EXEC_FILE, or a
+     derivative of EXEC_FILE.
+
+     It can also happen that EXEC_FILE is NULL.  This is mostly a bit of an
+     edge case where GDB has attached to a running process, and couldn't
+     figure out the filename for the executable.  If the user then does
+     'run' we could end up with EXEC_FILE being NULL.  If this happens then
+     the only option is to use the 'remote exec-file' setting.
+
+     If INFO.SECOND is ::VALUE_FROM_REMOTE or ::VALUE_FROM_GDB then there
+     is a value in 'remote exec-file', we should not do anything with
+     EXEC_FILE and just retain the 'remote exec-file' value.
+
+     If INFO.SECOND is ::UNSET_VALUE then the user hasn't 'set remote
+     exec-file' value, and the remote target has specifically told us (via
+     the qExecAndArgs packet) that it has no default executable set.  In
+     this case, if GDB and the remote can see the same filesystem, we can
+     potentially use EXEC_FILE.
+
+     If INFO.SECOND is ::DEFAULT_VALUE then the user hasn't set a 'remote
+     exec-file' value, but the remote target was unable to tell us (maybe
+     the qExecAndArgs packet isn't supported) if it has a default
+     executable set.  We might be tempted to treat this like the
+     ::UNSET_VALUE case, however, this could potentially break backward
+     compatibility in the case where the remote does have a default
+     executable set.  To maintain compatibility, we send over the 'remote
+     exec-file' setting, whatever it might be.  */
+  if (exec_file != nullptr
+      && info.second == remote_exec_source::UNSET_VALUE)
+    {
+      /* If the user has set the core exec file to a file on the target
+	 then we can just strip the target prefix and use that as the
+	 remote exec file name.  */
+      if (is_target_filename (exec_file))
+	return exec_file + strlen (TARGET_SYSROOT_PREFIX);
+
+      /* If the target filesystem is local then the remote can see
+	 everything GDB can see.  In this case the remote should be able to
+	 access EXEC_FILE.  */
+      if (target_filesystem_is_local ())
+	return exec_file;
+
+      /* If the sysroot is not a target path, then GDB can see a copy of
+	 the remote target's filesystem, or if sysroot is empty, then the
+	 remote and GDB could be sharing a filesystem.
+
+	 In either case, by removing the sysroot from the front of
+	 EXEC_FILE, we can build a filename that the remote can see.  */
+      if (!is_target_filename (gdb_sysroot)
+	  && startswith (exec_file, gdb_sysroot.c_str ()))
+	{
+	  /* If the prefix is '/aa/' and EXEC_FILE is '/aa/bb/cc' then we
+	     only want t remove '/aa' from EXEC_FILE to leave '/bb/cc'.  */
+	  size_t len = gdb_sysroot.length ();
+	  while (len > 0 && IS_DIR_SEPARATOR (gdb_sysroot[len - 1]))
+	    --len;
+
+	  return exec_file + len;
+	}
+    }
+
+  /* The user has set the remote exec-file, or GDB doesn't think the remote
+     target and GDB can see the same filesystem.  */
+  return info.first;
+}
+
 /* In the extended protocol we want to be able to do things like
    "run" and have them basically work as expected.  So we need
    a special create_inferior function.  We support changing the
@@ -11327,7 +11452,9 @@ extended_remote_target::create_inferior (const char *exec_file,
   int run_worked;
   char *stop_reply;
   struct remote_state *rs = get_remote_state ();
-  const std::string &remote_exec_file = get_remote_exec_file ();
+
+  std::string remote_exec_file
+    = get_exec_file_for_create_inferior (exec_file);
 
   /* If running asynchronously, register the target file descriptor
      with the event loop.  */
diff --git a/gdb/testsuite/gdb.server/ext-run.exp b/gdb/testsuite/gdb.server/ext-run.exp
index f4ff546c393..18205e11ec8 100644
--- a/gdb/testsuite/gdb.server/ext-run.exp
+++ b/gdb/testsuite/gdb.server/ext-run.exp
@@ -30,43 +30,147 @@ if {[build_executable $testfile.exp $testfile $srcfile debug] == -1} {
 # allow_xml_test must be called while gdb is not running.
 set do_xml_test [allow_xml_test]
 
-save_vars { GDBFLAGS } {
-    # If GDB and GDBserver are both running locally, set the sysroot to avoid
-    # reading files via the remote protocol.
-    if { ![is_remote host] && ![is_remote target] } {
-	set GDBFLAGS "$GDBFLAGS -ex \"set sysroot\""
+# This is used as an override function.
+proc do_nothing {} { return 0 }
+
+# Start an exetended-remote gdbserver, connect to it, and then use
+# 'run' to start an inferior.
+#
+# If CLEAR_SYSROOT is true then the 'set sysroot' command is issued,
+# clearing the sysroot, otherwise the sysroot is left unchanged.
+#
+# If SET_REMOTE_EXEC is true then the 'set remote-exec ...' command is
+# issued to point GDB at the executable on the target (after copying
+# the executable over).  Otherwise, we rely on GDB and gdbserver being
+# able to see the same filesystem, remote exec-file is not set, and
+# GDB will just use the path to the executable.
+proc do_test { clear_sysroot set_remote_exec fetch_exec_and_args } {
+
+    # If we don't clear the sysroot, then the sysroot will remain as
+    # 'target:'.  In this case, if we don't 'set remote exec-file'
+    # then GDB will not be able to start a remote inferior.
+    if { !$clear_sysroot && !$set_remote_exec } {
+	return
     }
 
     clean_restart $::testfile
-}
 
-# Make sure we're disconnected, in case we're testing with an
-# extended-remote board, therefore already connected.
-gdb_test "disconnect" ".*"
+    # Disable, or enable, use of the qExecAndArgs packet.
+    gdb_test "set remote fetch-exec-and-args-packet ${fetch_exec_and_args}" \
+	".*"
 
-set target_exec [gdbserver_download_current_prog]
-gdbserver_start_extended
+    # Make sure we're disconnected, in case we're testing with an
+    # extended-remote board, therefore already connected.
+    gdb_test "disconnect" ".*"
 
-gdb_test_no_output "set remote exec-file $target_exec" "set remote exec-file"
+    if { $clear_sysroot } {
+	gdb_test_no_output "set sysroot" \
+	    "clear sysroot"
+    } else {
+	set sysroot "UNKNOWN"
+	gdb_test_multiple "show sysroot" "" {
+	    -re -wrap "^The current system root is \"(\[^\r\n\]*)\"\\." {
+		set sysroot $expect_out(1,string)
+		pass $gdb_test_name
+	    }
+	}
 
-gdb_breakpoint main
-gdb_test "run" "Breakpoint.* main .*" "continue to main"
-
-if { [istarget *-*-linux*] } {
-    # On Linux, gdbserver can also report the list of processes.
-    # But only if xml support is compiled in.
-    if { $do_xml_test } {
-	# This is done in a way to avoid the timeout that can occur from
-	# applying .* regexp to large output.
-	gdb_test_lines "info os processes" "get process list" \
-	    "^pid +user +command.*\r\n1 +root +\[/a-z\]*(init|systemd|bash)"
+	if { $sysroot eq "" } {
+	    gdb_test_no_output "set sysroot target:"
+	} elseif { $sysroot ne "target:" } {
+	    unsupported "unexpected sysroot value"
+	    return
+	}
     }
-}
 
-gdb_test "kill" "" "kill" "Kill the program being debugged. .y or n. " "y"
+    gdbserver_start_extended
 
-gdb_load $binfile
-gdb_test "monitor help" "The following monitor commands.*" \
+    # Check the 'remote exec-file' setting before we (possibly) set it
+    # ourselves.
+    if { !$fetch_exec_and_args } {
+	set suffix "the default remote executable will be used"
+    } elseif { !$clear_sysroot} {
+	set suffix "the remote has no default executable set"
+    } else {
+	set file_re [string_to_regexp $::binfile]
+	set suffix "using automatic value \"$file_re\""
+    }
+    gdb_test "show remote exec-file" \
+	"The remote exec-file is unset, ${suffix}\\." \
+	"check remote exec-file is unset"
+
+    if { $set_remote_exec } {
+	set target_exec [gdbserver_download_current_prog]
+	gdb_test_no_output "set remote exec-file $target_exec" \
+	    "set remote exec-file"
+
+	# Check GDB reflect the value we just set.
+	set file_re [string_to_regexp $target_exec]
+	gdb_test "show remote exec-file" \
+	    "The remote exec-file is \"$file_re\"\\." \
+	    "check remote exec-file after set"
+    }
+
+    gdb_breakpoint main
+    gdb_test_multiple "run" "continue to main" {
+	-re -wrap "Breakpoint.* main .*" {
+	    pass $gdb_test_name
+	}
+	-re -wrap "Running the default executable on the remote target failed; try \"set remote exec-file\"." {
+
+	    # If 'set remote exec-file' has been used then we should
+	    # not get here.
+	    gdb_assert {!$set_remote_exec} \
+		"confirm remote exec-file is not set"
+
+	    if {!$fetch_exec_and_args} {
+		# We deliberately disabled GDB's ability to know that
+		# the remote doesn't have a default executable set (by
+		# disabling the qDefaultExecAndArgs packet).  We got
+		# the result we expected, but the inferior is not
+		# running, so we're done with this phase of testing.
+		pass $gdb_test_name
+		return
+	    }
+	}
+    }
+
+    if { [istarget *-*-linux*] } {
+	# On Linux, gdbserver can also report the list of processes.
+	# But only if xml support is compiled in.
+	if { $::do_xml_test } {
+	    # This is done in a way to avoid the timeout that can occur from
+	    # applying .* regexp to large output.
+	    gdb_test_lines "info os processes" "get process list" \
+		"^pid +user +command.*\r\n1 +root +\[/a-z\]*(init|systemd|bash)"
+	}
+    }
+
+    gdb_test "kill" "" "kill" "Kill the program being debugged. .y or n. " "y"
+
+    gdb_load $::binfile
+    gdb_test "monitor help" "The following monitor commands.*" \
         "load new file without any gdbserver inferior"
 
-gdb_test_no_output "monitor exit"
+    gdb_test_no_output "monitor exit"
+}
+
+set clear_sysroot_modes { false }
+set set_remote_exec_modes { true }
+if {![is_remote target] && ![is_remote host]} {
+    lappend set_remote_exec_modes false
+    lappend clear_sysroot_modes true
+}
+
+# This override prevents GDB from automatically setting the 'remote
+# exec-file' when using the extended-remote protocol.  If we want the
+# exec-file set, then this test takes care of it.
+with_override extended_gdbserver_load_last_file do_nothing {
+    foreach_with_prefix clear_sysroot $clear_sysroot_modes {
+	foreach_with_prefix set_remote_exec $set_remote_exec_modes {
+	    foreach_with_prefix fetch_exec_and_args { on off } {
+		do_test $clear_sysroot $set_remote_exec $fetch_exec_and_args
+	    }
+	}
+    }
+}

base-commit: 3c724596812882cc0c3b653330551fae132d6475
-- 
2.47.1