gdb: Support some escaping of args with startup-with-shell being off

diff --git a/gdb/nat/fork-inferior.c b/gdb/nat/fork-inferior.c
index 41765b102bc..65cfab83c98 100644
--- a/gdb/nat/fork-inferior.c
+++ b/gdb/nat/fork-inferior.c
@@ -27,6 +27,7 @@ 
 #include "gdbsupport/signals-state-save-restore.h"
 #include "gdbsupport/gdb_tilde_expand.h"
 #include "gdbsupport/gdb_signals.h"
+#include "gdbsupport/buildargv.h"
 #include <vector>
 
 extern char **environ;
@@ -42,6 +43,11 @@  class execv_argv
   execv_argv (const char *exec_file, const std::string &allargs,
 	      const char *shell_file);
 
+  ~execv_argv ()
+  {
+    free_vector_argv (m_argv);
+  }
+
   /* Return a pointer to the built argv, in the type expected by
      execv.  The result is (only) valid for as long as this execv_argv
      object is live.  We return a "char **" because that's the type
@@ -50,7 +56,7 @@  class execv_argv
      strings to which the array point.  */
   char **argv ()
   {
-    return const_cast<char **> (&m_argv[0]);
+    return m_argv.data ();
   }
 
 private:
@@ -69,66 +75,28 @@  class execv_argv
 		       const std::string &allargs,
 		       const char *shell_file);
 
-  /* The argument vector built.  Holds non-owning pointers.  Elements
-     either point to the strings passed to the execv_argv ctor, or
-     inside M_STORAGE.  */
-  std::vector<const char *> m_argv;
-
-  /* Storage.  In the no-shell case, this contains a copy of the
-     arguments passed to the ctor, split by '\0'.  In the shell case,
-     this contains the quoted shell command.  I.e., SHELL_COMMAND in
-     {"$SHELL" "-c", SHELL_COMMAND, NULL}.  */
-  std::string m_storage;
+  /* The argument vector.  This owns the strings within it.  */
+  std::vector<char *> m_argv;
 };
 
