'struct agent_expr *' -> unique_ptr<agent_expr>

This patch makes the gen_* functions return a unique_ptr instead of
raw pointer:

  typedef gdb::unique_ptr<agent_expr> agent_expr_up;

and then adjusts the codebase throughout to stop using
make_cleanup_free_agent_expr.

The cond_bytecode and cmd_bytecode fields of struct bp_location are
owning pointers, so they're changed to be unique_ptr's instead of raw
pointers.

gdb/ChangeLog:
2016-11-08  Pedro Alves  <palves@redhat.com>

	* ax-gdb.c (is_nontrivial_conversion): Use agent_expr_up.
	(gen_trace_for_var, gen_trace_for_expr, gen_eval_for_expr)
	(gen_trace_for_return_address, gen_printf): Use and return an
	agent_expr_up.  Don't use make_cleanup_free_agent_expr.
	(agent_eval_command_one, maint_agent_printf_command): Use
	agent_expr_up.  Don't use make_cleanup_free_agent_expr.
	* ax-gdb.h (gen_trace_for_expr, gen_trace_for_var)
	(gen_trace_for_return_address, gen_eval_for_expr, gen_printf): Use
	agent_expr_up.
	* ax-general.c (new_agent_expr): Rename to ...
	(agent_expr::agent_expr): ... this, and now a constructor.
	(free_agent_expr): Rename to ...
	(agent_expr::~agent_exp): ... this, and now a destructor.
	(do_free_agent_expr_cleanup, make_cleanup_free_agent_expr):
	Delete.
	* ax.h (struct agent_expr): Add ctor/dtor.
	(agent_expr_up): New typedef.
	(new_agent_expr, free_agent_expr, make_cleanup_free_agent_expr):
	Delete declarations.
	* breakpoint.c (parse_cond_to_aexpr): Use and return an
	agent_expr_up.  Don't use make_cleanup_free_agent_expr.
	(build_target_condition_list): Adjust to use agent_expr_up.
	(parse_cmd_to_aexpr): Use and return an agent_expr_up.  Don't use
	make_cleanup_free_agent_expr.
	(build_target_command_list): Adjust to use agent_expr_up.
	(force_breakpoint_reinsertion): Adjust to use agent_expr_up.
	(bp_location_dtor): Remove unnecessary free_agent_expr and xfree
	calls.
	* breakpoint.h (struct bp_target_info) <cond_bytecode,
	cmd_bytecode>: Now agent_expr_up's.
	* remote.c (remote_download_tracepoint): Adjust to use
	agent_expr_up and remove use of make_cleanup_free_agent_expr.
	* tracepoint.c (validate_actionline, collect_symbol): Adjust to
	use agent_expr_up and remove uses of make_cleanup_free_agent_expr.
	(collection_list::~collection_list): Call delete instead of
	free_agent_expr.
	(encode_actions_1): Adjust to use agent_expr_up and remove uses of
	make_cleanup_free_agent_expr.
	(add_aexpr): Change parameter type to agent_expr_up; Return a raw
	agent_expr pointer.

10 files changed, 194 insertions, 254 deletions

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index b19a07f..8c7d06f 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,48 @@
 2016-11-08  Pedro Alves  <palves@redhat.com>
 
+	* ax-gdb.c (is_nontrivial_conversion): Use agent_expr_up.
+	(gen_trace_for_var, gen_trace_for_expr, gen_eval_for_expr)
+	(gen_trace_for_return_address, gen_printf): Use and return an
+	agent_expr_up.  Don't use make_cleanup_free_agent_expr.
+	(agent_eval_command_one, maint_agent_printf_command): Use
+	agent_expr_up.  Don't use make_cleanup_free_agent_expr.
+	* ax-gdb.h (gen_trace_for_expr, gen_trace_for_var)
+	(gen_trace_for_return_address, gen_eval_for_expr, gen_printf): Use
+	agent_expr_up.
+	* ax-general.c (new_agent_expr): Rename to ...
+	(agent_expr::agent_expr): ... this, and now a constructor.
+	(free_agent_expr): Rename to ...
+	(agent_expr::~agent_exp): ... this, and now a destructor.
+	(do_free_agent_expr_cleanup, make_cleanup_free_agent_expr):
+	Delete.
+	* ax.h (struct agent_expr): Add ctor/dtor.
+	(agent_expr_up): New typedef.
+	(new_agent_expr, free_agent_expr, make_cleanup_free_agent_expr):
+	Delete declarations.
+	* breakpoint.c (parse_cond_to_aexpr): Use and return an
+	agent_expr_up.  Don't use make_cleanup_free_agent_expr.
+	(build_target_condition_list): Adjust to use agent_expr_up.
+	(parse_cmd_to_aexpr): Use and return an agent_expr_up.  Don't use
+	make_cleanup_free_agent_expr.
+	(build_target_command_list): Adjust to use agent_expr_up.
+	(force_breakpoint_reinsertion): Adjust to use agent_expr_up.
+	(bp_location_dtor): Remove unnecessary free_agent_expr and xfree
+	calls.
+	* breakpoint.h (struct bp_target_info) <cond_bytecode,
+	cmd_bytecode>: Now agent_expr_up's.
+	* remote.c (remote_download_tracepoint): Adjust to use
+	agent_expr_up and remove use of make_cleanup_free_agent_expr.
+	* tracepoint.c (validate_actionline, collect_symbol): Adjust to
+	use agent_expr_up and remove uses of make_cleanup_free_agent_expr.
+	(collection_list::~collection_list): Call delete instead of
+	free_agent_expr.
+	(encode_actions_1): Adjust to use agent_expr_up and remove uses of
+	make_cleanup_free_agent_expr.
+	(add_aexpr): Change parameter type to agent_expr_up; Return a raw
+	agent_expr pointer.
+
+2016-11-08  Pedro Alves  <palves@redhat.com>
+
 	* ada-lang.c (ada_name_for_lookup, type_as_string): Use and return
 	std::string.
 	(type_as_string_and_cleanup): Delete.
