]> git.ipfire.org Git - thirdparty/binutils-gdb.git/commitdiff
gdbserver: convert program_args to a single string
authorAndrew Burgess <aburgess@redhat.com>
Fri, 3 Nov 2023 21:40:29 +0000 (21:40 +0000)
committerAndrew Burgess <aburgess@redhat.com>
Wed, 15 Jan 2025 10:07:50 +0000 (10:07 +0000)
This commit changes how gdbserver stores the inferior arguments from
being a vector of separate arguments into a single string with all of
the arguments combined together.

Making this change might feel a little strange; intuitively it feels
like we would be better off storing the arguments as a vector, but
this change is part of a larger series of work that aims to improve
GDB's inferior argument handling.  The full series was posted here:

  https://inbox.sourceware.org/gdb-patches/cover.1730731085.git.aburgess@redhat.com

But asking people to review a 14 patch series in unreasonable, so I'm
instead posting the patches in smaller batches.  This patch can stand
alone, and I do think this change makes sense on its own:

First, GDB already stores the inferior arguments as a single string,
so doing this moves gdbserver into line with GDB.  The common code
into which gdbserver calls requires the arguments to be a single
string, so currently each target's create_inferior implementation
merged the arguments anyway, so all this commit really does is move
the merging up the call stack, and store the merged result rather than
storing the separate parts.

However, the biggest reason for why this commit is needed, is an issue
with passing arguments from GDB to gdbserver when starting a new
inferior.

Consider:

  (gdb) set args $VAR
  (gdb) run
  ...

When using a native target the inferior will see the value of $VAR
expanded by the shell GDB uses to start the inferior.  However, if
using an extended-remote target the inferior will see literally $VAR,
the unexpanded name of the variable, the reason for this is that,
although GDB sends '$VAR' to gdbserver, when gdbserver receives this,
it converts this to '\$VAR', which prevents the variable from being
expanded by the shell.

The reason for this is that construct_inferior_arguments escapes all
special shell characters within its arguments, and it is
construct_inferior_arguments that is used to combine the separate
arguments into a single string.

In the future I will change construct_inferior_arguments so that
it can apply different escaping strategies.  When this happens we will
want to escape arguments coming from the gdbserver command line
differently than arguments coming from GDB (via a vRun packet), which
means we need to call construct_inferior_arguments earlier, at the
point where we know if the arguments came from the gdbserver command
line, or from the vRun packet.

This argument escaping issue is discussed in PR gdb/28392.

This commit doesn't fix any issues, nor does it change
construct_inferior_arguments to actually do different escaping, that
will all come later.  This is purely a restructuring.

There should be no user visible changes after this commit.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28392

Tested-By: Guinevere Larsen <guinevere@redhat.com>
Approved-By: Simon Marchi <simon.marchi@efficios.com>
gdbserver/linux-low.cc
gdbserver/linux-low.h
gdbserver/netbsd-low.cc
gdbserver/netbsd-low.h
gdbserver/server.cc
gdbserver/target.h
gdbserver/win32-low.cc
gdbserver/win32-low.h

index 50ce2b449270465c97d734b1310e26885dd18feb..65268a6ee6cd6906bcb0605d303777e7e75dbdbd 100644 (file)
@@ -974,7 +974,7 @@ linux_ptrace_fun ()
 
 int
 linux_process_target::create_inferior (const char *program,
-                                      const std::vector<char *> &program_args)
+                                      const std::string &program_args)
 {
   client_state &cs = get_client_state ();
   struct lwp_info *new_lwp;
@@ -984,10 +984,9 @@ linux_process_target::create_inferior (const char *program,
   {
     maybe_disable_address_space_randomization restore_personality
       (cs.disable_randomization);
-    std::string str_program_args = construct_inferior_arguments (program_args);
 
     pid = fork_inferior (program,
-                        str_program_args.c_str (),
+                        program_args.c_str (),
                         get_environ ()->envp (), linux_ptrace_fun,
                         NULL, NULL, NULL, NULL);
   }
index 5be00b8c98cb87fb12277ac269d804e1efe602e1..75af38d898fe7bf93da17224ab6d14b64a98f1ae 100644 (file)
@@ -141,7 +141,7 @@ class linux_process_target : public process_stratum_target
 public:
 
   int create_inferior (const char *program,
-                      const std::vector<char *> &program_args) override;
+                      const std::string &program_args) override;
 
   void post_create_inferior () override;
 
index 04e103563e793a06fbd90a7bbcb139da8a4b9a63..9e7314b02a94157e4e08116c8c90e3f64bf9e00f 100644 (file)
@@ -78,11 +78,9 @@ netbsd_ptrace_fun ()
 
 int
 netbsd_process_target::create_inferior (const char *program,
-                                       const std::vector<char *> &program_args)
+                                       const std::string &program_args)
 {
-  std::string str_program_args = construct_inferior_arguments (program_args);
-
-  pid_t pid = fork_inferior (program, str_program_args.c_str (),
+  pid_t pid = fork_inferior (program, program_args.c_str (),
                             get_environ ()->envp (), netbsd_ptrace_fun,
                             nullptr, nullptr, nullptr, nullptr);
 
index 53200ddffc42a8102f66f0808289589e65af8762..aef1ce40cb90a3c914e51f2c7a4500c4baab0aa1 100644 (file)
@@ -42,7 +42,7 @@ class netbsd_process_target : public process_stratum_target
 public:
 
   int create_inferior (const char *program,
-                      const std::vector<char *> &program_args) override;
+                      const std::string &program_args) override;
 
   void post_create_inferior () override;
 
index bc591599b0299fe5257e0ca779b6ad1a46703b34..c1b18cc947e85617287da2be3e2e4487b2e4c10c 100644 (file)
@@ -121,7 +121,11 @@ private:
   /* The program name, adjusted if needed.  */
   std::string m_path;
 } program_path;
