[05/10] gdb: detect when gdbserver has no default executable set

Message ID 954d30bdef59ad6fa33d42c04b62a21f1e212eb7.1692200989.git.aburgess@redhat.com
State New
Headers
Series Improve GDB/gdbserver experience when using a local gdbserver |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 fail Patch failed to apply
linaro-tcwg-bot/tcwg_gdb_build--master-arm fail Patch failed to apply
linaro-tcwg-bot/tcwg_gdb_check--master-arm fail Patch failed to apply
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 fail Patch failed to apply

Commit Message

Andrew Burgess Aug. 16, 2023, 3:55 p.m. UTC
  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(-)
  

Patch

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
+    }
 }