From 2f13f758b07ec2f65892f3708b5f44a922fceef8 Mon Sep 17 00:00:00 2001 From: Joel Brobecker Date: Tue, 20 Feb 2018 17:20:08 +0400 Subject: [PATCH] WIP: GDBserver no longer working when given an executable's basename Recent versions of GDB are no longer able to start an executable when the path to that executable is a basename. Eg: $ gdbserver --once :4444 simple_main zsh:1: command not found: simple_main During startup program exited with code 127. Exiting This is a regression which was introduced by the following commit: commit 2090129c36c7e582943b7d300968d19b46160d84 Date: Thu Dec 22 21:11:11 2016 -0500 Subject: Share fork_inferior et al with gdbserver Prior to this patch, each host that GDBserver was supporting had its own create_inferior code, but the code in linux-low.c prior to the change gives the idea of what one might be doing: execv (program, allargs); if (errno == ENOENT) execvp (program, allargs); First we do an "execv", which doesn't search the path, but just uses the program name as is. This is how our scenario used to work. But if the executable couldn't be located with just the executable name, then we'd try with execvp, which searches the PATH. Today, this code has been factorized into a common implementation of fork_inferior, which basically does: if (exec_fun != NULL) (*exec_fun) (argv[0], &argv[0], env); else execvp (argv[0], &argv[0]); (The "exec_fun" case is only used in the case of Darwin, where we have to provide the equivalent of the execvp function). One could feel tempted to implement the same kind of approach in fork_inferior, but it's not going to work, because we now support running the inferior through the shell (so the execv would apply to the shell, not the inferior). Instead, what this patch does is explicitly handle the case where the executable name is a basename, and that filename exists in the current working directory - in that case, we implicitly prepend that working directory and use that to execute it. This is only a prototype, as you will see that there is a bit of a C++-ification happening in execv_argv to make memory management a little easier. If the principle of the patch is accepted, I will likely split the patches in two, and also probably C++-ify the code that quotes the arguments. This patch has been validated on x86_64-linux, using both the official testsuite as well as AdaCore's testsuite. We ran the testsuite first in standalone native mode, and then using the native-gdbserver board file. Change-Id: I97268aacc724e07b3f8aff8d69b59c9f65615257 --- gdb/common/filestuff.c | 31 ++++++++++++++++++++ gdb/common/filestuff.h | 6 ++++ gdb/nat/fork-inferior.c | 75 ++++++++++++++++++++++++++++++++++--------------- gdb/source.c | 33 ---------------------- 4 files changed, 90 insertions(+), 55 deletions(-) diff --git a/gdb/common/filestuff.c b/gdb/common/filestuff.c index f5a754ffa66..e6e046dea12 100644 --- a/gdb/common/filestuff.c +++ b/gdb/common/filestuff.c @@ -145,6 +145,37 @@ fdwalk (int (*func) (void *, int), void *arg) #endif /* HAVE_FDWALK */ +/* See filestuff.h. */ + +int +is_regular_file (const char *name, int *errno_ptr) +{ + struct stat st; + const int status = stat (name, &st); + + /* Stat should never fail except when the file does not exist. + If stat fails, analyze the source of error and return True + unless the file does not exist, to avoid returning false results + on obscure systems where stat does not work as expected. */ + + if (status != 0) + { + if (errno != ENOENT) + return 1; + *errno_ptr = ENOENT; + return 0; + } + + if (S_ISREG (st.st_mode)) + return 1; + + if (S_ISDIR (st.st_mode)) + *errno_ptr = EISDIR; + else + *errno_ptr = EINVAL; + return 0; +} + /* A vector holding all the fds open when notice_open_fds was called. We diff --git a/gdb/common/filestuff.h b/gdb/common/filestuff.h index 92a2a5f4c70..64a0b963308 100644 --- a/gdb/common/filestuff.h +++ b/gdb/common/filestuff.h @@ -19,6 +19,12 @@ #ifndef FILESTUFF_H #define FILESTUFF_H +/* Return True if the file NAME exists and is a regular file. + If the result is false then *ERRNO_PTR is set to a useful value assuming + we're expecting a regular file. */ + +extern int is_regular_file (const char *name, int *errno_ptr); + /* Note all the file descriptors which are open when this is called. These file descriptors will not be closed by close_most_fds. */ diff --git a/gdb/nat/fork-inferior.c b/gdb/nat/fork-inferior.c index 8b59387fa5a..4f089121fb9 100644 --- a/gdb/nat/fork-inferior.c +++ b/gdb/nat/fork-inferior.c @@ -27,6 +27,8 @@ #include "signals-state-save-restore.h" #include "gdb_tilde_expand.h" #include +#include "filenames.h" +#include "host-defs.h" extern char **environ; @@ -42,7 +44,7 @@ public: /* EXEC_FILE is the file to run. ALLARGS is a string containing the arguments to the program. If starting with a shell, SHELL_FILE is the shell to run. Otherwise, SHELL_FILE is NULL. */ - execv_argv (const char *exec_file, const std::string &allargs, + execv_argv (const std::string &exec_file, const std::string &allargs, const char *shell_file); /* Return a pointer to the built argv, in the type expected by @@ -63,12 +65,12 @@ private: /* Used when building an argv for a straight execv call, without going via the shell. */ - void init_for_no_shell (const char *exec_file, + void init_for_no_shell (const std::string &exec_file, const std::string &allargs); /* Used when building an argv for execing a shell that execs the child program. */ - void init_for_shell (const char *exec_file, + void init_for_shell (const std::string &exec_file, const std::string &allargs, const char *shell_file); @@ -93,7 +95,7 @@ private: M_STORAGE. */ void -execv_argv::init_for_no_shell (const char *exec_file, +execv_argv::init_for_no_shell (const std::string &exec_file, const std::string &allargs) { @@ -103,7 +105,7 @@ execv_argv::init_for_no_shell (const char *exec_file, allocations and string dups when 1 is sufficient. */ std::string &args_copy = m_storage = allargs; - m_argv.push_back (exec_file); + m_argv.push_back (exec_file.c_str ()); for (size_t cur_pos = 0; cur_pos < args_copy.size ();) { @@ -163,7 +165,7 @@ escape_bang_in_quoted_argument (const char *shell_file) /* See declaration. */ -execv_argv::execv_argv (const char *exec_file, +execv_argv::execv_argv (const std::string &exec_file, const std::string &allargs, const char *shell_file) { @@ -176,7 +178,7 @@ execv_argv::execv_argv (const char *exec_file, /* See declaration. */ void -execv_argv::init_for_shell (const char *exec_file, +execv_argv::init_for_shell (const std::string &exec_file, const std::string &allargs, const char *shell_file) { @@ -205,7 +207,7 @@ execv_argv::init_for_shell (const char *exec_file, on IRIX 4.0.1 can't deal with it. So we only quote it if we need to. */ bool need_to_quote; - const char *p = exec_file; + const char *p = exec_file.c_str(); while (1) { switch (*p) @@ -239,7 +241,7 @@ execv_argv::init_for_shell (const char *exec_file, if (need_to_quote) { shell_command += '\''; - for (p = exec_file; *p != '\0'; ++p) + for (p = exec_file.c_str (); *p != '\0'; ++p) { if (*p == '\'') shell_command += "'\\''"; @@ -295,7 +297,7 @@ fork_inferior (const char *exec_file_arg, const std::string &allargs, /* 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; + std::string exec_file; char **save_our_env; int i; int save_errno; @@ -309,6 +311,47 @@ fork_inferior (const char *exec_file_arg, const std::string &allargs, else exec_file = exec_file_arg; + /* Check if the user wants to set a different working directory for + the inferior. */ + inferior_cwd = get_inferior_cwd (); + + if (inferior_cwd != NULL) + { + /* Expand before forking because between fork and exec, the child + process may only execute async-signal-safe operations. */ + expanded_inferior_cwd = gdb_tilde_expand (inferior_cwd); + inferior_cwd = expanded_inferior_cwd.c_str (); + } + + /* If EXEC_FILE is a basename, and that executable can be found in + the current working directory, then assume this is the executable + we want to run (that is, do not search the PATH). This ensures + we preserve the traditional behavior where "gdb BASENAME_EXE" or + "gdbserver BASENAME_EXE", where BASENAME_EXE is the basename of + an executable, executes the BASENAME_EXE from the current working + directory, instead of either finding it on the PATH, or causing + an error because it could not be found in the PATH. */ + if (exec_file == lbasename (exec_file.c_str ())) + { + const char *base_dir = inferior_cwd; + if (base_dir == NULL) + base_dir = getcwd (NULL, 0); + if (base_dir != NULL) + { + std::string exec_fullpath (base_dir); + if (! IS_DIR_SEPARATOR (base_dir[strlen (base_dir) - 1])) + exec_fullpath.append(SLASH_STRING); + exec_fullpath.append(exec_file); + + int unused_errno; + if (is_regular_file (exec_fullpath.c_str(), &unused_errno)) + exec_file = exec_fullpath; + } + else + warning (_("Error finding name of working directory: %s"), + safe_strerror (errno)); + } + /* 'startup_with_shell' is declared in inferior.h and bound to the "set startup-with-shell" option. If 0, we'll just do a fork/exec, no shell, so don't bother figuring out what shell. */ @@ -342,18 +385,6 @@ fork_inferior (const char *exec_file_arg, const std::string &allargs, the parent and child flushing the same data after the fork. */ gdb_flush_out_err (); - /* Check if the user wants to set a different working directory for - the inferior. */ - inferior_cwd = get_inferior_cwd (); - - if (inferior_cwd != NULL) - { - /* Expand before forking because between fork and exec, the child - process may only execute async-signal-safe operations. */ - expanded_inferior_cwd = gdb_tilde_expand (inferior_cwd); - inferior_cwd = expanded_inferior_cwd.c_str (); - } - /* If there's any initialization of the target layers that must happen to prepare to handle the child we're about fork, do it now... */ diff --git a/gdb/source.c b/gdb/source.c index c6cb860fa64..e1281f978bb 100644 --- a/gdb/source.c +++ b/gdb/source.c @@ -669,39 +669,6 @@ info_source_command (const char *ignore, int from_tty) } -/* Return True if the file NAME exists and is a regular file. - If the result is false then *ERRNO_PTR is set to a useful value assuming - we're expecting a regular file. */ - -static int -is_regular_file (const char *name, int *errno_ptr) -{ - struct stat st; - const int status = stat (name, &st); - - /* Stat should never fail except when the file does not exist. - If stat fails, analyze the source of error and return True - unless the file does not exist, to avoid returning false results - on obscure systems where stat does not work as expected. */ - - if (status != 0) - { - if (errno != ENOENT) - return 1; - *errno_ptr = ENOENT; - return 0; - } - - if (S_ISREG (st.st_mode)) - return 1; - - if (S_ISDIR (st.st_mode)) - *errno_ptr = EISDIR; - else - *errno_ptr = EINVAL; - return 0; -} - /* Open a file named STRING, searching path PATH (dir names sep by some char) using mode MODE in the calls to open. You cannot use this function to create files (O_CREAT). -- 2.11.0