[RFA 1/2] Fix regressions for multi breakpoints command line setting/clearing

Tom Tromey tom@tromey.com
Thu Aug 9 04:57:00 GMT 2018


Tom> Yeah, I'd rather do the deeper fix, because otherwise we will have an
Tom> API that's difficult to use correctly.

Tom> But if I can't get it finished soon, I'll approve this.

I was very ill and so this didn't happen on quite the schedule I had hoped.
But, here's the patch I came up with.  Tested by the buildbot.  Let me
know what you think.

One oddity I noticed is that, currently, command repetition is global,
whereas it seems like it would be better per-ui.

Tom

commit d057d1227b386214d3a9527dfe61bf26e2512d69
Author: Tom Tromey <tom@tromey.com>
Date:   Sat Jul 28 11:03:09 2018 -0600

    Fix use-after-free in number_or_range_parser
    
    -fsanitize=address showed a use-after-free in number_or_range_parser.
    
    The cause was that handle_line_of_input could stash the input into
    "saved_command_line", and then this could be freed by reentrant calls.
    
    This fixes the bug by locating an "outer enough" caller to hold the
    storage for the command line, and then threading "struct buffer *"
    arguments through the call stack as needed.
    
    2018-08-08  Tom Tromey  <tom@tromey.com>
    
            * top.c (read_command_file): Update.
            (command_line_input): Add cmd_line_buffer argument.
            (execute_command): Check server_command, not saved_command_line,
            to see if repeating.
            * python/python.c (execute_gdb_command): Update.
            * python/py-gdb-readline.c (gdbpy_readline_wrapper): Update.
            * python/py-breakpoint.c (bppy_set_commands): Update.
            * mi/mi-cmd-break.c (mi_cmd_break_commands): Update.
            * linespec.c (decode_line_2): Update.
            * event-top.c (handle_line_of_input): Do not return
            saved_command_line.
            * defs.h (command_line_input): Add struct buffer * argument.
            * common/buffer.h (struct auto_buffer): New.
            * cli/cli-script.h (read_command_lines_1): Add struct buffer * to
            callback type.
            * cli/cli-script.c (read_next_line): Add "storage" argument.
            (recurse_read_control_structure): Update.  Use auto_buffer.
            (read_command_lines_1): Update.
            * breakpoint.c (read_uploaded_action): Update signature.
            * ada-lang.c (get_selections): Update.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 4d0593f163..fe32581b9b 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,26 @@
+2018-08-08  Tom Tromey  <tom@tromey.com>
+
+	* top.c (read_command_file): Update.
+	(command_line_input): Add cmd_line_buffer argument.
+	(execute_command): Check server_command, not saved_command_line,
+	to see if repeating.
+	* python/python.c (execute_gdb_command): Update.
+	* python/py-gdb-readline.c (gdbpy_readline_wrapper): Update.
+	* python/py-breakpoint.c (bppy_set_commands): Update.
+	* mi/mi-cmd-break.c (mi_cmd_break_commands): Update.
+	* linespec.c (decode_line_2): Update.
+	* event-top.c (handle_line_of_input): Do not return
+	saved_command_line.
+	* defs.h (command_line_input): Add struct buffer * argument.
+	* common/buffer.h (struct auto_buffer): New.
+	* cli/cli-script.h (read_command_lines_1): Add struct buffer * to
+	callback type.
+	* cli/cli-script.c (read_next_line): Add "storage" argument.
+	(recurse_read_control_structure): Update.  Use auto_buffer.
+	(read_command_lines_1): Update.
+	* breakpoint.c (read_uploaded_action): Update signature.
+	* ada-lang.c (get_selections): Update.
+
 2018-08-08  Simon Marchi  <simon.marchi@ericsson.com>
 
 	* target.c (str_comma_list_concat_elem): Fix typo in comment.
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 07a0536684..c4914837f0 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -62,6 +62,7 @@
 #include "cli/cli-utils.h"
 #include "common/function-view.h"
 #include "common/byte-vector.h"
