From patchwork Thu Aug 9 04:57:31 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tom Tromey X-Patchwork-Id: 28810 Received: (qmail 13006 invoked by alias); 9 Aug 2018 04:57:37 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 12990 invoked by uid 89); 9 Aug 2018 04:57:36 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.5 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=locating, multi, deeper, *result X-HELO: gateway23.websitewelcome.com Received: from gateway23.websitewelcome.com (HELO gateway23.websitewelcome.com) (192.185.49.124) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 09 Aug 2018 04:57:33 +0000 Received: from cm15.websitewelcome.com (cm15.websitewelcome.com [100.42.49.9]) by gateway23.websitewelcome.com (Postfix) with ESMTP id 488C656 for ; Wed, 8 Aug 2018 23:57:32 -0500 (CDT) Received: from box5379.bluehost.com ([162.241.216.53]) by cmsmtp with SMTP id nd0ufVoXybXuJnd0ufuduK; Wed, 08 Aug 2018 23:57:32 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=tromey.com; s=default; h=Content-Type:MIME-Version:Message-ID:In-Reply-To:Date: References:Subject:Cc:To:From:Sender:Reply-To:Content-Transfer-Encoding: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=e/dbwOwRloimhggRLgU8O3iqg+MXbHAEaLTnkKWIB1U=; b=Lt0h+j5pyALkYdlevhpbCtZVJ2 mC7awJKzjCyaBsZOCaA6lr3+CeyW20dLomRRIU2Lv1Ix1ik9vDqKnmybXLoFXgYmBdXP3BIs6ws2A /3UQN8RiapMg34qriuK6qJQXe; Received: from 75-166-85-72.hlrn.qwest.net ([75.166.85.72]:36188 helo=bapiya) by box5379.bluehost.com with esmtpsa (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.91) (envelope-from ) id 1fnd0t-003OKd-Vm; Wed, 08 Aug 2018 23:57:32 -0500 From: Tom Tromey To: Tom Tromey Cc: Philippe Waroquiers , gdb-patches@sourceware.org Subject: Re: [RFA 1/2] Fix regressions for multi breakpoints command line setting/clearing References: <20180802212613.29813-1-philippe.waroquiers@skynet.be> <20180802212613.29813-2-philippe.waroquiers@skynet.be> <87sh3v1ezc.fsf@tromey.com> Date: Wed, 08 Aug 2018 22:57:31 -0600 In-Reply-To: <87sh3v1ezc.fsf@tromey.com> (Tom Tromey's message of "Fri, 03 Aug 2018 12:28:55 -0600") Message-ID: <87lg9gi1c4.fsf@tromey.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1.50 (gnu/linux) MIME-Version: 1.0 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 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 * 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 + + * 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 * 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 /* 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 read_next_line_func, + (gdb::function_view read_next_line_func, struct command_line *current_cmd, gdb::function_view 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 read_next_line_func, - struct command_line *current_cmd, - gdb::function_view validator) +recurse_read_control_structure + (gdb::function_view read_next_line_func, + struct command_line *current_cmd, + gdb::function_view validator) { enum misc_command_type val; enum command_control_type ret; @@ -1095,8 +1096,9 @@ recurse_read_control_structure (gdb::function_view 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 read_next_line_func, - int parse_commands, - gdb::function_view validator) +read_command_lines_1 + (gdb::function_view read_next_line_func, + int parse_commands, + gdb::function_view 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 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); extern counted_command_line read_command_lines_1 - (gdb::function_view, int, + (gdb::function_view, int, gdb::function_view); 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 /* 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) {