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 05/10] gdb: detect when gdbserver has no default executable set
Date: Wed, 16 Aug 2023 16:55:01 +0100	[thread overview]
Message-ID: <954d30bdef59ad6fa33d42c04b62a21f1e212eb7.1692200989.git.aburgess@redhat.com> (raw)
In-Reply-To: <cover.1692200989.git.aburgess@redhat.com>

This commit extends the use of the new qDefaultExecAndArgs
packet (added in the previous commit) so that GDB now understands when
it is connected to a remote server that doesn't have a default
executable set.  We don't do much with this information right now,
other than produce more useful text for 'show remote exec-file'.

Here I've connected to a gdbserver with no default executable set:

  (gdb) show remote exec-file
  The remote exec-file is "", the remote has no default executable set.
  (gdb) file /tmp/hello.x
  Reading symbols from /tmp/hello.x...
  (gdb) run
  Starting program: /tmp/hello.x
  Running the default executable on the remote target failed; try "set remote exec-file"?
  (gdb)

I think that all makes sense.
---
 gdb/remote.c                                  | 120 ++++++++++++++----
 .../gdb.server/fetch-exec-and-args.exp        |  58 ++++++++-
 2 files changed, 150 insertions(+), 28 deletions(-)

diff --git a/gdb/remote.c b/gdb/remote.c
index 870153731b0..038980476c0 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -743,6 +743,12 @@ struct remote_exec_and_args_info
     return m_state == state::SET;
   }
 
+  /* Is this object in state::UNSET?  */
+  bool is_unset () const
+  {
+    return m_state == state::UNSET;
+  }
+
   /* Return the argument string.  Only call when is_set returns true.  */
   const std::string &args () const
   {
@@ -1419,8 +1425,58 @@ is_remote_target (process_stratum_target *target)
   return as_remote_target (target) != nullptr;
 }
 
+/* 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.  */
+
+enum class remote_exec_source
+{
+  /* The remote exec-file has it's default (empty string) value, neither
+     the user, nor the remote target have tried to set the value yet.  */
+  DEFAULT_VALUE,
+
+  /* The remote exec-file value was set based on information fetched from
+     the remote target.  We warn the user if we are replacing a value they
+     supplied with one fetched from the remote target.  */
+  VALUE_FROM_REMOTE,
+
+  /* The remote exec-file value was set either directly by the user, or by
+     GDB after the inferior performed an exec.  */
+  VALUE_FROM_GDB,
+
+  /* The remote exec-file has it's default (empty string) value, but this
+     is because the user hasn't supplied a value yet, and the remote target
+     has specifically told GDB that it has no default executable available.  */
+  UNSET_VALUE,
+};
+
+/* Data held per program-space to represent the remote exec-file path.  The
+   first item in the pair is the exec-file path, this is set either by the
+   user with 'set remote exec-file', or automatically by GDB when
+   connecting to a remote target.
+
+   The second item in the pair is an enum flag that indicates where the
+   path value came from, or, when the path is the empty string, what this
+   actually means.  See show_remote_exec_file for details.  */
+using remote_exec_file_info = std::pair<std::string, remote_exec_source>;
+
 /* Per-program-space data key.  */
-static const registry<program_space>::key<std::string> remote_pspace_data;
+static const registry<program_space>::key<remote_exec_file_info>
+  remote_pspace_data;
+
+/* Retrieve the remote_exec_file_info object for PSPACE.  If no such object
+   yet exists then create a new one using the default constructor.  */
+
+static remote_exec_file_info &
+get_remote_exec_file_info (program_space *pspace)
+{
+  remote_exec_file_info *info = remote_pspace_data.get (pspace);
+  if (info == nullptr)
+    info = remote_pspace_data.emplace (pspace, "",
+				       remote_exec_source::DEFAULT_VALUE);
+  gdb_assert (info != nullptr);
+  return *info;
+}
 
 /* The size to align memory write packets, when practical.  The protocol
    does not guarantee any alignment, and gdb will generate short
@@ -1743,23 +1799,23 @@ remote_target::get_remote_state ()
 /* Fetch the remote exec-file from the current program space.  */
 
 static const std::string &
