From bb6203bf1db908741de9355a6ae6969324e381d8 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Alexandra=20H=C3=A1jkov=C3=A1?= Date: Thu, 20 May 2021 20:55:35 +0200 Subject: [PATCH] cli-script: use unique_ptr to not leak next struct In cli/cli-script.c, process_next_line() allocates memory which will eventually end up being assigned to the 'next' field in struct command_line. However, in a case recurse_read_control_structure returns 'invalid_control' this memory is leaked. This commit uses std::unique_ptr as appropriate to prevent this leakage. This issue was found by coverity scanning. gdb/ChangeLog: * cli/cli-script.h (command_line_up): New unique_ptr typedef. * cli/cli-script.c (multi_line_command_p): Use unique_ptr command_line_up instead of struct command_line. (build_command_line): Likewise. (get_command_line): Update the cmd function call parameter. (process_next_line): Use unique_ptr command_line_up instead of struct command_line. (recurse_read_control_structure): Change the the type of next to command_line_up. (read_command_lines_1): Change type of `next' to be command_line_up and update all references of `next' accordingly. --- gdb/ChangeLog | 16 ++++++++++ gdb/cli/cli-script.c | 72 ++++++++++++++++++++++---------------------- gdb/cli/cli-script.h | 3 ++ 3 files changed, 55 insertions(+), 36 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index de0fd2d6f1..916b17e395 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,19 @@ +2021-05-20 Alexandra Hájková + Pedro Alves + + * cli/cli-script.h (command_line_up): New unique_ptr typedef. + * cli/cli-script.c (multi_line_command_p): Use unique_ptr + command_line_up instead of struct command_line. + (build_command_line): Likewise. + (get_command_line): Update the cmd function call parameter. + (process_next_line): Use unique_ptr command_line_up instead + of struct command_line. + (recurse_read_control_structure): Change the the type of + next to command_line_up. + (read_command_lines_1): Change type of `next' to be + command_line_up and update all references of `next' + accordingly. + 2021-05-20 Alexandra Hájková * MAINTAINERS (Write After Approval): Add myself. diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c index ff93523f4d..9846367790 100644 --- a/gdb/cli/cli-script.c +++ b/gdb/cli/cli-script.c @@ -155,7 +155,7 @@ multi_line_command_p (enum command_control_type type) /* Allocate, initialize a new command line structure for one of the control commands (if/while). */ -static struct command_line * +static command_line_up build_command_line (enum command_control_type type, const char *args) { if (args == NULL || *args == '\0') @@ -171,7 +171,7 @@ build_command_line (enum command_control_type type, const char *args) } gdb_assert (args != NULL); - return new struct command_line (type, xstrdup (args)); + return command_line_up (new command_line (type, xstrdup (args))); } /* Build and return a new command structure for the control commands @@ -181,7 +181,7 @@ counted_command_line get_command_line (enum command_control_type type, const char *arg) { /* Allocate and build a new command line structure. */ - counted_command_line cmd (build_command_line (type, arg), + counted_command_line cmd (build_command_line (type, arg).release (), command_lines_deleter ()); /* Read in the body of this command. */ @@ -957,7 +957,7 @@ line_first_arg (const char *p) Otherwise, only "end" is recognized. */ static enum misc_command_type -process_next_line (const char *p, struct command_line **command, +process_next_line (const char *p, command_line_up *command, int parse_commands, gdb::function_view validator) @@ -1055,9 +1055,9 @@ process_next_line (const char *p, struct command_line **command, *command = build_command_line (guile_control, ""); } else if (p_end - p == 10 && startswith (p, "loop_break")) - *command = new struct command_line (break_control); + *command = command_line_up (new command_line (break_control)); else if (p_end - p == 13 && startswith (p, "loop_continue")) - *command = new struct command_line (continue_control); + *command = command_line_up (new command_line (continue_control)); else not_handled = 1; } @@ -1065,22 +1065,12 @@ process_next_line (const char *p, struct command_line **command, if (!parse_commands || not_handled) { /* A normal command. */ - *command = new struct command_line (simple_control, - savestring (p, p_end - p)); + *command = command_line_up (new command_line (simple_control, + savestring (p, p_end - p))); } if (validator) - { - try - { - validator ((*command)->line); - } - catch (const gdb_exception &ex) - { - free_command_lines (command); - throw; - } - } + validator ((*command)->line); /* Nothing special. */ return ok_command; @@ -1097,10 +1087,11 @@ recurse_read_control_structure (gdb::function_view read_next_li { enum misc_command_type val; enum command_control_type ret; - struct command_line *child_tail, *next; + struct command_line *child_tail; counted_command_line *current_body = ¤t_cmd->body_list_0; + command_line_up next; - child_tail = NULL; + child_tail = nullptr; /* Sanity checks. */ if (current_cmd->control_type == simple_control) @@ -1111,8 +1102,8 @@ recurse_read_control_structure (gdb::function_view read_next_li { dont_repeat (); - next = NULL; - val = process_next_line (read_next_line_func (), &next, + next = nullptr; + val = process_next_line (read_next_line_func (), &next, current_cmd->control_type != python_control && current_cmd->control_type != guile_control && current_cmd->control_type != compile_control, @@ -1144,7 +1135,7 @@ recurse_read_control_structure (gdb::function_view read_next_li && current_body == ¤t_cmd->body_list_0) { current_body = ¤t_cmd->body_list_1; - child_tail = NULL; + child_tail = nullptr; continue; } else @@ -1154,21 +1145,26 @@ recurse_read_control_structure (gdb::function_view read_next_li } } - if (child_tail) + /* Transfer ownership of NEXT to the command's body list. */ + if (child_tail != nullptr) { - child_tail->next = next; + child_tail->next = next.release (); + child_tail = child_tail->next; } else - *current_body = counted_command_line (next, command_lines_deleter ()); - - child_tail = next; + { + child_tail = next.get (); + *current_body = counted_command_line (next.release (), + command_lines_deleter ()); + } /* If the latest line is another control structure, then recurse on it. */ - if (multi_line_command_p (next->control_type)) + if (multi_line_command_p (child_tail->control_type)) { control_level++; - ret = recurse_read_control_structure (read_next_line_func, next, + ret = recurse_read_control_structure (read_next_line_func, + child_tail, validator); control_level--; @@ -1240,10 +1236,11 @@ read_command_lines_1 (gdb::function_view read_next_line_func, int parse_commands, gdb::function_view validator) { - struct command_line *tail, *next; + struct command_line *tail; counted_command_line head (nullptr, command_lines_deleter ()); enum command_control_type ret; enum misc_command_type val; + command_line_up next; control_level = 0; tail = NULL; @@ -1273,7 +1270,7 @@ read_command_lines_1 (gdb::function_view read_next_line_func, if (multi_line_command_p (next->control_type)) { control_level++; - ret = recurse_read_control_structure (read_next_line_func, next, + ret = recurse_read_control_structure (read_next_line_func, next.get (), validator); control_level--; @@ -1281,15 +1278,18 @@ read_command_lines_1 (gdb::function_view read_next_line_func, break; } + /* Transfer ownership of NEXT to the HEAD list. */ if (tail) { - tail->next = next; + tail->next = next.release (); + tail = tail->next; } else { - head = counted_command_line (next, command_lines_deleter ()); + tail = next.get (); + head = counted_command_line (next.release (), + command_lines_deleter ()); } - tail = next; } dont_repeat (); diff --git a/gdb/cli/cli-script.h b/gdb/cli/cli-script.h index 8624bf5583..8c98fdde65 100644 --- a/gdb/cli/cli-script.h +++ b/gdb/cli/cli-script.h @@ -66,6 +66,9 @@ struct command_lines_deleter /* A reference-counted struct command_line. */ typedef std::shared_ptr counted_command_line; +/* A unique_ptr specialization for command_line. */ +typedef std::unique_ptr command_line_up; + /* * Structure for saved commands lines (for breakpoints, defined commands, etc). */ -- 2.34.1