[13/16] gdb: allow 'set args' and run commands to contain newlines

  When starting GDB it is possible to set an inferior argument that
contains a newline, for example:

  shell> gdb --args my.app "abc
  > def"
  ...
  (gdb) show args
  Argument list to give program being debugged when it is started is "abc'
  'def".

However, once GDB is started, the only way to install an argument
containing a newline is to use the Python API.

This commit changes that.

After this commit 'set args' as well as 'run', 'start', and 'starti',
will now accept multi-line inferior arguments, e.g.:

  (gdb) set args "abc
  > def"
  (gdb) show args
  Argument list to give program being debugged when it is started is ""abc
  def"".

And also:

  (gdb) run "abc
  > def"
  ... etc ...

Once GDB has presented the secondary prompt to gather the remaining
inferior arguments then it is possible for the user to quit argument
entry by sending SIGINT (usually, Ctrl-c).  For the 'set args' case
this will abort the argument change, leaving the arguments as they
were previously.  For the run style commands, this aborts the run
command completely, the inferior is not changed, and the partially
collected arguments are not installed.
---
 gdb/NEWS                                 |  10 ++
 gdb/doc/gdb.texinfo                      |  20 ++++
 gdb/infcmd.c                             | 128 +++++++++++++++++++++--
 gdb/testsuite/gdb.base/inferior-args.exp |  71 ++++++++++++-
 4 files changed, 219 insertions(+), 10 deletions(-)
  

> From: Andrew Burgess <aburgess@redhat.com>
> Cc: Andrew Burgess <aburgess@redhat.com>, Michael Weghorn <m.weghorn@posteo.de>
> Date: Tue,  9 Jan 2024 14:26:36 +0000
> 
>  gdb/NEWS                                 |  10 ++
>  gdb/doc/gdb.texinfo                      |  20 ++++
>  gdb/infcmd.c                             | 128 +++++++++++++++++++++--
>  gdb/testsuite/gdb.base/inferior-args.exp |  71 ++++++++++++-
>  4 files changed, 219 insertions(+), 10 deletions(-)

Thanks, the documentation parts are okay.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>
  

On 1/9/24 06:26, Andrew Burgess wrote:

> After this commit 'set args' as well as 'run', 'start', and 'starti',
> will now accept multi-line inferior arguments, e.g.:
> 
>    (gdb) set args "abc
>    > def"
>    (gdb) show args
>    Argument list to give program being debugged when it is started is ""abc
>    def"".
> 

While playing with this, I noticed some odd behavior. I don't know if
this was introduced by your patch or (more likely) I just noticed it
with your patch. [I don't think there's any reason to block this patch
pending a solution in either case.]

I'm noticing some extra newlines output with this new command if you
use ^d to interrupt entering input:

(gdb) set args "
 > <!-- hit ^d -->quit
(gdb) show args

Argument list to give program being debugged when it is started is "".
(gdb)

Notice the extra newline output after the last command.

