[v2,4/5] Implement "set cwd" command
Commit Message
This is the actual implementation of the "set/show cwd" commands. The
way they work is:
- If the user sets the inferior's cwd by using "set cwd", then this
directory is saved into current_inferior ()->cwd and is used when
the inferior is started (see below).
- If the user doesn't set the inferior's cwd by using "set cwd", but
rather use the "cd" command as before, the this directory is
inherited by the inferior because GDB will have chdir'd into it.
The way the directory is changed before the inferior execution is by
calling "gdb_chdir" after the call to fork/vfork on "fork_inferior",
but before the actual execution. This way, we'll make sure that GDB's
cwd is not affected by the user set cwd.
gdb/ChangeLog:
yyyy-mm-dd Sergio Durigan Junior <sergiodj@redhat.com>
* NEWS (New commands): Mention "set/show cwd".
* cli/cli-cmds.c (_initialize_cli_cmds): Mention "set cwd" on
"cd" command's help text.
* common/common-inferior.h (get_inferior_cwd): New prototype.
* infcmd.c: Include "gdb_chdir.h" and "readline/tilde.h".
(inferior_cwd_scratch): New global variable.
(set_inferior_cwd): New function.
(get_inferior_cwd): Likewise.
(set_cwd_command): Likewise.
(show_cwd_command): Likewise.
(_initialize_infcmd): Add "set/show cwd" commands.
* inferior.h (class inferior) <cwd>: New field.
* nat/fork-inferior.c: Include "gdb_chdir.h".
(fork_inferior): Change inferior's cwd before its execution.
* windows-nat.c (windows_create_inferior): Pass inferior's cwd
to CreateProcess.
gdb/gdbserver/ChangeLog:
yyyy-mm-dd Sergio Durigan Junior <sergiodj@redhat.com>
* inferiors.c (get_inferior_cwd): New function.
* inferiors.h (struct process_info) <cwd>: New field.
gdb/doc/ChangeLog:
yyyy-mm-dd Sergio Durigan Junior <sergiodj@redhat.com>
* gdb.texinfo (Starting your Program) <The working directory.>:
Mention new "set cwd" command.
(Working Directory) <Your Program's Working Directory>:
Rephrase to explain that "set cwd" exists and is the default
way to change the inferior's cwd.
gdb/testsuite/ChangeLog:
yyyy-mm-dd Sergio Durigan Junior <sergiodj@redhat.com>
* gdb.base/set-cwd.c: New file.
* gdb.base/set-cwd.exp: Likewise.
---
gdb/NEWS | 3 ++
gdb/cli/cli-cmds.c | 6 +--
gdb/common/common-inferior.h | 4 ++
gdb/doc/gdb.texinfo | 34 ++++++++++----
gdb/gdbserver/inferiors.c | 11 +++++
gdb/infcmd.c | 75 +++++++++++++++++++++++++++++++
gdb/inferior.h | 4 ++
gdb/nat/fork-inferior.c | 17 +++++++
gdb/testsuite/gdb.base/set-cwd.c | 29 ++++++++++++
gdb/testsuite/gdb.base/set-cwd.exp | 90 ++++++++++++++++++++++++++++++++++++++
gdb/windows-nat.c | 5 ++-
11 files changed, 265 insertions(+), 13 deletions(-)
create mode 100644 gdb/testsuite/gdb.base/set-cwd.c
create mode 100644 gdb/testsuite/gdb.base/set-cwd.exp
Comments
On 09/19/2017 05:28 AM, Sergio Durigan Junior wrote:
> diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
> index 653dd56a64..f086f10aea 100644
> --- a/gdb/cli/cli-cmds.c
> +++ b/gdb/cli/cli-cmds.c
> @@ -1716,9 +1716,9 @@ The commands below can be used to select other frames by number or address."),
> Print working directory. This is used for your program as well."));
>
> c = add_cmd ("cd", class_files, cd_command, _("\
> -Set working directory to DIR for debugger and program being debugged.\n\
> -The change does not take effect for the program being debugged\n\
> -until the next time it is started."), &cmdlist);
> +Set working directory to DIR for debugger.\n\
I think it'd be nice to add a sentence here suggesting what the
current working directory affects.
> +In order to change the inferior's current working directory, the recommended\n\
> +way is to use the \"set cwd\" command."), &cmdlist);
> set_cmd_completer (c, filename_completer);
>
> add_com ("echo", class_support, echo_command, _("\
> diff --git a/gdb/common/common-inferior.h b/gdb/common/common-inferior.h
> index 87c13009ed..515a8c0f4e 100644
> --- a/gdb/common/common-inferior.h
> +++ b/gdb/common/common-inferior.h
> @@ -30,4 +30,8 @@ extern const char *get_exec_wrapper ();
> otherwise return 0 in that case. */
> extern char *get_exec_file (int err);
>
> +/* Return the inferior's current working directory. If nothing has
> + been set, then return NULL. */
> +extern const char *get_inferior_cwd ();
> +
> #endif /* ! COMMON_INFERIOR_H */
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index af5fe6f46c..c513a49c26 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -2057,8 +2057,9 @@ environment} to change parts of the environment that affect
> your program. @xref{Environment, ,Your Program's Environment}.
>
> @item The @emph{working directory.}
> -Your program inherits its working directory from @value{GDBN}. You can set
> -the @value{GDBN} working directory with the @code{cd} command in @value{GDBN}.
> +You can set your program's working directory with the command
> +@code{set cwd}. If you do not set any working directory with this
> +command, your program will inherit @value{GDBN}'s working directory.
> @xref{Working Directory, ,Your Program's Working Directory}.
>
> @item The @emph{standard input and output.}
> @@ -2424,19 +2425,36 @@ variables to files that are only run when you sign on, such as
> @section Your Program's Working Directory
>
> @cindex working directory (of your program)
> -Each time you start your program with @code{run}, it inherits its
> -working directory from the current working directory of @value{GDBN}.
> -The @value{GDBN} working directory is initially whatever it inherited
> -from its parent process (typically the shell), but you can specify a new
> -working directory in @value{GDBN} with the @code{cd} command.
> +Each time you start your program with @code{run}, the inferior will be
> +initialized with the current working directory specified by the
> +@code{set cwd} command. If no directory has been specified by this
> +command, then the inferior will inherit @value{GDBN}'s current working
> +directory as its working directory.
> +
> +You can also change @value{GDBN}'s current working directory by using
> +the @code{cd} command.
>
> The @value{GDBN} working directory also serves as a default for the commands
> that specify files for @value{GDBN} to operate on. @xref{Files, ,Commands to
> Specify Files}.
>
> @table @code
> +@kindex set cwd
> +@cindex change inferior's working directory
> +@item set cwd @r{[}@var{directory}@r{]}
> +Set the inferior's working directory to @var{directory}. If not
> +given, @var{directory} uses @file{'~'}. This has no effect on
> +@value{GDBN}'s working directory.
I think we're missing a sentence somewhere in the manual saying that
this setting only takes effect the next time you start the program,
like it is said in "help cd" currently.
> +
> +@kindex show cwd
> +@cindex show inferior's working directory
> +@item show cwd
> +Show the inferior's working directory. If no directory has been
> +specified by @code{set cwd}, then the default inferior's working
> +directory is the same as @value{GDBN}'s working directory.
> +
> @kindex cd
> -@cindex change working directory
> +@cindex change @value{GDBN}'s working directory
> @item cd @r{[}@var{directory}@r{]}
> Set the @value{GDBN} working directory to @var{directory}. If not
> given, @var{directory} uses @file{'~'}.
> diff --git a/gdb/gdbserver/inferiors.c b/gdb/gdbserver/inferiors.c
> index 72f0412757..e78ad4faf1 100644
> --- a/gdb/gdbserver/inferiors.c
> +++ b/gdb/gdbserver/inferiors.c
> @@ -29,6 +29,9 @@ struct thread_info *current_thread;
>
> #define get_thread(inf) ((struct thread_info *)(inf))
>
> +/* The current working directory used to start the inferior. */
> +static const char *current_inferior_cwd = NULL;
> +
> void
> add_inferior_to_list (struct inferior_list *list,
> struct inferior_list_entry *new_inferior)
> @@ -445,3 +448,11 @@ switch_to_thread (ptid_t ptid)
> if (!ptid_equal (ptid, minus_one_ptid))
> current_thread = find_thread_ptid (ptid);
> }
> +
> +/* See common/common-inferior.h. */
> +
> +const char *
> +get_inferior_cwd ()
> +{
> + return current_inferior_cwd;
> +}
> diff --git a/gdb/infcmd.c b/gdb/infcmd.c
> index 25cf025426..178a3ca15f 100644
> --- a/gdb/infcmd.c
> +++ b/gdb/infcmd.c
> @@ -59,6 +59,8 @@
> #include "top.h"
> #include "interps.h"
> #include "common/gdb_optional.h"
> +#include "gdb_chdir.h"
> +#include "readline/tilde.h"
>
> /* Local functions: */
>
> @@ -111,6 +113,10 @@ static void run_command (char *, int);
>
> static char *inferior_args_scratch;
>
> +/* Scratch area where the new cwd will be stored by 'set cwd'. */
> +
> +static char *inferior_cwd_scratch;
> +
> /* Scratch area where 'set inferior-tty' will store user-provided value.
> We'll immediate copy it into per-inferior storage. */
>
> @@ -246,6 +252,56 @@ show_args_command (struct ui_file *file, int from_tty,
> deprecated_show_value_hack (file, from_tty, c, get_inferior_args ());
> }
>
> +/* Set the inferior current working directory. This directory will be
> + entered by GDB before executing the inferior. */
The second sentence is talking about implementation details of Unix-like
targets, which doesn't really belong here.
> +
> +static void
> +set_inferior_cwd (const char *cwd)
> +{
> + struct inferior *inf = current_inferior ();
> +
> + gdb_assert (inf != NULL);
> + xfree ((void *) inf->cwd);
> + inf->cwd = tilde_expand (*cwd != '\0' ? cwd : "~");
So the inferior owns the string? Wouldn't it be better if
inf->cwd were a unique_ptr? I don't see anywhere releasing
the cwd string when an inferior is deleted.
> +
> +/* Handle the 'show cwd' command. */
> +
> +static void
> +show_cwd_command (struct ui_file *file, int from_tty,
> + struct cmd_list_element *c, const char *value)
> +{
> + const char *cwd = get_inferior_cwd ();
> +
> + if (cwd == NULL)
> + {
> + /* To maintain backwards compatibility, we use
> + 'current_directory' here, which is set by the "cd"
> + command. */
> + cwd = current_directory;
> + }
This doesn't look like the right thing to show, to me.
It'll only make sense for native debugging, not remote debugging,
right? I mean, when remote debugging, the inferior won't inherit
gdb's cwd. Right?
> +
> + fprintf_filtered (gdb_stdout,
> + _("Current working directory that will be used "
> + "when starting the inferior is \"%s\".\n"), cwd);
> +}
> +
>
> index 6d020f73c4..bcd1e54a03 100644
> --- a/gdb/inferior.h
> +++ b/gdb/inferior.h
> @@ -355,6 +355,10 @@ public:
> should never be freed. */
> char **argv = NULL;
>
> + /* The current working directory that will be used when starting
> + this inferior. */
> + const char *cwd = NULL;
See comment above in set_inferior_cwd. Who owns this string?
Looks like removing the inferior leaks this?
Can this be a unique_xmalloc_ptr ?
> @@ -376,6 +377,22 @@ fork_inferior (const char *exec_file_arg, const std::string &allargs,
> UIs. */
> close_most_fds ();
>
> + const char *cwd = get_inferior_cwd ();
> +
> + if (cwd != NULL)
> + {
> + TRY
> + {
> + gdb_chdir (cwd);
> + }
> + CATCH (ex, RETURN_MASK_ERROR)
> + {
> + warning ("%s", ex.message);
> + _exit (0177);
> + }
> + END_CATCH
> + }
This is the fork-child path, and as such we should only be calling
async-signal-safe code. And throwing C++ exceptions is not async-signal-safe.
I think an easy solution is to call gdb_tilde_expand _before_ forking, and
then call plain old chdir in the child, and then call
trace_start_error_with_name instead of perror_with_name on failure:
if (chdir (expanded_dir.c_str ()) < 0)
trace_start_error_with_name (expanded_dir.c_str ());
With that, I'm not sure whether gdb_chdir is still very useful
compared to using gdb_tilde_expand + chdir in the "cd" command too.
> +++ b/gdb/testsuite/gdb.base/set-cwd.c
> @@ -0,0 +1,29 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> + Copyright 2017 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/>. */
> +
> +#include <stdio.h>
> +#include <unistd.h>
> +
> +int
> +main (int argc, char *argv[])
> +{
> + char dir[BUFSIZ];
> +
> + getcwd (dir, BUFSIZ);
Hmm, BUFSIZ came back? :-) Please use something like PATH_MAX
with a fallback to 1024 or some such.
> +
> + return 0; /* break-here */
> +}
> diff --git a/gdb/testsuite/gdb.base/set-cwd.exp b/gdb/testsuite/gdb.base/set-cwd.exp
> new file mode 100644
> index 0000000000..3a6ffd3862
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/set-cwd.exp
> @@ -0,0 +1,90 @@
> +# This testcase is part of GDB, the GNU debugger.
> +
> +# Copyright 2017 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/>.
> +
> +if { ![isnative] || [is_remote target] || [is_remote host]
> + || [target_info gdb_protocol] == "extended-remote" } then {
> + untested "not implemented on gdbserver"
The skip on [is_remote host] surely has nothing to do with gdbserver, right?
Please split that up to a separate untested call with a meaningful gdb.sum
output message. The rest of the checks don't look exactly right,
but I'll ignore that because surely you'll be changing it in the following
patch.
> + return
> +}
> +
> +standard_testfile
> +
> +if { [prepare_for_testing "failed to prepare" $testfile $srcfile debug] } {
> + return -1
> +}
> +
> +# Test that tilde expansion works fine.
> +
> +proc test_tilde_expansion { } {
> + if { [info exists ::env(HOME)] } {
> + with_test_prefix "test tilde expansion" {
> + set home $::env(HOME)
> +
> + gdb_test_no_output "set cwd ~/test" "set cwd to ~/test dir"
> +
> + gdb_test "show cwd" \
> + "Current working directory that will be used when starting the inferior is \"${home}/test\"\." \
> + "show cwd shows expanded tilde"
> + }
> + }
> +}
> +
> +# Test that when we "set cwd" the inferior will be started under the
> +# correct working directory and GDB will not be affected by this.
> +
> +proc test_cd_into_dir { } {
> + global decimal gdb_prompt
> +
> + with_test_prefix "test cd into temp dir" {
> + gdb_test_multiple "pwd" "pwd before run" {
> + -re "Working directory \(.*\)\.\r\n$gdb_prompt $" {
> + set gdb_cwd_before_run $expect_out(1,string)
> + }
> + }
Is the above fails, then gdb_cwd_before_run is left unset,
and the next reference causes a TCL error.
> +
> + set tmpdir [standard_output_file ""]
> +
> + gdb_test_no_output "set cwd $tmpdir" "set cwd to temp dir"
> +
> + if { ![runto_main] } {
> + untested "could not run to main"
> + return -1
> + }
> +
> + gdb_breakpoint [gdb_get_line_number "break-here"]
> + gdb_continue_to_breakpoint "break-here" ".* break-here .*"
> +
> + gdb_test "print dir" "\\\$$decimal = \"$tmpdir\", .*" \
> + "inferior cwd is correctly set"
> +
> + gdb_test_multiple "pwd" "pwd after run" {
> + -re "Working directory \(.*\)\.\r\n$gdb_prompt $" {
> + set gdb_cwd_after_run $expect_out(1,string)
> + }
> + }
Likewise.
> +
> + set test "GDB cwd is unchanged after running inferior"
> + if { [string equal $gdb_cwd_before_run $gdb_cwd_after_run] } {
> + pass $test
> + } else {
> + fail $test
> + }
You can use gdb_assert instead of this if/else pass/fail.
Thanks,
Pedro Alves
On Wednesday, September 20 2017, Pedro Alves wrote:
> On 09/19/2017 05:28 AM, Sergio Durigan Junior wrote:
>
>> diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
>> index 653dd56a64..f086f10aea 100644
>> --- a/gdb/cli/cli-cmds.c
>> +++ b/gdb/cli/cli-cmds.c
>> @@ -1716,9 +1716,9 @@ The commands below can be used to select other frames by number or address."),
>> Print working directory. This is used for your program as well."));
>>
>> c = add_cmd ("cd", class_files, cd_command, _("\
>> -Set working directory to DIR for debugger and program being debugged.\n\
>> -The change does not take effect for the program being debugged\n\
>> -until the next time it is started."), &cmdlist);
>> +Set working directory to DIR for debugger.\n\
>
>
> I think it'd be nice to add a sentence here suggesting what the
> current working directory affects.
Done. Here's the new text:
c = add_cmd ("cd", class_files, cd_command, _("\
Set working directory to DIR for debugger.\n\
The debugger's current working directory specifies where scripts and other\n\
files that can be loaded by GDB are located.\n\
In order to change the inferior's current working directory, the recommended\n\
way is to use the \"set cwd\" command."), &cmdlist);
>> +In order to change the inferior's current working directory, the recommended\n\
>> +way is to use the \"set cwd\" command."), &cmdlist);
>> set_cmd_completer (c, filename_completer);
>>
>> add_com ("echo", class_support, echo_command, _("\
>> diff --git a/gdb/common/common-inferior.h b/gdb/common/common-inferior.h
>> index 87c13009ed..515a8c0f4e 100644
>> --- a/gdb/common/common-inferior.h
>> +++ b/gdb/common/common-inferior.h
>> @@ -30,4 +30,8 @@ extern const char *get_exec_wrapper ();
>> otherwise return 0 in that case. */
>> extern char *get_exec_file (int err);
>>
>> +/* Return the inferior's current working directory. If nothing has
>> + been set, then return NULL. */
>> +extern const char *get_inferior_cwd ();
>> +
>> #endif /* ! COMMON_INFERIOR_H */
>> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
>> index af5fe6f46c..c513a49c26 100644
>> --- a/gdb/doc/gdb.texinfo
>> +++ b/gdb/doc/gdb.texinfo
>> @@ -2057,8 +2057,9 @@ environment} to change parts of the environment that affect
>> your program. @xref{Environment, ,Your Program's Environment}.
>>
>> @item The @emph{working directory.}
>> -Your program inherits its working directory from @value{GDBN}. You can set
>> -the @value{GDBN} working directory with the @code{cd} command in @value{GDBN}.
>> +You can set your program's working directory with the command
>> +@code{set cwd}. If you do not set any working directory with this
>> +command, your program will inherit @value{GDBN}'s working directory.
>> @xref{Working Directory, ,Your Program's Working Directory}.
>>
>> @item The @emph{standard input and output.}
>> @@ -2424,19 +2425,36 @@ variables to files that are only run when you sign on, such as
>> @section Your Program's Working Directory
>>
>> @cindex working directory (of your program)
>> -Each time you start your program with @code{run}, it inherits its
>> -working directory from the current working directory of @value{GDBN}.
>> -The @value{GDBN} working directory is initially whatever it inherited
>> -from its parent process (typically the shell), but you can specify a new
>> -working directory in @value{GDBN} with the @code{cd} command.
>> +Each time you start your program with @code{run}, the inferior will be
>> +initialized with the current working directory specified by the
>> +@code{set cwd} command. If no directory has been specified by this
>> +command, then the inferior will inherit @value{GDBN}'s current working
>> +directory as its working directory.
>> +
>> +You can also change @value{GDBN}'s current working directory by using
>> +the @code{cd} command.
>>
>> The @value{GDBN} working directory also serves as a default for the commands
>> that specify files for @value{GDBN} to operate on. @xref{Files, ,Commands to
>> Specify Files}.
>>
>> @table @code
>> +@kindex set cwd
>> +@cindex change inferior's working directory
>> +@item set cwd @r{[}@var{directory}@r{]}
>> +Set the inferior's working directory to @var{directory}. If not
>> +given, @var{directory} uses @file{'~'}. This has no effect on
>> +@value{GDBN}'s working directory.
>
>
> I think we're missing a sentence somewhere in the manual saying that
> this setting only takes effect the next time you start the program,
> like it is said in "help cd" currently.
Here's what the new text looks like:
@table @code
@kindex set cwd
@cindex change inferior's working directory
@item set cwd @r{[}@var{directory}@r{]}
Set the inferior's working directory to @var{directory}. If not
given, @var{directory} uses @file{'~'}. This has no effect on
@value{GDBN}'s working directory. This setting only takes effect the
next time you start the inferior.
>> +
>> +@kindex show cwd
>> +@cindex show inferior's working directory
>> +@item show cwd
>> +Show the inferior's working directory. If no directory has been
>> +specified by @code{set cwd}, then the default inferior's working
>> +directory is the same as @value{GDBN}'s working directory.
>> +
>> @kindex cd
>> -@cindex change working directory
>> +@cindex change @value{GDBN}'s working directory
>> @item cd @r{[}@var{directory}@r{]}
>> Set the @value{GDBN} working directory to @var{directory}. If not
>> given, @var{directory} uses @file{'~'}.
>> diff --git a/gdb/gdbserver/inferiors.c b/gdb/gdbserver/inferiors.c
>> index 72f0412757..e78ad4faf1 100644
>> --- a/gdb/gdbserver/inferiors.c
>> +++ b/gdb/gdbserver/inferiors.c
>> @@ -29,6 +29,9 @@ struct thread_info *current_thread;
>>
>> #define get_thread(inf) ((struct thread_info *)(inf))
>>
>> +/* The current working directory used to start the inferior. */
>> +static const char *current_inferior_cwd = NULL;
>> +
>> void
>> add_inferior_to_list (struct inferior_list *list,
>> struct inferior_list_entry *new_inferior)
>> @@ -445,3 +448,11 @@ switch_to_thread (ptid_t ptid)
>> if (!ptid_equal (ptid, minus_one_ptid))
>> current_thread = find_thread_ptid (ptid);
>> }
>> +
>> +/* See common/common-inferior.h. */
>> +
>> +const char *
>> +get_inferior_cwd ()
>> +{
>> + return current_inferior_cwd;
>> +}
>> diff --git a/gdb/infcmd.c b/gdb/infcmd.c
>> index 25cf025426..178a3ca15f 100644
>> --- a/gdb/infcmd.c
>> +++ b/gdb/infcmd.c
>> @@ -59,6 +59,8 @@
>> #include "top.h"
>> #include "interps.h"
>> #include "common/gdb_optional.h"
>> +#include "gdb_chdir.h"
>> +#include "readline/tilde.h"
>>
>> /* Local functions: */
>>
>> @@ -111,6 +113,10 @@ static void run_command (char *, int);
>>
>> static char *inferior_args_scratch;
>>
>> +/* Scratch area where the new cwd will be stored by 'set cwd'. */
>> +
>> +static char *inferior_cwd_scratch;
>> +
>> /* Scratch area where 'set inferior-tty' will store user-provided value.
>> We'll immediate copy it into per-inferior storage. */
>>
>> @@ -246,6 +252,56 @@ show_args_command (struct ui_file *file, int from_tty,
>> deprecated_show_value_hack (file, from_tty, c, get_inferior_args ());
>> }
>>
>> +/* Set the inferior current working directory. This directory will be
>> + entered by GDB before executing the inferior. */
>
> The second sentence is talking about implementation details of Unix-like
> targets, which doesn't really belong here.
True. I will remove it.
>> +
>> +static void
>> +set_inferior_cwd (const char *cwd)
>> +{
>> + struct inferior *inf = current_inferior ();
>> +
>> + gdb_assert (inf != NULL);
>> + xfree ((void *) inf->cwd);
>> + inf->cwd = tilde_expand (*cwd != '\0' ? cwd : "~");
>
> So the inferior owns the string? Wouldn't it be better if
> inf->cwd were a unique_ptr? I don't see anywhere releasing
> the cwd string when an inferior is deleted.
Right, the inferior owns the string. And indeed, there's no place
free'ing the string when the inferior goes away. I think the unique_ptr
idea is good, I'll implement it.
>> +
>> +/* Handle the 'show cwd' command. */
>> +
>> +static void
>> +show_cwd_command (struct ui_file *file, int from_tty,
>> + struct cmd_list_element *c, const char *value)
>> +{
>> + const char *cwd = get_inferior_cwd ();
>> +
>> + if (cwd == NULL)
>> + {
>> + /* To maintain backwards compatibility, we use
>> + 'current_directory' here, which is set by the "cd"
>> + command. */
>> + cwd = current_directory;
>> + }
>
> This doesn't look like the right thing to show, to me.
> It'll only make sense for native debugging, not remote debugging,
> right? I mean, when remote debugging, the inferior won't inherit
> gdb's cwd. Right?
That's right. Maybe a better text for when cwd == NULL would be:
You have not set the inferior's current working directory. The
inferior will inherit GDB's cwd if native debugging, or gdbserver's
cwd if remote debugging.
WDYT?
>> +
>> + fprintf_filtered (gdb_stdout,
>> + _("Current working directory that will be used "
>> + "when starting the inferior is \"%s\".\n"), cwd);
>> +}
>> +
>>
>
>> index 6d020f73c4..bcd1e54a03 100644
>> --- a/gdb/inferior.h
>> +++ b/gdb/inferior.h
>> @@ -355,6 +355,10 @@ public:
>> should never be freed. */
>> char **argv = NULL;
>>
>> + /* The current working directory that will be used when starting
>> + this inferior. */
>> + const char *cwd = NULL;
>
> See comment above in set_inferior_cwd. Who owns this string?
> Looks like removing the inferior leaks this?
> Can this be a unique_xmalloc_ptr ?
As explained above, I'll implement it as a unique_ptr.
>
>> @@ -376,6 +377,22 @@ fork_inferior (const char *exec_file_arg, const std::string &allargs,
>> UIs. */
>> close_most_fds ();
>>
>> + const char *cwd = get_inferior_cwd ();
>> +
>> + if (cwd != NULL)
>> + {
>> + TRY
>> + {
>> + gdb_chdir (cwd);
>> + }
>> + CATCH (ex, RETURN_MASK_ERROR)
>> + {
>> + warning ("%s", ex.message);
>> + _exit (0177);
>> + }
>> + END_CATCH
>> + }
>
> This is the fork-child path, and as such we should only be calling
> async-signal-safe code. And throwing C++ exceptions is not async-signal-safe.
> I think an easy solution is to call gdb_tilde_expand _before_ forking, and
> then call plain old chdir in the child, and then call
> trace_start_error_with_name instead of perror_with_name on failure:
>
> if (chdir (expanded_dir.c_str ()) < 0)
> trace_start_error_with_name (expanded_dir.c_str ());
>
> With that, I'm not sure whether gdb_chdir is still very useful
> compared to using gdb_tilde_expand + chdir in the "cd" command too.
Done.
>> +++ b/gdb/testsuite/gdb.base/set-cwd.c
>> @@ -0,0 +1,29 @@
>> +/* This testcase is part of GDB, the GNU debugger.
>> +
>> + Copyright 2017 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/>. */
>> +
>> +#include <stdio.h>
>> +#include <unistd.h>
>> +
>> +int
>> +main (int argc, char *argv[])
>> +{
>> + char dir[BUFSIZ];
>> +
>> + getcwd (dir, BUFSIZ);
>
> Hmm, BUFSIZ came back? :-) Please use something like PATH_MAX
> with a fallback to 1024 or some such.
Hm, right, I forgot to remove it from the testcase. I'll use 4096 (the
default for PATH_MAX), it should be enough.
>> +
>> + return 0; /* break-here */
>> +}
>> diff --git a/gdb/testsuite/gdb.base/set-cwd.exp b/gdb/testsuite/gdb.base/set-cwd.exp
>> new file mode 100644
>> index 0000000000..3a6ffd3862
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.base/set-cwd.exp
>> @@ -0,0 +1,90 @@
>> +# This testcase is part of GDB, the GNU debugger.
>> +
>> +# Copyright 2017 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/>.
>> +
>> +if { ![isnative] || [is_remote target] || [is_remote host]
>> + || [target_info gdb_protocol] == "extended-remote" } then {
>> + untested "not implemented on gdbserver"
>
> The skip on [is_remote host] surely has nothing to do with gdbserver, right?
I think I was trying to be "overzealous" here, but "[is_remote host]"
would only make sense if this test was being run on gdbserver, right?
Having said that, I don't think it's correct to check for that here.
> Please split that up to a separate untested call with a meaningful gdb.sum
> output message. The rest of the checks don't look exactly right,
> but I'll ignore that because surely you'll be changing it in the following
> patch.
What I really want to do here is to disable this test on remote
debugging. Sometimes I get the whole "target/host/native/remote" thing
right, sometimes I get confused about it.
Given your comment on patch #5, I guess I should be checking only for
[use_gdb_stub] and go from that. I'll make a few tests here to see what
works for my case.
>
>> + return
>
>
>> +}
>> +
>> +standard_testfile
>> +
>> +if { [prepare_for_testing "failed to prepare" $testfile $srcfile debug] } {
>> + return -1
>> +}
>> +
>> +# Test that tilde expansion works fine.
>> +
>> +proc test_tilde_expansion { } {
>> + if { [info exists ::env(HOME)] } {
>> + with_test_prefix "test tilde expansion" {
>> + set home $::env(HOME)
>> +
>> + gdb_test_no_output "set cwd ~/test" "set cwd to ~/test dir"
>> +
>> + gdb_test "show cwd" \
>> + "Current working directory that will be used when starting the inferior is \"${home}/test\"\." \
>> + "show cwd shows expanded tilde"
>> + }
>> + }
>> +}
>> +
>> +# Test that when we "set cwd" the inferior will be started under the
>> +# correct working directory and GDB will not be affected by this.
>> +
>> +proc test_cd_into_dir { } {
>> + global decimal gdb_prompt
>> +
>> + with_test_prefix "test cd into temp dir" {
>> + gdb_test_multiple "pwd" "pwd before run" {
>> + -re "Working directory \(.*\)\.\r\n$gdb_prompt $" {
>> + set gdb_cwd_before_run $expect_out(1,string)
>> + }
>> + }
>
> Is the above fails, then gdb_cwd_before_run is left unset,
> and the next reference causes a TCL error.
I've added a catch-all case after the first one, which fails and returns
-1.
>> +
>> + set tmpdir [standard_output_file ""]
>> +
>> + gdb_test_no_output "set cwd $tmpdir" "set cwd to temp dir"
>> +
>> + if { ![runto_main] } {
>> + untested "could not run to main"
>> + return -1
>> + }
>> +
>> + gdb_breakpoint [gdb_get_line_number "break-here"]
>> + gdb_continue_to_breakpoint "break-here" ".* break-here .*"
>> +
>> + gdb_test "print dir" "\\\$$decimal = \"$tmpdir\", .*" \
>> + "inferior cwd is correctly set"
>> +
>> + gdb_test_multiple "pwd" "pwd after run" {
>> + -re "Working directory \(.*\)\.\r\n$gdb_prompt $" {
>> + set gdb_cwd_after_run $expect_out(1,string)
>> + }
>> + }
>
> Likewise.
Likewise my comment above.
>
>> +
>> + set test "GDB cwd is unchanged after running inferior"
>> + if { [string equal $gdb_cwd_before_run $gdb_cwd_after_run] } {
>> + pass $test
>> + } else {
>> + fail $test
>> + }
>
> You can use gdb_assert instead of this if/else pass/fail.
Hm, good point. Done.
Thanks,
@@ -64,6 +64,9 @@ QStartupWithShell
* New commands
+set|show cwd
+ Set and show the current working directory for the inferior.
+
set|show compile-gcc
Set and show compilation command used for compiling and injecting code
with the 'compile' commands.
@@ -1716,9 +1716,9 @@ The commands below can be used to select other frames by number or address."),
Print working directory. This is used for your program as well."));
c = add_cmd ("cd", class_files, cd_command, _("\
-Set working directory to DIR for debugger and program being debugged.\n\
-The change does not take effect for the program being debugged\n\
-until the next time it is started."), &cmdlist);
+Set working directory to DIR for debugger.\n\
+In order to change the inferior's current working directory, the recommended\n\
+way is to use the \"set cwd\" command."), &cmdlist);
set_cmd_completer (c, filename_completer);
add_com ("echo", class_support, echo_command, _("\
@@ -30,4 +30,8 @@ extern const char *get_exec_wrapper ();
otherwise return 0 in that case. */
extern char *get_exec_file (int err);
+/* Return the inferior's current working directory. If nothing has
+ been set, then return NULL. */
+extern const char *get_inferior_cwd ();
+
#endif /* ! COMMON_INFERIOR_H */
@@ -2057,8 +2057,9 @@ environment} to change parts of the environment that affect
your program. @xref{Environment, ,Your Program's Environment}.
@item The @emph{working directory.}
-Your program inherits its working directory from @value{GDBN}. You can set
-the @value{GDBN} working directory with the @code{cd} command in @value{GDBN}.
+You can set your program's working directory with the command
+@code{set cwd}. If you do not set any working directory with this
+command, your program will inherit @value{GDBN}'s working directory.
@xref{Working Directory, ,Your Program's Working Directory}.
@item The @emph{standard input and output.}
@@ -2424,19 +2425,36 @@ variables to files that are only run when you sign on, such as
@section Your Program's Working Directory
@cindex working directory (of your program)
-Each time you start your program with @code{run}, it inherits its
-working directory from the current working directory of @value{GDBN}.
-The @value{GDBN} working directory is initially whatever it inherited
-from its parent process (typically the shell), but you can specify a new
-working directory in @value{GDBN} with the @code{cd} command.
+Each time you start your program with @code{run}, the inferior will be
+initialized with the current working directory specified by the
+@code{set cwd} command. If no directory has been specified by this
+command, then the inferior will inherit @value{GDBN}'s current working
+directory as its working directory.
+
+You can also change @value{GDBN}'s current working directory by using
+the @code{cd} command.
The @value{GDBN} working directory also serves as a default for the commands
that specify files for @value{GDBN} to operate on. @xref{Files, ,Commands to
Specify Files}.
@table @code
+@kindex set cwd
+@cindex change inferior's working directory
+@item set cwd @r{[}@var{directory}@r{]}
+Set the inferior's working directory to @var{directory}. If not
+given, @var{directory} uses @file{'~'}. This has no effect on
+@value{GDBN}'s working directory.
+
+@kindex show cwd
+@cindex show inferior's working directory
+@item show cwd
+Show the inferior's working directory. If no directory has been
+specified by @code{set cwd}, then the default inferior's working
+directory is the same as @value{GDBN}'s working directory.
+
@kindex cd
-@cindex change working directory
+@cindex change @value{GDBN}'s working directory
@item cd @r{[}@var{directory}@r{]}
Set the @value{GDBN} working directory to @var{directory}. If not
given, @var{directory} uses @file{'~'}.
@@ -29,6 +29,9 @@ struct thread_info *current_thread;
#define get_thread(inf) ((struct thread_info *)(inf))
+/* The current working directory used to start the inferior. */
+static const char *current_inferior_cwd = NULL;
+
void
add_inferior_to_list (struct inferior_list *list,
struct inferior_list_entry *new_inferior)
@@ -445,3 +448,11 @@ switch_to_thread (ptid_t ptid)
if (!ptid_equal (ptid, minus_one_ptid))
current_thread = find_thread_ptid (ptid);
}
+
+/* See common/common-inferior.h. */
+
+const char *
+get_inferior_cwd ()
+{
+ return current_inferior_cwd;
+}
@@ -59,6 +59,8 @@
#include "top.h"
#include "interps.h"
#include "common/gdb_optional.h"
+#include "gdb_chdir.h"
+#include "readline/tilde.h"
/* Local functions: */
@@ -111,6 +113,10 @@ static void run_command (char *, int);
static char *inferior_args_scratch;
+/* Scratch area where the new cwd will be stored by 'set cwd'. */
+
+static char *inferior_cwd_scratch;
+
/* Scratch area where 'set inferior-tty' will store user-provided value.
We'll immediate copy it into per-inferior storage. */
@@ -246,6 +252,56 @@ show_args_command (struct ui_file *file, int from_tty,
deprecated_show_value_hack (file, from_tty, c, get_inferior_args ());
}
+/* Set the inferior current working directory. This directory will be
+ entered by GDB before executing the inferior. */
+
+static void
+set_inferior_cwd (const char *cwd)
+{
+ struct inferior *inf = current_inferior ();
+
+ gdb_assert (inf != NULL);
+ xfree ((void *) inf->cwd);
+ inf->cwd = tilde_expand (*cwd != '\0' ? cwd : "~");
+}
+
+/* See common/common-inferior.h. */
+
+const char *
+get_inferior_cwd ()
+{
+ return current_inferior ()->cwd;
+}
+
+/* Handle the 'set cwd' command. */
+
+static void
+set_cwd_command (char *args, int from_tty, struct cmd_list_element *c)
+{
+ set_inferior_cwd (inferior_cwd_scratch);
+}
+
+/* Handle the 'show cwd' command. */
+
+static void
+show_cwd_command (struct ui_file *file, int from_tty,
+ struct cmd_list_element *c, const char *value)
+{
+ const char *cwd = get_inferior_cwd ();
+
+ if (cwd == NULL)
+ {
+ /* To maintain backwards compatibility, we use
+ 'current_directory' here, which is set by the "cd"
+ command. */
+ cwd = current_directory;
+ }
+
+ fprintf_filtered (gdb_stdout,
+ _("Current working directory that will be used "
+ "when starting the inferior is \"%s\".\n"), cwd);
+}
+
/* Compute command-line string given argument vector. This does the
same shell processing as fork_inferior. */
@@ -3214,6 +3270,25 @@ Follow this command with any number of args, to be passed to the program."),
gdb_assert (c != NULL);
set_cmd_completer (c, filename_completer);
+ cmd_name = "cwd";
+ add_setshow_string_noescape_cmd (cmd_name, class_run,
+ &inferior_cwd_scratch, _("\
+Set the current working directory to be used when the inferior is started.\n\
+Changing this setting does not have any effect on inferiors that are\n\
+already running."),
+ _("\
+Show the current working directory that is used when the inferior is started."),
+ _("\
+Use this command to change the current working directory that will be used\n\
+when the inferior is started. This setting does not affect GDB's current\n\
+working directory."),
+ set_cwd_command,
+ show_cwd_command,
+ &setlist, &showlist);
+ c = lookup_cmd (&cmd_name, setlist, "", -1, 1);
+ gdb_assert (c != NULL);
+ set_cmd_completer (c, filename_completer);
+
c = add_cmd ("environment", no_class, environment_info, _("\
The environment to give the program, or one variable's value.\n\
With an argument VAR, prints the value of environment variable VAR to\n\
@@ -355,6 +355,10 @@ public:
should never be freed. */
char **argv = NULL;
+ /* The current working directory that will be used when starting
+ this inferior. */
+ const char *cwd = NULL;
+
/* The name of terminal device to use for I/O. */
char *terminal = NULL;
@@ -25,6 +25,7 @@
#include "common-inferior.h"
#include "common-gdbthread.h"
#include "signals-state-save-restore.h"
+#include "gdb_chdir.h"
#include <vector>
extern char **environ;
@@ -376,6 +377,22 @@ fork_inferior (const char *exec_file_arg, const std::string &allargs,
UIs. */
close_most_fds ();
+ const char *cwd = get_inferior_cwd ();
+
+ if (cwd != NULL)
+ {
+ TRY
+ {
+ gdb_chdir (cwd);
+ }
+ CATCH (ex, RETURN_MASK_ERROR)
+ {
+ warning ("%s", ex.message);
+ _exit (0177);
+ }
+ END_CATCH
+ }
+
if (debug_fork)
sleep (debug_fork);
new file mode 100644
@@ -0,0 +1,29 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2017 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/>. */
+
+#include <stdio.h>
+#include <unistd.h>
+
+int
+main (int argc, char *argv[])
+{
+ char dir[BUFSIZ];
+
+ getcwd (dir, BUFSIZ);
+
+ return 0; /* break-here */
+}
new file mode 100644
@@ -0,0 +1,90 @@
+# This testcase is part of GDB, the GNU debugger.
+
+# Copyright 2017 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/>.
+
+if { ![isnative] || [is_remote target] || [is_remote host]
+ || [target_info gdb_protocol] == "extended-remote" } then {
+ untested "not implemented on gdbserver"
+ return
+}
+
+standard_testfile
+
+if { [prepare_for_testing "failed to prepare" $testfile $srcfile debug] } {
+ return -1
+}
+
+# Test that tilde expansion works fine.
+
+proc test_tilde_expansion { } {
+ if { [info exists ::env(HOME)] } {
+ with_test_prefix "test tilde expansion" {
+ set home $::env(HOME)
+
+ gdb_test_no_output "set cwd ~/test" "set cwd to ~/test dir"
+
+ gdb_test "show cwd" \
+ "Current working directory that will be used when starting the inferior is \"${home}/test\"\." \
+ "show cwd shows expanded tilde"
+ }
+ }
+}
+
+# Test that when we "set cwd" the inferior will be started under the
+# correct working directory and GDB will not be affected by this.
+
+proc test_cd_into_dir { } {
+ global decimal gdb_prompt
+
+ with_test_prefix "test cd into temp dir" {
+ gdb_test_multiple "pwd" "pwd before run" {
+ -re "Working directory \(.*\)\.\r\n$gdb_prompt $" {
+ set gdb_cwd_before_run $expect_out(1,string)
+ }
+ }
+
+ set tmpdir [standard_output_file ""]
+
+ gdb_test_no_output "set cwd $tmpdir" "set cwd to temp dir"
+
+ if { ![runto_main] } {
+ untested "could not run to main"
+ return -1
+ }
+
+ gdb_breakpoint [gdb_get_line_number "break-here"]
+ gdb_continue_to_breakpoint "break-here" ".* break-here .*"
+
+ gdb_test "print dir" "\\\$$decimal = \"$tmpdir\", .*" \
+ "inferior cwd is correctly set"
+
+ gdb_test_multiple "pwd" "pwd after run" {
+ -re "Working directory \(.*\)\.\r\n$gdb_prompt $" {
+ set gdb_cwd_after_run $expect_out(1,string)
+ }
+ }
+
+ set test "GDB cwd is unchanged after running inferior"
+ if { [string equal $gdb_cwd_before_run $gdb_cwd_after_run] } {
+ pass $test
+ } else {
+ fail $test
+ }
+ }
+}
+
+test_cd_into_dir
+test_tilde_expansion
@@ -2461,6 +2461,7 @@ windows_create_inferior (struct target_ops *ops, const char *exec_file,
BOOL ret;
DWORD flags = 0;
const char *inferior_io_terminal = get_inferior_io_terminal ();
+ const char *inferior_cwd = get_inferior_cwd ();
if (!exec_file)
error (_("No executable specified, use `target exec'."));
@@ -2574,7 +2575,7 @@ windows_create_inferior (struct target_ops *ops, const char *exec_file,
TRUE, /* inherit handles */
flags, /* start flags */
w32_env, /* environment */
- NULL, /* current directory */
+ inferior_cwd, /* current directory */
&si,
&pi);
if (w32_env)
@@ -2697,7 +2698,7 @@ windows_create_inferior (struct target_ops *ops, const char *exec_file,
TRUE, /* inherit handles */
flags, /* start flags */
w32env, /* environment */
- NULL, /* current directory */
+ inferior_cwd, /* current directory */
&si,
&pi);
if (tty != INVALID_HANDLE_VALUE)