+#include "common/buffer.h"
 #include <algorithm>
 
 /* Define whether or not the C operator '/' truncates towards zero for
@@ -4041,7 +4042,8 @@ get_selections (int *choices, int n_choices, int max_results,
   if (prompt == NULL)
     prompt = "> ";
 
-  args = command_line_input (prompt, 0, annotation_suffix);
+  auto_buffer storage;
+  args = command_line_input (prompt, 0, annotation_suffix, &storage);
 
   if (args == NULL)
     error_no_arg (_("one or more choice numbers"));
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 8f0feaa474..0bfc8193dd 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -14679,7 +14679,7 @@ static struct uploaded_tp *this_utp;
 static int next_cmd;
 
 static char *
-read_uploaded_action (void)
+read_uploaded_action (struct buffer *)
 {
   char *rslt = nullptr;
 
diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c
index 6f31a40019..dd897ea258 100644
--- a/gdb/cli/cli-script.c
+++ b/gdb/cli/cli-script.c
@@ -40,14 +40,14 @@
 
 static enum command_control_type
 recurse_read_control_structure
-    (gdb::function_view<const char * ()> read_next_line_func,
+    (gdb::function_view<const char * (struct buffer *)> read_next_line_func,
      struct command_line *current_cmd,
      gdb::function_view<void (const char *)> validator);
 
 static void do_define_command (const char *comname, int from_tty,
 			       const counted_command_line *commands);
 
-static char *read_next_line (void);
+static char *read_next_line (struct buffer *);
 
 /* Level of control structure when reading.  */
 static int control_level;
@@ -880,7 +880,7 @@ user_args::insert_args (const char *line) const
    from stdin.  */
 
 static char *
-read_next_line (void)
+read_next_line (struct buffer *storage)
 {
   struct ui *ui = current_ui;
   char *prompt_ptr, control_prompt[256];
@@ -903,7 +903,7 @@ read_next_line (void)
   else
     prompt_ptr = NULL;
 
-  return command_line_input (prompt_ptr, from_tty, "commands");
+  return command_line_input (prompt_ptr, from_tty, "commands", storage);
 }
 
 /* Return true if CMD's name is NAME.  */