-get_remote_exec_file (void)
+get_remote_exec_file ()
 {
-  const std::string *remote_exec_file
-    = remote_pspace_data.get (current_program_space);
-  if (remote_exec_file == nullptr)
-    remote_exec_file = remote_pspace_data.emplace (current_program_space);
-  return *remote_exec_file;
+  const remote_exec_file_info &info
+    = get_remote_exec_file_info (current_program_space);
+  return info.first;
 }
 
 /* Set the remote exec file for PSPACE.  */
 
 static void
 set_pspace_remote_exec_file (struct program_space *pspace,
-			     const std::string &filename)
+			     const std::string &filename,
+			     remote_exec_source source)
 {
-  remote_pspace_data.clear (pspace);
-  remote_pspace_data.emplace (pspace, filename);
+  remote_exec_file_info &info = get_remote_exec_file_info (pspace);
+  info.first = filename;
+  info.second = source;
 }
 
 /* The "set remote exec-file" callback.  */
@@ -1767,8 +1823,8 @@ set_pspace_remote_exec_file (struct program_space *pspace,
 static void
 set_remote_exec_file_cb (const std::string &filename)
 {
-  set_pspace_remote_exec_file (current_program_space,
-			       filename);
+  set_pspace_remote_exec_file (current_program_space, filename,
+			       remote_exec_source::VALUE_FROM_GDB);
 }
 
 /* Get the value for the 'set remote exec-file' user setting.  */
@@ -1785,12 +1841,18 @@ static void
 show_remote_exec_file (struct ui_file *file, int from_tty,
 		       struct cmd_list_element *cmd, const char *value)
 {
-  const std::string &filename = get_remote_exec_file ();
-  if (filename.empty ())
-    gdb_printf (file, _("The remote exec-file is \"\", the default remote "
-			"executable will be used.\n"));
+  const remote_exec_file_info &info
+    = get_remote_exec_file_info (current_program_space);
+
+  if (info.second == remote_exec_source::DEFAULT_VALUE)
+    gdb_printf (file, _("The remote exec-file is \"\", 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 \"\", the remote has "
+			"no default executable set.\n"));
   else
-    gdb_printf (file, "The remote exec-file is \"%s\".\n", filename.c_str ());
+    gdb_printf (file, _("The remote exec-file is \"%s\".\n"),
+		info.first.c_str ());
 }
 
 static int
@@ -5134,14 +5196,27 @@ remote_target::start_remote_1 (int from_tty, int extended_p)
   if (exec_and_args.is_set ())
     {
       current_inferior ()->set_args (exec_and_args.args ());
-      const std::string &remote_exec = get_remote_exec_file ();
-      if (!remote_exec.empty () && remote_exec != exec_and_args.exec ())
+      remote_exec_file_info &info
+	= get_remote_exec_file_info (current_program_space);
+      if (info.second == remote_exec_source::VALUE_FROM_GDB
+	  && info.first != exec_and_args.exec ())
 	warning (_("updating 'remote exec-file' to '%ps' to match "
 		   "remote target"),
 		 styled_string (file_name_style.style (),
 				exec_and_args.exec ().c_str ()));
-      set_pspace_remote_exec_file (current_program_space,
-				   exec_and_args.exec ());
+      info.first = exec_and_args.exec ();
+      info.second = remote_exec_source::VALUE_FROM_REMOTE;
+    }
+  else if (exec_and_args.is_unset ())
+    {
+      remote_exec_file_info &info
+	= get_remote_exec_file_info (current_program_space);
+      if (info.second == remote_exec_source::DEFAULT_VALUE
+	  || info.second == remote_exec_source::VALUE_FROM_REMOTE)
+	{
+	  info.first.clear ();
+	  info.second = remote_exec_source::UNSET_VALUE;
+	}
     }
 
   if (extended_p)
@@ -6426,7 +6501,8 @@ remote_target::follow_exec (inferior *follow_inf, ptid_t ptid,
   if (is_target_filename (execd_pathname))
     execd_pathname += strlen (TARGET_SYSROOT_PREFIX);
 
-  set_pspace_remote_exec_file (follow_inf->pspace, execd_pathname);
+  set_pspace_remote_exec_file (follow_inf->pspace, execd_pathname,
+			       remote_exec_source::VALUE_FROM_GDB);
 }
 
 /* Same as remote_detach, but don't send the "D" packet; just disconnect.  */
diff --git a/gdb/testsuite/gdb.server/fetch-exec-and-args.exp b/gdb/testsuite/gdb.server/fetch-exec-and-args.exp
index 22c22d0c0b2..93c8d23575b 100644
--- a/gdb/testsuite/gdb.server/fetch-exec-and-args.exp
+++ b/gdb/testsuite/gdb.server/fetch-exec-and-args.exp
@@ -56,8 +56,13 @@ proc check_show_args { packet } {
 # output, and is converted to a regexp by this function.
 proc check_remote_exec_file { packet filename } {
     if { $filename eq "" } {
-	set remote_exec_re \
-	    "The remote exec-file is \"\", the default remote executable will be used\\."
+	if { $packet } {
+	    set remote_exec_re \
+		"The remote exec-file is \"\", the remote has no default executable set\\."
+	} else {
+	    set remote_exec_re \
+		"The remote exec-file is \"\", the default remote executable will be used\\."
+	}
     } else {
 	set remote_exec_re \
 	    "The remote exec-file is \"[string_to_regexp $filename]\"\\."
@@ -195,13 +200,54 @@ proc_with_prefix test_remote_exec_warning {} {
     }
 }
 
+# Start GDB.  When PACKET is 'off' disable the qDefaultExecAndArgs
+# packet, otherwise, when PACKET is 'on' enable the packet.
+#
+# Start gdbserver in extended-remote mode, but don't provide a
+# filename when starting gdbserver.
+#
+# Connect to the remote server, and check 'show remote exec-file'.
+proc_with_prefix test_server_with_no_exec { packet set_remote_exec } {
+    clean_restart
+
+    gdb_test "disconnect" ".*"
+
+    gdb_file_cmd $::binfile
+
+    gdb_test "set remote fetch-exec-and-args ${packet}" \
+	"Support for the 'qDefaultExecAndArgs' packet on future remote targets is set to \"${packet}\"\\."
+
+    # Start gdbserver.
+    set target_exec [gdbserver_download_current_prog]
+
+    if { $set_remote_exec } {
+	gdb_test_no_output "set remote exec-file $target_exec" \
+	    "set remote exec-file"
+	set expected_filename $target_exec
+    } else {
+	set expected_filename ""
+    }
+
+    gdbserver_start_extended
+
+    check_remote_exec_file $packet $expected_filename
+}
+
 # This override prevents the remote exec-file from being set when
 # using the extended-remote protocol.  This is harmless when using
 # other boards.
 with_override extended_gdbserver_load_last_file do_nothing {
-    foreach_with_prefix packet { on off } {
-	test_exec_and_arg_fetch ${packet}
-    }
+    # This override stops GDB connecting to the gdbserver as soon as
+    # GDB is started when testing with the extended-remote protocol.
+    with_override gdbserver_start_multi do_nothing {
+	foreach_with_prefix packet { on off } {
+	    test_exec_and_arg_fetch ${packet}
+
+	    foreach_with_prefix set_remote_exec { true false } {
+		test_server_with_no_exec $packet $set_remote_exec
+	    }
+	}
 
-    test_remote_exec_warning
+	test_remote_exec_warning
+    }
 }
-- 
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 ` Andrew Burgess [this message]
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 ` [PATCH 09/10] gdb: use exec_file with remote targets when possible Andrew Burgess
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=954d30bdef59ad6fa33d42c04b62a21f1e212eb7.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).