public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Simon Marchi <simon.marchi@efficios.com>
To: gdb-patches@sourceware.org
Cc: Simon Marchi <simon.marchi@efficios.com>
Subject: [PATCH 4/4] gdb: remove get_exec_file
Date: Thu, 30 May 2024 14:53:56 -0400	[thread overview]
Message-ID: <20240530185509.265901-5-simon.marchi@efficios.com> (raw)
In-Reply-To: <20240530185509.265901-1-simon.marchi@efficios.com>

I believe that the get_exec_file function is unnecessary, and the code
can be simplified if we remove it.

Consider for instance when you "run" a program on Linux with native
debugging.

 1. run_command_1 obtains the executable file from
    `current_program_space->exec_filename ()`
 2. it passes it to `run_target->create_inferior()`, which is
    `inf_ptrace_target::create_inferior()` in this case, which then
    passes it to `fork_inferior()`
 3. `fork_inferior()` then has a fallback, where if the passed exec file
    is nullptr, it gets its from `get_exec_file()`.
 4. `get_exec_file()` returns `current_program_space->exec_filename ()`
    - just like the things we started with - or errors out if the
    current program space doesn't have a specified executable.

If there's no exec filename passed in step 1, there's not going to be
any in step 4, so it seems pointless to call `get_exec_file()`, we could
just error out when `exec_file` is nullptr.  But we can't error out
directly in `fork_inferior()`, since the error is GDB-specific, and that
function is shared with GDBserver.

Speaking of GDBserver, all code paths that lead to `fork_inferior()`
provide a non-nullptr exec file.

Therefore, to simplify things:

 - Make `fork_inferior()` assume that the passed exec file is not
   nullptr, don't call `get_exec_file()`
 - Change some targets (darwin-nat, go32-nat, gnu-nat, inf-ptrace,
   nto-procfs, procfs) to error out when the exec file passed to their
   create_inferior method is nullptr.  Some targets are fine with a
   nullptr exec file, so we can't check that in `run_command_1()`.
 - Add the `no_executable_specified_error()` function, which re-uses the
   error message that `get_exec_file()` had.
 - Change some targets (go32-nat, nto-procfs) to not call
   `get_exec_file()`, since it's pointless for the same reason as in the
   example above, if it returns, it's going the be the same value as the
   `exec_file` parameter.  Just rely on `exec_file`.
 - Remove the final use of `get_exec_file()`, in `load_command()`.
 - Remove the `get_exec_file()` implementations in GDB and GDBserver and
   remove the shared declaration.

Change-Id: I601c16498e455f7baa1f111a179da2f6c913baa3
---
 gdb/corefile.c               | 13 -------------
 gdb/darwin-nat.c             |  3 +++
 gdb/exec.c                   |  9 +++++++++
 gdb/exec.h                   |  5 +++++
 gdb/gnu-nat.c                |  3 +++
 gdb/go32-nat.c               |  6 ++----
 gdb/inf-ptrace.c             |  3 +++
 gdb/nat/fork-inferior.c      | 16 +++++-----------
 gdb/nto-procfs.c             |  7 +++++--
 gdb/procfs.c                 |  3 +++
 gdb/symfile.c                |  4 +++-
 gdbserver/server.cc          | 11 -----------
 gdbsupport/common-inferior.h |  5 -----
 13 files changed, 41 insertions(+), 47 deletions(-)

diff --git a/gdb/corefile.c b/gdb/corefile.c
index aa35e3aa4e1f..96052cfdc6d4 100644
--- a/gdb/corefile.c
+++ b/gdb/corefile.c
@@ -76,19 +76,6 @@ validate_files (void)
     }
 }
 
-/* See gdbsupport/common-inferior.h.  */
-
-const char *
-get_exec_file ()
-{
-  if (current_program_space->exec_filename () != nullptr)
-    return current_program_space->exec_filename ();
-
-  error (_("No executable file specified.\n\
-Use the \"file\" or \"exec-file\" command."));
-}
-\f
-
 std::string
 memory_error_message (enum target_xfer_status err,
 		      struct gdbarch *gdbarch, CORE_ADDR memaddr)