> diff --git a/gdb/infcmd.c b/gdb/infcmd.c
> index a3c32792491..bad8736aad2 100644
> --- a/gdb/infcmd.c
> +++ b/gdb/infcmd.c
> @@ -120,11 +120,112 @@ show_inferior_tty_command (struct ui_file *file, int from_tty,
>   		"is \"%s\".\n"), inferior_tty.c_str ());
>   }
>   
> +/* Return true if the inferior argument string ARGS represents a "complete"
> +   set of arguments.  Arguments are considered complete so long as they
> +   don't contain unbalanced single or double quoted strings.  Unbalanced
> +   means that a single or double quoted argument is started, but not
> +   finished.  */
> +
> +static bool
> +args_complete_p (const std::string &args)
> +{
> +  const char *input = args.c_str ();
> +  bool squote = false, dquote = false;
> +
> +  while (*input != '\0')
> +    {
> +      input = skip_spaces (input);
> +
> +      if (squote)
> +	{
> +	  /* Inside a single quoted argument, look for the closing single
> +	     quote.  */
> +	  if (*input == '\'')
> +	    squote = false;
> +	}
> +      else if (dquote)
> +	{
> +	  /* If we see either '\"' or '\\' within a double quoted argument
> +	     then skip both characters (one is skipped here, and the other
> +	     at the end of the loop).  We need to skip the '\"' so that we
> +	     don't consider the '"' as closing the double quoted argument,
> +	     and we don't skip the entire '\\' then we'll only skip the
> +	     first '\', in which case we might see the second '\' as a '\"'
> +	     sequence, which would be wrong.  */
> +	  if (*input == '\\' && strchr ("\"\\", *(input + 1)) != nullptr)
> +	    ++input;
> +	  /* Otherwise, just look for the closing double quote.  */
> +	  else if (*input == '"')
> +	    dquote = false;
> +	}
> +      else
> +	{
> +	  /* Outside of either a single or double quoted argument, we need
> +	     to check for '\"', '\'', and '\\'.  The escaped quotes we
> +	     obviously need to skip so we don't think that we have started
> +	     a quoted argument.  The '\\' we need to skip so we don't just
> +	     skip the first '\' and then incorrectly consider the second
> +	     '\' are part of a '\"' or '\'' sequence.  */
> +	  if (*input == '\\' && strchr ("\"\\'", *(input + 1)) != nullptr)
> +	    ++input;
> +	  /* Otherwise, check for the start of a single or double quoted
> +	     argument.  */
> +	  else if (*input == '\'')
> +	    squote = true;
> +	  else if (*input == '"')
> +	    dquote = true;
> +	}
> +
> +      ++input;
> +    }
> +
> +  return (!dquote && !squote);
> +}
> +
> +/* ... */

^^^ Did you forget to finish your comment here?

Keith

