[PATCHv2 08/10] gdb: remote filesystem can be local to GDB in some cases

[Andrew Burgess <aburgess@redhat.com>]

This commit makes use of the qMachineId packet (added in the previous
commit) to detect when a remote target is actually running on the same
host as GDB.

If GDB detects that the remote target is running on the same host as
GDB then GDB is able to avoid fetching files from the remote, and can
instead, access the files directly, which is much quicker.

There is a new setting to control this behaviour:

  set remote local-filesystem auto|on|off
  show remote local-filesystem

By default this setting is 'auto', in which case GDB will use the
result of the qMachineId packet to choose between acting like 'on' or
'off', but the user can force this on (maybe for targets that don't
support qMachineId?), or can force it off if this feature is causing
problems.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>
---
 gdb/NEWS                                     |  25 ++++
 gdb/doc/gdb.texinfo                          |  30 ++++
 gdb/remote.c                                 | 148 ++++++++++++++-----
 gdb/testsuite/gdb.server/server-local-fs.c   |  22 +++
 gdb/testsuite/gdb.server/server-local-fs.exp | 138 +++++++++++++++++
 gdb/testsuite/gdb.server/sysroot.exp         |  90 ++++++-----
 6 files changed, 371 insertions(+), 82 deletions(-)
 create mode 100644 gdb/testsuite/gdb.server/server-local-fs.c
 create mode 100644 gdb/testsuite/gdb.server/server-local-fs.exp

diff --git a/gdb/NEWS b/gdb/NEWS
index 2b1f265f5b8..36ca93398f6 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -113,6 +113,13 @@
   subsequent runs of the inferior will use the same arguments as the
   first run.
 
+* For remote targets that support the qMachineId packet, if GDB
+  believes that the remote target is on the same host as GDB itself,
+  and that GDB can access the same files as the remote target, then
+  GDB will take advantage of this to access files directly, rather
+  than copying them from the remote target.  This behaviour can be
+  disable using the new command 'set remote local-filesystem off'.
+
 * New commands
 
 set debug breakpoint on|off
@@ -140,6 +147,24 @@ show always-read-ctf
 info main
   Get main symbol to identify entry point into program.
 
+set remote local-filesystem on|off|auto
+show remote local-filesystem
+  When 'on' GDB will assume that its local filesystem is the same
+  filesystem as the remote target, this effectively means GDB will
+  ignore any 'target:' prefix in the sysroot setting.  When 'off' GDB
+  will use the sysroot value to determine if a path is remote or not;
+  a sysroot starting 'target:' indicates that paths should be treated
+  as remote.
+
+  The default value for this setting is 'auto', in this mode GDB will
+  make use of the qMachineId packet to determine if the remote target
+  is on the same host as GDB or not.  For remote targets that don't
+  support qMachineId, or in cases where the qMachineId indicates that
+  the remote target is truly remote, GDB will behave as if this
+  setting is 'off'.  Only when qMachineId is supported, and qMachineId
+  indicates the remote target is on the same host as GDB, will GDB
+  treat this setting as 'on'.
+
 * New convenience function "$_shell", to execute a shell command and
   return the result.  This lets you run shell commands in expressions.
   Some examples:
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 6b417ab85d8..bc41d3dbedd 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -21949,6 +21949,7 @@
 @cindex system root, alternate
 @kindex set solib-absolute-prefix
 @kindex set sysroot
+@anchor{set sysroot}
 @item set sysroot @var{path}
 Use @var{path} as the system root for the program being debugged.  Any
 absolute shared library paths will be prefixed with @var{path}; many
@@ -21977,6 +21978,15 @@
 named @file{target:} or @file{remote:}, you need to use some
 equivalent variant of the name like @file{./target:}.
 
+If @value{GDBN} is connected to a remote target, but the remote target
+is running on the same host as @value{GDBN}, then @value{GDBN} is able
+to avoid fetching files from the remote target, effectively ignoring
+the @file{target:} prefix in the sysroot.  @value{GDBN} uses the
+@code{qMachineId} packet (@pxref{Machine-Id Packet}) to determine if a
+remote target and @value{GDBN} are running on the same host or not.
+This behaviour can be controlled using @code{set remote
+local-filesystem} (@pxref{set remote local-filesystem}).
+
 For targets with an MS-DOS based filesystem, such as MS-Windows,
 @value{GDBN} tries prefixing a few variants of the target
 absolute file name with @var{path}.  But first, on Unix hosts,