-/* Create argument vector for straight call to execvp.  Breaks up
-   ALLARGS into an argument vector suitable for passing to execvp and
-   stores it in M_ARGV.  E.g., on "run a b c d" this routine would get
-   as input the string "a b c d", and as output it would fill in
-   M_ARGV with the four arguments "a", "b", "c", "d".  Each argument
-   in M_ARGV points to a substring of a copy of ALLARGS stored in
-   M_STORAGE.  */
+/* Create argument vector for straight call to execvp.  Breaks up ALLARGS
+   into an argument vector suitable for passing to execvp and stores it in
+   M_ARGV.  EXEC_FILE is the executable to be run.
+
+   E.g., if EXEC_FILE is "foo", and the user does "run a b c d" then
+   ALLARGS would be "a b c d", and this function would fill M_ARGV with
+   give arguments "foo", "a", "b", "c", and "d".  */
 
 void
 execv_argv::init_for_no_shell (const char *exec_file,
 			       const std::string &allargs)
 {
+  m_argv.push_back (xstrdup (exec_file));
 
-  /* Save/work with a copy stored in our storage.  The pointers pushed
-     to M_ARGV point directly into M_STORAGE, which is modified in
-     place with the necessary NULL terminators.  This avoids N heap
-     allocations and string dups when 1 is sufficient.  */
-  std::string &args_copy = m_storage = allargs;
+  gdb_argv argv (allargs.c_str ());
 
-  m_argv.push_back (exec_file);
-
-  for (size_t cur_pos = 0; cur_pos < args_copy.size ();)
-    {
-      /* Skip whitespace-like chars.  */
-      std::size_t pos = args_copy.find_first_not_of (" \t\n", cur_pos);
-
-      if (pos != std::string::npos)
-	cur_pos = pos;
-
-      /* Find the position of the next separator.  */
-      std::size_t next_sep = args_copy.find_first_of (" \t\n", cur_pos);
-
-      if (next_sep == std::string::npos)
-	{
-	  /* No separator found, which means this is the last
-	     argument.  */
-	  next_sep = args_copy.size ();
-	}
-      else
-	{
-	  /* Replace the separator with a terminator.  */
-	  args_copy[next_sep++] = '\0';
-	}
-
-      m_argv.push_back (&args_copy[cur_pos]);
-
-      cur_pos = next_sep;
-    }
+  for (const auto &a : argv)
+    m_argv.push_back (xstrdup (a));
 
   /* NULL-terminate the vector.  */
   m_argv.push_back (NULL);
@@ -182,11 +150,7 @@  execv_argv::init_for_shell (const char *exec_file,
   /* We're going to call a shell.  */
   bool escape_bang = escape_bang_in_quoted_argument (shell_file);
 
-  /* We need to build a new shell command string, and make argv point
-     to it.  So build it in the storage.  */
-  std::string &shell_command = m_storage;
-
-  shell_command = "exec ";
+  std::string shell_command = "exec ";
 
   /* Add any exec wrapper.  That may be a program name with arguments,
      so the user must handle quoting.  */
@@ -256,9 +220,9 @@  execv_argv::init_for_shell (const char *exec_file,
      "-c" says to interpret the next arg as a shell command to
      execute, and this command is "exec <target-program> <args>".  */
   m_argv.reserve (4);
-  m_argv.push_back (shell_file);
-  m_argv.push_back ("-c");
-  m_argv.push_back (shell_command.c_str ());
+  m_argv.push_back (xstrdup (shell_file));
+  m_argv.push_back (xstrdup ("-c"));
+  m_argv.push_back (xstrdup (shell_command.c_str ()));
   m_argv.push_back (NULL);
 }
 
diff --git a/gdb/testsuite/gdb.base/args.exp b/gdb/testsuite/gdb.base/args.exp
index f9235f2f7b0..34d722a7941 100644
--- a/gdb/testsuite/gdb.base/args.exp
+++ b/gdb/testsuite/gdb.base/args.exp
@@ -29,30 +29,54 @@  if {[build_executable $testfile.exp $testfile $srcfile] == -1} {
     return -1
 }
 
+set startup_with_shell_modes { "on" }
+if {![gdb_protocol_is_remote]} {
+    lappend startup_with_shell_modes "off"
+} else {
+    # Some of these tests will not work when using the remote protocol
+    # due to bug PR gdb/28392.
+    unsupported "gdbserver 'startup-with-shell off' broken PR gdb/28392"
+}
+
 # NAME is the name to use for the tests and ARGLIST is the list of
 # arguments that are passed to GDB when it is started.
+#
+# The optional RE_LIST is the list of patterns to check the arguments
+# against, these patterns should match ARGLIST.  If the arguments are
+# expected to show up unmodified in the test output then RE_LIST can
+# be dropped, and this proc will reuse ARGLIST.
+
+proc args_test { name arglist {re_list {}} } {
+
+    # If RE_LIST is not supplied then we can reuse ARGLIST, this
+    # implies that the arguments will appear unmodified in the test
+    # output.
+    if {[llength $re_list] == 0} {
+	set re_list $arglist
+    }
 
-proc args_test { name arglist } {
-    save_vars { ::GDBFLAGS } {
-	set ::GDBFLAGS "$::GDBFLAGS --args $::binfile $arglist"
+    foreach_with_prefix startup_with_shell $::startup_with_shell_modes {
+	save_vars { ::GDBFLAGS } {
+	    set ::GDBFLAGS "$::GDBFLAGS --args $::binfile $arglist"
 
-	clean_restart $::binfile
+	    clean_restart $::binfile
 
-	runto_main
-	gdb_breakpoint [gdb_get_line_number "set breakpoint here"]
-	gdb_continue_to_breakpoint "breakpoint for $name"
+	    gdb_test_no_output "set startup-with-shell ${startup_with_shell}" \
+		"set startup-with-shell for $name"
 
-	set expected_len [expr 1 + [llength $arglist]]
-	gdb_test "print argc" "\\\$$::decimal = $expected_len" "argc for $name"
+	    runto_main
+	    gdb_breakpoint [gdb_get_line_number "set breakpoint here"]
+	    gdb_continue_to_breakpoint "breakpoint for $name"
 
-	set i 1
-	foreach arg $arglist {
-	    if { $arg eq "\n" } {
-		set arg {\\n}
+	    set expected_len [expr 1 + [llength $re_list]]
+	    gdb_test "print argc" "\\\$$::decimal = $expected_len" "argc for $name"
+
+	    set i 1
+	    foreach arg $re_list {
+		gdb_test "print argv\[$i\]" "\\\$$::decimal = $::hex \"$arg\"" \
+		    "argv\[$i\] for $name"
+		set i [expr $i + 1]
 	    }
-	    gdb_test "print argv\[$i\]" "\\\$$::decimal = $::hex \"$arg\"" \
-		"argv\[$i\] for $name"
-	    set i [expr $i + 1]
 	}
     }
 }
@@ -78,6 +102,10 @@  args_test "two empty with single quotes" {{1} {''} {''} {3}}
 
 # Try with arguments containing literal newlines.
 
-args_test "one newline" {{1} "\n" {3}}
+args_test "one newline" {{1} "\n" {3}} {1 \\\\n 3}
+
+args_test "two newlines" {{1} "\n" "\n" {3}} {1 \\\\n \\\\n 3}
+
+args_test "lone single quote" {{1} \' {3}}
 
-args_test "two newlines" {{1} "\n" "\n" {3}}
+args_test "lone double quote" {{1} \" {3}} {1 \\\\\" 3}
diff --git a/gdb/testsuite/gdb.base/inferior-args.exp b/gdb/testsuite/gdb.base/inferior-args.exp
index b165fb83839..79b73e61b33 100644
--- a/gdb/testsuite/gdb.base/inferior-args.exp
+++ b/gdb/testsuite/gdb.base/inferior-args.exp
@@ -25,16 +25,27 @@  if {[build_executable "failed to prepare" $testfile $srcfile \
     return
 }
 
-proc do_test { method } {
+# STARTUP_WITH_SHELL is either 'on' or 'off' and determines if the
+# inferior is started under a shell or not.  INFERIOR_ARGS is the list
+# of inferior arguments.  EXPECTED_RESULTS is the list of expected
+# results, one for each argument.
+#
+# When STUB_SUITABLE is true this test is suitable for use with
+# gdbserver, i.e. INFERIOR_ARGS can be passed through to
+# gdbserver_start via gdb_run_cmd.  Some of the weird quoting used in
+# some of the tests doesn't seem to play well with gdbserver_start.
+# This is a TCL issue, not a gdbserver issue.  Manually testing with
+# gdbserver shows no problems.  It's just that when we try to invoke
+# gdbserver from TCL the argument quoting gets messed up.  For tests
+# that are problematic, STUB_SUITABLE is false.
+proc do_test { method startup_with_shell inferior_args expected_results \
+		   stub_suitable } {
     global binfile hex
 
-    # The second arg is an empty string on purpose.  The last argument
-    # must be the empty argument -- we once had a bug where that
-    # wouldn't work!
-    set inferior_args { "first arg" "" "third-arg" "'" "\"" " " "" }
-
     clean_restart $binfile
 
+    gdb_test_no_output "set startup-with-shell $startup_with_shell"
+
     if { $method == "start" } {
 	# The start command does not make sense for a stub.
 	if { [use_gdb_stub] } {
@@ -80,6 +91,10 @@  proc do_test { method } {
 	    return -1
 	}
 
+	if { [use_gdb_stub] && !$stub_suitable } {
+	    return
+	}
+
 	# The run command does not make sense for a stub, but GDB_RUN_CMD
 	# does the right thing when the target is a stub (start the stub,
 	# connect to it, and "continue").
@@ -110,18 +125,74 @@  proc do_test { method } {
 	error "invalid method $method"
     }
 
+    set argc [expr [llength $expected_results] + 1]
+
     # Now that we are stopped at main, inspect argc/argv.
-    gdb_test "print argc" " = 8"
-    gdb_test "print argv\[0\]" " = $hex \".*\""
-    gdb_test "print argv\[1\]" " = $hex \"first arg\""
-    gdb_test "print argv\[2\]" " = $hex \"\""
-    gdb_test "print argv\[3\]" " = $hex \"third-arg\""
-    gdb_test "print argv\[4\]" " = $hex \"'\""
-    gdb_test "print argv\[5\]" " = $hex \"\\\\\"\""
-    gdb_test "print argv\[6\]" " = $hex \" \""
-    gdb_test "print argv\[7\]" " = $hex \"\""
+    gdb_test "print argc" " = $argc"
+    gdb_test "print argv\[0\]" " = $hex \"\[^\r\n\]+\""
+    for { set i 1 } { $i < $argc } { incr i } {
+	set idx [expr $i - 1]
+	gdb_test "print argv\[$i\]" " = [lindex $expected_results $idx]"
+    }
+}
+
+set test_desc_list []
+
+# test one
+# --------
+#
+# The second arg is an empty string on purpose.  The last argument
+# must be the empty argument -- we once had a bug where that wouldn't
+# work!
+lappend test_desc_list [list "test one" \
+			    true \
+			    { "first arg" "" "third-arg" "'" "\"" " " "" } \
+			    [list "$hex \"first arg\"" \
+				 "$hex \"\"" \
+				 "$hex \"third-arg\"" \
+				 "$hex \"'\"" \
+				 "$hex \"\\\\\"\"" \
+				 "$hex \" \"" \
+				 "$hex \"\"" ]]
+
+# test two
+# --------
+#
+# The argument being passed here is '"', that is a single double quote
+# contained within single quotes.
+#
+# I build the test descriptor using this mess of code to avoid having
+# unbalanced quotes, which messes up indentation and syntax
+# highlighting within (at least) emacs.  The 'format' of ascii code 34
+# gives us the double quote character.  Then I have to jump through
+# the rest of this mess in order to avoid TCL escaping the quote for
+# me.  It's super important that what we send to GDB is '"' not '\"'.
+set item [list "test two" false]
+set cmd [format "lappend item \{ '%c' '\\%c' \}" 34 34]
+eval $cmd
+set bs "\\\\"
+lappend item [list "$hex \"$bs\"\"" "$hex \"$bs$bs$bs\"\""]
+lappend test_desc_list $item
+
+set startup_with_shell_modes { "on" }
+if {![gdb_protocol_is_remote]} {
+    lappend startup_with_shell_modes "off"
+} else {
+    # Due to PR gdb/28392 gdbserver doesn't currently support having
+    # startup-with-shell off, and then attempting to pass arguments
+    # containing whitespace.
+    unsupported "bug gdb/28392: gdbserver doesn't support this"
 }
 
-foreach_with_prefix method { "start" "starti" "run" "set args" } {
-    do_test $method
+
+foreach desc $test_desc_list {
+    lassign $desc name stub_suitable args re_list
+    with_test_prefix $name {
+	foreach_with_prefix set_method { "start" "starti" "run" "set args" } {
+	    foreach_with_prefix startup_with_shell $startup_with_shell_modes {
+		do_test $set_method $startup_with_shell $args $re_list \
+		    $stub_suitable
+	    }
+	}
+    }
 }
diff --git a/gdb/testsuite/gdb.base/startup-with-shell.exp b/gdb/testsuite/gdb.base/startup-with-shell.exp
index 87a755904f6..495c43eeaee 100644
--- a/gdb/testsuite/gdb.base/startup-with-shell.exp
+++ b/gdb/testsuite/gdb.base/startup-with-shell.exp
@@ -43,55 +43,134 @@  proc initial_setup_simple { startup_with_shell run_args } {
     clean_restart $binfile
 
     gdb_test_no_output "set startup-with-shell $startup_with_shell"
+    gdb_test_no_output "set print characters unlimited"
 
     gdb_test_no_output "set args $run_args" \
 	"set args \$run_args"
 
-    set test "inferior started"
-    if { [runto_main] } {
-	pass $test
-    } else {
-	fail $test
+    return [runto_main]
+}
+
+# Start GDB, set the inferior arguments to ARGS, and then run to main.
+# Once at main, read the first argument from the inferior and compare
+# it to ON_RE if startup-with-shell is on, otherwise compare to
+# OFF_RE.
+#
+# If PROBLEMATIC_ON is true then when startup-with-shell is on we
+# expect the comparison to fail, so setup an xfail.
+#
+# If PROBLEMATIC_OFF is true then when startup-with-shell is off we
+# expect the comparison to fail, so setup an xfail.
+#
+# TESTNAME is a string used in the test names.
+proc run_test { args on_re off_re testname { problematic_on false } \
+		    { problematic_off false } } {
+    foreach startup_with_shell { "on" "off" } {
+	with_test_prefix "$testname, startup_with_shell: ${startup_with_shell}" {
+	    if {![initial_setup_simple $startup_with_shell $args]} {
+		return -1
+	    }
+
+	    if { $startup_with_shell } {
+		set re $on_re
+		set problematic $problematic_on
+	    } else {
+		set re $off_re
+		set problematic $problematic_off
+	    }
+
+	    if { $problematic } {
+		setup_xfail "*-*-*" gdb/28392
+	    }
+
+	    gdb_test "print argv\[1\]" "\\\$$::decimal = $::hex $re" $testname
+	}
     }
 }
 
+# This is like the run_test proc except that RE is used as the
+# expected argument regexp when startup-with-shell is both on and off.
+# For the other arguments, see run_test.
+proc run_test_same { args re testname { problematic_on false } \
+			 { problematic_off false } } {
+    run_test $args $re $re $testname $problematic_on $problematic_off
+}
+
+# The regexp to match a single '\' character.
+set bs "\\\\"
+
 # Are we using 'remote' or 'extended-remote' protocol?
 set is_remote_p [gdb_protocol_is_remote]
 
 ## Run the actual tests
 
-with_test_prefix "startup_with_shell = on; run_args = *.unique-extension" {
-    initial_setup_simple "on" "$unique_file_dir/*.unique-extension"
-    gdb_test_no_output "set print characters unlimited"
-    if { $is_remote_p } {
-	setup_xfail "*-*-*" gdb/28392
-    }
-    gdb_test "print argv\[1\]" "\\\$$decimal = $hex \"$unique_file\"" \
-	"first argument expanded"
-}
+run_test "$unique_file_dir/*.unique-extension" \
+    "\"$unique_file\"" \
+    "\"$unique_file_dir/\\\*\.unique-extension\"" \
+    "arg is glob" \
+    $is_remote_p
 
-with_test_prefix "startup_with_shell = off; run_args = *.unique-extension" {
-    initial_setup_simple "off" "$unique_file_dir/*.unique-extension"
-    gdb_test_no_output "set print characters unlimited"
-    gdb_test "print argv\[1\]" "\\\$$decimal = $hex \"$unique_file_dir/\\\*\.unique-extension\"" \
-	"first argument not expanded"
-}
+run_test_same "$unique_file_dir/\\*.unique-extension" \
+    "\"$unique_file_dir/\\\*\.unique-extension\"" \
+    "arg is escaped glob"
 
-with_test_prefix "startup_with_shell = on; run_args = \$TEST" {
+save_vars { env(TEST) } {
     set env(TEST) "1234"
-    initial_setup_simple "on" "\$TEST"
-    if { $is_remote_p } {
-	setup_xfail "*-*-*" gdb/28392
-    }
-    gdb_test "print argv\[1\]" "\\\$$decimal = $hex \"1234\"" \
-	"testing first argument"
-    unset env(TEST)
+    run_test "\$TEST" \
+	"\"1234\"" \
+	"\"\\\$TEST\"" \
+	"arg is shell variable" \
+	$is_remote_p
+
+    run_test_same "\\\$TEST" \
+	"\"\\\$TEST\"" \
+	"arg is escaped shell variable"
 }
 
-with_test_prefix "startup_with_shell = off; run_args = \$TEST" {
-    set env(TEST) "1234"
-    initial_setup_simple "off" "\$TEST"
-    gdb_test "print argv\[1\]" "\\\$$decimal = $hex \"\\\$TEST\"" \
-	"testing first argument"
-    unset env(TEST)
-}
+run_test_same "\"\\a\"" \
+    "\"${bs}${bs}a\"" \
+    "retain backslash in double quote arg" \
+    false $is_remote_p
+
+run_test_same "'\\a'" \
+    "\"${bs}${bs}a\"" \
+    "retain backslash in single quote arg" \
+    false $is_remote_p
+
+run_test_same "\"\\\$\"" \
+    "\"\\\$\"" \
+    "'\$' can be escaped in double quote arg"
+
+run_test_same "'\\\$'" \
+    "\"${bs}${bs}\\\$\"" \
+    "'\$' is not escaped in single quote arg" \
+    false $is_remote_p
+
+run_test_same "\"\\`\"" \
+    "\"\\`\"" \
+    "'`' can be escaped in double quote arg"
+
+run_test_same "'\\`'" \
+    "\"${bs}${bs}`\"" \
+    "'`' is not escaped in single quote arg" \
+    false $is_remote_p
+
+run_test_same "\"\\\"\"" \
+    "\"${bs}\"\"" \
+    "'\"' can be escaped in double quote arg" \
+    false $is_remote_p
+
+run_test_same "'\\\"'" \
+    "\"${bs}${bs}${bs}\"\"" \
+    "'\"' is not escaped in single quote arg" \
+    false $is_remote_p
+
+run_test_same "\"\\\\\"" \
+    "\"${bs}${bs}\"" \
+    "'\\' can be escaped in double quote arg" \
+    false $is_remote_p
+
+run_test_same "'\\\\'" \
+    "\"${bs}${bs}${bs}${bs}\"" \
+    "'\\' is not escaped in single quote arg" \
+    false $is_remote_p