diff --git a/gdb/ax-gdb.c b/gdb/ax-gdb.c
index ccd16f2..1816ba6 100644
--- a/gdb/ax-gdb.c
+++ b/gdb/ax-gdb.c
@@ -910,7 +910,7 @@ gen_conversion (struct agent_expr *ax, struct type *from, struct type *to)
 static int
 is_nontrivial_conversion (struct type *from, struct type *to)
 {
-  struct agent_expr *ax = new_agent_expr (NULL, 0);
+  agent_expr_up ax (new agent_expr (NULL, 0));
   int nontrivial;
 
   /* Actually generate the code, and see if anything came out.  At the
@@ -919,9 +919,8 @@ is_nontrivial_conversion (struct type *from, struct type *to)
      floating point and the like, it may not be.  Doing things this
      way allows this function to be independent of the logic in
      gen_conversion.  */
-  gen_conversion (ax, from, to);
+  gen_conversion (ax.get (), from, to);
   nontrivial = ax->len > 0;
-  free_agent_expr (ax);
   return nontrivial;
 }
 
@@ -2391,38 +2390,28 @@ gen_expr_binop_rest (struct expression *exp,
    variable's name, and no parsed expression; for instance, when the
    name comes from a list of local variables of a function.  */
 
-struct agent_expr *
+agent_expr_up
 gen_trace_for_var (CORE_ADDR scope, struct gdbarch *gdbarch,
 		   struct symbol *var, int trace_string)
 {
-  struct cleanup *old_chain = 0;
-  struct agent_expr *ax = new_agent_expr (gdbarch, scope);
+  agent_expr_up ax (new agent_expr (gdbarch, scope));
   struct axs_value value;
 
-  old_chain = make_cleanup_free_agent_expr (ax);
-
   ax->tracing = 1;
   ax->trace_string = trace_string;
-  gen_var_ref (gdbarch, ax, &value, var);
+  gen_var_ref (gdbarch, ax.get (), &value, var);
 
   /* If there is no actual variable to trace, flag it by returning
      an empty agent expression.  */
   if (value.optimized_out)
-    {
-      do_cleanups (old_chain);
-      return NULL;
-    }
+    return agent_expr_up ();
 
   /* Make sure we record the final object, and get rid of it.  */
-  gen_traced_pop (gdbarch, ax, &value);
+  gen_traced_pop (gdbarch, ax.get (), &value);
 
   /* Oh, and terminate.  */
-  ax_simple (ax, aop_end);
+  ax_simple (ax.get (), aop_end);
 
-  /* We have successfully built the agent expr, so cancel the cleanup
-     request.  If we add more cleanups that we always want done, this
-     will have to get more complicated.  */
-  discard_cleanups (old_chain);
   return ax;
 }
 
@@ -2433,33 +2422,27 @@ gen_trace_for_var (CORE_ADDR scope, struct gdbarch *gdbarch,
    record the value of all memory touched by the expression.  The
    caller can then use the ax_reqs function to discover which
    registers it relies upon.  */
-struct agent_expr *
+
+agent_expr_up
 gen_trace_for_expr (CORE_ADDR scope, struct expression *expr,
 		    int trace_string)
 {
-  struct cleanup *old_chain = 0;
-  struct agent_expr *ax = new_agent_expr (expr->gdbarch, scope);
+  agent_expr_up ax (new agent_expr (expr->gdbarch, scope));
   union exp_element *pc;
   struct axs_value value;
 
-  old_chain = make_cleanup_free_agent_expr (ax);
-
   pc = expr->elts;
   ax->tracing = 1;
   ax->trace_string = trace_string;
   value.optimized_out = 0;
-  gen_expr (expr, &pc, ax, &value);
+  gen_expr (expr, &pc, ax.get (), &value);
 
   /* Make sure we record the final object, and get rid of it.  */
-  gen_traced_pop (expr->gdbarch, ax, &value);
+  gen_traced_pop (expr->gdbarch, ax.get (), &value);
 
   /* Oh, and terminate.  */
-  ax_simple (ax, aop_end);
+  ax_simple (ax.get (), aop_end);
 
-  /* We have successfully built the agent expr, so cancel the cleanup
-     request.  If we add more cleanups that we always want done, this
-     will have to get more complicated.  */
-  discard_cleanups (old_chain);
   return ax;
 }
 
@@ -2470,58 +2453,44 @@ gen_trace_for_expr (CORE_ADDR scope, struct expression *expr,
    gen_trace_for_expr does.  The generated bytecode sequence leaves
    the result of expression evaluation on the top of the stack.  */
 
-struct agent_expr *
+agent_expr_up
 gen_eval_for_expr (CORE_ADDR scope, struct expression *expr)
 {
-  struct cleanup *old_chain = 0;
-  struct agent_expr *ax = new_agent_expr (expr->gdbarch, scope);
+  agent_expr_up ax (new agent_expr (expr->gdbarch, scope));
   union exp_element *pc;
   struct axs_value value;
 
-  old_chain = make_cleanup_free_agent_expr (ax);
-
   pc = expr->elts;
   ax->tracing = 0;
   value.optimized_out = 0;
-  gen_expr (expr, &pc, ax, &value);
+  gen_expr (expr, &pc, ax.get (), &value);
 
-  require_rvalue (ax, &value);
+  require_rvalue (ax.get (), &value);
 
   /* Oh, and terminate.  */
-  ax_simple (ax, aop_end);
+  ax_simple (ax.get (), aop_end);
 
-  /* We have successfully built the agent expr, so cancel the cleanup
-     request.  If we add more cleanups that we always want done, this
-     will have to get more complicated.  */
-  discard_cleanups (old_chain);
   return ax;
 }
 
-struct agent_expr *
+agent_expr_up
 gen_trace_for_return_address (CORE_ADDR scope, struct gdbarch *gdbarch,
 			      int trace_string)
 {
-  struct cleanup *old_chain = 0;
-  struct agent_expr *ax = new_agent_expr (gdbarch, scope);
+  agent_expr_up ax (new agent_expr (gdbarch, scope));
   struct axs_value value;
 
-  old_chain = make_cleanup_free_agent_expr (ax);
-
   ax->tracing = 1;
   ax->trace_string = trace_string;
 
-  gdbarch_gen_return_address (gdbarch, ax, &value, scope);
+  gdbarch_gen_return_address (gdbarch, ax.get (), &value, scope);
 
   /* Make sure we record the final object, and get rid of it.  */
-  gen_traced_pop (gdbarch, ax, &value);
+  gen_traced_pop (gdbarch, ax.get (), &value);
 
   /* Oh, and terminate.  */
-  ax_simple (ax, aop_end);
+  ax_simple (ax.get (), aop_end);
 
-  /* We have successfully built the agent expr, so cancel the cleanup
-     request.  If we add more cleanups that we always want done, this
-     will have to get more complicated.  */
-  discard_cleanups (old_chain);
   return ax;
 }
 
@@ -2529,21 +2498,18 @@ gen_trace_for_return_address (CORE_ADDR scope, struct gdbarch *gdbarch,
    evaluate the arguments and pass everything to a special
    bytecode.  */
 
-struct agent_expr *
+agent_expr_up
 gen_printf (CORE_ADDR scope, struct gdbarch *gdbarch,
 	    CORE_ADDR function, LONGEST channel,
 	    const char *format, int fmtlen,
 	    struct format_piece *frags,
 	    int nargs, struct expression **exprs)
 {
-  struct cleanup *old_chain = 0;
-  struct agent_expr *ax = new_agent_expr (gdbarch, scope);
+  agent_expr_up ax (new agent_expr (gdbarch, scope));
   union exp_element *pc;
   struct axs_value value;
   int tem;
 
-  old_chain = make_cleanup_free_agent_expr (ax);
-
   /* We're computing values, not doing side effects.  */
   ax->tracing = 0;
 
@@ -2553,26 +2519,21 @@ gen_printf (CORE_ADDR scope, struct gdbarch *gdbarch,
     {
       pc = exprs[tem]->elts;
       value.optimized_out = 0;
-      gen_expr (exprs[tem], &pc, ax, &value);
-      require_rvalue (ax, &value);
+      gen_expr (exprs[tem], &pc, ax.get (), &value);
+      require_rvalue (ax.get (), &value);
     }
 
   /* Push function and channel.  */
-  ax_const_l (ax, channel);
-  ax_const_l (ax, function);
+  ax_const_l (ax.get (), channel);
+  ax_const_l (ax.get (), function);
 
   /* Issue the printf bytecode proper.  */
-  ax_simple (ax, aop_printf);
-  ax_raw_byte (ax, nargs);
-  ax_string (ax, format, fmtlen);
+  ax_simple (ax.get (), aop_printf);
+  ax_raw_byte (ax.get (), nargs);
+  ax_string (ax.get (), format, fmtlen);
 
   /* And terminate.  */
-  ax_simple (ax, aop_end);
-
-  /* We have successfully built the agent expr, so cancel the cleanup
-     request.  If we add more cleanups that we always want done, this
-     will have to get more complicated.  */
-  discard_cleanups (old_chain);
+  ax_simple (ax.get (), aop_end);
 
   return ax;
 }
@@ -2580,8 +2541,6 @@ gen_printf (CORE_ADDR scope, struct gdbarch *gdbarch,
 static void
 agent_eval_command_one (const char *exp, int eval, CORE_ADDR pc)
 {
-  struct cleanup *old_chain = 0;
-  struct agent_expr *agent;
   const char *arg;
   int trace_string = 0;
 
@@ -2591,33 +2550,33 @@ agent_eval_command_one (const char *exp, int eval, CORE_ADDR pc)
         exp = decode_agent_options (exp, &trace_string);
     }
 
+  agent_expr_up agent;
+
   arg = exp;
   if (!eval && strcmp (arg, "$_ret") == 0)
     {
-      agent = gen_trace_for_return_address (pc, get_current_arch (),
-					    trace_string);
-      old_chain = make_cleanup_free_agent_expr (agent);
+      agent = gdb::move (gen_trace_for_return_address (pc, get_current_arch (),
+						       trace_string));
     }
   else
     {
       expression_up expr = parse_exp_1 (&arg, pc, block_for_pc (pc), 0);
+
       if (eval)
 	{
 	  gdb_assert (trace_string == 0);
-	  agent = gen_eval_for_expr (pc, expr.get ());
+	  agent = gdb::move (gen_eval_for_expr (pc, expr.get ()));
 	}
       else
-	agent = gen_trace_for_expr (pc, expr.get (), trace_string);
-      old_chain = make_cleanup_free_agent_expr (agent);
+	agent = gdb::move (gen_trace_for_expr (pc, expr.get (), trace_string));
     }
 
-  ax_reqs (agent);
-  ax_print (gdb_stdout, agent);
+  ax_reqs (agent.get ());
+  ax_print (gdb_stdout, agent.get ());
 
   /* It would be nice to call ax_reqs here to gather some general info
      about the expression, and then print out the result.  */
 
-  do_cleanups (old_chain);
   dont_repeat ();
 }
 
@@ -2695,7 +2654,6 @@ maint_agent_printf_command (char *exp, int from_tty)
 {
   struct cleanup *old_chain = 0;
   struct expression *argvec[100];
-  struct agent_expr *agent;
   struct frame_info *fi = get_current_frame ();	/* need current scope */
   const char *cmdrest;
   const char *format_start, *format_end;
@@ -2755,12 +2713,12 @@ maint_agent_printf_command (char *exp, int from_tty)
     }
 
 
-  agent = gen_printf (get_frame_pc (fi), get_current_arch (), 0, 0,
-		      format_start, format_end - format_start,
-		      fpieces, nargs, argvec);
-  make_cleanup_free_agent_expr (agent);
-  ax_reqs (agent);
-  ax_print (gdb_stdout, agent);
+  agent_expr_up agent = gen_printf (get_frame_pc (fi), get_current_arch (),
+				    0, 0,
+				    format_start, format_end - format_start,
+				    fpieces, nargs, argvec);
+  ax_reqs (agent.get ());
+  ax_print (gdb_stdout, agent.get ());
 
   /* It would be nice to call ax_reqs here to gather some general info
      about the expression, and then print out the result.  */
diff --git a/gdb/ax-gdb.h b/gdb/ax-gdb.h
index dacacfc..0d791e9 100644
--- a/gdb/ax-gdb.h
+++ b/gdb/ax-gdb.h
@@ -101,17 +101,17 @@ struct axs_value
    record the value of all memory touched by the expression, and leave
    no values on the stack.  The caller can then use the ax_reqs
    function to discover which registers the expression uses.  */
-extern struct agent_expr *gen_trace_for_expr (CORE_ADDR, struct expression *,
-					      int);
+extern agent_expr_up gen_trace_for_expr (CORE_ADDR, struct expression *,
+					 int);
 
-extern struct agent_expr *gen_trace_for_var (CORE_ADDR, struct gdbarch *,
-					     struct symbol *, int);
+extern agent_expr_up gen_trace_for_var (CORE_ADDR, struct gdbarch *,
+					struct symbol *, int);
 
-extern struct agent_expr *gen_trace_for_return_address (CORE_ADDR,
-							struct gdbarch *,
-							int);
+extern agent_expr_up gen_trace_for_return_address (CORE_ADDR,
+						   struct gdbarch *,
+						   int);
 
-extern struct agent_expr *gen_eval_for_expr (CORE_ADDR, struct expression *);
+extern agent_expr_up gen_eval_for_expr (CORE_ADDR, struct expression *);
 
 extern void gen_expr (struct expression *exp, union exp_element **pc,
 		      struct agent_expr *ax, struct axs_value *value);
@@ -119,9 +119,9 @@ extern void gen_expr (struct expression *exp, union exp_element **pc,
 extern void require_rvalue (struct agent_expr *ax, struct axs_value *value);
 
 struct format_piece;
-extern struct agent_expr *gen_printf (CORE_ADDR, struct gdbarch *,
-				      CORE_ADDR, LONGEST, const char *, int,
-				      struct format_piece *,
-				      int, struct expression **);
+extern agent_expr_up gen_printf (CORE_ADDR, struct gdbarch *,
+				 CORE_ADDR, LONGEST, const char *, int,
+				 struct format_piece *,
+				 int, struct expression **);
 
 #endif /* AX_GDB_H */
diff --git a/gdb/ax-general.c b/gdb/ax-general.c
index 7f27a45..35225f6 100644
--- a/gdb/ax-general.c
+++ b/gdb/ax-general.c
@@ -37,52 +37,30 @@ static void generic_ext (struct agent_expr *x, enum agent_op op, int n);
 
 /* Functions for building expressions.  */
 
-/* Allocate a new, empty agent expression.  */
-struct agent_expr *
-new_agent_expr (struct gdbarch *gdbarch, CORE_ADDR scope)
+agent_expr::agent_expr (struct gdbarch *gdbarch, CORE_ADDR scope)
 {
-  struct agent_expr *x = XNEW (struct agent_expr);
-
-  x->len = 0;
-  x->size = 1;			/* Change this to a larger value once
+  this->len = 0;
+  this->size = 1;		/* Change this to a larger value once
 				   reallocation code is tested.  */
-  x->buf = (unsigned char *) xmalloc (x->size);
+  this->buf = (unsigned char *) xmalloc (this->size);
 
-  x->gdbarch = gdbarch;
-  x->scope = scope;
+  this->gdbarch = gdbarch;
+  this->scope = scope;
 
   /* Bit vector for registers used.  */
-  x->reg_mask_len = 1;
-  x->reg_mask = XCNEWVEC (unsigned char, x->reg_mask_len);
-
-  x->tracing = 0;
-  x->trace_string = 0;
+  this->reg_mask_len = 1;
+  this->reg_mask = XCNEWVEC (unsigned char, this->reg_mask_len);
 
-  return x;
+  this->tracing = 0;
+  this->trace_string = 0;
 }
 
-/* Free a agent expression.  */
-void
-free_agent_expr (struct agent_expr *x)
+agent_expr::~agent_expr ()
 {
-  xfree (x->buf);
-  xfree (x->reg_mask);
-  xfree (x);
+  xfree (this->buf);
+  xfree (this->reg_mask);
 }
 
-static void
-do_free_agent_expr_cleanup (void *x)
-{
-  free_agent_expr ((struct agent_expr *) x);
-}
-
-struct cleanup *
-make_cleanup_free_agent_expr (struct agent_expr *x)
-{
-  return make_cleanup (do_free_agent_expr_cleanup, x);
-}
-
-
 /* Make sure that X has room for at least N more bytes.  This doesn't
    affect the length, just the allocated size.  */
 static void
diff --git a/gdb/ax.h b/gdb/ax.h
index 536efe8..4b90229 100644
--- a/gdb/ax.h
+++ b/gdb/ax.h
@@ -94,6 +94,11 @@ union agent_val
 /* A buffer containing a agent expression.  */
 struct agent_expr
   {
+    /* Construct an empty agent expression.  */
+    explicit agent_expr (struct gdbarch *gdbarch, CORE_ADDR scope);
+
+    ~agent_expr ();
+
     /* The bytes of the expression.  */
     unsigned char *buf;
 
@@ -162,6 +167,9 @@ struct agent_expr
     int trace_string;
   };
 
+/* An agent_expr owning pointer.  */
+typedef gdb::unique_ptr<agent_expr> agent_expr_up;
+
 /* Pointer to an agent_expr structure.  */
 typedef struct agent_expr *agent_expr_p;
 
@@ -183,13 +191,6 @@ enum agent_op
 
 /* Functions for building expressions.  */
 
-/* Allocate a new, empty agent expression.  */
-extern struct agent_expr *new_agent_expr (struct gdbarch *, CORE_ADDR);
-
-/* Free a agent expression.  */
-extern void free_agent_expr (struct agent_expr *);
-extern struct cleanup *make_cleanup_free_agent_expr (struct agent_expr *);
-
 /* Append a raw byte to EXPR.  */
 extern void ax_raw_byte (struct agent_expr *expr, gdb_byte byte);
 
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 45f00d1..c883145 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -2256,19 +2256,19 @@ unduplicated_should_be_inserted (struct bp_location *bl)
    by the bytecode interpreter.  Return NULL if there was
    any error during parsing.  */
 
-static struct agent_expr *
+static agent_expr_up
 parse_cond_to_aexpr (CORE_ADDR scope, struct expression *cond)
 {
-  struct agent_expr *aexpr = NULL;
-
-  if (!cond)
+  if (cond == NULL)
     return NULL;
 
+  agent_expr_up aexpr;
+
   /* We don't want to stop processing, so catch any errors
      that may show up.  */
   TRY
     {
-      aexpr = gen_eval_for_expr (scope, cond);
+      aexpr = gdb::move (gen_eval_for_expr (scope, cond));
     }
 
   CATCH (ex, RETURN_MASK_ERROR)
@@ -2276,7 +2276,6 @@ parse_cond_to_aexpr (CORE_ADDR scope, struct expression *cond)
       /* If we got here, it means the condition could not be parsed to a valid
 	 bytecode expression and thus can't be evaluated on the target's side.
 	 It's no use iterating through the conditions.  */
-      return NULL;
     }
   END_CATCH
 
@@ -2321,14 +2320,12 @@ build_target_condition_list (struct bp_location *bl)
 	{
 	  if (modified)
 	    {
-	      struct agent_expr *aexpr;
-
 	      /* Re-parse the conditions since something changed.  In that
 		 case we already freed the condition bytecodes (see
 		 force_breakpoint_reinsertion).  We just
 		 need to parse the condition to bytecodes again.  */
-	      aexpr = parse_cond_to_aexpr (bl->address, loc->cond.get ());
-	      loc->cond_bytecode = aexpr;
+	      loc->cond_bytecode = parse_cond_to_aexpr (bl->address,
+							loc->cond.get ());
 	    }
 
 	  /* If we have a NULL bytecode expression, it means something
@@ -2359,8 +2356,7 @@ build_target_condition_list (struct bp_location *bl)
 	      if (!loc->cond_bytecode)
 		return;
 
-	      free_agent_expr (loc->cond_bytecode);
-	      loc->cond_bytecode = NULL;
+	      loc->cond_bytecode.reset ();
 	    }
 	}
     }
@@ -2378,7 +2374,7 @@ build_target_condition_list (struct bp_location *bl)
 	/* Add the condition to the vector.  This will be used later to send the
 	   conditions to the target.  */
 	VEC_safe_push (agent_expr_p, bl->target_info.conditions,
-		       loc->cond_bytecode);
+		       loc->cond_bytecode.get ());
     }
 
   return;
@@ -2388,19 +2384,18 @@ build_target_condition_list (struct bp_location *bl)
    bytecode suitable for evaluation by the bytecode interpreter.
    Return NULL if there was any error during parsing.  */
 
-static struct agent_expr *
+static agent_expr_up
 parse_cmd_to_aexpr (CORE_ADDR scope, char *cmd)
 {
   struct cleanup *old_cleanups = 0;
   struct expression **argvec;
-  struct agent_expr *aexpr = NULL;
   const char *cmdrest;
   const char *format_start, *format_end;
   struct format_piece *fpieces;
   int nargs;
   struct gdbarch *gdbarch = get_current_arch ();
 
-  if (!cmd)
+  if (cmd == NULL)
     return NULL;
 
   cmdrest = cmd;
@@ -2450,20 +2445,21 @@ parse_cmd_to_aexpr (CORE_ADDR scope, char *cmd)
 	++cmdrest;
     }
 
+  agent_expr_up aexpr;
+
   /* We don't want to stop processing, so catch any errors
      that may show up.  */
   TRY
     {
-      aexpr = gen_printf (scope, gdbarch, 0, 0,
-			  format_start, format_end - format_start,
-			  fpieces, nargs, argvec);
+      aexpr = gdb::move (gen_printf (scope, gdbarch, 0, 0,
+				     format_start, format_end - format_start,
+				     fpieces, nargs, argvec));
     }
   CATCH (ex, RETURN_MASK_ERROR)
     {
       /* If we got here, it means the command could not be parsed to a valid
 	 bytecode expression and thus can't be evaluated on the target's side.
 	 It's no use iterating through the other commands.  */
-      aexpr = NULL;
     }
   END_CATCH
 
@@ -2520,15 +2516,13 @@ build_target_command_list (struct bp_location *bl)
 	{
 	  if (modified)
 	    {
-	      struct agent_expr *aexpr;
-
 	      /* Re-parse the commands since something changed.  In that
 		 case we already freed the command bytecodes (see
 		 force_breakpoint_reinsertion).  We just
 		 need to parse the command to bytecodes again.  */
-	      aexpr = parse_cmd_to_aexpr (bl->address,
-					  loc->owner->extra_string);
-	      loc->cmd_bytecode = aexpr;
+	      loc->cmd_bytecode
+		= parse_cmd_to_aexpr (bl->address,
+				      loc->owner->extra_string);
 	    }
 
 	  /* If we have a NULL bytecode expression, it means something
@@ -2556,8 +2550,7 @@ build_target_command_list (struct bp_location *bl)
 	      if (loc->cmd_bytecode == NULL)
 		return;
 
-	      free_agent_expr (loc->cmd_bytecode);
-	      loc->cmd_bytecode = NULL;
+	      loc->cmd_bytecode.reset ();
 	    }
 	}
     }
@@ -2575,7 +2568,7 @@ build_target_command_list (struct bp_location *bl)
 	/* Add the command to the vector.  This will be used later
 	   to send the commands to the target.  */
 	VEC_safe_push (agent_expr_p, bl->target_info.tcommands,
-		       loc->cmd_bytecode);
+		       loc->cmd_bytecode.get ());
     }
 
   bl->target_info.persist = 0;
@@ -12408,11 +12401,7 @@ force_breakpoint_reinsertion (struct bp_location *bl)
 
       /* Free the agent expression bytecode as well.  We will compute
 	 it later on.  */
-      if (loc->cond_bytecode)
-	{
-	  free_agent_expr (loc->cond_bytecode);
-	  loc->cond_bytecode = NULL;
-	}
+      loc->cond_bytecode.reset ();
     }
 }
 /* Called whether new breakpoints are created, or existing breakpoints
@@ -12898,8 +12887,6 @@ say_where (struct breakpoint *b)
 static void
 bp_location_dtor (struct bp_location *self)
 {
-  if (self->cond_bytecode)
-    free_agent_expr (self->cond_bytecode);
   xfree (self->function_name);
 
   VEC_free (agent_expr_p, self->target_info.conditions);
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 60ffbf7..8c2dfaf 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -344,7 +344,7 @@ struct bp_location
   /* Conditional expression in agent expression
      bytecode form.  This is used for stub-side breakpoint
      condition evaluation.  */
-  struct agent_expr *cond_bytecode;
+  agent_expr_up cond_bytecode;
 
   /* Signals that the condition has changed since the last time
      we updated the global location list.  This means the condition
@@ -361,7 +361,7 @@ struct bp_location
 
   enum condition_status condition_changed;
 
-  struct agent_expr *cmd_bytecode;
+  agent_expr_up cmd_bytecode;
 
   /* Signals that breakpoint conditions and/or commands need to be
      re-synched with the target.  This has no use other than
diff --git a/gdb/remote.c b/gdb/remote.c
index 29096a0..419ebf9 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -12260,8 +12260,6 @@ remote_download_tracepoint (struct target_ops *self, struct bp_location *loc)
   char **stepping_actions;
   int ndx;
   struct cleanup *old_chain = NULL;
-  struct agent_expr *aexpr;
-  struct cleanup *aexpr_chain = NULL;
   char *pkt;
   struct breakpoint *b = loc->owner;
   struct tracepoint *t = (struct tracepoint *) b;
@@ -12332,15 +12330,13 @@ remote_download_tracepoint (struct target_ops *self, struct bp_location *loc)
 	 capabilities at definition time.  */
       if (remote_supports_cond_tracepoints ())
 	{
-	  aexpr = gen_eval_for_expr (tpaddr, loc->cond.get ());
-	  aexpr_chain = make_cleanup_free_agent_expr (aexpr);
+	  agent_expr_up aexpr = gen_eval_for_expr (tpaddr, loc->cond.get ());
 	  xsnprintf (buf + strlen (buf), BUF_SIZE - strlen (buf), ":X%x,",
 		     aexpr->len);
 	  pkt = buf + strlen (buf);
 	  for (ndx = 0; ndx < aexpr->len; ++ndx)
 	    pkt = pack_hex_byte (pkt, aexpr->buf[ndx]);
 	  *pkt = '\0';
-	  do_cleanups (aexpr_chain);
 	}
       else
 	warning (_("Target does not support conditional tracepoints, "
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index 7ff41f9..3d28606 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -702,7 +702,6 @@ validate_actionline (const char *line, struct breakpoint *b)
   const char *tmp_p;
   const char *p;
   struct bp_location *loc;
-  struct agent_expr *aexpr;
   struct tracepoint *t = (struct tracepoint *) b;
 
   /* If EOF is typed, *line is NULL.  */
@@ -775,17 +774,16 @@ validate_actionline (const char *line, struct breakpoint *b)
 	      /* We have something to collect, make sure that the expr to
 		 bytecode translator can handle it and that it's not too
 		 long.  */
-	      aexpr = gen_trace_for_expr (loc->address, exp.get (), trace_string);
-	      old_chain = make_cleanup_free_agent_expr (aexpr);
+	      agent_expr_up aexpr = gen_trace_for_expr (loc->address,
+							exp.get (),
+							trace_string);
 
 	      if (aexpr->len > MAX_AGENT_EXPR_LEN)
 		error (_("Expression is too complicated."));
 
-	      ax_reqs (aexpr);
-
-	      report_agent_reqs_errors (aexpr);
+	      ax_reqs (aexpr.get ());
 
-	      do_cleanups (old_chain);
+	      report_agent_reqs_errors (aexpr.get ());
 	    }
 	}
       while (p && *p++ == ',');
@@ -810,16 +808,13 @@ validate_actionline (const char *line, struct breakpoint *b)
 	      /* We have something to evaluate, make sure that the expr to
 		 bytecode translator can handle it and that it's not too
 		 long.  */
-	      aexpr = gen_eval_for_expr (loc->address, exp.get ());
-	      old_chain = make_cleanup_free_agent_expr (aexpr);
+	      agent_expr_up aexpr = gen_eval_for_expr (loc->address, exp.get ());
 
 	      if (aexpr->len > MAX_AGENT_EXPR_LEN)
 		error (_("Expression is too complicated."));
 
-	      ax_reqs (aexpr);
-	      report_agent_reqs_errors (aexpr);
-
-	      do_cleanups (old_chain);
+	      ax_reqs (aexpr.get ());
+	      report_agent_reqs_errors (aexpr.get ());
 	    }
 	}
       while (p && *p++ == ',');