-static std::vector<char *> program_args;
+
+/* All program arguments are merged into a single string.  */
+
+static std::string program_args;
+
 static std::string wrapper_argv;
 
 /* The PID of the originally created or attached inferior.  Used to
@@ -3458,9 +3462,8 @@ handle_v_run (char *own_buf)
   else
     program_path.set (new_program_name.get ());
 
-  /* Free the old argv and install the new one.  */
-  free_vector_argv (program_args);
-  program_args = new_argv;
+  program_args = construct_inferior_arguments (new_argv);
+  free_vector_argv (new_argv);
 
   try
     {
@@ -4346,12 +4349,11 @@ captured_main (int argc, char *argv[])
 
   if (pid == 0 && *next_arg != NULL)
     {
-      int i, n;
-
-      n = argc - (next_arg - argv);
       program_path.set (next_arg[0]);
-      for (i = 1; i < n; i++)
-       program_args.push_back (xstrdup (next_arg[i]));
+
+      int n = argc - (next_arg - argv);
+      program_args
+       = construct_inferior_arguments ({&next_arg[1], &next_arg[n]});
 
       /* Wait till we are at first instruction in program.  */
       target_create_inferior (program_path.get (), program_args);
index 3643b9110dac760f3aadabe481b6f360d7210f5e..1a013bb83db53c5ccec5b545e47ade110a4c08da 100644 (file)
@@ -77,13 +77,13 @@ public:
   /* Start a new process.
 
      PROGRAM is a path to the program to execute.
-     PROGRAM_ARGS is a standard NULL-terminated array of arguments,
-     to be passed to the inferior as ``argv'' (along with PROGRAM).
+     PROGRAM_ARGS is a string containing all of the arguments that will be
+     used to start the inferior.
 
      Returns the new PID on success, -1 on failure.  Registers the new
      process with the process list.  */
   virtual int create_inferior (const char *program,
-                              const std::vector<char *> &program_args) = 0;
+                              const std::string &program_args) = 0;
 
   /* Do additional setup after a new process is created, including
      exec-wrapper completion.  */
index da858b65e6f7fba5501153e56430d567c1753a20..3cfae74d61a7ca891a3a86aa03746c871077ce78 100644 (file)
@@ -490,14 +490,11 @@ create_process (const char *program, char *args,
   return ret;
 }
 
-/* Start a new process.
-   PROGRAM is the program name.
-   PROGRAM_ARGS is the vector containing the inferior's args.
-   Returns the new PID on success, -1 on failure.  Registers the new
-   process with the process list.  */
+/* See target.h.  */
+
 int
 win32_process_target::create_inferior (const char *program,
-                                      const std::vector<char *> &program_args)
+                                      const std::string &program_args)
 {
   client_state &cs = get_client_state ();
 #ifndef USE_WIN32API
@@ -508,8 +505,7 @@ win32_process_target::create_inferior (const char *program,
   DWORD flags;
   PROCESS_INFORMATION pi;
   DWORD err;
-  std::string str_program_args = construct_inferior_arguments (program_args);
-  char *args = (char *) str_program_args.c_str ();
+  char *args = (char *) program_args.c_str ();
 
   /* win32_wait needs to know we're not attaching.  */
   windows_process.attaching = 0;
index daed16a6ae6a097ad4383c7ebe8b3cd3752b5eae..e9dd6adc5a827a86981b2e0d3191c10a46956eee 100644 (file)
@@ -101,7 +101,7 @@ class win32_process_target : public process_stratum_target
 public:
 
   int create_inferior (const char *program,
-                      const std::vector<char *> &program_args) override;
+                      const std::string &program_args) override;
 
   int attach (unsigned long pid) override;