> + > +static std::string
> +get_complete_args (std::string args)
> +{
> +  /* If the user wants an argument containing a newline then they need to
> +     do so within quotes.  Use args_complete_p to check if the ARGS string
> +     contains balanced double and single quotes.  If not then prompt the
> +     user for additional arguments and append this to ARGS.  */
> +  const char *prompt = nullptr;
> +  while (!args_complete_p (args))
> +    {
> +      if (prompt == nullptr)
> +	{
> +	  prompt = getenv ("PS2");
> +	  if (prompt == nullptr)
> +	    prompt = "> ";
> +	}
> +
> +      std::string buffer;
> +      const char *content = command_line_input (buffer, prompt, "set_args");
> +      if (content == nullptr)
> +	return {};
> +
> +      args += "\n" + buffer;
> +    }
> +
> +  return args;
> +}
> +
>   /* Store the new value passed to 'set args'.  */
>   
>   static void
> -set_args_value (const std::string &args)
> +set_args_value (const std::string &args_in)
>   {
> +  std::string args;
> +
> +  if (!args_in.empty ())
> +    {
> +      args = get_complete_args (args_in);
> +      if (args.empty ())
> +	return;
> +    }
> +
>     current_inferior ()->set_args (args);
>   }
>   
> @@ -376,6 +477,21 @@ run_command_1 (const char *args, int from_tty, enum run_how run_how)
>   
>     dont_repeat ();
>   
> +  gdb::unique_xmalloc_ptr<char> stripped = strip_bg_char (args, &async_exec);
> +  args = stripped.get ();
> +
> +  std::string inf_args;
> +  /* If there were other args, beside '&', process them.  */
> +  if (args != nullptr)
> +    {
> +      /* If ARGS is only a partial argument string then this call will
> +	 interactively read more arguments from the user.  If the user
> +	 quits then we shouldn't start the inferior.  */
> +      inf_args = get_complete_args (args);
> +      if (inf_args.empty ())
> +	return;
> +    }
> +
>     scoped_disable_commit_resumed disable_commit_resumed ("running");
>   
>     kill_if_already_running (from_tty);
> @@ -397,9 +513,6 @@ run_command_1 (const char *args, int from_tty, enum run_how run_how)
>     reopen_exec_file ();
>     reread_symbols (from_tty);
>   
> -  gdb::unique_xmalloc_ptr<char> stripped = strip_bg_char (args, &async_exec);
> -  args = stripped.get ();
> -
>     /* Do validation and preparation before possibly changing anything
>        in the inferior.  */
>   
> @@ -412,6 +525,9 @@ run_command_1 (const char *args, int from_tty, enum run_how run_how)
>   
>     /* Done.  Can now set breakpoints, change inferior args, etc.  */
>   
> +  if (!inf_args.empty ())
> +    current_inferior ()->set_args (inf_args);
> +
>     /* Insert temporary breakpoint in main function if requested.  */
>     if (run_how == RUN_STOP_AT_MAIN)
>       {
> @@ -433,10 +549,6 @@ run_command_1 (const char *args, int from_tty, enum run_how run_how)
>        the user has to manually nuke all symbols between runs if they
>        want them to go away (PR 2207).  This is probably reasonable.  */
>   
> -  /* If there were other args, beside '&', process them.  */
> -  if (args != nullptr)
> -    current_inferior ()->set_args (args);
> -
>     if (from_tty)
>       {
>         uiout->field_string (nullptr, "Starting program");
> diff --git a/gdb/testsuite/gdb.base/inferior-args.exp b/gdb/testsuite/gdb.base/inferior-args.exp
> index 6c22ecb3c54..b259e4a4217 100644
> --- a/gdb/testsuite/gdb.base/inferior-args.exp
> +++ b/gdb/testsuite/gdb.base/inferior-args.exp
> @@ -211,12 +211,15 @@ lappend test_desc_list [list "test four" \
>   			    [list "$hex \"'\"" \
>   				 "$hex \"\\\\\"\""]]
>   
> -# Run all tests in the global TEST_DESC_LIST.
> +# Run all tests in the global TEST_DESC_LIST, as well as some tests of
> +# inferior arguments containing newlines.
>   proc run_all_tests {} {
> +    set all_methods { "start" "starti" "run" "set args" }
> +
>       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 set_method $all_methods {
>   		foreach_with_prefix startup_with_shell { on off } {
>   		    do_test $set_method $startup_with_shell $args $re_list \
>   			$stub_suitable
> @@ -224,6 +227,70 @@ proc run_all_tests {} {
>   	    }
>   	}
>       }
> +
> +    # Check the multi-line argument entry.  This isn't going to work when
> +    # using the gdbstub, as the only way to set arguments in this case is
> +    # via the gdbserver command line, which isn't what we're testing here.
> +    if { ![use_gdb_stub] } {
> +	foreach_with_prefix set_method $all_methods {
> +	    clean_restart $::binfile
> +
> +	    # First check that we can abort entering multi-line arguments.
> +	    set saw_prompt false
> +	    gdb_test_multiple "$set_method \"ab" "abort argument entry" {
> +		-re "^$set_method \"ab\r\n" {
> +		    exp_continue
> +		}
> +		-re "^> $" {
> +		    set saw_prompt true
> +		    send_gdb "\004"
> +		    exp_continue
> +		}
> +		-re "quit\r\n$::gdb_prompt $" {
> +		    gdb_assert {$saw_prompt} \
> +			$gdb_test_name
> +		}
> +	    }
> +
> +	    # Now place a breakpoint on main.
> +	    if { ![gdb_breakpoint "main" message] } {
> +		fail "could not set breakpoint on main"
> +		continue
> +	    }
> +
> +	    # And actually enter some multi-line arguments.
> +	    set saw_prompt false
> +	    gdb_test_multiple "$set_method \"xy" "complete argument entry" {
> +		-re "^$set_method \"xy\r\n" {
> +		    exp_continue
> +		}
> +		-re "^> $" {
> +		    set saw_prompt true
> +		    send_gdb "12\"\n"
> +		    exp_continue
> +		}
> +
> +		-re "$::gdb_prompt $" {
> +		    gdb_assert { $saw_prompt } \
> +			$gdb_test_name
> +		}
> +	    }
> +
> +	    # For the two methods that don't automatically run to main,
> +	    # poke the inferior along to main.
> +	    if { $set_method == "set args" } {
> +		if { ![runto_main] } {
> +		    continue
> +		}
> +	    } elseif { $set_method == "starti" } {
> +		gdb_continue_to_breakpoint "b/p in main"
> +	    }
> +
> +	    # And check we correctly see the argument containing a newline.
> +	    gdb_test "print argc" " = 2"
> +	    gdb_test "print argv\[1\]" " = $::hex \"xy\\\\n12\""
> +	}
> +    }
>   }
>   
>   run_all_tests