diff --git a/gdb/darwin-nat.c b/gdb/darwin-nat.c
index c70cd44bb7fb..d1bcf940dd11 100644
--- a/gdb/darwin-nat.c
+++ b/gdb/darwin-nat.c
@@ -1968,6 +1968,9 @@ darwin_nat_target::create_inferior (const char *exec_file,
 				    const std::string &allargs,
 				    char **env, int from_tty)
 {
+  if (exec_file == nullptr)
+    no_executable_specified_error ();
+
   std::optional<scoped_restore_tmpl<bool>> restore_startup_with_shell;
   darwin_nat_target *the_target = this;
 
diff --git a/gdb/exec.c b/gdb/exec.c
index 496f3b130708..63eee4297bc4 100644
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -504,6 +504,15 @@ exec_file_attach (const char *filename, int from_tty)
   gdb::observers::executable_changed.notify (current_program_space, reload_p);
 }
 
+/* See exec.h.  */
+
+void
+no_executable_specified_error ()
+{
+  error (_("No executable file specified.\n\
+Use the \"file\" or \"exec-file\" command."));
+}
+
 /*  Process the first arg in ARGS as the new exec file.
 
    Note that we have to explicitly ignore additional args, since we can
diff --git a/gdb/exec.h b/gdb/exec.h
index f667f23600d8..0c1f60495d72 100644
--- a/gdb/exec.h
+++ b/gdb/exec.h
@@ -105,4 +105,9 @@ extern void print_section_info (const std::vector<target_section> *table,
 extern void try_open_exec_file (const char *exec_file_host,
 				struct inferior *inf,
 				symfile_add_flags add_flags);
+
+/* Report a "No executable file specified" error.  */
+
+extern void no_executable_specified_error ();
+
 #endif
