[RFA,2/8] Use counted_command_line everywhere

Message ID 20180419191539.661-3-tom@tromey.com
State Superseded
Headers show
Series
  • Various command-related improvements
Related show

Commit Message

Tom Tromey April 19, 2018, 7:15 p.m.
Currently command lines are reference counted using shared_ptr only
when attached to breakpoints.  This patch changes gdb to use
shared_ptr in commands as well.  This allows for the removal of
copy_command_lines.

Note that the change to execute_user_command explicitly makes a new
reference to the command line.  This will be used in a later patch.

This simplifies struct command_line based on the observation that a
given command can have at most two child bodies: an "if" can have both
"then" and "else" parts.  Perhaps the names I've chosen for the
replacements here are not very good -- your input requested.

ChangeLog
2018-04-19  Tom Tromey  <tom@tromey.com>

	* tracepoint.c (all_tracepoint_actions): Rename from
	all_tracepoint_actions_and_cleanup.  Change return type.
	(actions_command, encode_actions_1, encode_actions)
	(trace_dump_actions, tdump_command): Update.
	* remote.c (remote_download_command_source): Update.
	* python/python.c (gdbpy_eval_from_control_command)
	(python_command, python_interactive_command): Update.
	* mi/mi-cmd-break.c (mi_cmd_break_commands): Update.
	* guile/guile.c (guile_command)
	(gdbscm_eval_from_control_command, guile_command): Update.
	* compile/compile.c (compile_code_command)
	(compile_print_command, compile_to_object): Update.
	* cli/cli-script.h (struct command_lines_deleter): New.
	(counted_command_line): New typedef.
	(struct command_line): Add constructor, destructor.
	<body_list>: Remove.
	<body_list_0, body_list_1>: New members.
	(command_line_up): Remove typedef.
	(read_command_lines, read_command_lines_1, get_command_line):
	Update.
	(copy_command_lines): Don't declare.
	* cli/cli-script.c (build_command_line): Use "new".
	(get_command_line): Return counted_command_line.
	(print_command_lines, execute_user_command)
	(execute_control_command_1, while_command, if_command): Update.
	(realloc_body_list): Remove.
	(process_next_line, recurse_read_control_structure): Update.
	(read_command_lines, read_command_lines_1): Return counted_command_line.
	(free_command_lines): Use "delete".
	(copy_command_lines): Remove.
	(define_command, document_command, show_user_1): Update.
	* cli/cli-decode.h (struct cmd_list_element) <user_commands>: Now
	a counted_command_line.
	* breakpoint.h (counted_command_line): Remove typedef.
	(breakpoint_set_commands): Update.
	* breakpoint.c (check_no_tracepoint_commands)
	(validate_commands_for_breakpoint): Update.
	(breakpoint_set_commands): Change commands to be a
	counted_command_line.
	(commands_command_1, update_dprintf_command_list)
	(create_tracepoint_from_upload): Update.
---
 gdb/ChangeLog         |  44 ++++++++++++
 gdb/breakpoint.c      |  30 ++++----
 gdb/breakpoint.h      |   6 +-
 gdb/cli/cli-decode.h  |   3 +-
 gdb/cli/cli-script.c  | 190 ++++++++++++--------------------------------------
 gdb/cli/cli-script.h  |  66 +++++++++++-------
 gdb/compile/compile.c |   6 +-
 gdb/guile/guile.c     |   8 +--
 gdb/mi/mi-cmd-break.c |   2 +-
 gdb/python/python.c   |   8 +--
 gdb/remote.c          |   2 +-
 gdb/tracepoint.c      |  62 +++++++---------
 12 files changed, 184 insertions(+), 243 deletions(-)

-- 
2.13.6

Comments

Pedro Alves April 24, 2018, 4:43 p.m. | #1
On 04/19/2018 08:15 PM, Tom Tromey wrote:
> Currently command lines are reference counted using shared_ptr only

> when attached to breakpoints.  This patch changes gdb to use

> shared_ptr in commands as well.  This allows for the removal of

> copy_command_lines.

> 

> Note that the change to execute_user_command explicitly makes a new

> reference to the command line.  This will be used in a later patch.

> 

> This simplifies struct command_line based on the observation that a

> given command can have at most two child bodies: an "if" can have both

> "then" and "else" parts.  Perhaps the names I've chosen for the

> replacements here are not very good -- your input requested.

> 


Names look fine to me.