@@ -1075,9 +1075,10 @@ process_next_line (const char *p, struct command_line **command,
    obtain lines of the command.  */
 
 static enum command_control_type
-recurse_read_control_structure (gdb::function_view<const char * ()> read_next_line_func,
-				struct command_line *current_cmd,
-				gdb::function_view<void (const char *)> validator)
+recurse_read_control_structure
+  (gdb::function_view<const char * (struct buffer *)> read_next_line_func,
+   struct command_line *current_cmd,
+   gdb::function_view<void (const char *)> validator)
 {
   enum misc_command_type val;
   enum command_control_type ret;
@@ -1095,8 +1096,9 @@ recurse_read_control_structure (gdb::function_view<const char * ()> read_next_li
     {
       dont_repeat ();
 
+      auto_buffer storage;
       next = NULL;
-      val = process_next_line (read_next_line_func (), &next, 
+      val = process_next_line (read_next_line_func (&storage), &next,
 			       current_cmd->control_type != python_control
 			       && current_cmd->control_type != guile_control
 			       && current_cmd->control_type != compile_control,
@@ -1223,9 +1225,10 @@ 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,
-		      int parse_commands,
-		      gdb::function_view<void (const char *)> validator)
+read_command_lines_1
+  (gdb::function_view<const char * (struct buffer *)> read_next_line_func,
+   int parse_commands,
+   gdb::function_view<void (const char *)> validator)
 {
   struct command_line *tail, *next;
   counted_command_line head (nullptr, command_lines_deleter ());
@@ -1238,7 +1241,8 @@ 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,
+      auto_buffer storage;
+      val = process_next_line (read_next_line_func (&storage), &next, parse_commands,
 			       validator);
 
       /* Ignore blank lines or comments.  */
diff --git a/gdb/cli/cli-script.h b/gdb/cli/cli-script.h
index 736ebb3a7b..fecba181b5 100644
--- a/gdb/cli/cli-script.h
+++ b/gdb/cli/cli-script.h
@@ -109,7 +109,7 @@ private:
 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<const char * (struct buffer *)>, int,
      gdb::function_view<void (const char *)>);
 
 
diff --git a/gdb/common/buffer.h b/gdb/common/buffer.h
index 9806fd8ad8..d93793422d 100644
--- a/gdb/common/buffer.h
+++ b/gdb/common/buffer.h
@@ -65,4 +65,22 @@ void buffer_xml_printf (struct buffer *buffer, const char *format, ...)
 #define buffer_grow_str0(BUFFER,STRING)			\
   buffer_grow (BUFFER, STRING, strlen (STRING) + 1)
 
+/* A buffer that frees itself on scope exit.  */
+struct auto_buffer : public buffer
+{
+  auto_buffer ()
+  {
+    buffer_init (this);
+  }
+
+  ~auto_buffer ()
+  {
+    buffer_free (this);
+  }
+
+private:
+
+  DISABLE_COPY_AND_ASSIGN (auto_buffer);
+};
+
 #endif
diff --git a/gdb/defs.h b/gdb/defs.h
index 4cf83f0d44..f066f9ec49 100644
--- a/gdb/defs.h
+++ b/gdb/defs.h
@@ -309,7 +309,8 @@ typedef void initialize_file_ftype (void);
 
 extern char *gdb_readline_wrapper (const char *);
 
-extern char *command_line_input (const char *, int, const char *);
+extern char *command_line_input (const char *, int, const char *,
+				 struct buffer *);
 
 extern void print_prompt (void);
 
diff --git a/gdb/event-top.c b/gdb/event-top.c
index 5852089f09..457488f3c6 100644
--- a/gdb/event-top.c
+++ b/gdb/event-top.c
@@ -714,7 +714,12 @@ handle_line_of_input (struct buffer *cmd_line_buffer,
   for (p1 = cmd; *p1 == ' ' || *p1 == '\t'; p1++)
     ;
   if (repeat && *p1 == '\0')
-    return saved_command_line;
+    {
+      xfree (buffer_finish (cmd_line_buffer));
+      buffer_grow_str0 (cmd_line_buffer, saved_command_line);
+      cmd_line_buffer->used_size = 0;
+      return cmd_line_buffer->buffer;
+    }
 
   /* Add command to history if appropriate.  Note: lines consisting
      solely of comments are also added to the command history.  This
@@ -731,10 +736,8 @@ handle_line_of_input (struct buffer *cmd_line_buffer,
     {
       xfree (saved_command_line);
       saved_command_line = xstrdup (cmd);
-      return saved_command_line;
     }
-  else
-    return cmd;
+  return cmd;
 }
 
 /* Handle a complete line of input.  This is called by the callback
diff --git a/gdb/linespec.c b/gdb/linespec.c
index 790ddf4740..314ff7dcdb 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -46,6 +46,7 @@
 #include "location.h"
 #include "common/function-view.h"
 #include "common/def-vector.h"
+#include "common/buffer.h"
 #include <algorithm>
 
 /* An enumeration of the various things a user might attempt to
@@ -1554,7 +1555,8 @@ decode_line_2 (struct linespec_state *self,
     {
       prompt = "> ";
     }
-  args = command_line_input (prompt, 0, "overload-choice");
+  auto_buffer storage;
+  args = command_line_input (prompt, 0, "overload-choice", &storage);
 
   if (args == 0 || *args == 0)
     error_no_arg (_("one or more choice numbers"));
diff --git a/gdb/mi/mi-cmd-break.c b/gdb/mi/mi-cmd-break.c
index 8897117bb8..1b77f47dde 100644
--- a/gdb/mi/mi-cmd-break.c
+++ b/gdb/mi/mi-cmd-break.c
@@ -494,7 +494,7 @@ mi_cmd_break_commands (const char *command, char **argv, int argc)
 
   int count = 1;
   auto reader
-    = [&] ()
+    = [&] (struct buffer *)
       {
 	const char *result = nullptr;
 	if (count < argc)
diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index e1db674647..6baf3f0a7c 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -530,7 +530,7 @@ bppy_set_commands (PyObject *self, PyObject *newvalue, void *closure)
       bool first = true;
       char *save_ptr = nullptr;
       auto reader
-	= [&] ()
+	= [&] (struct buffer *)
 	  {
 	    const char *result = strtok_r (first ? commands.get () : nullptr,
 					   "\n", &save_ptr);
diff --git a/gdb/python/py-gdb-readline.c b/gdb/python/py-gdb-readline.c
index a95be41c49..097c772d29 100644
--- a/gdb/python/py-gdb-readline.c
+++ b/gdb/python/py-gdb-readline.c
@@ -38,10 +38,11 @@ gdbpy_readline_wrapper (FILE *sys_stdin, FILE *sys_stdout,
 {
   int n;
   char *p = NULL, *q;
+  auto_buffer buffer;
 
   TRY
     {
-      p = command_line_input (prompt, 0, "python");
+      p = command_line_input (prompt, 0, "python", &buffer);
     }
   /* Handle errors by raising Python exceptions.  */
   CATCH (except, RETURN_MASK_ALL)
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 20fc674f20..2c9d80cb4d 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -592,7 +592,7 @@ execute_gdb_command (PyObject *self, PyObject *args, PyObject *kw)
       bool first = true;
       char *save_ptr = nullptr;
       auto reader
-	= [&] ()
+	= [&] (struct buffer *)
 	  {
 	    const char *result = strtok_r (first ? &arg_copy[0] : nullptr,
 					   "\n", &save_ptr);
diff --git a/gdb/top.c b/gdb/top.c
index de1a335e40..593cd133df 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -416,9 +416,10 @@ read_command_file (FILE *stream)
   while (ui->instream != NULL && !feof (ui->instream))
     {
       char *command;
+      auto_buffer storage;
 
       /* Get a command-line.  This calls the readline package.  */
-      command = command_line_input (NULL, 0, NULL);
+      command = command_line_input (NULL, 0, NULL, &storage);
       if (command == NULL)
 	break;
       command_handler (command);
@@ -634,7 +635,7 @@ execute_command (const char *p, int from_tty)
       /* If this command has been post-hooked, run the hook last.  */
       execute_cmd_post_hook (c);
 
-      if (repeat_arguments != NULL && cmd_start == saved_command_line)
+      if (repeat_arguments != NULL && !server_command)
 	{
 	  gdb_assert (strlen (args_pointer) >= strlen (repeat_arguments));
 	  strcpy (saved_command_line + (args_pointer - cmd_start),
@@ -1155,9 +1156,9 @@ gdb_safe_append_history (void)
     }
 }
 
-/* Read one line from the command input stream `instream' into a local
-   static buffer.  The buffer is made bigger as necessary.  Returns
-   the address of the start of the line.
+/* Read one line from the command input stream `instream' into a
+   buffer.  The buffer is made bigger as necessary.  Returns the
+   address of the start of the line.
 
    NULL is returned for end of file.
 
@@ -1170,10 +1171,9 @@ gdb_safe_append_history (void)
 
 char *
 command_line_input (const char *prompt_arg, int repeat,
-		    const char *annotation_suffix)
+		    const char *annotation_suffix,
+		    struct buffer *cmd_line_buffer)
 {
-  static struct buffer cmd_line_buffer;
-  static int cmd_line_buffer_initialized;
   struct ui *ui = current_ui;
   const char *prompt = prompt_arg;
   char *cmd;
@@ -1201,14 +1201,8 @@ command_line_input (const char *prompt_arg, int repeat,
       prompt = local_prompt;
     }
 
-  if (!cmd_line_buffer_initialized)
-    {
-      buffer_init (&cmd_line_buffer);
-      cmd_line_buffer_initialized = 1;
-    }
-
   /* Starting a new command line.  */
-  cmd_line_buffer.used_size = 0;
+  cmd_line_buffer->used_size = 0;
 
 #ifdef SIGTSTP
   if (job_control)
@@ -1254,7 +1248,7 @@ command_line_input (const char *prompt_arg, int repeat,
 	  rl = gdb_readline_no_editing (prompt);
 	}
 
-      cmd = handle_line_of_input (&cmd_line_buffer, rl,
+      cmd = handle_line_of_input (cmd_line_buffer, rl,
 				  repeat, annotation_suffix);
       if (cmd == (char *) EOF)
 	{



More information about the Gdb-patches mailing list