gdb: remove static buffer in command_line_input

[I sent this earlier today, but I don't see it in the archives.
Resending it through a different computer / SMTP.]

The use of the static buffer in command_line_input is becoming
problematic, as explained here [1].  In short, with this patch [2] that
attempt to fix a post-hook bug, when running gdb.base/commands.exp, we
hit a case where we read a "define" command line from a script file
using command_command_line_input.  The command line is stored in
command_line_input's static buffer.  Inside the define command's
execution, we read the lines inside the define using command_line_input,
which overwrites the define command, in command_line_input's static
buffer.  After the execution of the define command, execute_command does
a command look up to see if a post-hook is registered.  For that, it
uses a now stale pointer that used to point to the define command, in
the static buffer, causing a use-after-free.  Note that the pointer in
execute_command points to the dynamically-allocated buffer help by the
static buffer in command_line_input, not to the static object itself,
hence why we see a use-after-free.

Fix that by removing the static buffer.  I initially changed
command_line_input and other related functions to return an std::string,
which is the obvious but naive solution.  The thing is that some callees
don't need to return an allocated string, so this this an unnecessary
pessimization.  I changed it to passing in a reference to an std::string
buffer, which the callee can use if it needs to return
dynamically-allocated content.  It fills the buffer and returns a
pointers to the C string inside.  The callees that don't need to return
dynamically-allocated content simply don't use it.

So, it started with modifying command_line_input as described above, all
the other changes derive directly from that.

One slightly shady thing is in handle_line_of_input, where we now pass a
pointer to an std::string's internal buffer to readline's history_value
function, which takes a `char *`.  I'm pretty sure that this function
does not modify the input string, because I was able to change it (with
enough massaging) to take a `const char *`.

A subtle change is that we now clear a UI's line buffer using a
SCOPE_EXIT in command_line_handler, after executing the command.
This was previously done by this line in handle_line_of_input:

  /* We have a complete command line now.  Prepare for the next
     command, but leave ownership of memory to the buffer .  */
  cmd_line_buffer->used_size = 0;

I think the new way is clearer.

[1] https://inbox.sourceware.org/gdb-patches/becb8438-81ef-8ad8-cc42-fcbfaea8cddd@simark.ca/
[2] https://inbox.sourceware.org/gdb-patches/20221213112241.621889-1-jan.vrany@labware.com/

Change-Id: I8fc89b1c69870c7fc7ad9c1705724bd493596300
Reviewed-By: Tom Tromey <tom@tromey.com>

2 files changed, 20 insertions, 10 deletions

diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c
index 2101d6f..88afd1a 100644
--- a/gdb/cli/cli-script.c
+++ b/gdb/cli/cli-script.c
@@ -44,7 +44,7 @@
 
 static enum command_control_type
 recurse_read_control_structure
-    (gdb::function_view<const char * ()> read_next_line_func,
+    (read_next_line_ftype read_next_line_func,
      struct command_line *current_cmd,
      gdb::function_view<void (const char *)> validator);
 
@@ -54,7 +54,7 @@ static void do_define_command (const char *comname, int from_tty,
 static void do_document_command (const char *comname, int from_tty,
                                  const counted_command_line *commands);
 
-static const char *read_next_line ();
+static const char *read_next_line (std::string &buffer);
 
 /* Level of control structure when reading.  */
 static int control_level;
@@ -894,7 +894,7 @@ user_args::insert_args (const char *line) const
    from stdin.  */
 
 static const char *
-read_next_line ()
+read_next_line (std::string &buffer)
 {
   struct ui *ui = current_ui;
   char *prompt_ptr, control_prompt[256];
@@ -917,7 +917,7 @@ read_next_line ()
   else
     prompt_ptr = NULL;
 
-  return command_line_input (prompt_ptr, "commands");
+  return command_line_input (buffer, prompt_ptr, "commands");
 }
 
 /* Given an input line P, skip the command and return a pointer to the
@@ -1064,7 +1064,7 @@ process_next_line (const char *p, command_line_up *command,
    obtain lines of the command.  */
 
 static enum command_control_type
-recurse_read_control_structure (gdb::function_view<const char * ()> read_next_line_func,
+recurse_read_control_structure (read_next_line_ftype read_next_line_func,
 				struct command_line *current_cmd,
 				gdb::function_view<void (const char *)> validator)
 {
@@ -1085,8 +1085,9 @@ recurse_read_control_structure (gdb::function_view<const char * ()> read_next_li
     {
       dont_repeat ();
 
+      std::string buffer;
       next = nullptr;
-      val = process_next_line (read_next_line_func (), &next,
+      val = process_next_line (read_next_line_func (buffer), &next,
 			       current_cmd->control_type != python_control
 			       && current_cmd->control_type != guile_control
 			       && current_cmd->control_type != compile_control,
@@ -1215,7 +1216,7 @@ read_command_lines (const char *prompt_arg, int from_tty, int parse_commands,
    obtained using READ_NEXT_LINE_FUNC.  */
 
 counted_command_line
-read_command_lines_1 (gdb::function_view<const char * ()> read_next_line_func,
+read_command_lines_1 (read_next_line_ftype read_next_line_func,
 		      int parse_commands,
 		      gdb::function_view<void (const char *)> validator)
 {
@@ -1231,7 +1232,9 @@ read_command_lines_1 (gdb::function_view<const char * ()> read_next_line_func,
   while (1)
     {
       dont_repeat ();
-      val = process_next_line (read_next_line_func (), &next, parse_commands,
+
+      std::string buffer;
+      val = process_next_line (read_next_line_func (buffer), &next, parse_commands,
 			       validator);
 
       /* Ignore blank lines or comments.  */
diff --git a/gdb/cli/cli-script.h b/gdb/cli/cli-script.h
index 2b9483f..d9ca7de 100644
--- a/gdb/cli/cli-script.h
+++ b/gdb/cli/cli-script.h
@@ -112,11 +112,18 @@ private:
   }
 };
 
+/* Prototype for a function to call to get one more input line.
+
+   If the function needs to return a dynamically allocated string, it can place
+   in the passed-in buffer, and return a pointer to it.  Otherwise, it can
+   simply ignore it.  */
+
+using read_next_line_ftype = gdb::function_view<const char * (std::string &)>;
+
 extern counted_command_line read_command_lines
     (const char *, int, int, gdb::function_view<void (const char *)>);
 extern counted_command_line read_command_lines_1
-    (gdb::function_view<const char * ()>, int,
-     gdb::function_view<void (const char *)>);
+    (read_next_line_ftype, int, gdb::function_view<void (const char *)>);
 
 
 /* Exported to cli/cli-cmds.c */