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

Tom Tromey tom@tromey.com
Fri Aug 10 03:13:00 GMT 2018


Tom> I'm still going to write a history expansion test, because there should
Tom> be at least one.

Here's a patch with a test.

Tom

commit aea20ede47dfca4156f19c0b41e3a1b11e724c20
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.
    
    Due to some review comments by Philippe, I noticed that there were no
    existing tests for history expansion, so this patch adds one.
    
    gdb/ChangeLog
    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.
    
    gdb/testsuite/ChangeLog
    2018-08-09  Tom Tromey  <tom@tromey.com>
    
            * gdb.base/history.exp: New file.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 338813ab37..57747b4527 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-09  Jim Wilson  <jimw@sifive.com>
 
 	* Makefile.in (ALL_TARGET_OBS): Add riscv-linux-tdep.c.
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/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index bd3c3bfec5..1e9d767f64 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,7 @@
+2018-08-09  Tom Tromey  <tom@tromey.com>
+
+	* gdb.base/history.exp: New file.
+
 2018-08-09  Andrew Burgess  <andrew.burgess@embecosm.com>
 
 	* gdb.base/vla-optimized-out.exp: Add new test.
diff --git a/gdb/testsuite/gdb.base/history.exp b/gdb/testsuite/gdb.base/history.exp
new file mode 100644
index 0000000000..bdd65a7cc7
--- /dev/null
+++ b/gdb/testsuite/gdb.base/history.exp
@@ -0,0 +1,32 @@
+# Copyright 2018 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This file is part of the gdb testsuite.
+
+# Test history expansion.
+
+gdb_start
+
+# These tests require readline support.
+if { ![readline_is_used] } {
+    unsupported "readline isn't used."
+    return -1
+}
+
+gdb_test_no_output "set history expansion on"
+
+gdb_test "print 23" " = 23"
+
+gdb_test "!!" " = 23" "history expansion"
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