[06/16] gdbsupport: have construct_inferior_arguments take an escape function
Commit Message
This commit is a refactor in order to make later commits in this
series easier, and continues the work started in the previous commit.
The construct_inferior_arguments function takes a vector of strings
and does two things:
1. Applies escaping to each string, and
2. Combines all the individual strings into a single string, placing
a space between each.
The construct_inferior_arguments function is used in a couple of
places within GDB and gdbserver, and in each place, the same escaping
algorithm is used; it has to be, the algorithm is hard-coded into the
function construct_inferior_arguments.
However, in later commits I will propose that in different situations
we should actually apply different escaping algorithms, depending on
whether the incoming arguments are already escaped or not.
As a simple concrete example, if we invoke current GDB like this:
$ gdb -q --args /tmp/application '$SHELL'
...
(gdb) show args
Argument list to give program being debugged when it is started is "\$SHELL".
Here you can see an example of the default escaping used by
construct_inferior_arguments, special shell characters are
escaped (the '$' becomes '\$').
The downside of this is that the application will see a literal
'$SHELL' string, not the shell expanded $SHELL variable
value (e.g. /bin/bash). There's currently no way to get the second
behaviour from the GDB command line.
This is discussed in PR gdb/28392.
If we wanted to add a new GDB command line flag, maybe like this:
$ gdb -q --no-escape-args /tmp/application '$SHELL'
...
(gdb) show args
Argument list to give program being debugged when it is started is "$SHELL".
Now, the argument string contains '$SHELL', with no escaping. When
the inferior is run (with startup-with-shell on), the application will
see the shell expanded value of $SHELL.
To achieve this we need construct_inferior_arguments to apply a
different escaping algorithm.
Another example of where the current construct_inferior_arguments
behaviour is not correct is in gdbserver. Like GDB, arguments from
the command line are escaped, however, we currently also escape
arguments arriving in the vRun packet.
Thus, if in GDB we do this:
(gdb) set args $SHELL
And then run an extended-remote target, the $SHELL will be escaped
when it is passed to gdbserver, gdbserver will then hold \$SHELL,
which means the inferior will see a literal '$SHELL' string, rather
than the shell expanded value of the variable. This is a real bug
that exists today in GDB/gdbserver.
Given the upcoming patches, I think there is more than a boolean
choice between two escaping algorithms, so, at least for now, I
propose that construct_inferior_arguments should take a function
pointer for a function that will perform the argument escaping.
If it turns out that this is excessive then it is easy enough to fold
the algorithm selecting back into construct_inferior_arguments and
instead pass a bool or enum to choose the correct algorithm, but I
think this approach is simple enough.
After this commit construct_inferior_arguments merges all the
arguments into a single string, using the worker function to escape
each argument in turn.
However, this commit doesn't actually fix any of the above issues.
This is a restructuring commit. All this commit does is change
construct_inferior_arguments to take the escaping function, I've split
the existing escaping function out from construct_inferior_arguments,
and I've updated every call to construct_inferior_arguments to pass in
the one and only escaping function. As a result there should be no
change in behaviour after this commit.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28392
---
gdb/inferior.c | 5 ++-
gdb/inferior.h | 3 +-
gdb/main.c | 4 +-
gdb/python/py-inferior.c | 2 +-
gdbserver/server.cc | 6 ++-
gdbsupport/common-inferior.cc | 81 ++++++++++++++++++++---------------
gdbsupport/common-inferior.h | 17 ++++++--
7 files changed, 73 insertions(+), 45 deletions(-)
@@ -168,9 +168,10 @@ inferior::tty ()
/* See inferior.h. */
void
-inferior::set_args (gdb::array_view<char * const> args)
+inferior::set_args (gdb::array_view<char * const> args,
+ escape_args_func escape_func)
{
- set_args (construct_inferior_arguments (args));
+ set_args (construct_inferior_arguments (args, escape_func));
}
void
@@ -526,7 +526,8 @@ class inferior : public refcounted_object,
};
/* Set the argument string from some strings. */
- void set_args (gdb::array_view<char * const> args);
+ void set_args (gdb::array_view<char * const> args,
+ escape_args_func escape_func);
/* Get the argument string to use when running this inferior.
@@ -1084,8 +1084,8 @@ captured_main_1 (struct captured_main_args *context)
symarg = argv[optind];
execarg = argv[optind];
++optind;
- current_inferior ()->set_args
- (gdb::array_view<char * const> (&argv[optind], argc - optind));
+ gdb::array_view<char * const> arg_view (&argv[optind], argc - optind);
+ current_inferior ()->set_args (arg_view, escape_shell_characters);
}
else
{
@@ -919,7 +919,7 @@ infpy_set_args (PyObject *self, PyObject *value, void *closure)
for (const auto &arg : args)
argvec.push_back (arg.get ());
gdb::array_view<char * const> view (argvec.data (), argvec.size ());
- inf->inferior->set_args (view);
+ inf->inferior->set_args (view, escape_shell_characters);
}
else
{
@@ -3437,7 +3437,8 @@ handle_v_run (char *own_buf)
else
program_path.set (new_program_name.get ());
- program_args = construct_inferior_arguments (new_argv);
+ program_args = construct_inferior_arguments (new_argv,
+ escape_shell_characters);
free_vector_argv (new_argv);
target_create_inferior (program_path.get (), program_args);
@@ -4319,7 +4320,8 @@ captured_main (int argc, char *argv[])
std::vector<char *> temp_arg_vector;
for (i = 1; i < n; i++)
temp_arg_vector.push_back (next_arg[i]);
- program_args = construct_inferior_arguments (temp_arg_vector);
+ program_args = construct_inferior_arguments (temp_arg_vector,
+ escape_shell_characters);
/* Wait till we are at first instruction in program. */
target_create_inferior (program_path.get (), program_args);
@@ -28,7 +28,26 @@ bool startup_with_shell = true;
/* See common-inferior.h. */
std::string
-construct_inferior_arguments (gdb::array_view<char * const> argv)
+construct_inferior_arguments (gdb::array_view<char * const> argv,
+ escape_args_func escape_func)
+{
+ std::string result;
+
+ for (const char *a : argv)
+ {
+ if (!result.empty ())
+ result += " ";
+
+ result += escape_func (a);
+ }
+
+ return result;
+}
+
+/* See common-inferior.h. */
+
+std::string
+escape_shell_characters (const char *arg)
{
std::string result;
@@ -44,55 +63,49 @@ construct_inferior_arguments (gdb::array_view<char * const> argv)
static const char special[] = "\"!#$&*()\\|[]{}<>?'`~^; \t\n";
static const char quote = '\'';
#endif
- for (int i = 0; i < argv.size (); ++i)
+
+ /* Need to handle empty arguments specially. */
+ if (arg[0] == '\0')
+ {
+ result += quote;
+ result += quote;
+ }
+ else
{
- if (i > 0)
- result += ' ';
+#ifdef __MINGW32__
+ bool quoted = false;
- /* Need to handle empty arguments specially. */
- if (argv[i][0] == '\0')
+ if (strpbrk (arg, special))
{
- result += quote;
+ quoted = true;
result += quote;
}
- else
+#endif
+ for (const char *cp = arg; *cp; ++cp)
{
-#ifdef __MINGW32__
- bool quoted = false;
-
- if (strpbrk (argv[i], special))
+ if (*cp == '\n')
{
- quoted = true;
+ /* A newline cannot be quoted with a backslash (it just
+ disappears), only by putting it inside quotes. */
+ result += quote;
+ result += '\n';
result += quote;
}
-#endif
- for (char *cp = argv[i]; *cp; ++cp)
+ else
{
- if (*cp == '\n')
- {
- /* A newline cannot be quoted with a backslash (it
- just disappears), only by putting it inside
- quotes. */
- result += quote;
- result += '\n';
- result += quote;
- }
- else
- {
#ifdef __MINGW32__
- if (*cp == quote)
+ if (*cp == quote)
#else
- if (strchr (special, *cp) != NULL)
+ if (strchr (special, *cp) != NULL)
#endif
- result += '\\';
- result += *cp;
- }
+ result += '\\';
+ result += *cp;
}
+ }
#ifdef __MINGW32__
- if (quoted)
- result += quote;
+ if (quoted)
+ result += quote;
#endif
- }
}
return result;
@@ -57,9 +57,20 @@ extern const std::string &get_inferior_cwd ();
the target is started up with a shell. */
extern bool startup_with_shell;
-/* Compute command-line string given argument vector. This does the
- same shell processing as fork_inferior. */
+/* The type for a function used by construct_inferior_arguments to add any
+ quoting needed to an individual argument before combining all the
+ arguments into a single string. */
+using escape_args_func = std::string (*) (const char *arg);
+
+/* Return a version of ARG that has special shell characters escaped. */
+extern std::string escape_shell_characters (const char *arg);
+
+/* Pass each element of ARGS through ESCAPE_FUNC and combine the results
+ into a single string, separating each element with a single space
+ character. */
+
extern std::string
-construct_inferior_arguments (gdb::array_view<char * const>);
+construct_inferior_arguments (gdb::array_view<char * const> args,
+ escape_args_func escape_func);
#endif /* COMMON_COMMON_INFERIOR_H */