public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Andrew Burgess <aburgess@redhat.com>
To: gdb-patches@sourceware.org
Cc: Andrew Burgess <aburgess@redhat.com>
Subject: [PATCH 09/10] gdb: use exec_file with remote targets when possible
Date: Wed, 16 Aug 2023 16:55:05 +0100	[thread overview]
Message-ID: <ce3b65bec26b9d9c71784d21c5bb2e0728d1ebcf.1692200989.git.aburgess@redhat.com> (raw)
In-Reply-To: <cover.1692200989.git.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 is always used
in preference to the file set with the 'file' command.

Similarly, if the qDefaultExecAndArgs packet has succeeded, and GDB
knows that the remote target has a default executable set, then this
will be used in preference to the file set with the 'file' command;
this preserves GDB's existing behaviour.

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 either, the sysroot has been
updated by the user and no longer contains a 'target:' prefix, or, the
the remote_target::filesystem_is_local function returns true (see the
implementation of that function for details of when this can happen).

If all of these conditions are met, then GDB will use the file set
with the 'file' command when starting a new inferior, in all other
cases, GDB will use the file set with 'set remote exec-file'.
---
 gdb/remote.c                         |  52 ++++++++++-
 gdb/testsuite/gdb.server/ext-run.exp | 131 +++++++++++++++++++++------
 2 files changed, 154 insertions(+), 29 deletions(-)

diff --git a/gdb/remote.c b/gdb/remote.c
index 1d5e098e91f..e2d21c65b70 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1396,6 +1396,9 @@ class extended_remote_target final : public remote_target
 
   void post_attach (int) override;
   bool supports_disable_randomization () override;
+
+private:
+  std::string get_exec_file_for_create_inferior (const char *exec_file);
 };
 
 struct stop_reply : public notif_event
@@ -11031,6 +11034,51 @@ directory: %s"),
     }
 }
 