> @@ -2663,11 +2663,10 @@ trace_dump_actions (struct command_line *action,

>  

>        if (cmd_cfunc_eq (cmd, while_stepping_pseudocommand))

>  	{

> -	  int i;

> -

> -	  for (i = 0; i < action->body_count; ++i)

> -	    trace_dump_actions (action->body_list[i],

> -				1, stepping_frame, from_tty);

> +	  trace_dump_actions (action->body_list_0.get (),

> +			      1, stepping_frame, from_tty);

> +	  trace_dump_actions (action->body_list_1.get (),

> +			      1, stepping_frame, from_tty);


Hmm, this looked suspicious.  I'm not seeing why would a
while-stepping action have two body lists.  I guess this
happens to work because trace_dump_actions does nothing
if ACTION is NULL.

> @@ -2797,17 +2792,13 @@ all_tracepoint_actions_and_cleanup (struct breakpoint *t)

>    if (*default_collect)

>      {

>        struct command_line *default_collect_action;

> -      char *default_collect_line;

> -

> -      default_collect_line = xstrprintf ("collect %s", default_collect);

> -      make_cleanup (xfree, default_collect_line);

> +      gdb::unique_xmalloc_ptr<char> default_collect_line

> +	(xstrprintf ("collect %s", default_collect));


string_printf ?

>  

> -      validate_actionline (default_collect_line, t);

> -      default_collect_action = XNEW (struct command_line);

> -      make_cleanup (xfree, default_collect_action);

> -      default_collect_action->next = actions;

> -      default_collect_action->line = default_collect_line;

> -      actions = default_collect_action;

> +      validate_actionline (default_collect_line.get (), t);

> +      actions.reset (new struct command_line (simple_control,

> +					      default_collect_line.release ()),

> +		     command_lines_deleter ());

>      }


LGTM.

It's a shame that we can't use std::make_shared to avoid
the separate control block allocation.

Thanks,
Pedro Alves
Tom Tromey April 24, 2018, 11:11 p.m. | #2
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:


>> -	  for (i = 0; i < action->body_count; ++i)

>> -	    trace_dump_actions (action->body_list[i],

>> -				1, stepping_frame, from_tty);

>> +	  trace_dump_actions (action->body_list_0.get (),

>> +			      1, stepping_frame, from_tty);

>> +	  trace_dump_actions (action->body_list_1.get (),

>> +			      1, stepping_frame, from_tty);


Pedro> Hmm, this looked suspicious.  I'm not seeing why would a
Pedro> while-stepping action have two body lists.  I guess this
Pedro> happens to work because trace_dump_actions does nothing
Pedro> if ACTION is NULL.

Yeah.  If you prefer I can add an assert that body_list_1==nullptr,
which seems like it ought to always be true.

>> +      gdb::unique_xmalloc_ptr<char> default_collect_line

>> +	(xstrprintf ("collect %s", default_collect));


Pedro> string_printf ?

I'll do that.

Pedro> It's a shame that we can't use std::make_shared to avoid
Pedro> the separate control block allocation.

It could be done if we changed the representation a bit, but it looked
like a pain to me, so I didn't.

Tom
Tom Tromey April 24, 2018, 11:18 p.m. | #3
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:


>> +      gdb::unique_xmalloc_ptr<char> default_collect_line

>> +	(xstrprintf ("collect %s", default_collect));


Pedro> string_printf ?

Actually, a few lines later:

>> +      actions.reset (new struct command_line (simple_control,

>> +					      default_collect_line.release ()),


So this couldn't use string_printf without additional work.

Tom

Patch

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 11b89bcf88..2cb9c4b657 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -1015,8 +1015,8 @@  check_no_tracepoint_commands (struct command_line *commands)
 	error (_("The 'while-stepping' command can "
 		 "only be used for tracepoints"));
 
-      for (i = 0; i < c->body_count; ++i)
-	check_no_tracepoint_commands ((c->body_list)[i]);
+      check_no_tracepoint_commands (c->body_list_0.get ());
+      check_no_tracepoint_commands (c->body_list_1.get ());
 
       /* Not that command parsing removes leading whitespace and comment
 	 lines and also empty lines.  So, we only need to check for
@@ -1127,8 +1127,8 @@  validate_commands_for_breakpoint (struct breakpoint *b,
 	{
 	  struct command_line *c2;
 
-	  gdb_assert (while_stepping->body_count == 1);
-	  c2 = while_stepping->body_list[0];
+	  gdb_assert (while_stepping->body_list_1 == nullptr);
+	  c2 = while_stepping->body_list_0.get ();
 	  for (; c2; c2 = c2->next)
 	    {
 	      if (c2->control_type == while_stepping_control)
@@ -1168,7 +1168,7 @@  static_tracepoints_here (CORE_ADDR addr)
 
 void
 breakpoint_set_commands (struct breakpoint *b, 
-			 command_line_up &&commands)
+			 counted_command_line &&commands)
 {
   validate_commands_for_breakpoint (b, commands.get ());
 
@@ -1248,7 +1248,7 @@  commands_command_1 (const char *arg, int from_tty,
        if (cmd == NULL)
 	 {
 	   if (control != NULL)
-	     cmd = copy_command_lines (control->body_list[0]);
+	     cmd = control->body_list_0;
 	   else
 	     {
 	       std::string str
@@ -8781,18 +8781,12 @@  update_dprintf_command_list (struct breakpoint *b)
 		    _("Invalid dprintf style."));
 
   gdb_assert (printf_line != NULL);
-  /* Manufacture a printf sequence.  */
-  {
-    struct command_line *printf_cmd_line = XNEW (struct command_line);
-
-    printf_cmd_line->control_type = simple_control;
-    printf_cmd_line->body_count = 0;
-    printf_cmd_line->body_list = NULL;
-    printf_cmd_line->next = NULL;
-    printf_cmd_line->line = printf_line;
 
-    breakpoint_set_commands (b, command_line_up (printf_cmd_line));
-  }
+  /* Manufacture a printf sequence.  */
+  struct command_line *printf_cmd_line
+    = new struct command_line (simple_control, printf_line);
+  breakpoint_set_commands (b, counted_command_line (printf_cmd_line,
+						    command_lines_deleter ()));
 }
 
 /* Update all dprintf commands, making their command lists reflect
@@ -14796,7 +14790,7 @@  create_tracepoint_from_upload (struct uploaded_tp *utp)
      function.  */
   if (!utp->cmd_strings.empty ())
     {
-      command_line_up cmd_list;
+      counted_command_line cmd_list;
 
       this_utp = utp;
       next_cmd = 0;
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 062e390469..f3c2599286 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -661,10 +661,6 @@  enum watchpoint_triggered
 typedef struct bp_location *bp_location_p;
 DEF_VEC_P(bp_location_p);
 
-/* A reference-counted struct command_line. This is an implementation
-   detail to the breakpoints module.  */
-typedef std::shared_ptr<command_line> counted_command_line;
-
 /* Some targets (e.g., embedded PowerPC) need two debug registers to set
    a watchpoint over a memory region.  If this flag is true, GDB will use
    only one register per watchpoint, thus assuming that all acesses that
@@ -1479,7 +1475,7 @@  extern void disable_breakpoint (struct breakpoint *);
 extern void enable_breakpoint (struct breakpoint *);
 
 extern void breakpoint_set_commands (struct breakpoint *b, 
-				     command_line_up &&commands);
+				     counted_command_line &&commands);
 
 extern void breakpoint_set_silent (struct breakpoint *b, int silent);
 
diff --git a/gdb/cli/cli-decode.h b/gdb/cli/cli-decode.h
index a3208e1fc6..e18b709767 100644
--- a/gdb/cli/cli-decode.h
+++ b/gdb/cli/cli-decode.h
@@ -24,6 +24,7 @@ 
 /* Include the public interfaces.  */
 #include "command.h"
 #include "gdb_regex.h"
+#include "cli-script.h"
 
 #if 0
 /* FIXME: cagney/2002-03-17: Once cmd_type() has been removed, ``enum
@@ -213,7 +214,7 @@  struct cmd_list_element
     const char *const *enums = nullptr;
 
     /* Pointer to command strings of user-defined commands */
-    struct command_line *user_commands = nullptr;
+    counted_command_line user_commands;
 
     /* Pointer to command that is hooked by this one, (by hook_pre)
        so the hook can be removed when this one is deleted.  */
diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c
index cdfda113a7..b066da7d60 100644
--- a/gdb/cli/cli-script.c
+++ b/gdb/cli/cli-script.c
@@ -134,32 +134,23 @@  multi_line_command_p (enum command_control_type type)
 static struct command_line *
 build_command_line (enum command_control_type type, const char *args)
 {
-  struct command_line *cmd;
-
   if ((args == NULL || *args == '\0')
       && (type == if_control || type == while_control))
     error (_("if/while commands require arguments."));
   gdb_assert (args != NULL);
 
-  cmd = XNEW (struct command_line);
-  cmd->next = NULL;
-  cmd->control_type = type;
-
-  cmd->body_count = 1;
-  cmd->body_list = XCNEWVEC (struct command_line *, cmd->body_count);
-  cmd->line = xstrdup (args);
-
-  return cmd;
+  return new struct command_line (type, xstrdup (args));
 }
 
 /* Build and return a new command structure for the control commands
    such as "if" and "while".  */
 
-command_line_up
+counted_command_line
 get_command_line (enum command_control_type type, const char *arg)
 {
   /* Allocate and build a new command line structure.  */
-  command_line_up cmd (build_command_line (type, arg));
+  counted_command_line cmd (build_command_line (type, arg),
+			    command_lines_deleter ());
 
   /* Read in the body of this command.  */
   if (recurse_read_control_structure (read_next_line, cmd.get (), 0, 0)
@@ -228,7 +219,7 @@  print_command_lines (struct ui_out *uiout, struct command_line *cmd,
 	  else
 	    uiout->field_string (NULL, list->line);
 	  uiout->text ("\n");
-	  print_command_lines (uiout, *list->body_list, depth + 1);
+	  print_command_lines (uiout, list->body_list_0.get (), depth + 1);
 	  if (depth)
 	    uiout->spaces (2 * depth);
 	  uiout->field_string (NULL, "end");
@@ -244,16 +235,16 @@  print_command_lines (struct ui_out *uiout, struct command_line *cmd,
 	  uiout->field_fmt (NULL, "if %s", list->line);
 	  uiout->text ("\n");
 	  /* The true arm.  */
-	  print_command_lines (uiout, list->body_list[0], depth + 1);
+	  print_command_lines (uiout, list->body_list_0.get (), depth + 1);
 
 	  /* Show the false arm if it exists.  */
-	  if (list->body_count == 2)
+	  if (list->body_list_1 != nullptr)
 	    {
 	      if (depth)
 		uiout->spaces (2 * depth);
 	      uiout->field_string (NULL, "else");
 	      uiout->text ("\n");
-	      print_command_lines (uiout, list->body_list[1], depth + 1);
+	      print_command_lines (uiout, list->body_list_1.get (), depth + 1);
 	    }
 
 	  if (depth)
@@ -273,7 +264,7 @@  print_command_lines (struct ui_out *uiout, struct command_line *cmd,
 	  else
 	    uiout->field_string (NULL, "commands");
 	  uiout->text ("\n");
-	  print_command_lines (uiout, *list->body_list, depth + 1);
+	  print_command_lines (uiout, list->body_list_0.get (), depth + 1);
 	  if (depth)
 	    uiout->spaces (2 * depth);
 	  uiout->field_string (NULL, "end");
@@ -287,7 +278,7 @@  print_command_lines (struct ui_out *uiout, struct command_line *cmd,
 	  uiout->field_string (NULL, "python");
 	  uiout->text ("\n");
 	  /* Don't indent python code at all.  */
-	  print_command_lines (uiout, *list->body_list, 0);
+	  print_command_lines (uiout, list->body_list_0.get (), 0);
 	  if (depth)
 	    uiout->spaces (2 * depth);
 	  uiout->field_string (NULL, "end");
@@ -300,7 +291,7 @@  print_command_lines (struct ui_out *uiout, struct command_line *cmd,
 	{
 	  uiout->field_string (NULL, "compile expression");
 	  uiout->text ("\n");
-	  print_command_lines (uiout, *list->body_list, 0);
+	  print_command_lines (uiout, list->body_list_0.get (), 0);
 	  if (depth)
 	    uiout->spaces (2 * depth);
 	  uiout->field_string (NULL, "end");
@@ -313,7 +304,7 @@  print_command_lines (struct ui_out *uiout, struct command_line *cmd,
 	{
 	  uiout->field_string (NULL, "guile");
 	  uiout->text ("\n");
-	  print_command_lines (uiout, *list->body_list, depth + 1);
+	  print_command_lines (uiout, list->body_list_0.get (), depth + 1);
 	  if (depth)
 	    uiout->spaces (2 * depth);
 	  uiout->field_string (NULL, "end");
@@ -377,14 +368,17 @@  void
 execute_user_command (struct cmd_list_element *c, const char *args)
 {
   struct ui *ui = current_ui;
-  struct command_line *cmdlines;
+  counted_command_line cmdlines_copy;
   enum command_control_type ret;
   extern unsigned int max_user_call_depth;
 
-  cmdlines = c->user_commands;
-  if (cmdlines == 0)
+  /* Ensure that the user commands can't be deleted while they are
+     executing.  */
+  cmdlines_copy = c->user_commands;
+  if (cmdlines_copy == 0)
     /* Null command */
     return;
+  struct command_line *cmdlines = cmdlines_copy.get ();
 
   scoped_user_args_level push_user_args (args);
 
@@ -527,7 +521,7 @@  execute_control_command_1 (struct command_line *cmd)
 	      break;
 
 	    /* Execute the body of the while statement.  */
-	    current = *cmd->body_list;
+	    current = cmd->body_list_0.get ();
 	    while (current)
 	      {
 		scoped_restore save_nesting
@@ -581,9 +575,9 @@  execute_control_command_1 (struct command_line *cmd)
 	/* Choose which arm to take commands from based on the value
 	   of the conditional expression.  */
 	if (value_true (val))
-	  current = *cmd->body_list;
-	else if (cmd->body_count == 2)
-	  current = *(cmd->body_list + 1);
+	  current = cmd->body_list_0.get ();
+	else if (cmd->body_list_1 != nullptr)
+	  current = cmd->body_list_1.get ();
 	value_free_to_mark (val_mark);
 
 	/* Execute commands in the given arm.  */
@@ -665,7 +659,7 @@  static void
 while_command (const char *arg, int from_tty)
 {
   control_level = 1;
-  command_line_up command = get_command_line (while_control, arg);
+  counted_command_line command = get_command_line (while_control, arg);
 
   if (command == NULL)
     return;
@@ -682,7 +676,7 @@  static void
 if_command (const char *arg, int from_tty)
 {
   control_level = 1;
-  command_line_up command = get_command_line (if_control, arg);
+  counted_command_line command = get_command_line (if_control, arg);
 
   if (command == NULL)
     return;
@@ -828,31 +822,6 @@  user_args::insert_args (const char *line) const
 }
 
 
-/* Expand the body_list of COMMAND so that it can hold NEW_LENGTH
-   code bodies.  This is typically used when we encounter an "else"
-   clause for an "if" command.  */
-
-static void
-realloc_body_list (struct command_line *command, int new_length)
-{
-  int n;
-  struct command_line **body_list;
-
-  n = command->body_count;
-
-  /* Nothing to do?  */
-  if (new_length <= n)
-    return;
-
-  body_list = XCNEWVEC (struct command_line *, new_length);
-
-  memcpy (body_list, command->body_list, sizeof (struct command_line *) * n);
-
-  xfree (command->body_list);
-  command->body_list = body_list;
-  command->body_count = new_length;
-}
-
 /* Read next line from stdin.  Passed to read_command_line_1 and
    recurse_read_control_structure whenever we need to read commands
    from stdin.  */
@@ -1012,23 +981,9 @@  process_next_line (char *p, struct command_line **command, int parse_commands,
 	  *command = build_command_line (guile_control, "");
 	}
       else if (p_end - p == 10 && startswith (p, "loop_break"))
-	{
-	  *command = XNEW (struct command_line);
-	  (*command)->next = NULL;
-	  (*command)->line = NULL;
-	  (*command)->control_type = break_control;
-	  (*command)->body_count = 0;
-	  (*command)->body_list = NULL;
-	}
+	*command = new struct command_line (break_control);
       else if (p_end - p == 13 && startswith (p, "loop_continue"))
-	{
-	  *command = XNEW (struct command_line);
-	  (*command)->next = NULL;
-	  (*command)->line = NULL;
-	  (*command)->control_type = continue_control;
-	  (*command)->body_count = 0;
-	  (*command)->body_list = NULL;
-	}
+	*command = new struct command_line (continue_control);
       else
 	not_handled = 1;
     }
@@ -1036,12 +991,8 @@  process_next_line (char *p, struct command_line **command, int parse_commands,
   if (!parse_commands || not_handled)
     {
       /* A normal command.  */
-      *command = XNEW (struct command_line);
-      (*command)->next = NULL;
-      (*command)->line = savestring (p, p_end - p);
-      (*command)->control_type = simple_control;
-      (*command)->body_count = 0;
-      (*command)->body_list = NULL;
+      *command = new struct command_line (simple_control,
+					  savestring (p, p_end - p));
     }
 
   if (validator)
@@ -1053,7 +1004,7 @@  process_next_line (char *p, struct command_line **command, int parse_commands,
 	}
       CATCH (ex, RETURN_MASK_ALL)
 	{
-	  xfree (*command);
+	  free_command_lines (command);
 	  throw_exception (ex);
 	}
       END_CATCH
@@ -1073,21 +1024,17 @@  recurse_read_control_structure (char * (*read_next_line_func) (void),
 				void (*validator)(char *, void *),
 				void *closure)
 {
-  int current_body, i;
   enum misc_command_type val;
   enum command_control_type ret;
   struct command_line **body_ptr, *child_tail, *next;
+  counted_command_line *current_body = &current_cmd->body_list_0;
 
   child_tail = NULL;
-  current_body = 1;
 
   /* Sanity checks.  */
   if (current_cmd->control_type == simple_control)
     error (_("Recursed on a simple control type."));
 
-  if (current_body > current_cmd->body_count)
-    error (_("Allocated body is smaller than this command type needs."));
-
   /* Read lines from the input stream and build control structures.  */
   while (1)
     {
@@ -1123,10 +1070,9 @@  recurse_read_control_structure (char * (*read_next_line_func) (void),
       if (val == else_command)
 	{
 	  if (current_cmd->control_type == if_control
-	      && current_body == 1)
+	      && current_body == &current_cmd->body_list_0)
 	    {
-	      realloc_body_list (current_cmd, 2);
-	      current_body = 2;
+	      current_body = &current_cmd->body_list_1;
 	      child_tail = NULL;
 	      continue;
 	    }
@@ -1142,14 +1088,7 @@  recurse_read_control_structure (char * (*read_next_line_func) (void),
 	  child_tail->next = next;
 	}
       else
-	{
-	  body_ptr = current_cmd->body_list;
-	  for (i = 1; i < current_body; i++)
-	    body_ptr++;
-
-	  *body_ptr = next;
-
-	}
+	*current_body = counted_command_line (next, command_lines_deleter ());
 
       child_tail = next;
 
@@ -1183,7 +1122,7 @@  recurse_read_control_structure (char * (*read_next_line_func) (void),
 
 #define END_MESSAGE "End with a line saying just \"end\"."
 
-command_line_up
+counted_command_line
 read_command_lines (char *prompt_arg, int from_tty, int parse_commands,
 		    void (*validator)(char *, void *), void *closure)
 {
@@ -1205,7 +1144,7 @@  read_command_lines (char *prompt_arg, int from_tty, int parse_commands,
 
   /* Reading commands assumes the CLI behavior, so temporarily
      override the current interpreter with CLI.  */
-  command_line_up head;
+  counted_command_line head (nullptr, command_lines_deleter ());
   if (current_interp_named_p (INTERP_CONSOLE))
     head = read_command_lines_1 (read_next_line, parse_commands,
 				 validator, closure);
@@ -1228,12 +1167,12 @@  read_command_lines (char *prompt_arg, int from_tty, int parse_commands,
 /* Act the same way as read_command_lines, except that each new line is
    obtained using READ_NEXT_LINE_FUNC.  */
 
-command_line_up
+counted_command_line
 read_command_lines_1 (char * (*read_next_line_func) (void), int parse_commands,
 		      void (*validator)(char *, void *), void *closure)
 {
   struct command_line *tail, *next;
-  command_line_up head;
+  counted_command_line head (nullptr, command_lines_deleter ());
   enum command_control_type ret;
   enum misc_command_type val;
 
@@ -1279,7 +1218,7 @@  read_command_lines_1 (char * (*read_next_line_func) (void), int parse_commands,
 	}
       else
 	{
-	  head.reset (next);
+	  head = counted_command_line (next, command_lines_deleter ());
 	}
       tail = next;
     }
@@ -1299,54 +1238,15 @@  free_command_lines (struct command_line **lptr)
 {
   struct command_line *l = *lptr;
   struct command_line *next;
-  struct command_line **blist;
-  int i;
 
   while (l)
     {
-      if (l->body_count > 0)
-	{
-	  blist = l->body_list;
-	  for (i = 0; i < l->body_count; i++, blist++)
-	    free_command_lines (blist);
-	}
       next = l->next;
-      xfree (l->line);
-      xfree (l);
+      delete l;
       l = next;
     }
   *lptr = NULL;
 }
-
-command_line_up
-copy_command_lines (struct command_line *cmds)
-{
-  struct command_line *result = NULL;
-
-  if (cmds)
-    {
-      result = XNEW (struct command_line);
-
-      result->next = copy_command_lines (cmds->next).release ();
-      result->line = xstrdup (cmds->line);
-      result->control_type = cmds->control_type;
-      result->body_count = cmds->body_count;
-      if (cmds->body_count > 0)
-        {
-          int i;
-
-          result->body_list = XNEWVEC (struct command_line *, cmds->body_count);
-
-          for (i = 0; i < cmds->body_count; i++)
-            result->body_list[i]
-	      = copy_command_lines (cmds->body_list[i]).release ();
-        }
-      else
-        result->body_list = NULL;
-    }
-
-  return command_line_up (result);
-}
 
 /* Validate that *COMNAME is a valid name for a command.  Return the
    containing command list, in case it starts with a prefix command.
@@ -1483,15 +1383,12 @@  define_command (const char *comname, int from_tty)
 
   xsnprintf (tmpbuf, sizeof (tmpbuf),
 	     "Type commands for definition of \"%s\".", comfull);
-  command_line_up cmds = read_command_lines (tmpbuf, from_tty, 1, 0, 0);
-
-  if (c && c->theclass == class_user)
-    free_command_lines (&c->user_commands);
+  counted_command_line cmds = read_command_lines (tmpbuf, from_tty, 1, 0, 0);
 
   newc = add_cmd (comname, class_user, user_defined_command,
 		  (c && c->theclass == class_user)
 		  ? c->doc : xstrdup ("User-defined."), list);
-  newc->user_commands = cmds.release ();
+  newc->user_commands = std::move (cmds);
 
   /* If this new command is a hook, then mark both commands as being
      tied.  */
@@ -1534,7 +1431,8 @@  document_command (const char *comname, int from_tty)
 
   xsnprintf (tmpbuf, sizeof (tmpbuf), "Type documentation for \"%s\".",
 	     comfull);
-  command_line_up doclines = read_command_lines (tmpbuf, from_tty, 0, 0, 0);
+  counted_command_line doclines = read_command_lines (tmpbuf, from_tty,
+						      0, 0, 0);
 
   if (c->doc)
     xfree ((char *) c->doc);
@@ -1611,7 +1509,7 @@  show_user_1 (struct cmd_list_element *c, const char *prefix, const char *name,
       return;
     }
 
-  cmdlines = c->user_commands;
+  cmdlines = c->user_commands.get ();
   fprintf_filtered (stream, "User command \"%s%s\":\n", prefix, name);
 
   if (!cmdlines)
diff --git a/gdb/cli/cli-script.h b/gdb/cli/cli-script.h
index 518c8a80b8..58dede2342 100644
--- a/gdb/cli/cli-script.h
+++ b/gdb/cli/cli-script.h
@@ -45,12 +45,38 @@  enum command_control_type
   invalid_control
 };
 
+struct command_line;
+
+extern void free_command_lines (struct command_line **);
+
+/* A deleter for command_line that calls free_command_lines.  */
+
+struct command_lines_deleter
+{
+  void operator() (command_line *cmd_lines) const
+  {
+    free_command_lines (&cmd_lines);
+  }
+};
+
+/* A reference-counted struct command_line.  */
+typedef std::shared_ptr<command_line> counted_command_line;
+
 /* * Structure for saved commands lines (for breakpoints, defined
    commands, etc).  */
 
 struct command_line
 {
-  struct command_line *next;
+  explicit command_line (command_control_type type_, char *line_ = nullptr)
+    : line (line_),
+      control_type (type_)
+  {
+    memset (&control_u, 0, sizeof (control_u));
+  }
+
+  DISABLE_COPY_AND_ASSIGN (command_line);
+
+  struct command_line *next = nullptr;
   char *line;
   enum command_control_type control_type;
   union
@@ -63,36 +89,28 @@  struct command_line
       compile;
     }
   control_u;
-  /* * The number of elements in body_list.  */
-  int body_count;
   /* * For composite commands, the nested lists of commands.  For
      example, for "if" command this will contain the then branch and
      the else branch, if that is available.  */
-  struct command_line **body_list;
-};
+  counted_command_line body_list_0;
+  counted_command_line body_list_1;
 
-extern void free_command_lines (struct command_line **);
+private:
 
-/* A deleter for command_line that calls free_command_lines.  */
+  friend void free_command_lines (struct command_line **);
 
-struct command_lines_deleter
-{
-  void operator() (command_line *cmd_lines) const
+  ~command_line ()
   {
-    free_command_lines (&cmd_lines);
+    xfree (line);
   }
 };
 
-/* A unique pointer to a command_line.  */
-
-typedef std::unique_ptr<command_line, command_lines_deleter> command_line_up;
-
-extern command_line_up read_command_lines (char *, int, int,
-					   void (*)(char *, void *),
-					   void *);
-extern command_line_up read_command_lines_1 (char * (*) (void), int,
-					     void (*)(char *, void *),
-					     void *);
+extern counted_command_line read_command_lines (char *, int, int,
+						void (*)(char *, void *),
+						void *);
+extern counted_command_line read_command_lines_1 (char * (*) (void), int,
+						  void (*)(char *, void *),
+						  void *);
 
 
 /* Exported to cli/cli-cmds.c */
@@ -112,14 +130,12 @@  extern enum command_control_type
 extern enum command_control_type
 	execute_control_command_untraced (struct command_line *cmd);
 
-extern command_line_up get_command_line (enum command_control_type,
-					 const char *);
+extern counted_command_line get_command_line (enum command_control_type,
+					      const char *);
 
 extern void print_command_lines (struct ui_out *,
 				 struct command_line *, unsigned int);
 
-extern command_line_up copy_command_lines (struct command_line *cmds);
-
 /* Exported to gdb/infrun.c */
 
 extern void execute_user_command (struct cmd_list_element *c, const char *args);
diff --git a/gdb/compile/compile.c b/gdb/compile/compile.c
index 7f35272872..01c0bc4b39 100644
--- a/gdb/compile/compile.c
+++ b/gdb/compile/compile.c
@@ -149,7 +149,7 @@  compile_code_command (const char *arg, int from_tty)
     eval_compile_command (NULL, arg, scope, NULL);
   else
     {
-      command_line_up l = get_command_line (compile_control, "");
+      counted_command_line l = get_command_line (compile_control, "");
 
       l->control_u.compile.scope = scope;
       execute_control_command_untraced (l.get ());
@@ -187,7 +187,7 @@  compile_print_command (const char *arg, int from_tty)
     eval_compile_command (NULL, arg, scope, &fmt);
   else
     {
-      command_line_up l = get_command_line (compile_control, "");
+      counted_command_line l = get_command_line (compile_control, "");
 
       l->control_u.compile.scope = scope;
       l->control_u.compile.scope_data = &fmt;
@@ -512,7 +512,7 @@  compile_to_object (struct command_line *cmd, const char *cmd_string,
     {
       struct command_line *iter;
 
-      for (iter = cmd->body_list[0]; iter; iter = iter->next)
+      for (iter = cmd->body_list_0.get (); iter; iter = iter->next)
 	{
 	  input_buf.puts (iter->line);
 	  input_buf.puts ("\n");
diff --git a/gdb/guile/guile.c b/gdb/guile/guile.c
index 8fc12dd876..0bbbf6eac1 100644
--- a/gdb/guile/guile.c
+++ b/gdb/guile/guile.c
@@ -209,7 +209,7 @@  guile_command (const char *arg, int from_tty)
     }
   else
     {
-      command_line_up l = get_command_line (guile_control, "");
+      counted_command_line l = get_command_line (guile_control, "");
 
       execute_control_command_untraced (l.get ());
     }
@@ -256,12 +256,12 @@  gdbscm_eval_from_control_command
   char *script, *msg;
   struct cleanup *cleanup;
 
-  if (cmd->body_count != 1)
+  if (cmd->body_list_1 != nullptr)
     error (_("Invalid \"guile\" block structure."));
 
   cleanup = make_cleanup (null_cleanup, NULL);
 
-  script = compute_scheme_string (cmd->body_list[0]);
+  script = compute_scheme_string (cmd->body_list_0.get ());
   msg = gdbscm_safe_eval_string (script, 0);
   xfree (script);
   if (msg != NULL)
@@ -408,7 +408,7 @@  guile_command (const char *arg, int from_tty)
     {
       /* Even if Guile isn't enabled, we still have to slurp the
 	 command list to the corresponding "end".  */
-      command_line_up l = get_command_line (guile_control, "");
+      counted_command_line l = get_command_line (guile_control, "");
 
       execute_control_command_untraced (l.get ());
     }
diff --git a/gdb/mi/mi-cmd-break.c b/gdb/mi/mi-cmd-break.c
index c1f5e2d924..1772fad43c 100644
--- a/gdb/mi/mi-cmd-break.c
+++ b/gdb/mi/mi-cmd-break.c
@@ -489,7 +489,7 @@  mi_read_next_line (void)
 void
 mi_cmd_break_commands (const char *command, char **argv, int argc)
 {
-  command_line_up break_command;
+  counted_command_line break_command;
   char *endptr;
   int bnum;
   struct breakpoint *b;
diff --git a/gdb/python/python.c b/gdb/python/python.c
index 9eae8a1aef..45b6516fc2 100644
--- a/gdb/python/python.c
+++ b/gdb/python/python.c
@@ -385,12 +385,12 @@  gdbpy_eval_from_control_command (const struct extension_language_defn *extlang,
 {
   int ret;
 
-  if (cmd->body_count != 1)
+  if (cmd->body_list_1 != nullptr)
     error (_("Invalid \"python\" block structure."));
 
   gdbpy_enter enter_py (get_current_arch (), current_language);
 
-  std::string script = compute_python_string (cmd->body_list[0]);
+  std::string script = compute_python_string (cmd->body_list_0.get ());
   ret = PyRun_SimpleString (script.c_str ());
   if (ret)
     error (_("Error while executing Python code."));
@@ -413,7 +413,7 @@  python_command (const char *arg, int from_tty)
     }
   else
     {
-      command_line_up l = get_command_line (python_control, "");
+      counted_command_line l = get_command_line (python_control, "");
 
       execute_control_command_untraced (l.get ());
     }
@@ -1582,7 +1582,7 @@  python_interactive_command (const char *arg, int from_tty)
     error (_("Python scripting is not supported in this copy of GDB."));
   else
     {
-      command_line_up l = get_command_line (python_control, "");
+      counted_command_line l = get_command_line (python_control, "");
 
       execute_control_command_untraced (l.get ());
     }
diff --git a/gdb/remote.c b/gdb/remote.c
index f54a38833b..47c546c3d1 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -12238,7 +12238,7 @@  remote_download_command_source (int num, ULONGEST addr,
       if (cmd->control_type == while_control
 	  || cmd->control_type == while_stepping_control)
 	{
-	  remote_download_command_source (num, addr, *cmd->body_list);
+	  remote_download_command_source (num, addr, cmd->body_list_0.get ());
 
 	  QUIT;	/* Allow user to bail out with ^C.  */
 	  strcpy (rs->buf, "QTDPsrc:");
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index 7e173ce75d..0c3842199f 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -167,8 +167,7 @@  char *trace_stop_notes = NULL;
 struct collection_list;
 static char *mem2hex (gdb_byte *, char *, int);
 
-static struct command_line *
-  all_tracepoint_actions_and_cleanup (struct breakpoint *t);
+static counted_command_line all_tracepoint_actions (struct breakpoint *);
 
 static struct trace_status trace_status;
 
@@ -579,8 +578,9 @@  actions_command (const char *args, int from_tty)
 	string_printf ("Enter actions for tracepoint %d, one per line.",
 		       t->number);
 
-      command_line_up l = read_command_lines (&tmpbuf[0], from_tty, 1,
-					      check_tracepoint_command, t);
+      counted_command_line l = read_command_lines (&tmpbuf[0], from_tty, 1,
+						   check_tracepoint_command,
+						   t);
       breakpoint_set_commands (t, std::move (l));
     }
   /* else just return */
@@ -1437,7 +1437,7 @@  encode_actions_1 (struct command_line *action,
 	     here.  */
 	  gdb_assert (stepping_list);
 
-	  encode_actions_1 (action->body_list[0], tloc, frame_reg,
+	  encode_actions_1 (action->body_list_0.get (), tloc, frame_reg,
 			    frame_offset, stepping_list, NULL);
 	}
       else
@@ -1453,17 +1453,17 @@  encode_actions (struct bp_location *tloc,
 		struct collection_list *tracepoint_list,
 		struct collection_list *stepping_list)
 {
-  struct command_line *actions;
   int frame_reg;
   LONGEST frame_offset;
 
   gdbarch_virtual_frame_pointer (tloc->gdbarch,
 				 tloc->address, &frame_reg, &frame_offset);
 
-  actions = all_tracepoint_actions_and_cleanup (tloc->owner);
-
-  encode_actions_1 (actions, tloc, frame_reg, frame_offset,
+  counted_command_line actions = all_tracepoint_actions (tloc->owner);
+  encode_actions_1 (actions.get (), tloc, frame_reg, frame_offset,
 		    tracepoint_list, stepping_list);
+  encode_actions_1 (breakpoint_commands (tloc->owner), tloc,
+		    frame_reg, frame_offset, tracepoint_list, stepping_list);
 
   tracepoint_list->finish ();
   stepping_list->finish ();
@@ -2663,11 +2663,10 @@  trace_dump_actions (struct command_line *action,
 
       if (cmd_cfunc_eq (cmd, while_stepping_pseudocommand))
 	{
-	  int i;
-
-	  for (i = 0; i < action->body_count; ++i)
-	    trace_dump_actions (action->body_list[i],
-				1, stepping_frame, from_tty);
+	  trace_dump_actions (action->body_list_0.get (),
+			      1, stepping_frame, from_tty);
+	  trace_dump_actions (action->body_list_1.get (),
+			      1, stepping_frame, from_tty);
 	}
       else if (cmd_cfunc_eq (cmd, collect_pseudocommand))
 	{
@@ -2778,16 +2777,12 @@  get_traceframe_location (int *stepping_frame_p)
   return t->loc;
 }
 
-/* Return all the actions, including default collect, of a tracepoint
-   T.  It constructs cleanups into the chain, and leaves the caller to
-   handle them (call do_cleanups).  */
+/* Return the default collect actions of a tracepoint T.  */
 
-static struct command_line *
-all_tracepoint_actions_and_cleanup (struct breakpoint *t)
+static counted_command_line
+all_tracepoint_actions (struct breakpoint *t)
 {
-  struct command_line *actions;
-
-  actions = breakpoint_commands (t);
+  counted_command_line actions (nullptr, command_lines_deleter ());
 
   /* If there are default expressions to collect, make up a collect
      action and prepend to the action list to encode.  Note that since
@@ -2797,17 +2792,13 @@  all_tracepoint_actions_and_cleanup (struct breakpoint *t)
   if (*default_collect)
     {
       struct command_line *default_collect_action;
-      char *default_collect_line;
-
-      default_collect_line = xstrprintf ("collect %s", default_collect);
-      make_cleanup (xfree, default_collect_line);
+      gdb::unique_xmalloc_ptr<char> default_collect_line
+	(xstrprintf ("collect %s", default_collect));
 
-      validate_actionline (default_collect_line, t);
-      default_collect_action = XNEW (struct command_line);
-      make_cleanup (xfree, default_collect_action);
-      default_collect_action->next = actions;
-      default_collect_action->line = default_collect_line;
-      actions = default_collect_action;
+      validate_actionline (default_collect_line.get (), t);
+      actions.reset (new struct command_line (simple_control,
+					      default_collect_line.release ()),
+		     command_lines_deleter ());
     }
 
   return actions;
@@ -2820,7 +2811,6 @@  tdump_command (const char *args, int from_tty)
 {
   int stepping_frame = 0;
   struct bp_location *loc;
-  struct command_line *actions;
 
   /* This throws an error is not inspecting a trace frame.  */
   loc = get_traceframe_location (&stepping_frame);
@@ -2834,9 +2824,11 @@  tdump_command (const char *args, int from_tty)
 
   select_frame (get_current_frame ());
 
-  actions = all_tracepoint_actions_and_cleanup (loc->owner);
+  counted_command_line actions = all_tracepoint_actions (loc->owner);
 
-  trace_dump_actions (actions, 0, stepping_frame, from_tty);
+  trace_dump_actions (actions.get (), 0, stepping_frame, from_tty);
+  trace_dump_actions (breakpoint_commands (loc->owner), 0, stepping_frame,
+		      from_tty);
 }
 
 /* Encode a piece of a tracepoint's source-level definition in a form