@@ -1045,10 +1040,10 @@ collection_list::collect_symbol (struct symbol *sym,
   /* Expressions are the most general case.  */
   if (treat_as_expr)
     {
-      struct agent_expr *aexpr;
       struct cleanup *old_chain1 = NULL;
 
-      aexpr = gen_trace_for_var (scope, gdbarch, sym, trace_string);
+      agent_expr_up aexpr = gen_trace_for_var (scope, gdbarch,
+					       sym, trace_string);
 
       /* It can happen that the symbol is recorded as a computed
 	 location, but it's been optimized away and doesn't actually
@@ -1060,33 +1055,28 @@ collection_list::collect_symbol (struct symbol *sym,
 	  return;
 	}
 
-      old_chain1 = make_cleanup_free_agent_expr (aexpr);
+      ax_reqs (aexpr.get ());
 
-      ax_reqs (aexpr);
-
-      report_agent_reqs_errors (aexpr);
-
-      discard_cleanups (old_chain1);
-      add_aexpr (aexpr);
+      report_agent_reqs_errors (aexpr.get ());
 
       /* Take care of the registers.  */
       if (aexpr->reg_mask_len > 0)
 	{
-	  int ndx1, ndx2;
-
-	  for (ndx1 = 0; ndx1 < aexpr->reg_mask_len; ndx1++)
+	  for (int ndx1 = 0; ndx1 < aexpr->reg_mask_len; ndx1++)
 	    {
 	      QUIT;	/* Allow user to bail out with ^C.  */
 	      if (aexpr->reg_mask[ndx1] != 0)
 		{
 		  /* Assume chars have 8 bits.  */
-		  for (ndx2 = 0; ndx2 < 8; ndx2++)
+		  for (int ndx2 = 0; ndx2 < 8; ndx2++)
 		    if (aexpr->reg_mask[ndx1] & (1 << ndx2))
 		      /* It's used -- record it.  */
 		      add_register (ndx1 * 8 + ndx2);
 		}
 	    }
 	}
+
+      add_aexpr (gdb::move (aexpr));
     }
 }
 
