This is the mail archive of the
gdb-cvs@sourceware.org
mailing list for the GDB project.
[binutils-gdb] Make gdbserver work with filename-only binaries
- From: Sergio Durigan Junior <sergiodj at sourceware dot org>
- To: gdb-cvs at sourceware dot org
- Date: 28 Feb 2018 16:38:49 -0000
- Subject: [binutils-gdb] Make gdbserver work with filename-only binaries
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=25e3c82c0e927398e759e2d5e35623012b8683f7
commit 25e3c82c0e927398e759e2d5e35623012b8683f7
Author: Sergio Durigan Junior <sergiodj@redhat.com>
Date: Fri Feb 9 18:54:41 2018 -0500
Make gdbserver work with filename-only binaries
Simon mentioned on IRC that, after the startup-with-shell feature has
been implemented on gdbserver, it is not possible to specify a
filename-only binary, like:
$ gdbserver :1234 a.out
/bin/bash: line 0: exec: a.out: not found
During startup program exited with code 127.
Exiting
This happens on systems where the current directory "." is not listed
in the PATH environment variable. Although including "." in the PATH
variable is a possible workaround, this can be considered a regression
because before startup-with-shell it was possible to use only the
filename (due to reason that gdbserver used "exec*" directly).
The idea of the patch is to verify if the program path provided by the
user (or by the remote protocol) contains a directory separator
character. If it doesn't, it means we're dealing with a filename-only
binary, so we call "gdb_abspath" to properly expand it and transform
it into a full path. Otherwise, we leave the program path untouched.
This mimicks the behaviour seen on GDB (look at "openp" and
"attach_inferior", for example).
I am also submitting a testcase which exercises the scenario described
above. This test requires gdbserver to be executed in a different CWD
than the original, so I also created a helper function, "with_cwd" (on
testsuite/lib/gdb.exp), which takes care of cd'ing into and out of the
specified dir.
Built and regtested on BuildBot, without regressions.
gdb/ChangeLog:
2018-02-28 Sergio Durigan Junior <sergiodj@redhat.com>
Simon Marchi <simon.marchi@polymtl.ca>
* common/common-utils.c: Include "sys/stat.h".
(is_regular_file): Move here from "source.c"; change return
type to "bool".
* common/common-utils.h (is_regular_file): New prototype.
* common/pathstuff.c (contains_dir_separator): New function.
* common/pathstuff.h (contains_dir_separator): New prototype.
* source.c: Don't include "sys/stat.h".
(is_regular_file): Move to "common/common-utils.c".
gdb/gdbserver/ChangeLog:
2018-02-28 Sergio Durigan Junior <sergiodj@redhat.com>
* server.c: Include "filenames.h" and "pathstuff.h".
(program_name): Delete variable.
(program_path): New anonymous class.
(get_exec_wrapper): Use "program_path" instead of
"program_name".
(handle_v_run): Likewise.
(captured_main): Likewise.
(process_serial_event): Likewise.
gdb/testsuite/ChangeLog:
2018-02-28 Sergio Durigan Junior <sergiodj@redhat.com>
* gdb.server/abspath.exp: New file.
* lib/gdb.exp (with_cwd): New procedure.
Diff:
---
gdb/ChangeLog | 12 ++++++++
gdb/common/common-utils.c | 32 ++++++++++++++++++++
gdb/common/common-utils.h | 5 ++++
gdb/common/pathstuff.c | 14 +++++++++
gdb/common/pathstuff.h | 4 +++
gdb/gdbserver/ChangeLog | 11 +++++++
gdb/gdbserver/server.c | 53 ++++++++++++++++++++++++--------
gdb/source.c | 34 ---------------------
gdb/testsuite/ChangeLog | 5 ++++
gdb/testsuite/gdb.server/abspath.exp | 58 ++++++++++++++++++++++++++++++++++++
gdb/testsuite/lib/gdb.exp | 24 +++++++++++++++
11 files changed, 205 insertions(+), 47 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index f27a096..5926515 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,4 +1,16 @@
2018-02-28 Sergio Durigan Junior <sergiodj@redhat.com>
+ Simon Marchi <simon.marchi@polymtl.ca>
+
+ * common/common-utils.c: Include "sys/stat.h".
+ (is_regular_file): Move here from "source.c"; change return
+ type to "bool".
+ * common/common-utils.h (is_regular_file): New prototype.
+ * common/pathstuff.c (contains_dir_separator): New function.
+ * common/pathstuff.h (contains_dir_separator): New prototype.
+ * source.c: Don't include "sys/stat.h".
+ (is_regular_file): Move to "common/common-utils.c".
+
+2018-02-28 Sergio Durigan Junior <sergiodj@redhat.com>
* Makefile.in (COMMON_SFILES): Add "common/pathstuff.c".
(HFILES_NO_SRCDIR): Add "common/pathstuff.h".
diff --git a/gdb/common/common-utils.c b/gdb/common/common-utils.c
index ae2dd9d..80de826 100644
--- a/gdb/common/common-utils.c
+++ b/gdb/common/common-utils.c
@@ -20,6 +20,7 @@
#include "common-defs.h"
#include "common-utils.h"
#include "host-defs.h"
+#include <sys/stat.h>
#include <ctype.h>
/* The xmalloc() (libiberty.h) family of memory management routines.
@@ -408,3 +409,34 @@ stringify_argv (const std::vector<char *> &args)
return ret;
}
+
+/* See common/common-utils.h. */
+
+bool
+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 true;
+ *errno_ptr = ENOENT;
+ return false;
+ }
+
+ if (S_ISREG (st.st_mode))
+ return true;
+
+ if (S_ISDIR (st.st_mode))
+ *errno_ptr = EISDIR;
+ else
+ *errno_ptr = EINVAL;
+ return false;
+}
diff --git a/gdb/common/common-utils.h b/gdb/common/common-utils.h
index 2320318..5408c35 100644
--- a/gdb/common/common-utils.h
+++ b/gdb/common/common-utils.h
@@ -146,4 +146,9 @@ in_inclusive_range (T value, T low, T high)
return value >= low && value <= high;
}
+/* 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 bool is_regular_file (const char *name, int *errno_ptr);
+
#endif
diff --git a/gdb/common/pathstuff.c b/gdb/common/pathstuff.c
index 02f6e44..fc574dc 100644
--- a/gdb/common/pathstuff.c
+++ b/gdb/common/pathstuff.c
@@ -140,3 +140,17 @@ gdb_abspath (const char *path)
? "" : SLASH_STRING,
path, (char *) NULL));
}
+
+/* See common/pathstuff.h. */
+
+bool
+contains_dir_separator (const char *path)
+{
+ for (; *path != '\0'; path++)
+ {
+ if (IS_DIR_SEPARATOR (*path))
+ return true;
+ }
+
+ return false;
+}
diff --git a/gdb/common/pathstuff.h b/gdb/common/pathstuff.h
index 3cb02c8..9f26127 100644
--- a/gdb/common/pathstuff.h
+++ b/gdb/common/pathstuff.h
@@ -46,4 +46,8 @@ extern gdb::unique_xmalloc_ptr<char>
extern gdb::unique_xmalloc_ptr<char> gdb_abspath (const char *path);
+/* Return whether PATH contains a directory separator character. */
+
+extern bool contains_dir_separator (const char *path);
+
#endif /* PATHSTUFF_H */
diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
index f6b578b..8c3b02e 100644
--- a/gdb/gdbserver/ChangeLog
+++ b/gdb/gdbserver/ChangeLog
@@ -1,5 +1,16 @@
2018-02-28 Sergio Durigan Junior <sergiodj@redhat.com>
+ * server.c: Include "filenames.h" and "pathstuff.h".
+ (program_name): Delete variable.
+ (program_path): New anonymous class.
+ (get_exec_wrapper): Use "program_path" instead of
+ "program_name".
+ (handle_v_run): Likewise.
+ (captured_main): Likewise.
+ (process_serial_event): Likewise.
+
+2018-02-28 Sergio Durigan Junior <sergiodj@redhat.com>
+
* Makefile.in (SFILES): Add "$(srcdir)/common/pathstuff.c".
(OBJS): Add "pathstuff.o".
* server.c (current_directory): New global variable.
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index 922d526..7745027 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.c
@@ -39,6 +39,8 @@
#include "common-inferior.h"
#include "job-control.h"
#include "environ.h"
+#include "filenames.h"
+#include "pathstuff.h"
#include "common/selftest.h"
@@ -112,7 +114,35 @@ static int vCont_supported;
space randomization feature before starting an inferior. */
int disable_randomization = 1;
-static char *program_name = NULL;
+static struct {
+ /* Set the PROGRAM_PATH. Here we adjust the path of the provided
+ binary if needed. */
+ void set (gdb::unique_xmalloc_ptr<char> &&path)
+ {
+ m_path = std::move (path);
+
+ /* Make sure we're using the absolute path of the inferior when
+ creating it. */
+ if (!contains_dir_separator (m_path.get ()))
+ {
+ int reg_file_errno;
+
+ /* Check if the file is in our CWD. If it is, then we prefix
+ its name with CURRENT_DIRECTORY. Otherwise, we leave the
+ name as-is because we'll try searching for it in $PATH. */
+ if (is_regular_file (m_path.get (), ®_file_errno))
+ m_path = gdb_abspath (m_path.get ());
+ }
+ }
+
+ /* Return the PROGRAM_PATH. */
+ char *get ()
+ { return m_path.get (); }
+
+private:
+ /* The program name, adjusted if needed. */
+ gdb::unique_xmalloc_ptr<char> m_path;
+} program_path;
static std::vector<char *> program_args;
static std::string wrapper_argv;
@@ -269,10 +299,10 @@ get_exec_wrapper ()
char *
get_exec_file (int err)
{
- if (err && program_name == NULL)
+ if (err && program_path.get () == NULL)
error (_("No executable file specified."));
- return program_name;
+ return program_path.get ();
}
/* See server.h. */
@@ -3003,7 +3033,7 @@ handle_v_run (char *own_buf)
{
/* GDB didn't specify a program to run. Use the program from the
last run with the new argument list. */
- if (program_name == NULL)
+ if (program_path.get () == NULL)
{
write_enn (own_buf);
free_vector_argv (new_argv);
@@ -3011,16 +3041,13 @@ handle_v_run (char *own_buf)
}
}
else
- {
- xfree (program_name);
- program_name = new_program_name;
- }
+ program_path.set (gdb::unique_xmalloc_ptr<char> (new_program_name));
/* Free the old argv and install the new one. */
free_vector_argv (program_args);
program_args = new_argv;
- create_inferior (program_name, program_args);
+ create_inferior (program_path.get (), program_args);
if (last_status.kind == TARGET_WAITKIND_STOPPED)
{
@@ -3765,13 +3792,13 @@ captured_main (int argc, char *argv[])
int i, n;
n = argc - (next_arg - argv);
- program_name = xstrdup (next_arg[0]);
+ program_path.set (gdb::unique_xmalloc_ptr<char> (xstrdup (next_arg[0])));
for (i = 1; i < n; i++)
program_args.push_back (xstrdup (next_arg[i]));
program_args.push_back (NULL);
/* Wait till we are at first instruction in program. */
- create_inferior (program_name, program_args);
+ create_inferior (program_path.get (), program_args);
/* We are now (hopefully) stopped at the first instruction of
the target process. This assumes that the target process was
@@ -4288,9 +4315,9 @@ process_serial_event (void)
fprintf (stderr, "GDBserver restarting\n");
/* Wait till we are at 1st instruction in prog. */
- if (program_name != NULL)
+ if (program_path.get () != NULL)
{
- create_inferior (program_name, program_args);
+ create_inferior (program_path.get (), program_args);
if (last_status.kind == TARGET_WAITKIND_STOPPED)
{
diff --git a/gdb/source.c b/gdb/source.c
index 04ee3b3..6979fb2 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -29,7 +29,6 @@
#include "filestuff.h"
#include <sys/types.h>
-#include <sys/stat.h>
#include <fcntl.h>
#include "gdbcore.h"
#include "gdb_regex.h"
@@ -670,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).
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 59bbc6e..63c6263 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2018-02-28 Sergio Durigan Junior <sergiodj@redhat.com>
+
+ * gdb.server/abspath.exp: New file.
+ * lib/gdb.exp (with_cwd): New procedure.
+
2018-02-28 Simon Marchi <simon.marchi@ericsson.com>
* lib/gdb.exp (gdb_is_target_1): Add prompt_regexp parameter and
diff --git a/gdb/testsuite/gdb.server/abspath.exp b/gdb/testsuite/gdb.server/abspath.exp
new file mode 100644
index 0000000..726ff58
--- /dev/null
+++ b/gdb/testsuite/gdb.server/abspath.exp
@@ -0,0 +1,58 @@
+# This testcase is part of GDB, the GNU debugger.
+
+# Copyright 2018 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+# Test that gdbserver performs path expansion/adjustment when we
+# provide just a filename (without any path specifications) to it.
+
+load_lib gdbserver-support.exp
+
+standard_testfile normal.c
+
+if { [skip_gdbserver_tests] } {
+ return 0
+}
+
+# Because we're relying on being able to change our CWD before
+# executing gdbserver, we just run if we're not testing with a remote
+# target.
+if { [is_remote target] } {
+ return 0
+}
+
+if { [prepare_for_testing "failed to prepare" $testfile $srcfile debug] } {
+ return -1
+}
+
+# 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]
+
+# Switch to where $target_exec lives, and execute gdbserver from
+# there.
+with_cwd "[file dirname $target_exec]" {
+ set target_execname [file tail $target_exec]
+ set res [gdbserver_start "" $target_execname]
+
+ set gdbserver_protocol [lindex $res 0]
+ set gdbserver_gdbport [lindex $res 1]
+ gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport
+
+ gdb_breakpoint main
+ gdb_test "continue" "Breakpoint $decimal.* main.*" "continue to main"
+}
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 9102e54..4d48f5e 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -2054,6 +2054,30 @@ proc save_vars { vars body } {
}
}
+# Run tests in BODY with the current working directory (CWD) set to
+# DIR. When BODY is finished, restore the original CWD. Return the
+# result of BODY.
+#
+# This procedure doesn't check if DIR is a valid directory, so you
+# have to make sure of that.
+
+proc with_cwd { dir body } {
+ set saved_dir [pwd]
+ verbose -log "Switching to directory $dir (saved CWD: $saved_dir)."
+ cd $dir
+
+ set code [catch {uplevel 1 $body} result]
+
+ verbose -log "Switching back to $saved_dir."
+ cd $saved_dir
+
+ if {$code == 1} {
+ global errorInfo errorCode
+ return -code $code -errorinfo $errorInfo -errorcode $errorCode $result
+ } else {
+ return -code $code $result
+ }
+}
# Run tests in BODY with GDB prompt and variable $gdb_prompt set to
# PROMPT. When BODY is finished, restore GDB prompt and variable