diff --git a/gdb/gnu-nat.c b/gdb/gnu-nat.c
index 198fc42c2178..1d3e523a9631 100644
--- a/gdb/gnu-nat.c
+++ b/gdb/gnu-nat.c
@@ -2103,6 +2103,9 @@ gnu_nat_target::create_inferior (const char *exec_file,
 				 char **env,
 				 int from_tty)
 {
+  if (exec_file == nullptr)
+    no_executable_specified_error ();
+
   struct inf *inf = cur_inf ();
   inferior *inferior = current_inferior ();
   int pid;
diff --git a/gdb/go32-nat.c b/gdb/go32-nat.c
index f59a7ed5a2bc..84533669b53b 100644
--- a/gdb/go32-nat.c
+++ b/gdb/go32-nat.c
@@ -681,10 +681,8 @@ go32_nat_target::create_inferior (const char *exec_file,
   int result;
   const char *args = allargs.c_str ();
 
-  /* If no exec file handed to us, get it from the exec-file command -- with
-     a good, common error message if none is specified.  */
-  if (exec_file == 0)
-    exec_file = get_exec_file ();
+  if (exec_file == nullptr)
+    no_executable_specified_error ();
 
   resume_signal = -1;
   resume_is_step = 0;
diff --git a/gdb/inf-ptrace.c b/gdb/inf-ptrace.c
index ce303eb87eaf..acb80af9bff1 100644
--- a/gdb/inf-ptrace.c
+++ b/gdb/inf-ptrace.c
@@ -75,6 +75,9 @@ inf_ptrace_target::create_inferior (const char *exec_file,
 				    const std::string &allargs,
 				    char **env, int from_tty)
 {
+  if (exec_file == nullptr)
+    no_executable_specified_error ();
+
   inferior *inf = current_inferior ();
 
   /* Do not change either targets above or the same target if already present.
diff --git a/gdb/nat/fork-inferior.c b/gdb/nat/fork-inferior.c
index 172eef0f93b0..2fd9cba52ee0 100644
--- a/gdb/nat/fork-inferior.c
+++ b/gdb/nat/fork-inferior.c
@@ -265,26 +265,20 @@ execv_argv::init_for_shell (const char *exec_file,
 /* See nat/fork-inferior.h.  */
 
 pid_t
-fork_inferior (const char *exec_file_arg, const std::string &allargs,
-	       char **env, traceme_ftype traceme_fun,
-	       init_trace_ftype init_trace_fun, pre_trace_ftype pre_trace_fun,
-	       const char *shell_file_arg, exec_ftype exec_fun)
+fork_inferior (const char *exec_file, const std::string &allargs, char **env,
+	       traceme_ftype traceme_fun, init_trace_ftype init_trace_fun,
+	       pre_trace_ftype pre_trace_fun, const char *shell_file_arg,
+	       exec_ftype exec_fun)
 {
   pid_t pid;
   /* Set debug_fork then attach to the child while it sleeps, to debug.  */
   int debug_fork = 0;
   const char *shell_file;
-  const char *exec_file;
   char **save_our_env;
   int i;
   int save_errno;
 
-  /* If no exec file handed to us, get it from the exec-file command
-     -- with a good, common error message if none is specified.  */
-  if (exec_file_arg == NULL)
-    exec_file = get_exec_file ();
-  else
-    exec_file = exec_file_arg;
+  gdb_assert (exec_file != nullptr);
 
   /* 'startup_with_shell' is declared in inferior.h and bound to the
      "set startup-with-shell" option.  If 0, we'll just do a
diff --git a/gdb/nto-procfs.c b/gdb/nto-procfs.c
index 9ec214771271..c3108746d946 100644
--- a/gdb/nto-procfs.c
+++ b/gdb/nto-procfs.c
@@ -1179,6 +1179,9 @@ nto_procfs_target::create_inferior (const char *exec_file,
 				    const std::string &allargs,
 				    char **env, int from_tty)
 {
+  if (exec_file == nullptr)
+    no_executable_specified_error ();
+
   struct inheritance inherit;
   pid_t pid;
   int flags, errn;
@@ -1190,9 +1193,9 @@ nto_procfs_target::create_inferior (const char *exec_file,
 
   argv = xmalloc ((allargs.size () / (unsigned) 2 + 2) *
 		  sizeof (*argv));
-  argv[0] = const_cast<char *> (get_exec_file ());
+  argv[0] = exec_file;
   args = xstrdup (allargs.c_str ());
-  breakup_args (args, (exec_file != NULL) ? &argv[1] : &argv[0]);
+  breakup_args (args, &argv[1]);
 
   argv = nto_parse_redirection (argv, &in, &out, &err);
 
diff --git a/gdb/procfs.c b/gdb/procfs.c
index 93d1113893d2..a9a26c6b94cc 100644
--- a/gdb/procfs.c
+++ b/gdb/procfs.c
@@ -2760,6 +2760,9 @@ procfs_target::create_inferior (const char *exec_file,
 				const std::string &allargs,
 				char **env, int from_tty)
 {
+  if (exec_file == nullptr)
+    no_executable_specified_error ();
+
   const char *shell_file = get_shell ();
   char *tryname;
   int pid;
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 78e68139c6f7..a49ef9f24303 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -1843,7 +1843,9 @@ load_command (const char *arg, int from_tty)
     {
       const char *parg, *prev;
 
-      arg = get_exec_file ();
+      arg = current_program_space->exec_filename ();
+      if (arg == nullptr)
+	no_executable_specified_error ();
 
       /* We may need to quote this string so buildargv can pull it
 	 apart.  */
diff --git a/gdbserver/server.cc b/gdbserver/server.cc
index 82f4318457cb..6f9d2a85609b 100644
--- a/gdbserver/server.cc
+++ b/gdbserver/server.cc
@@ -282,17 +282,6 @@ get_exec_wrapper ()
   return !wrapper_argv.empty () ? wrapper_argv.c_str () : NULL;
 }
 
-/* See gdbsupport/common-inferior.h.  */
-
-const char *
-get_exec_file ()
-{
-  if (program_path.get () == NULL)
-    error (_("No executable file specified."));
-
-  return program_path.get ();
-}
-
 /* See server.h.  */
 
 gdb_environ *
diff --git a/gdbsupport/common-inferior.h b/gdbsupport/common-inferior.h
index 9c19e9d9fdd9..cc625aec2368 100644
--- a/gdbsupport/common-inferior.h
+++ b/gdbsupport/common-inferior.h
@@ -27,11 +27,6 @@
    otherwise.  */
 extern const char *get_exec_wrapper ();
 
-/* Return the name of the executable file as a string.
-
-   Error out if no executable is specified.  */
-extern const char *get_exec_file ();
-
 /* Return the inferior's current working directory.
 
    If it is not set, the string is empty.  */
-- 
2.45.1


  parent reply	other threads:[~2024-05-30 18:55 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-30 18:53 [PATCH 0/4] Cleanups, " Simon Marchi
2024-05-30 18:53 ` [PATCH 1/4] gdb: make progspace::exec_filename private, add getter / setter Simon Marchi
2024-05-30 18:53 ` [PATCH 2/4] gdb: replace `get_exec_file (0)` calls with `current_program_space->exec_filename ()` Simon Marchi
2024-05-30 18:53 ` [PATCH 3/4] gdb: remove dead code in nto-procfs.c Simon Marchi
2024-05-30 18:53 ` Simon Marchi [this message]
2024-06-03 14:53 ` [PATCH 0/4] Cleanups, remove get_exec_file Tom Tromey
2024-06-08  3:16   ` Simon Marchi

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=20240530185509.265901-5-simon.marchi@efficios.com \
    --to=simon.marchi@efficios.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).