@@ -1193,7 +1183,7 @@ collection_list::collection_list ()
 collection_list::~collection_list ()
 {
   for (int ndx = 0; ndx < m_aexprs.size (); ndx++)
-    free_agent_expr (m_aexprs[ndx]);
+    delete m_aexprs[ndx];
 }
 
 /* Reduce a collection list to string form (for gdb protocol).  */
@@ -1351,7 +1341,6 @@ encode_actions_1 (struct command_line *action,
   int i;
   struct value *tempval;
   struct cmd_list_element *cmd;
-  struct agent_expr *aexpr;
 
   for (; action; action = action->next)
     {
@@ -1403,32 +1392,24 @@ encode_actions_1 (struct command_line *action,
 		}
 	      else if (0 == strncasecmp ("$_ret", action_exp, 5))
 		{
-		  struct cleanup *old_chain1 = NULL;
-
-		  aexpr = gen_trace_for_return_address (tloc->address,
-							target_gdbarch (),
-							trace_string);
-
-		  old_chain1 = make_cleanup_free_agent_expr (aexpr);
-
-		  ax_reqs (aexpr);
-		  report_agent_reqs_errors (aexpr);
+		  agent_expr_up aexpr
+		    = gen_trace_for_return_address (tloc->address,
+						    target_gdbarch (),
+						    trace_string);
 
-		  discard_cleanups (old_chain1);
-		  collect->add_aexpr (aexpr);
+		  ax_reqs (aexpr.get ());
+		  report_agent_reqs_errors (aexpr.get ());
 
 		  /* take care of the registers */
 		  if (aexpr->reg_mask_len > 0)
 		    {
-		      int ndx1, ndx2;
-
-		      for (ndx1 = 0; ndx1 < aexpr->reg_mask_len; ndx1++)
+		      for (int ndx1 = 0; ndx1 < aexpr->reg_mask_len; ndx1++)
 			{
 			  QUIT;	/* allow user to bail out with ^C */
 			  if (aexpr->reg_mask[ndx1] != 0)
 			    {
 			      /* assume chars have 8 bits */
-			      for (ndx2 = 0; ndx2 < 8; ndx2++)
+			      for (int ndx2 = 0; ndx2 < 8; ndx2++)
 				if (aexpr->reg_mask[ndx1] & (1 << ndx2))
 				  {
 				    /* It's used -- record it.  */
@@ -1438,6 +1419,7 @@ encode_actions_1 (struct command_line *action,
 			}
 		    }
 
+		  collect->add_aexpr (gdb::move (aexpr));
 		  action_exp = strchr (action_exp, ',');	/* more? */
 		}
 	      else if (0 == strncasecmp ("$_sdata", action_exp, 7))
@@ -1499,31 +1481,24 @@ encode_actions_1 (struct command_line *action,
 		      break;
 
 		    default:	/* Full-fledged expression.  */
-		      aexpr = gen_trace_for_expr (tloc->address, exp.get (),
-						  trace_string);
-
-		      old_chain1 = make_cleanup_free_agent_expr (aexpr);
+		      agent_expr_up aexpr = gen_trace_for_expr (tloc->address,
+								exp.get (),
+								trace_string);
 
-		      ax_reqs (aexpr);
+		      ax_reqs (aexpr.get ());
 
-		      report_agent_reqs_errors (aexpr);
-
-		      discard_cleanups (old_chain1);
-		      collect->add_aexpr (aexpr);
+		      report_agent_reqs_errors (aexpr.get ());
 
 		      /* Take care of the registers.  */
 		      if (aexpr->reg_mask_len > 0)
 			{
-			  int ndx1;
-			  int ndx2;
-
-			  for (ndx1 = 0; ndx1 < aexpr->reg_mask_len; ndx1++)
+			  for (int ndx1 = 0; ndx1 < aexpr->reg_mask_len; ndx1++)
 			    {
 			      QUIT;	/* Allow user to bail out with ^C.  */
 			      if (aexpr->reg_mask[ndx1] != 0)
 				{
 				  /* Assume chars have 8 bits.  */
-				  for (ndx2 = 0; ndx2 < 8; ndx2++)
+				  for (int ndx2 = 0; ndx2 < 8; ndx2++)
 				    if (aexpr->reg_mask[ndx1] & (1 << ndx2))
 				      {
 					/* It's used -- record it.  */
@@ -1533,6 +1508,7 @@ encode_actions_1 (struct command_line *action,
 			    }
 			}
 
+		      collect->add_aexpr (gdb::move (aexpr));
 		      collect->append_exp (exp.get ());
 		      break;
 		    }		/* switch */
@@ -1554,16 +1530,15 @@ encode_actions_1 (struct command_line *action,
 						   block_for_pc (tloc->address),
 						   1);
 
-		  aexpr = gen_eval_for_expr (tloc->address, exp.get ());
-		  old_chain1 = make_cleanup_free_agent_expr (aexpr);
+		  agent_expr_up aexpr = gen_eval_for_expr (tloc->address,
+							   exp.get ());
 
-		  ax_reqs (aexpr);
-		  report_agent_reqs_errors (aexpr);
+		  ax_reqs (aexpr.get ());
+		  report_agent_reqs_errors (aexpr.get ());
 
-		  discard_cleanups (old_chain1);
 		  /* Even though we're not officially collecting, add
 		     to the collect list anyway.  */
-		  collect->add_aexpr (aexpr);
+		  collect->add_aexpr (gdb::move (aexpr));
 		}		/* do */
 	    }
 	  while (action_exp && *action_exp++ == ',');
@@ -1625,9 +1600,9 @@ encode_actions_rsp (struct bp_location *tloc, char ***tdp_actions,
 }
 
 void
-collection_list::add_aexpr (struct agent_expr *aexpr)
+collection_list::add_aexpr (agent_expr_up aexpr)
 {
-  m_aexprs.push_back (aexpr);
+  m_aexprs.push_back (aexpr.release ());
 }
 
 static void
diff --git a/gdb/tracepoint.h b/gdb/tracepoint.h
index d3571b1..7dfd8cc 100644
--- a/gdb/tracepoint.h
+++ b/gdb/tracepoint.h
@@ -249,7 +249,9 @@ public:
 
   void append_exp (struct expression *exp);
 
-  void add_aexpr (struct agent_expr *aexpr);
+  /* Add AEXPR to the list, taking ownership.  */
+  void add_aexpr (agent_expr_up aexpr);
+
   void add_register (unsigned int regno);
   void add_memrange (int type, bfd_signed_vma base,
 		     unsigned long len);