@@ -24024,6 +24034,26 @@
 target system.  If it is not set, the target will use a default
 filename (e.g.@: the last program run).
 
+@anchor{set remote local-filesystem}
+@cindex filesystem, for remote target
+@item set remote local-filesystem @r{[}auto@r{|}on@r{|}off@r{]}
+@itemx show remote local-filesystem
+When set to @samp{on}, @value{GDBN} will ignore any @file{target:}
+prefix in the sysroot setting (@pxref{set sysroot,,Setting the
+sysroot}) when accessing files on a remote target.  The result of this
+is that @value{GDBN} will act as though @value{GDBN} and the remote
+target are sharing a filesystem.
+
+When set to @samp{off}, @value{GDBN} treats a @file{target:} prefix in
+the usual way; files will be copied from the remote target before
+being read.
+
+When set to @samp{auto}, which is the default, @value{GDBN} will use
+the @code{qMachineId} packet (@pxref{Machine-Id Packet}) to determine
+if the remote target and @value{GDBN} are running on the same host,
+and can access the same filesystem, in which case, this setting act as
+if @samp{on}.  Otherwise, this setting acts as if @samp{off}.
+
 @item set remote interrupt-sequence
 @cindex interrupt remote programs
 @cindex select Ctrl-C, BREAK or BREAK-g
diff --git a/gdb/remote.c b/gdb/remote.c
index 1a1e963f832..d5b758c888d 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1444,6 +1444,52 @@ is_remote_target (process_stratum_target *target)
   return as_remote_target (target) != nullptr;
 }
 
+/* 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);
+}
+
+/* Should GDB assume that the remote target is on the same local
+   filesystem?  For example, when starting a target using the '|'
+   notation, the target will be on the local machine.
+
+   When this is set to auto GDB will try to figure this out itself, while
+   setting this to true forces GDB to assume the remote is actually
+   local.  */
+
+static enum auto_boolean remote_filesystem_is_local = AUTO_BOOLEAN_AUTO;
+
+/* Implement 'show remote local-filesystem'.  */
+
+static void
+show_remote_local_filesystem (struct ui_file *file, int from_tty,
+			      struct cmd_list_element *cmd,
+			      const char *value)
+{
+  if (remote_filesystem_is_local == AUTO_BOOLEAN_AUTO)
+    {
+      remote_target *remote = get_current_remote_target ();
+
+      if (remote == nullptr)
+	gdb_printf (file, _("Whether the remote filesystem is local to GDB "
+			    "is \"auto\" (current target is not remote).\n"));
+      else if (target_filesystem_is_local ())
+	gdb_printf (file, _("Whether the remote filesystem is local to GDB "
+			    "is \"auto\" (currently \"on\").\n"));
+      else
+	gdb_printf (file, _("Whether the remote filesystem is local to GDB "
+			    "is \"auto\" (currently \"off\").\n"));
+    }
+  else
+    gdb_printf (file, _("Whether the remote filesystem is local to GDB "
+			"is \"%s\".\n"), value);
+}
+
 /* An enum used to track where the per-program-space remote exec-file data
    came from.  This is useful when deciding which warnings to give to the
    user.  */