+/* Return the path for the executable that GDB should ask the remote target
+   to use when starting an inferior.  EXEC_FILE is GDB's idea of the
+   current inferior (e.g. set with the 'file' command), however, this is
+   often not the right thing for a remote target to use, which is why GDB
+   also has the 'set remote exec-file' command.
+
+   If the user has 'set remote exec-file', then this function returns the
+   path the user set.  Otherwise, if it is possible that GDB and the remote
+   target can see the same filesystem, this function will return
+   EXEC_FILE.  */
+
+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);
+
+  /* If INFO.SECOND is remote_exec_source::DEFAULT_VALUE, then this
+     indicates that the remote target has failed to inform us if it has a
+     default-executable set or not, maybe the remote doesn't support the
+     qDefaultExecAndArgs packet?  In this case, we pass the empty string to
+     the remote and expect it to use the default executable (if one is
+     set).  */
+  if (exec_file != nullptr
+      && info.second == remote_exec_source::UNSET_VALUE)
+    {
+      /* If the sysroot has been set to something that does not have a
+	 'target:' prefix then GDB will not be trying to fetch files from
+	 the remote anyway, so we can assume that the executable is visible
+	 to both the remote and GDB.
+
+	 Otherwise, if GDB is able to determine that the remote filesystem
+	 is actually local, then we are still OK to use the local
+	 executable.  */
+      if (!is_target_filename (gdb_sysroot)
+	  || target_filesystem_is_local ())
+	return exec_file;
+    }
+
+  /* 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
@@ -11045,7 +11093,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 59ead666d7b..3a4bf3e7eb1 100644
--- a/gdb/testsuite/gdb.server/ext-run.exp
+++ b/gdb/testsuite/gdb.server/ext-run.exp
@@ -30,43 +30,118 @@ 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 } {
+    save_vars { ::GDBFLAGS } {
+	if { $clear_sysroot } {
+	    set ::GDBFLAGS "$::GDBFLAGS -ex \"set sysroot\""
+	}
+
+	clean_restart $::binfile
     }
 
-    clean_restart $binfile
-}
+    # Disable, or enable, use of the qDefaultExecAndArgs packet.
+    gdb_test "set remote fetch-exec-and-args-packet ${fetch_exec_and_args}" \
+	".*"
 
-# Make sure we're disconnected, in case we're testing with an
-# extended-remote board, therefore already connected.
-gdb_test "disconnect" ".*"
+    # Make sure we're disconnected, in case we're testing with an
+    # extended-remote board, therefore already connected.
+    gdb_test "disconnect" ".*"
 
-set target_exec [gdbserver_download_current_prog]
-gdbserver_start_extended
+    gdbserver_start_extended
 
-gdb_test_no_output "set remote exec-file $target_exec" "set remote exec-file"
+    gdb_test "show remote exec-file" \
+	"The remote exec-file is \"\"\[^\r\n\]*" \
+	"check remote exec-file is unset"
 
-gdb_breakpoint main
-gdb_test "run" "Breakpoint.* main .*" "continue to main"
+    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"
+    }
 
-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_sequence "info os processes" "get process list" \
-	    { "pid +user +command" "1 +root +\[/a-z\]*(init|systemd)" }
+    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
+	    } else {
+		# This means that the qMachineId packet is not supported,
+		# and GDB could not tell if it is running on the same host
+		# as gdbserver.  Confirm that qMachineId is showing as
+		# disabled, and then move onto the next testing mode.
+		gdb_test "show remote fetch-machine-id-packet" \
+		    "Support for the 'qMachineId' packet on the current remote target is \"auto\", currently disabled\\." \
+		    "confirm qMachineId packet is not supported"
+		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_sequence "info os processes" "get process list" \
+		{ "pid +user +command" "1 +root +\[/a-z\]*(init|systemd)" }
+	}
     }
-}
 
-gdb_test "kill" "" "kill" "Kill the program being debugged. .y or n. " "y"
+    gdb_test "kill" "" "kill" "Kill the program being debugged. .y or n. " "y"
 
-gdb_load $binfile
-gdb_test "monitor help" "The following monitor commands.*" \
+    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
+	    }
+	}
+    }
+}
-- 
2.25.4


  parent reply	other threads:[~2023-08-16 15:55 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-16 15:54 [PATCH 00/10] Improve GDB/gdbserver experience when using a local gdbserver Andrew Burgess
2023-08-16 15:54 ` [PATCH 01/10] gdb: have remote_target::extended_remote_run take the exec filename Andrew Burgess
2023-08-23  9:30   ` Alexandra Petlanova Hajkova
2023-08-16 15:54 ` [PATCH 02/10] gdb: improve how 'remote exec-file' is stored and accessed Andrew Burgess
2023-08-23  8:44   ` Alexandra Petlanova Hajkova
2023-08-16 15:54 ` [PATCH 03/10] gdb: improve show text and help text for 'remote exec-file' Andrew Burgess
2023-08-23 11:36   ` Mark Wielaard
2023-08-24  8:56   ` Alexandra Petlanova Hajkova
2023-08-16 15:55 ` [PATCH 04/10] gdb/gdbserver: add new qDefaultExecAndArgs packet Andrew Burgess
2023-08-16 16:36   ` Eli Zaretskii
2023-08-28 15:35   ` Tom Tromey
2023-08-16 15:55 ` [PATCH 05/10] gdb: detect when gdbserver has no default executable set Andrew Burgess
2023-08-16 15:55 ` [PATCH 06/10] gdb: make use of is_target_filename Andrew Burgess
2023-08-23 13:35   ` Mark Wielaard
2023-08-16 15:55 ` [PATCH 07/10] gdb: add qMachineId packet Andrew Burgess
2023-08-16 16:34   ` Eli Zaretskii
2023-08-25 14:49     ` Andrew Burgess
2023-08-25 15:01       ` Eli Zaretskii
2023-09-26 14:42         ` Andrew Burgess
2023-09-29  7:45           ` Eli Zaretskii
2023-08-22  2:39   ` Thiago Jung Bauermann
2023-08-23  9:24   ` Mark Wielaard
2023-08-23 11:36     ` Andrew Burgess
2023-08-28 16:06   ` Tom Tromey
2023-08-16 15:55 ` [PATCH 08/10] gdb: remote filesystem can be local to GDB in some cases Andrew Burgess
2023-08-16 16:40   ` Eli Zaretskii
2023-08-16 15:55 ` Andrew Burgess [this message]
2023-08-16 15:55 ` [PATCH 10/10] gdb: remote the get_remote_exec_file function Andrew Burgess
2023-08-23 13:42   ` Mark Wielaard
2023-08-22 10:41 ` [PATCH 00/10] Improve GDB/gdbserver experience when using a local gdbserver Alexandra Petlanova Hajkova
2023-08-23 14:32 ` Mark Wielaard
2023-08-23 15:26   ` Andrew Burgess
2023-08-25 15:34 ` [PATCHv2 " Andrew Burgess
2023-08-25 15:34   ` [PATCHv2 01/10] gdb: have remote_target::extended_remote_run take the exec filename Andrew Burgess
2023-08-25 15:34   ` [PATCHv2 02/10] gdb: improve how 'remote exec-file' is stored and accessed Andrew Burgess
2023-08-25 15:34   ` [PATCHv2 03/10] gdb: improve show text and help text for 'remote exec-file' Andrew Burgess
2023-08-25 15:34   ` [PATCHv2 04/10] gdb/gdbserver: add new qDefaultExecAndArgs packet Andrew Burgess
2023-08-26  6:46     ` Eli Zaretskii
2023-08-25 15:34   ` [PATCHv2 05/10] gdb: detect when gdbserver has no default executable set Andrew Burgess
2023-08-25 15:34   ` [PATCHv2 06/10] gdb: make use of is_target_filename Andrew Burgess
2023-08-25 15:34   ` [PATCHv2 07/10] gdb: add qMachineId packet Andrew Burgess
2023-08-26  6:54     ` Eli Zaretskii
2023-08-25 15:34   ` [PATCHv2 08/10] gdb: remote filesystem can be local to GDB in some cases Andrew Burgess
2023-08-26  6:49     ` Eli Zaretskii
2023-08-25 15:34   ` [PATCHv2 09/10] gdb: use exec_file with remote targets when possible Andrew Burgess
2023-08-25 15:34   ` [PATCHv2 10/10] gdb: remove the get_remote_exec_file function Andrew Burgess

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ce3b65bec26b9d9c71784d21c5bb2e0728d1ebcf.1692200989.git.aburgess@redhat.com \
    --to=aburgess@redhat.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).