@@ -1973,16 +2019,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.  */
@@ -13216,49 +13252,60 @@ remote_target::fileio_fstat (int fd, struct stat *st, fileio_error *remote_errno
 bool
 remote_target::filesystem_is_local ()
 {
-  /* Valgrind GDB presents itself as a remote target but works
-     on the local filesystem: it does not implement remote get
-     and users are not expected to set a sysroot.  To handle
-     this case we treat the remote filesystem as local if the
-     sysroot is exactly TARGET_SYSROOT_PREFIX and if the stub
-     does not support vFile:open.  */
-  if (gdb_sysroot == TARGET_SYSROOT_PREFIX)
+  if (remote_filesystem_is_local == AUTO_BOOLEAN_AUTO)
     {
-      packet_support ps = m_features.packet_support (PACKET_vFile_open);
+      /* If the remote appears to be on the same local machine then assume
+	 the filesystem is local.  */
+      struct remote_state *rs = get_remote_state ();
+      if (rs->remote_target_is_local_p)
+	return true;
 
-      if (ps == PACKET_SUPPORT_UNKNOWN)
+      /* Valgrind GDB presents itself as a remote target but works
+	 on the local filesystem: it does not implement remote get
+	 and users are not expected to set a sysroot.  To handle
+	 this case we treat the remote filesystem as local if the
+	 sysroot is exactly TARGET_SYSROOT_PREFIX and if the stub
+	 does not support vFile:open.  */
+      if (gdb_sysroot == TARGET_SYSROOT_PREFIX)
 	{
-	  int fd;
-	  fileio_error remote_errno;
+	  packet_support ps = m_features.packet_support (PACKET_vFile_open);
 
-	  /* Try opening a file to probe support.  The supplied
-	     filename is irrelevant, we only care about whether
-	     the stub recognizes the packet or not.  */
-	  fd = remote_hostio_open (NULL, "just probing",
-				   FILEIO_O_RDONLY, 0700, 0,
-				   &remote_errno);
+	  if (ps == PACKET_SUPPORT_UNKNOWN)
+	    {
+	      int fd;
+	      fileio_error remote_errno;
 
-	  if (fd >= 0)
-	    remote_hostio_close (fd, &remote_errno);
+	      /* Try opening a file to probe support.  The supplied
+		 filename is irrelevant, we only care about whether
+		 the stub recognizes the packet or not.  */
+	      fd = remote_hostio_open (NULL, "just probing",
+				       FILEIO_O_RDONLY, 0700, 0,
+				       &remote_errno);
 
-	  ps = m_features.packet_support (PACKET_vFile_open);
-	}
+	      if (fd >= 0)
+		remote_hostio_close (fd, &remote_errno);
 
-      if (ps == PACKET_DISABLE)
-	{
-	  static int warning_issued = 0;
+	      ps = m_features.packet_support (PACKET_vFile_open);
+	    }
 
-	  if (!warning_issued)
+	  if (ps == PACKET_DISABLE)
 	    {
-	      warning (_("remote target does not support file"
-			 " transfer, attempting to access files"
-			 " from local filesystem."));
-	      warning_issued = 1;
-	    }
+	      static int warning_issued = 0;
 
-	  return true;
+	      if (!warning_issued)
+		{
+		  warning (_("remote target does not support file"
+			     " transfer, attempting to access files"
+			     " from local filesystem."));
+		  warning_issued = 1;
+		}
+
+	      return true;
+	    }
 	}
     }
+  else if (remote_filesystem_is_local == AUTO_BOOLEAN_TRUE)
+    return true;
 
   return false;
 }
@@ -15891,6 +15938,25 @@ this setting is not used."),
 				   &remote_set_cmdlist,
 				   &remote_show_cmdlist);
 
+  add_setshow_auto_boolean_cmd ("local-filesystem", class_files,
+				&remote_filesystem_is_local, _("\
+Set whether the remote's filesystem is local to GDB."), _("\
+Show whether the remote's filesystem is local to GDB."), _("\
+When 'on', GDB assumes that the remote target's filesystem is the same\n\
+local filesystem as GDB sees.  GDB can avoid transferring files over\n\
+the remote protocol, and will instead access the files directly.\n\
+When 'off', GDB will always fetch files using the remote protocol,\n\
+e.g. when an inferior loads a library, GDB will read the libraries\n\
+debug information using the remote protocol, which is slower than\n\
+accessing the library directly.\n\
+\n\
+The default for the setting is 'auto', in which case GDB will try to\n\
+detect when the remote target is running on the same host as GDB."),
+				nullptr,
+				show_remote_local_filesystem,
+				&remote_set_cmdlist,
+				&remote_show_cmdlist);
+
   add_setshow_boolean_cmd ("range-stepping", class_run,
 			   &use_range_stepping, _("\
 Enable or disable range stepping."), _("\
diff --git a/gdb/testsuite/gdb.server/server-local-fs.c b/gdb/testsuite/gdb.server/server-local-fs.c
new file mode 100644
index 00000000000..439a0927f73
--- /dev/null
+++ b/gdb/testsuite/gdb.server/server-local-fs.c
@@ -0,0 +1,22 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2023 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+int
+main (void)
+{
+  return 0;	/* Break here.  */
+}
diff --git a/gdb/testsuite/gdb.server/server-local-fs.exp b/gdb/testsuite/gdb.server/server-local-fs.exp
new file mode 100644
index 00000000000..b830c06d033
--- /dev/null
+++ b/gdb/testsuite/gdb.server/server-local-fs.exp
@@ -0,0 +1,138 @@
+# This testcase is part of GDB, the GNU debugger.
+#
+# Copyright 2023 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Check that GDB can detect when the remote target and GDB running on
+# the same host and can automatically choose to ignore the 'target:'
+# prefix in the sysroot.
+
+load_lib gdbserver-support.exp
+
+standard_testfile
+
+require allow_gdbserver_tests
+
+# This test requires that GDB and the remote be on the same host.
+require {!is_remote host}
+
+if { [build_executable "failed to prepare" ${testfile}] } {
+    return -1
+}
+
+# Call after connecting to a remote target, return true if the
+# GDB/GDBserver combo supports the qMachineId packet, otherwise,
+# return false.
+
+proc supports_qMachineId {} {
+    gdb_test_multiple "show remote fetch-machine-id-packet" "" {
+	-re -wrap "Support for the 'qMachineId' packet on the current remote target is \"auto\", currently disabled\\." {
+	    return false
+	}
+	-re -wrap "Support for the 'qMachineId' packet on the current remote target is \"off\"\\." {
+	    return false
+	}
+	-re -wrap "Support for the 'qMachineId' packet on the current remote target is \"auto\", currently enabled\\." {
+	    return true
+	}
+	-re -wrap "Support for the 'qMachineId' packet on the current remote target is \"on\"\\." {
+	    return true
+	}
+    }
+}
+
+clean_restart $binfile
+
+# This test is all about ignoring the 'target:' prefix in the sysroot.
+# If the test is being run with some board that sets a specific
+# sysroot value then this test might now work, so just don't try.
+gdb_test_multiple "show sysroot" "" {
+    -re -wrap "The current system root is \"target:\"\\." {
+	pass $gdb_test_name
+    }
+    -re -wrap "The current system root is \"\"\\." {
+	gdb_test_no_output "set sysroot target:"
+	send_gdb "show sysroot\n"
+	exp_continue
+    }
+    -re -wrap "" {
+	# Any other sysroot setting is not going to work with this
+	# test.
+	unsupport "unsupported sysroot setting"
+	return
+    }
+}
+
+# Start a gdbserver and connect to it from GDB.  Check to see if GDB
+# downloads any files from the remote target.
+#
+# FS_MODE is used to set 'remote local-filesystem'.
+#
+# Depending on whether the target is remote or local, and the value of
+# FS_MODE, this test might, or might not, expect to see files being
+# downloaded.
+proc test_for_remote_read { fs_mode } {
+
+    clean_restart $::binfile
+
+    # We already checked that the sysroot setting either is already
+    # 'target:', or is '', but can be set to 'target:', so at this
+    # point we just force the sysroot setting.
+    gdb_test_no_output "set sysroot target:"
+
+    # Setup whether GDB thinks the remote filesystem is local or not.
+    gdb_test_no_output "set remote local-filesystem $fs_mode"
+
+    # Disconnect in case we are using extended-remote and are already
+    # connected.
+    gdb_test "disconnect" ".*"
+
+    # Start the gdbserver.
+    set res [gdbserver_spawn ""]
+    set gdbserver_protocol [lindex $res 0]
+    set gdbserver_gdbport [lindex $res 1]
+
+    # Connect to gdbserver.
+    gdb_test_multiple "target $gdbserver_protocol $gdbserver_gdbport" \
+	"connect to gdbserver" {
+	-re -wrap "Reading \[^\r\n\]+ from remote target\.\.\.\r\n.*" {
+	    if {$fs_mode eq "off"
+		|| [is_remote target]
+		|| ![supports_qMachineId]} {
+		pass $gdb_test_name
+	    } else {
+		fail $gdb_test_name
+	    }
+	}
+	-re -wrap "" {
+	    if {![is_remote target]
+		&& $fs_mode ne "off"} {
+		pass $gdb_test_name
+	    } else {
+		fail $gdb_test_name
+	    }
+	}
+    }
+}
+
+foreach_with_prefix fs_mode { auto on off } {
+    if {$fs_mode eq "on" && [is_remote target]} {
+	# It doesn't make sense to force GDB to try and use the local
+	# filesystem when the target is actually remote.
+	continue
+    }
+
+    test_for_remote_read $fs_mode
+}
diff --git a/gdb/testsuite/gdb.server/sysroot.exp b/gdb/testsuite/gdb.server/sysroot.exp
index 35f15d6c60e..7662787f705 100644
--- a/gdb/testsuite/gdb.server/sysroot.exp
+++ b/gdb/testsuite/gdb.server/sysroot.exp
@@ -42,53 +42,61 @@ if { ( ![is_remote host] && ![is_remote target ] )
 }
 lappend modes "remote"
 
-foreach_with_prefix sysroot $modes {
-    global srcdir
-    global subdir
-    global binfile
-
-    if { $sysroot == "local" } {
-	set sysroot_command "/"
-	set reading_symbols "Reading symbols from $host_binfile..."
-	set timeout_factor 1
-    } else {
-	set sysroot_command "target:"
-	set reading_symbols "Reading .*$target_binfile from remote target..."
-	set timeout_factor 5
-    }
+foreach_with_prefix fs_mode $modes {
+    foreach_with_prefix sysroot $modes {
+	global srcdir
+	global subdir
+	global binfile
 
-    # Reading debug info from the remote target can take a bit of time, so
-    # increase the timeout in that case.
-    with_timeout_factor $timeout_factor {
-	# Restart GDB.
-	clean_restart
+	if { $sysroot == "local" || $fs_mode == "local" } {
+	    set sysroot_command "/"
+	    set reading_symbols "Reading symbols from $host_binfile..."
+	    set timeout_factor 1
+	} else {
+	    set sysroot_command "target:"
+	    set reading_symbols "Reading .*$target_binfile from remote target..."
+	    set timeout_factor 5
+	}
 
-	# Make sure we're disconnected, in case we're testing with an
-	# extended-remote board, therefore already connected.
-	gdb_test "disconnect" ".*"
+	# Reading debug info from the remote target can take a bit of time, so
+	# increase the timeout in that case.
+	with_timeout_factor $timeout_factor {
+	    # Restart GDB.
+	    clean_restart
 
-	# Start GDBserver.
-	set res [gdbserver_start "" $target_binfile]
-	set gdbserver_protocol [lindex $res 0]
-	set gdbserver_gdbport [lindex $res 1]
+	    if { $fs_mode == "local" } {
+		gdb_test_no_output "set remote local-filesystem on"
+	    } else {
+		gdb_test_no_output "set remote local-filesystem off"
+	    }
 
-	# Set the sysroot.
-	gdb_test_no_output "set sysroot $sysroot_command"
+	    # Make sure we're disconnected, in case we're testing with an
+	    # extended-remote board, therefore already connected.
+	    gdb_test "disconnect" ".*"
 
-	# Connect to gdbserver, making sure GDB reads in the binary correctly.
-	set test "connect to remote and read binary"
-	if {[gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport $reading_symbols] == 0} {
-	    pass $test
-	} else {
-	    fail $test
-	}
+	    # Start GDBserver.
+	    set res [gdbserver_start "" $target_binfile]
+	    set gdbserver_protocol [lindex $res 0]
+	    set gdbserver_gdbport [lindex $res 1]
+
+	    # Set the sysroot.
+	    gdb_test_no_output "set sysroot $sysroot_command"
 
-	gdb_breakpoint main
-	gdb_test "continue" "Breakpoint $decimal.* main.*" "continue to main"
+	    # Connect to gdbserver, making sure GDB reads in the binary correctly.
+	    set test "connect to remote and read binary"
+	    if {[gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport $reading_symbols] == 0} {
+		pass $test
+	    } else {
+		fail $test
+	    }
 
-	# Test that we can stop inside a library.
-	gdb_breakpoint printf
-	gdb_test "continue" "Breakpoint $decimal.* (__)?printf.*" \
-	    "continue to printf"
+	    gdb_breakpoint main
+	    gdb_test "continue" "Breakpoint $decimal.* main.*" "continue to main"
+
+	    # Test that we can stop inside a library.
+	    gdb_breakpoint printf
+	    gdb_test "continue" "Breakpoint $decimal.* (__)?printf.*" \
+		"continue to printf"
+	}
     }
 }
-- 
2.25.4