'struct expression *' -> gdb::unique_xmalloc_ptr<expression>

This patch makes parse_expression and friends return a unique_ptr
instead of raw pointer [1]:

  typedef gdb::unique_malloc_ptr<expression> expression_up;

and then adjusts the codebase throughout to stop using cleanups to
manage lifetime of expression pointers.

Whenever I found a structure owning an expression pointer, I made it
store a unique_ptr instead of a raw pointer, which then requires using
new/delete of the holding structure, instead of XNEW/xfree.

[1] - I'd like to set the rule that types named with an "_up" suffix
      are unique_ptr typedefs.

Note I used gdb::unique_xmalloc_ptr instead of gdb::unique_ptr, simply
because we still use xmalloc instead of new to allocate expression
objects.  Once that's changed, all we need to do is change the
expression_up typedef and the smart pointer will then call delete
instead of xfree.

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

	* ada-lang.c (ada_read_renaming_var_value): Use expression_up.
	(struct ada_catchpoint_location) <excep_cond_expr>: Now an
	expression_up.
	(ada_catchpoint_location_dtor): Reset excep_cond_expr instead of
	using xfree.
	(create_excep_cond_exprs): Use expression_up and gdb::move.
	(allocate_location_exception): Use new instead of XNEW.
	(should_stop_exception): Likewise.  Adjust to use expression_up.
	(create_ada_exception_catchpoint): Use new instead of XNEW.
	* ax-gdb.c (agent_eval_command_one): Use expression_up instead of
	cleanups.
	(maint_agent_printf_command): Use expression_up.
	* break-catch-sig.c (create_signal_catchpoint): Use new instead of
	XNEW.
	* break-catch-syscall.c (create_syscall_event_catchpoint):
	Likewise.
	* break-catch-throw.c (handle_gnu_v3_exceptions): Use new instead
	of XCNEW.  Use gdb::unique_ptr instead of cleanups.
	* breakpoint.c (set_breakpoint_condition, update_watchpoint)
	(parse_cmd_to_aexpr, watchpoint_check)
	(bpstat_check_breakpoint_conditions, watchpoint_locations_match):
	Adjust to use expression_up.
	(init_bp_location): Adjust.
	(free_bp_location): Use delete instead of xfree.
	(set_raw_breakpoint_without_location, set_raw_breakpoint)
	(add_solib_catchpoint, create_fork_vfork_event_catchpoint)
	(new_single_step_breakpoint, create_breakpoint_sal): Use new
	instead of XNEW.
	(find_condition_and_thread): Adjust to use expression_up.
	(create_breakpoint): Use new instead of XNEW.
	(dtor_watchpoint): Don't xfree expression pointers, they're
	unique_ptr's now.
	(insert_watchpoint, remove_watchpoint): Adjust.
	(watch_command_1): Use expression_up.  Use new instead of XCNEW.
	(catch_exec_command_1): Use new instead of XNEW.
	(bp_location_dtor): Don't xfree expression pointers, they're
	unique_ptr's now.
	(base_breakpoint_allocate_location)
	(strace_marker_create_breakpoints_sal): Use new instead of XNEW.
	(delete_breakpoint): Use delete instead of xfree.
	* breakpoint.h (struct bp_location) <cond>: Now an
	unique_ptr<expression> instead of a raw pointer.
	(struct watchpoint) <exp, cond_exp>: Likewise.
	* cli/cli-script.c (execute_control_command): Use expression_up
	instead of cleanups.
	* dtrace-probe.c (dtrace_process_dof_probe): Use expression_up.
	* eval.c (parse_and_eval_address, parse_and_eval_long)
	(parse_and_eval, parse_to_comma_and_eval, parse_and_eval_type):
	Use expression_up instead of cleanups.
	* expression.h (expression_up): New typedef.
	(parse_expression, parse_expression_with_language, parse_exp_1):
	Change return type to expression_up.
	* mi/mi-main.c (mi_cmd_data_evaluate_expression)
	(print_variable_or_computed): Use expression_up.
	* objc-lang.c (print_object_command): Use expression_up instead of
	cleanups.
	* parse.c (parse_exp_1, parse_exp_in_context)
	(parse_exp_in_context_1, parse_expression)
	(parse_expression_with_language): Return an expression_up instead
	of a raw pointer.
	(parse_expression_for_completion): Use expression_up.
	* printcmd.c (struct display) <exp>: Now an expression_up instead
	of a raw pointer.
	(print_command_1, output_command_const, set_command, x_command):
	Use expression_up instead of cleanups.
	(display_command): Likewise.  Use new instead of XNEW.
	(free_display): Use delete instead of xfree.
	(do_one_display): Adjust to use expression_up.
	* remote.c (remote_download_tracepoint): Likewise.
	* stack.c (return_command): Likewise.
	* tracepoint.c (validate_actionline, encode_actions_1): Use
	expression_up instead of cleanups.
	* typeprint.c (whatis_exp, maintenance_print_type): Likewise.
	* value.c (init_if_undefined_command): Likewise.
	* varobj.c (struct varobj_root) <exp>: Now an expression_up
	instead of a raw pointer.
	(varobj_create): Adjust.
	(varobj_set_value): Use an expression_up instead of cleanups.
	(new_root_variable): Use new instead of XNEW.
	(free_variable): Use delete instead of xfree.
	(value_of_root_1): Use std::swap.

23 files changed, 257 insertions, 309 deletions

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 8b3efe8..67c0b07 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,89 @@
 2016-11-08  Pedro Alves  <palves@redhat.com>
 
+	* ada-lang.c (ada_read_renaming_var_value): Use expression_up.
+	(struct ada_catchpoint_location) <excep_cond_expr>: Now an
+	expression_up.
+	(ada_catchpoint_location_dtor): Reset excep_cond_expr instead of
+	using xfree.
+	(create_excep_cond_exprs): Use expression_up and gdb::move.
+	(allocate_location_exception): Use new instead of XNEW.
+	(should_stop_exception): Likewise.  Adjust to use expression_up.
+	(create_ada_exception_catchpoint): Use new instead of XNEW.
+	* ax-gdb.c (agent_eval_command_one): Use expression_up instead of
+	cleanups.
+	(maint_agent_printf_command): Use expression_up.
+	* break-catch-sig.c (create_signal_catchpoint): Use new instead of
+	XNEW.
+	* break-catch-syscall.c (create_syscall_event_catchpoint):
+	Likewise.
+	* break-catch-throw.c (handle_gnu_v3_exceptions): Use new instead
+	of XCNEW.  Use gdb::unique_ptr instead of cleanups.
+	* breakpoint.c (set_breakpoint_condition, update_watchpoint)
+	(parse_cmd_to_aexpr, watchpoint_check)
+	(bpstat_check_breakpoint_conditions, watchpoint_locations_match):
+	Adjust to use expression_up.
+	(init_bp_location): Adjust.
+	(free_bp_location): Use delete instead of xfree.
+	(set_raw_breakpoint_without_location, set_raw_breakpoint)
+	(add_solib_catchpoint, create_fork_vfork_event_catchpoint)
+	(new_single_step_breakpoint, create_breakpoint_sal): Use new
+	instead of XNEW.
+	(find_condition_and_thread): Adjust to use expression_up.
+	(create_breakpoint): Use new instead of XNEW.
+	(dtor_watchpoint): Don't xfree expression pointers, they're
+	unique_ptr's now.
+	(insert_watchpoint, remove_watchpoint): Adjust.
+	(watch_command_1): Use expression_up.  Use new instead of XCNEW.
+	(catch_exec_command_1): Use new instead of XNEW.
+	(bp_location_dtor): Don't xfree expression pointers, they're
+	unique_ptr's now.
+	(base_breakpoint_allocate_location)
+	(strace_marker_create_breakpoints_sal): Use new instead of XNEW.
+	(delete_breakpoint): Use delete instead of xfree.
+	* breakpoint.h (struct bp_location) <cond>: Now an
+	unique_ptr<expression> instead of a raw pointer.
+	(struct watchpoint) <exp, cond_exp>: Likewise.
+	* cli/cli-script.c (execute_control_command): Use expression_up
+	instead of cleanups.
+	* dtrace-probe.c (dtrace_process_dof_probe): Use expression_up.
+	* eval.c (parse_and_eval_address, parse_and_eval_long)
+	(parse_and_eval, parse_to_comma_and_eval, parse_and_eval_type):
+	Use expression_up instead of cleanups.
+	* expression.h (expression_up): New typedef.
+	(parse_expression, parse_expression_with_language, parse_exp_1):
+	Change return type to expression_up.
+	* mi/mi-main.c (mi_cmd_data_evaluate_expression)
+	(print_variable_or_computed): Use expression_up.
+	* objc-lang.c (print_object_command): Use expression_up instead of
+	cleanups.
+	* parse.c (parse_exp_1, parse_exp_in_context)
+	(parse_exp_in_context_1, parse_expression)
+	(parse_expression_with_language): Return an expression_up instead
+	of a raw pointer.
+	(parse_expression_for_completion): Use expression_up.
+	* printcmd.c (struct display) <exp>: Now an expression_up instead
+	of a raw pointer.
+	(print_command_1, output_command_const, set_command, x_command):
+	Use expression_up instead of cleanups.
+	(display_command): Likewise.  Use new instead of XNEW.
+	(free_display): Use delete instead of xfree.
+	(do_one_display): Adjust to use expression_up.
+	* remote.c (remote_download_tracepoint): Likewise.
+	* stack.c (return_command): Likewise.
+	* tracepoint.c (validate_actionline, encode_actions_1): Use
+	expression_up instead of cleanups.
+	* typeprint.c (whatis_exp, maintenance_print_type): Likewise.
+	* value.c (init_if_undefined_command): Likewise.
+	* varobj.c (struct varobj_root) <exp>: Now an expression_up
+	instead of a raw pointer.
+	(varobj_create): Adjust.
+	(varobj_set_value): Use an expression_up instead of cleanups.
+	(new_root_variable): Use new instead of XNEW.
+	(free_variable): Use delete instead of xfree.
+	(value_of_root_1): Use std::swap.
+
+2016-11-08  Pedro Alves  <palves@redhat.com>
+
 	* cli/cli-script.c (execute_control_command): Use std::string
 	instead of cleanups.
 	(locate_arg): Constify return type.
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index be2f47a..d1a39bc 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -4454,17 +4454,10 @@ ada_read_renaming_var_value (struct symbol *renaming_sym,
 			     const struct block *block)
 {
   const char *sym_name;
-  struct expression *expr;
-  struct value *value;
-  struct cleanup *old_chain = NULL;
 
   sym_name = SYMBOL_LINKAGE_NAME (renaming_sym);
-  expr = parse_exp_1 (&sym_name, 0, block, 0);
-  old_chain = make_cleanup (free_current_contents, &expr);
-  value = evaluate_expression (expr);
-
-  do_cleanups (old_chain);
-  return value;
+  expression_up expr = parse_exp_1 (&sym_name, 0, block, 0);
+  return evaluate_expression (expr.get ());
 }
 
 
@@ -12308,7 +12301,7 @@ struct ada_catchpoint_location
   /* The condition that checks whether the exception that was raised
      is the specific exception the user specified on catchpoint
      creation.  */
-  struct expression *excep_cond_expr;
+  expression_up excep_cond_expr;
 };
 
 /* Implement the DTOR method in the bp_location_ops structure for all
@@ -12319,7 +12312,7 @@ ada_catchpoint_location_dtor (struct bp_location *bl)
 {
   struct ada_catchpoint_location *al = (struct ada_catchpoint_location *) bl;
 
-  xfree (al->excep_cond_expr);
+  al->excep_cond_expr.reset ();
 }
 
 /* The vtable to be used in Ada catchpoint locations.  */
@@ -12371,7 +12364,7 @@ create_excep_cond_exprs (struct ada_catchpoint *c)
     {
       struct ada_catchpoint_location *ada_loc
 	= (struct ada_catchpoint_location *) bl;
-      struct expression *exp = NULL;
+      expression_up exp;
 
       if (!bl->shlib_disabled)
 	{
@@ -12380,26 +12373,20 @@ create_excep_cond_exprs (struct ada_catchpoint *c)
 	  s = cond_string;
 	  TRY
 	    {
-	      exp = parse_exp_1 (&s, bl->address,
-				 block_for_pc (bl->address), 0);
+	      exp = gdb::move (parse_exp_1 (&s, bl->address,
+					    block_for_pc (bl->address),
+					    0));
 	    }
 	  CATCH (e, RETURN_MASK_ERROR)
 	    {
 	      warning (_("failed to reevaluate internal exception condition "
 			 "for catchpoint %d: %s"),
 		       c->base.number, e.message);
-	      /* There is a bug in GCC on sparc-solaris when building with
-		 optimization which causes EXP to change unexpectedly
-		 (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56982).
-		 The problem should be fixed starting with GCC 4.9.
-		 In the meantime, work around it by forcing EXP back
-		 to NULL.  */
-	      exp = NULL;
 	    }
 	  END_CATCH
 	}
 
-      ada_loc->excep_cond_expr = exp;
+      ada_loc->excep_cond_expr = gdb::move (exp);
     }
 
   do_cleanups (old_chain);
@@ -12427,7 +12414,7 @@ allocate_location_exception (enum ada_exception_catchpoint_kind ex,
 {
   struct ada_catchpoint_location *loc;
 
-  loc = XNEW (struct ada_catchpoint_location);
+  loc = new ada_catchpoint_location ();
   init_bp_location (&loc->base, &ada_catchpoint_location_ops, self);
   loc->excep_cond_expr = NULL;
   return &loc->base;
@@ -12479,7 +12466,7 @@ should_stop_exception (const struct bp_location *bl)
       struct value *mark;
 
       mark = value_mark ();
-      stop = value_true (evaluate_expression (ada_loc->excep_cond_expr));
+      stop = value_true (evaluate_expression (ada_loc->excep_cond_expr.get ()));
       value_free_to_mark (mark);
     }
   CATCH (ex, RETURN_MASK_ALL)
@@ -13143,7 +13130,7 @@ create_ada_exception_catchpoint (struct gdbarch *gdbarch,
   struct symtab_and_line sal
     = ada_exception_sal (ex_kind, excep_string, &addr_string, &ops);
 
-  c = XNEW (struct ada_catchpoint);
+  c = new ada_catchpoint ();
   init_ada_exception_breakpoint (&c->base, gdbarch, sal, addr_string,
 				 ops, tempflag, disabled, from_tty);
   c->excep_string = excep_string;
diff --git a/gdb/ax-gdb.c b/gdb/ax-gdb.c
index 7c6cb64..ccd16f2 100644
--- a/gdb/ax-gdb.c
+++ b/gdb/ax-gdb.c
@@ -2581,7 +2581,6 @@ static void
 agent_eval_command_one (const char *exp, int eval, CORE_ADDR pc)
 {
   struct cleanup *old_chain = 0;
-  struct expression *expr;
   struct agent_expr *agent;
   const char *arg;
   int trace_string = 0;
@@ -2601,16 +2600,15 @@ agent_eval_command_one (const char *exp, int eval, CORE_ADDR pc)
     }
   else
     {
-      expr = parse_exp_1 (&arg, pc, block_for_pc (pc), 0);
-      old_chain = make_cleanup (free_current_contents, &expr);
+      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);
+	  agent = gen_eval_for_expr (pc, expr.get ());
 	}
       else
-	agent = gen_trace_for_expr (pc, expr, trace_string);
-      make_cleanup_free_agent_expr (agent);
+	agent = gen_trace_for_expr (pc, expr.get (), trace_string);
+      old_chain = make_cleanup_free_agent_expr (agent);
     }
 
   ax_reqs (agent);
@@ -2696,7 +2694,6 @@ static void
 maint_agent_printf_command (char *exp, int from_tty)
 {
   struct cleanup *old_chain = 0;
-  struct expression *expr;
   struct expression *argvec[100];
   struct agent_expr *agent;
   struct frame_info *fi = get_current_frame ();	/* need current scope */
@@ -2748,8 +2745,8 @@ maint_agent_printf_command (char *exp, int from_tty)
       const char *cmd1;
 
       cmd1 = cmdrest;
-      expr = parse_exp_1 (&cmd1, 0, (struct block *) 0, 1);
-      argvec[nargs] = expr;
+      expression_up expr = parse_exp_1 (&cmd1, 0, (struct block *) 0, 1);
+      argvec[nargs] = expr.release ();
       ++nargs;
       cmdrest = cmd1;
       if (*cmdrest == ',')
diff --git a/gdb/break-catch-sig.c b/gdb/break-catch-sig.c
index 296f900..e869a83 100644
--- a/gdb/break-catch-sig.c
+++ b/gdb/break-catch-sig.c
@@ -371,7 +371,7 @@ create_signal_catchpoint (int tempflag, VEC (gdb_signal_type) *filter,
   struct signal_catchpoint *c;
   struct gdbarch *gdbarch = get_current_arch ();
 
-  c = XNEW (struct signal_catchpoint);
+  c = new signal_catchpoint ();
   init_catchpoint (&c->base, gdbarch, tempflag, NULL, &signal_catchpoint_ops);
   c->signals_to_be_caught = filter;
   c->catch_all = catch_all;
diff --git a/gdb/break-catch-syscall.c b/gdb/break-catch-syscall.c
index 63b8cd2..a4df2ce 100644
--- a/gdb/break-catch-syscall.c
+++ b/gdb/break-catch-syscall.c
@@ -433,7 +433,7 @@ create_syscall_event_catchpoint (int tempflag, VEC(int) *filter,
   struct syscall_catchpoint *c;
   struct gdbarch *gdbarch = get_current_arch ();
 
-  c = XNEW (struct syscall_catchpoint);
+  c = new syscall_catchpoint ();
   init_catchpoint (&c->base, gdbarch, tempflag, NULL, ops);
   c->syscalls_to_be_caught = filter;
 
diff --git a/gdb/break-catch-throw.c b/gdb/break-catch-throw.c
index 153db71..cfec33b 100644
--- a/gdb/break-catch-throw.c
+++ b/gdb/break-catch-throw.c
@@ -393,8 +393,6 @@ static void
 handle_gnu_v3_exceptions (int tempflag, char *except_rx, char *cond_string,
 			  enum exception_event_kind ex_event, int from_tty)
 {
-  struct exception_catchpoint *cp;
-  struct cleanup *cleanup = make_cleanup (null_cleanup, NULL);
   regex_t *pattern = NULL;
 
   if (except_rx != NULL)
@@ -406,8 +404,7 @@ handle_gnu_v3_exceptions (int tempflag, char *except_rx, char *cond_string,
 			   _("invalid type-matching regexp"));
     }
 
-  cp = XCNEW (struct exception_catchpoint);
-  make_cleanup (xfree, cp);
+  gdb::unique_ptr<exception_catchpoint> cp (new exception_catchpoint ());
 
   init_catchpoint (&cp->base, get_current_arch (), tempflag, cond_string,
 		   &gnu_v3_exception_catchpoint_ops);
@@ -421,7 +418,7 @@ handle_gnu_v3_exceptions (int tempflag, char *except_rx, char *cond_string,
   re_set_exception_catchpoint (&cp->base);
 
   install_breakpoint (0, &cp->base, 1);
-  discard_cleanups (cleanup);
+  cp.release ();
 }
 
 /* Look for an "if" token in *STRING.  The "if" token must be preceded
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 38262c1..101f230 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -981,8 +981,7 @@ set_breakpoint_condition (struct breakpoint *b, const char *exp,
     {
       struct watchpoint *w = (struct watchpoint *) b;
 
-      xfree (w->cond_exp);
-      w->cond_exp = NULL;
+      w->cond_exp.reset ();
     }
   else
     {
@@ -990,8 +989,7 @@ set_breakpoint_condition (struct breakpoint *b, const char *exp,
 
       for (loc = b->loc; loc; loc = loc->next)
 	{
-	  xfree (loc->cond);
-	  loc->cond = NULL;
+	  loc->cond.reset ();
 
 	  /* No need to free the condition agent expression
 	     bytecode (if we have one).  We will handle this
@@ -1897,11 +1895,7 @@ update_watchpoint (struct watchpoint *b, int reparse)
     {
       const char *s;
 
-      if (b->exp)
-	{
-	  xfree (b->exp);
-	  b->exp = NULL;
-	}
+      b->exp.reset ();
       s = b->exp_string_reparse ? b->exp_string_reparse : b->exp_string;
       b->exp = parse_exp_1 (&s, 0, b->exp_valid_block, 0);
       /* If the meaning of expression itself changed, the old value is
@@ -1917,11 +1911,7 @@ update_watchpoint (struct watchpoint *b, int reparse)
 	 locations (re)created below.  */
       if (b->base.cond_string != NULL)
 	{
-	  if (b->cond_exp != NULL)
-	    {
-	      xfree (b->cond_exp);
-	      b->cond_exp = NULL;
-	    }
+	  b->cond_exp.reset ();
 
 	  s = b->base.cond_string;
 	  b->cond_exp = parse_exp_1 (&s, 0, b->cond_exp_valid_block, 0);
@@ -1953,7 +1943,7 @@ update_watchpoint (struct watchpoint *b, int reparse)
       struct value *val_chain, *v, *result, *next;
       struct program_space *frame_pspace;
 
-      fetch_subexp_value (b->exp, &pc, &v, &result, &val_chain, 0);
+      fetch_subexp_value (b->exp.get (), &pc, &v, &result, &val_chain, 0);
 
       /* Avoid setting b->val if it's already set.  The meaning of
 	 b->val is 'the last value' user saw, and we should update
@@ -2337,7 +2327,7 @@ build_target_condition_list (struct bp_location *bl)
 		 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);
+	      aexpr = parse_cond_to_aexpr (bl->address, loc->cond.get ());
 	      loc->cond_bytecode = aexpr;
 	    }
 
@@ -2402,7 +2392,7 @@ static struct agent_expr *
 parse_cmd_to_aexpr (CORE_ADDR scope, char *cmd)
 {
   struct cleanup *old_cleanups = 0;
-  struct expression *expr, **argvec;
+  struct expression **argvec;
   struct agent_expr *aexpr = NULL;
   const char *cmdrest;
   const char *format_start, *format_end;
@@ -2453,8 +2443,8 @@ parse_cmd_to_aexpr (CORE_ADDR scope, char *cmd)
       const char *cmd1;
 
       cmd1 = cmdrest;
-      expr = parse_exp_1 (&cmd1, scope, block_for_pc (scope), 1);
-      argvec[nargs++] = expr;
+      expression_up expr = parse_exp_1 (&cmd1, scope, block_for_pc (scope), 1);
+      argvec[nargs++] = expr.release ();
       cmdrest = cmd1;
       if (*cmdrest == ',')
 	++cmdrest;
@@ -5223,7 +5213,7 @@ watchpoint_check (void *p)
 	return WP_VALUE_CHANGED;
 
       mark = value_mark ();
-      fetch_subexp_value (b->exp, &pc, &new_val, NULL, NULL, 0);
+      fetch_subexp_value (b->exp.get (), &pc, &new_val, NULL, NULL, 0);
 
       if (b->val_bitsize != 0)
 	new_val = extract_bitfield_from_watchpoint_value (b, new_val);
@@ -5524,10 +5514,10 @@ bpstat_check_breakpoint_conditions (bpstat bs, ptid_t ptid)
     {
       struct watchpoint *w = (struct watchpoint *) b;
 
-      cond = w->cond_exp;
+      cond = w->cond_exp.get ();
     }
   else
-    cond = bl->cond;
+    cond = bl->cond.get ();
 
   if (cond && b->disposition != disp_del_at_next_stop)
     {
@@ -7150,12 +7140,12 @@ watchpoint_locations_match (struct bp_location *loc1,
        && target_can_accel_watchpoint_condition (loc1->address, 
 						 loc1->length,
 						 loc1->watchpoint_type,
-						 w1->cond_exp))
+						 w1->cond_exp.get ()))
       || (w2->cond_exp
 	  && target_can_accel_watchpoint_condition (loc2->address, 
 						    loc2->length,
 						    loc2->watchpoint_type,
-						    w2->cond_exp)))
+						    w2->cond_exp.get ())))
     return 0;
 
   /* Note that this checks the owner's type, not the location's.  In
@@ -7363,7 +7353,6 @@ init_bp_location (struct bp_location *loc, const struct bp_location_ops *ops,
 
   loc->ops = ops;
   loc->owner = owner;
-  loc->cond = NULL;
   loc->cond_bytecode = NULL;
   loc->shlib_disabled = 0;
   loc->enabled = 1;
@@ -7432,7 +7421,7 @@ static void
 free_bp_location (struct bp_location *loc)
 {
   loc->ops->dtor (loc);
-  xfree (loc);
+  delete loc;
 }
 
 /* Increment reference count.  */
@@ -7515,7 +7504,7 @@ set_raw_breakpoint_without_location (struct gdbarch *gdbarch,
 				     enum bptype bptype,
 				     const struct breakpoint_ops *ops)
 {
-  struct breakpoint *b = XNEW (struct breakpoint);
+  struct breakpoint *b = new breakpoint ();
 
   init_raw_breakpoint_without_location (b, gdbarch, bptype, ops);
   add_to_breakpoint_chain (b);
@@ -7631,7 +7620,7 @@ set_raw_breakpoint (struct gdbarch *gdbarch,
 		    struct symtab_and_line sal, enum bptype bptype,
 		    const struct breakpoint_ops *ops)
 {
-  struct breakpoint *b = XNEW (struct breakpoint);
+  struct breakpoint *b = new breakpoint ();
 
   init_raw_breakpoint (b, gdbarch, sal, bptype, ops);
   add_to_breakpoint_chain (b);
@@ -8566,7 +8555,7 @@ add_solib_catchpoint (char *arg, int is_load, int is_temp, int enabled)
     arg = "";
   arg = skip_spaces (arg);
 
-  c = XCNEW (struct solib_catchpoint);
+  c = new solib_catchpoint ();
   cleanup = make_cleanup (xfree, c);
 
   if (*arg != '\0')
@@ -8665,7 +8654,7 @@ create_fork_vfork_event_catchpoint (struct gdbarch *gdbarch,
 				    int tempflag, char *cond_string,
                                     const struct breakpoint_ops *ops)
 {
-  struct fork_catchpoint *c = XNEW (struct fork_catchpoint);
+  struct fork_catchpoint *c = new fork_catchpoint ();
 
   init_catchpoint (&c->base, gdbarch, tempflag, cond_string, ops);
 
@@ -8926,7 +8915,7 @@ enable_breakpoints_after_startup (void)
 static struct breakpoint *
 new_single_step_breakpoint (int thread, struct gdbarch *gdbarch)
 {
-  struct breakpoint *b = XNEW (struct breakpoint);
+  struct breakpoint *b = new breakpoint ();
 
   init_raw_breakpoint_without_location (b, gdbarch, bp_single_step,
 					&momentary_breakpoint_ops);
@@ -9434,11 +9423,11 @@ create_breakpoint_sal (struct gdbarch *gdbarch,
     {
       struct tracepoint *t;
 
-      t = XCNEW (struct tracepoint);
+      t = new tracepoint ();
       b = &t->base;
     }
   else
-    b = XNEW (struct breakpoint);
+    b = new breakpoint ();
 
   old_chain = make_cleanup (xfree, b);
 
@@ -9689,11 +9678,8 @@ find_condition_and_thread (const char *tok, CORE_ADDR pc,
 
       if (toklen >= 1 && strncmp (tok, "if", toklen) == 0)
 	{
-	  struct expression *expr;
-
 	  tok = cond_start = end_tok + 1;
-	  expr = parse_exp_1 (&tok, pc, block_for_pc (pc), 0);
-	  xfree (expr);
+	  parse_exp_1 (&tok, pc, block_for_pc (pc), 0);
 	  cond_end = tok;
 	  *cond_string = savestring (cond_start, cond_end - cond_start);
 	}
@@ -9938,11 +9924,11 @@ create_breakpoint (struct gdbarch *gdbarch,
 	{
 	  struct tracepoint *t;
 
-	  t = XCNEW (struct tracepoint);
+	  t = new tracepoint ();
 	  b = &t->base;
 	}
       else
-	b = XNEW (struct breakpoint);
+	b = new breakpoint ();
 
       init_raw_breakpoint_without_location (b, gdbarch, type_wanted, ops);
       b->location = copy_event_location (location);
@@ -10663,8 +10649,6 @@ dtor_watchpoint (struct breakpoint *self)
 {
   struct watchpoint *w = (struct watchpoint *) self;
 
-  xfree (w->cond_exp);
-  xfree (w->exp);
   xfree (w->exp_string);
   xfree (w->exp_string_reparse);
   value_free (w->val);
@@ -10716,7 +10700,7 @@ insert_watchpoint (struct bp_location *bl)
   int length = w->exact ? 1 : bl->length;
 
   return target_insert_watchpoint (bl->address, length, bl->watchpoint_type,
-				   w->cond_exp);
+				   w->cond_exp.get ());
 }
 
 /* Implement the "remove" breakpoint_ops method for hardware watchpoints.  */
@@ -10728,7 +10712,7 @@ remove_watchpoint (struct bp_location *bl, enum remove_bp_reason reason)
   int length = w->exact ? 1 : bl->length;
 
   return target_remove_watchpoint (bl->address, length, bl->watchpoint_type,
-				   w->cond_exp);
+				   w->cond_exp.get ());
 }
 
 static int
@@ -11159,7 +11143,6 @@ watch_command_1 (const char *arg, int accessflag, int from_tty,
 		 int just_location, int internal)
 {
   struct breakpoint *b, *scope_breakpoint = NULL;
-  struct expression *exp;
   const struct block *exp_valid_block = NULL, *cond_exp_valid_block = NULL;
   struct value *val, *mark, *result;
   int saved_bitpos = 0, saved_bitsize = 0;
@@ -11271,7 +11254,7 @@ watch_command_1 (const char *arg, int accessflag, int from_tty,
   expression = savestring (arg, exp_end - arg);
   back_to = make_cleanup (xfree, expression);
   exp_start = arg = expression;
-  exp = parse_exp_1 (&arg, 0, 0, 0);
+  expression_up exp = parse_exp_1 (&arg, 0, 0, 0);
   exp_end = arg;
   /* Remove trailing whitespace from the expression before saving it.
      This makes the eventual display of the expression string a bit
@@ -11280,7 +11263,7 @@ watch_command_1 (const char *arg, int accessflag, int from_tty,
     --exp_end;
 
   /* Checking if the expression is not constant.  */
-  if (watchpoint_exp_is_const (exp))
+  if (watchpoint_exp_is_const (exp.get ()))
     {
       int len;
 
@@ -11292,7 +11275,7 @@ watch_command_1 (const char *arg, int accessflag, int from_tty,
 
   exp_valid_block = innermost_block;
   mark = value_mark ();
-  fetch_subexp_value (exp, &pc, &val, &result, NULL, just_location);
+  fetch_subexp_value (exp.get (), &pc, &val, &result, NULL, just_location);
 
   if (val != NULL && just_location)
     {
@@ -11328,17 +11311,14 @@ watch_command_1 (const char *arg, int accessflag, int from_tty,
   toklen = end_tok - tok;
   if (toklen >= 1 && strncmp (tok, "if", toklen) == 0)
     {
-      struct expression *cond;
-
       innermost_block = NULL;
       tok = cond_start = end_tok + 1;
-      cond = parse_exp_1 (&tok, 0, 0, 0);
+      parse_exp_1 (&tok, 0, 0, 0);
 
       /* The watchpoint expression may not be local, but the condition
 	 may still be.  E.g.: `watch global if local > 0'.  */
       cond_exp_valid_block = innermost_block;
 
-      xfree (cond);
       cond_end = tok;
     }
   if (*tok)
@@ -11392,7 +11372,7 @@ watch_command_1 (const char *arg, int accessflag, int from_tty,
   else
     bp_type = bp_hardware_watchpoint;
 
-  w = XCNEW (struct watchpoint);
+  w = new watchpoint ();
   b = &w->base;
   if (use_mask)
     init_raw_breakpoint_without_location (b, NULL, bp_type,
@@ -11403,7 +11383,7 @@ watch_command_1 (const char *arg, int accessflag, int from_tty,
   b->thread = thread;
   b->disposition = disp_donttouch;
   b->pspace = current_program_space;
-  w->exp = exp;
+  w->exp = gdb::move (exp);
   w->exp_valid_block = exp_valid_block;
   w->cond_exp_valid_block = cond_exp_valid_block;
   if (just_location)
@@ -11951,7 +11931,7 @@ catch_exec_command_1 (char *arg, int from_tty,
   if ((*arg != '\0') && !isspace (*arg))
     error (_("Junk at end of arguments."));
 
-  c = XNEW (struct exec_catchpoint);
+  c = new exec_catchpoint ();
   init_catchpoint (&c->base, gdbarch, tempflag, cond_string,
 		   &catch_exec_breakpoint_ops);
   c->exec_pathname = NULL;
@@ -12919,7 +12899,6 @@ say_where (struct breakpoint *b)
 static void
 bp_location_dtor (struct bp_location *self)
 {
-  xfree (self->cond);
   if (self->cond_bytecode)
     free_agent_expr (self->cond_bytecode);
   xfree (self->function_name);
@@ -12952,7 +12931,7 @@ base_breakpoint_allocate_location (struct breakpoint *self)
 {
   struct bp_location *loc;
 
-  loc = XNEW (struct bp_location);
+  loc = new struct bp_location ();
   init_bp_location (loc, &bp_location_ops, self);
   return loc;
 }
@@ -13809,7 +13788,7 @@ strace_marker_create_breakpoints_sal (struct gdbarch *gdbarch,
       location = copy_event_location (canonical->location);
       old_chain = make_cleanup_delete_event_location (location);
 
-      tp = XCNEW (struct tracepoint);
+      tp = new tracepoint ();
       init_breakpoint_sal (&tp->base, gdbarch, expanded,
 			   location, NULL,
 			   cond_string, extra_string,
@@ -13951,7 +13930,7 @@ delete_breakpoint (struct breakpoint *bpt)
   /* On the chance that someone will soon try again to delete this
      same bp, we mark it as deleted before freeing its storage.  */
   bpt->type = bp_none;
-  xfree (bpt);
+  delete bpt;
 }
 
 static void
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 8e215b6..60ffbf7 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -339,7 +339,7 @@ struct bp_location
      different for different locations.  Only valid for real
      breakpoints; a watchpoint's conditional expression is stored in
      the owner breakpoint object.  */
-  struct expression *cond;
+  expression_up cond;
 
   /* Conditional expression in agent expression
      bytecode form.  This is used for stub-side breakpoint
@@ -794,12 +794,12 @@ struct watchpoint
   char *exp_string_reparse;
 
   /* The expression we are watching, or NULL if not a watchpoint.  */
-  struct expression *exp;
+  expression_up exp;
   /* The largest block within which it is valid, or NULL if it is
      valid anywhere (e.g. consists just of global symbols).  */
   const struct block *exp_valid_block;
   /* The conditional expression if any.  */
-  struct expression *cond_exp;
+  expression_up cond_exp;
   /* The largest block within which it is valid, or NULL if it is
      valid anywhere (e.g. consists just of global symbols).  */
   const struct block *cond_exp_valid_block;
diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c
index 7b73832..ce4d8cb 100644
--- a/gdb/cli/cli-script.c
+++ b/gdb/cli/cli-script.c
@@ -440,9 +440,7 @@ print_command_trace (const char *cmd)
 enum command_control_type
 execute_control_command (struct command_line *cmd)
 {
-  struct expression *expr;
   struct command_line *current;
-  struct cleanup *old_chain = make_cleanup (null_cleanup, 0);
   struct value *val;
   struct value *val_mark;
   int loop;
@@ -489,8 +487,7 @@ execute_control_command (struct command_line *cmd)
 
 	/* Parse the loop control expression for the while statement.  */
 	std::string new_line = insert_args (cmd->line);
-	expr = parse_expression (new_line.c_str ());
-	make_cleanup (free_current_contents, &expr);
+	expression_up expr = parse_expression (new_line.c_str ());
 
 	ret = simple_control;
 	loop = 1;
@@ -504,7 +501,7 @@ execute_control_command (struct command_line *cmd)
 
 	    /* Evaluate the expression.  */
 	    val_mark = value_mark ();
-	    val = evaluate_expression (expr);
+	    val = evaluate_expression (expr.get ());
 	    cond_result = value_true (val);
 	    value_free_to_mark (val_mark);
 
@@ -555,15 +552,14 @@ execute_control_command (struct command_line *cmd)
 
 	/* Parse the conditional for the if statement.  */
 	std::string new_line = insert_args (cmd->line);
-	expr = parse_expression (new_line.c_str ());
-	make_cleanup (free_current_contents, &expr);
+	expression_up expr = parse_expression (new_line.c_str ());
 
 	current = NULL;
 	ret = simple_control;
 
 	/* Evaluate the conditional.  */
 	val_mark = value_mark ();
-	val = evaluate_expression (expr);
+	val = evaluate_expression (expr.get ());
 
 	/* Choose which arm to take commands from based on the value
 	   of the conditional expression.  */
@@ -619,8 +615,6 @@ execute_control_command (struct command_line *cmd)
       break;
     }
 
-  do_cleanups (old_chain);
-
   return ret;
 }
 
diff --git a/gdb/dtrace-probe.c b/gdb/dtrace-probe.c
index 242e316..cbeeea8 100644
--- a/gdb/dtrace-probe.c
+++ b/gdb/dtrace-probe.c
@@ -413,7 +413,7 @@ dtrace_process_dof_probe (struct objfile *objfile,
       for (j = 0; j < ret->probe_argc; j++)
 	{
 	  struct dtrace_probe_arg arg;
-	  struct expression *expr = NULL;
+	  expression_up expr;
 
 	  /* Set arg.expr to ensure all fields in expr are initialized and
 	     the compiler will not warn when arg is used.  */
@@ -430,11 +430,11 @@ dtrace_process_dof_probe (struct objfile *objfile,
 
 	  TRY
 	    {
-	      expr = parse_expression_with_language (arg.type_str, language_c);
+	      expr = gdb::move (parse_expression_with_language (arg.type_str,
+								language_c));
 	    }
 	  CATCH (ex, RETURN_MASK_ERROR)
 	    {
-	      expr = NULL;
 	    }
 	  END_CATCH
 
diff --git a/gdb/eval.c b/gdb/eval.c
index 00a107c..f30b8e1 100644
--- a/gdb/eval.c
+++ b/gdb/eval.c
@@ -95,14 +95,9 @@ evaluate_subexp (struct type *expect_type, struct expression *exp,
 CORE_ADDR
 parse_and_eval_address (const char *exp)
 {
-  struct expression *expr = parse_expression (exp);
-  CORE_ADDR addr;
-  struct cleanup *old_chain =
-    make_cleanup (free_current_contents, &expr);
-
-  addr = value_as_address (evaluate_expression (expr));
-  do_cleanups (old_chain);
-  return addr;
+  expression_up expr = parse_expression (exp);
+
+  return value_as_address (evaluate_expression (expr.get ()));
 }
 
 /* Like parse_and_eval_address, but treats the value of the expression
@@ -110,27 +105,17 @@ parse_and_eval_address (const char *exp)
 LONGEST
 parse_and_eval_long (const char *exp)
 {
-  struct expression *expr = parse_expression (exp);
-  LONGEST retval;
-  struct cleanup *old_chain =
-    make_cleanup (free_current_contents, &expr);
-
-  retval = value_as_long (evaluate_expression (expr));
-  do_cleanups (old_chain);
-  return (retval);
+  expression_up expr = parse_expression (exp);
+
+  return value_as_long (evaluate_expression (expr.get ()));
 }
 
 struct value *
 parse_and_eval (const char *exp)
 {
-  struct expression *expr = parse_expression (exp);
-  struct value *val;
-  struct cleanup *old_chain =
-    make_cleanup (free_current_contents, &expr);
+  expression_up expr = parse_expression (exp);
 
-  val = evaluate_expression (expr);
-  do_cleanups (old_chain);
-  return val;
+  return evaluate_expression (expr.get ());
 }
 
 /* Parse up to a comma (or to a closeparen)
@@ -140,14 +125,9 @@ parse_and_eval (const char *exp)
 struct value *
 parse_to_comma_and_eval (const char **expp)
 {
-  struct expression *expr = parse_exp_1 (expp, 0, (struct block *) 0, 1);
-  struct value *val;
-  struct cleanup *old_chain =
-    make_cleanup (free_current_contents, &expr);
+  expression_up expr = parse_exp_1 (expp, 0, (struct block *) 0, 1);
 
-  val = evaluate_expression (expr);
-  do_cleanups (old_chain);
-  return val;
+  return evaluate_expression (expr.get ());
 }
 
 /* Evaluate an expression in internal prefix form
@@ -3104,14 +3084,13 @@ struct type *
 parse_and_eval_type (char *p, int length)
 {
   char *tmp = (char *) alloca (length + 4);
-  struct expression *expr;
 
   tmp[0] = '(';
   memcpy (tmp + 1, p, length);
   tmp[length + 1] = ')';
   tmp[length + 2] = '0';
   tmp[length + 3] = '\0';
-  expr = parse_expression (tmp);
+  expression_up expr = parse_expression (tmp);
   if (expr->elts[0].opcode != UNOP_CAST)
     error (_("Internal error in eval_type."));
   return expr->elts[1].type;
diff --git a/gdb/expression.h b/gdb/expression.h
index 4952d84..d8b8cba 100644
--- a/gdb/expression.h
+++ b/gdb/expression.h
@@ -85,6 +85,8 @@ struct expression
     union exp_element elts[1];
   };
 
+typedef gdb::unique_xmalloc_ptr<expression> expression_up;
+
 /* Macros for converting between number of expression elements and bytes
    to store that many expression elements.  */
 
@@ -95,16 +97,16 @@ struct expression
 
 /* From parse.c */
 
-extern struct expression *parse_expression (const char *);
+extern expression_up parse_expression (const char *);
 
-extern struct expression *parse_expression_with_language (const char *string,
-							  enum language lang);
+extern expression_up parse_expression_with_language (const char *string,
+						     enum language lang);
 
 extern struct type *parse_expression_for_completion (const char *, char **,
 						     enum type_code *);
 
-extern struct expression *parse_exp_1 (const char **, CORE_ADDR pc,
-				       const struct block *, int);
+extern expression_up parse_exp_1 (const char **, CORE_ADDR pc,
+				  const struct block *, int);
 
 /* For use by parsers; set if we want to parse an expression and
    attempt completion.  */
diff --git a/gdb/gnu-v3-abi.c b/gdb/gnu-v3-abi.c
index 0e037e6..eda9a11 100644
--- a/gdb/gnu-v3-abi.c
+++ b/gdb/gnu-v3-abi.c
@@ -1195,7 +1195,6 @@ gnuv3_get_type_from_type_info (struct value *type_info_ptr)
   char *type_name;
   struct cleanup *cleanup;
   struct value *type_val;
-  struct expression *expr;
   struct type *result;
 
   type_name = gnuv3_get_typename_from_type_info (type_info_ptr);
@@ -1206,10 +1205,9 @@ gnuv3_get_type_from_type_info (struct value *type_info_ptr)
      mis-parse.  Another approach might be to re-use the demangler's
      internal form to reconstruct the type somehow.  */
 
-  expr = parse_expression (type_name);
-  make_cleanup (xfree, expr);
+  expression_up expr = parse_expression (type_name);
 
-  type_val = evaluate_type (expr);
+  type_val = evaluate_type (expr.get ());
   result = value_type (type_val);
 
   do_cleanups (cleanup);
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 3b071af..2f189d5 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1362,7 +1362,6 @@ mi_cmd_data_write_register_values (char *command, char **argv, int argc)
 void
 mi_cmd_data_evaluate_expression (char *command, char **argv, int argc)
 {
-  struct expression *expr;
   struct cleanup *old_chain;
   struct value *val;
   struct ui_file *stb;
@@ -1376,11 +1375,9 @@ mi_cmd_data_evaluate_expression (char *command, char **argv, int argc)
     error (_("-data-evaluate-expression: "
 	     "Usage: -data-evaluate-expression expression"));
 
-  expr = parse_expression (argv[0]);
+  expression_up expr = parse_expression (argv[0]);
 
-  make_cleanup (free_current_contents, &expr);
-
-  val = evaluate_expression (expr);
+  val = evaluate_expression (expr.get ());
 
   /* Print the result of the expression evaluation.  */
   get_user_print_options (&opts);
@@ -2743,7 +2740,6 @@ mi_cmd_ada_task_info (char *command, char **argv, int argc)
 static void
 print_variable_or_computed (char *expression, enum print_values values)
 {
-  struct expression *expr;
   struct cleanup *old_chain;
   struct value *val;
   struct ui_file *stb;
@@ -2753,14 +2749,12 @@ print_variable_or_computed (char *expression, enum print_values values)
   stb = mem_fileopen ();
   old_chain = make_cleanup_ui_file_delete (stb);
 
-  expr = parse_expression (expression);
-
-  make_cleanup (free_current_contents, &expr);
+  expression_up expr = parse_expression (expression);
 
   if (values == PRINT_SIMPLE_VALUES)
-    val = evaluate_type (expr);
+    val = evaluate_type (expr.get ());
   else
-    val = evaluate_expression (expr);
+    val = evaluate_expression (expr.get ());
 
   if (values != PRINT_NO_VALUES)
     make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
diff --git a/gdb/objc-lang.c b/gdb/objc-lang.c
index b2844b9..43d83da 100644
--- a/gdb/objc-lang.c
+++ b/gdb/objc-lang.c
@@ -1193,14 +1193,11 @@ print_object_command (char *args, int from_tty)
 "The 'print-object' command requires an argument (an Objective-C object)");
 
   {
-    struct expression *expr = parse_expression (args);
-    struct cleanup *old_chain = 
-      make_cleanup (free_current_contents, &expr);
+    expression_up expr = parse_expression (args);
     int pc = 0;
 
     object = evaluate_subexp (builtin_type (expr->gdbarch)->builtin_data_ptr,
-			      expr, &pc, EVAL_NORMAL);
-    do_cleanups (old_chain);
+			      expr.get (), &pc, EVAL_NORMAL);
   }
 
   /* Validate the address for sanity.  */
diff --git a/gdb/parse.c b/gdb/parse.c
index 231eebf..e921438 100644
--- a/gdb/parse.c
+++ b/gdb/parse.c
@@ -115,12 +115,12 @@ static void free_funcalls (void *ignore);
 static int prefixify_subexp (struct expression *, struct expression *, int,
 			     int);
 
-static struct expression *parse_exp_in_context (const char **, CORE_ADDR,
-						const struct block *, int, 
-						int, int *);
-static struct expression *parse_exp_in_context_1 (const char **, CORE_ADDR,
-						  const struct block *, int,
-						  int, int *);
+static expression_up parse_exp_in_context (const char **, CORE_ADDR,
+					   const struct block *, int,
+					   int, int *);
+static expression_up parse_exp_in_context_1 (const char **, CORE_ADDR,
+					     const struct block *, int,
+					     int, int *);
 
 void _initialize_parse (void);
 
@@ -1108,14 +1108,14 @@ prefixify_subexp (struct expression *inexpr,
 
    If COMMA is nonzero, stop if a comma is reached.  */
 
-struct expression *
+expression_up
 parse_exp_1 (const char **stringptr, CORE_ADDR pc, const struct block *block,
 	     int comma)
 {
   return parse_exp_in_context (stringptr, pc, block, comma, 0, NULL);
 }
 
-static struct expression *
+static expression_up
 parse_exp_in_context (const char **stringptr, CORE_ADDR pc,
 		      const struct block *block,
 		      int comma, int void_context_p, int *out_subexp)
@@ -1131,7 +1131,7 @@ parse_exp_in_context (const char **stringptr, CORE_ADDR pc,
    left-hand-side of the struct op.  If not doing such completion, it
    is left untouched.  */
 
-static struct expression *
+static expression_up
 parse_exp_in_context_1 (const char **stringptr, CORE_ADDR pc,
 			const struct block *block,
 			int comma, int void_context_p, int *out_subexp)
@@ -1254,18 +1254,16 @@ parse_exp_in_context_1 (const char **stringptr, CORE_ADDR pc,
   discard_cleanups (old_chain);
 
   *stringptr = lexptr;
-  return ps.expout;
+  return expression_up (ps.expout);
 }
 
 /* Parse STRING as an expression, and complain if this fails
    to use up all of the contents of STRING.  */
 
-struct expression *
+expression_up
 parse_expression (const char *string)
 {
-  struct expression *exp;
-
-  exp = parse_exp_1 (&string, 0, 0, 0);
+  expression_up exp = parse_exp_1 (&string, 0, 0, 0);
   if (*string)
     error (_("Junk after end of expression."));
   return exp;
@@ -1274,11 +1272,10 @@ parse_expression (const char *string)
 /* Same as parse_expression, but using the given language (LANG)
    to parse the expression.  */
 
-struct expression *
+expression_up
 parse_expression_with_language (const char *string, enum language lang)
 {
   struct cleanup *old_chain = NULL;
-  struct expression *expr;
 
   if (current_language->la_language != lang)
     {
@@ -1286,7 +1283,7 @@ parse_expression_with_language (const char *string, enum language lang)
       set_language (lang);
     }
 
-  expr = parse_expression (string);
+  expression_up expr = parse_expression (string);
 
   if (old_chain != NULL)
     do_cleanups (old_chain);
@@ -1305,14 +1302,14 @@ struct type *
 parse_expression_for_completion (const char *string, char **name,
 				 enum type_code *code)
 {
-  struct expression *exp = NULL;
+  expression_up exp;
   struct value *val;
   int subexp;
 
   TRY
     {
       parse_completion = 1;
-      exp = parse_exp_in_context (&string, 0, 0, 0, 0, &subexp);
+      exp = gdb::move (parse_exp_in_context (&string, 0, 0, 0, 0, &subexp));
     }
   CATCH (except, RETURN_MASK_ERROR)
     {
@@ -1333,24 +1330,17 @@ parse_expression_for_completion (const char *string, char **name,
     }
 
   if (expout_last_struct == -1)
-    {
-      xfree (exp);
-      return NULL;
-    }
+    return NULL;
 
-  *name = extract_field_op (exp, &subexp);
+  *name = extract_field_op (exp.get (), &subexp);
   if (!*name)
-    {
-      xfree (exp);
-      return NULL;
-    }
+    return NULL;
 
   /* This might throw an exception.  If so, we want to let it
      propagate.  */
-  val = evaluate_subexpression_type (exp, subexp);
+  val = evaluate_subexpression_type (exp.get (), subexp);
   /* (*NAME) is a part of the EXP memory block freed below.  */
   *name = xstrdup (*name);
-  xfree (exp);
 
   return value_type (val);
 }
diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index 7180fda..b70ed19 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -119,7 +119,7 @@ struct display
     char *exp_string;
 
     /* Expression to be evaluated and displayed.  */
-    struct expression *exp;
+    expression_up exp;
 
     /* Item number of this auto-display item.  */
     int number;
@@ -1243,8 +1243,6 @@ print_value (struct value *val, const struct format_data *fmtp)
 static void
 print_command_1 (const char *exp, int voidprint)
 {
-  struct expression *expr;
-  struct cleanup *old_chain = make_cleanup (null_cleanup, NULL);
   struct value *val;
   struct format_data fmt;
 
@@ -1252,9 +1250,8 @@ print_command_1 (const char *exp, int voidprint)
 
   if (exp && *exp)
     {
-      expr = parse_expression (exp);
-      make_cleanup (free_current_contents, &expr);
-      val = evaluate_expression (expr);
+      expression_up expr = parse_expression (exp);
+      val = evaluate_expression (expr.get ());
     }
   else
     val = access_value_history (0);
@@ -1262,8 +1259,6 @@ print_command_1 (const char *exp, int voidprint)
   if (voidprint || (val && value_type (val) &&
 		    TYPE_CODE (value_type (val)) != TYPE_CODE_VOID))
     print_value (val, &fmt);
-
-  do_cleanups (old_chain);
 }
 
 static void
@@ -1292,8 +1287,6 @@ output_command (char *exp, int from_tty)
 void
 output_command_const (const char *exp, int from_tty)
 {
-  struct expression *expr;
-  struct cleanup *old_chain;
   char format = 0;
   struct value *val;
   struct format_data fmt;
@@ -1310,10 +1303,9 @@ output_command_const (const char *exp, int from_tty)
       format = fmt.format;
     }
 
-  expr = parse_expression (exp);
-  old_chain = make_cleanup (free_current_contents, &expr);
+  expression_up expr = parse_expression (exp);
 
-  val = evaluate_expression (expr);
+  val = evaluate_expression (expr.get ());
 
   annotate_value_begin (value_type (val));
 
@@ -1325,16 +1317,12 @@ output_command_const (const char *exp, int from_tty)
 
   wrap_here ("");
   gdb_flush (gdb_stdout);
-
-  do_cleanups (old_chain);
 }
 
 static void
 set_command (char *exp, int from_tty)
 {
-  struct expression *expr = parse_expression (exp);
-  struct cleanup *old_chain =
-    make_cleanup (free_current_contents, &expr);
+  expression_up expr = parse_expression (exp);
 
   if (expr->nelts >= 1)
     switch (expr->elts[0].opcode)
@@ -1352,8 +1340,7 @@ set_command (char *exp, int from_tty)
 	  (_("Expression is not an assignment (and might have no effect)"));
       }
 
-  evaluate_expression (expr);
-  do_cleanups (old_chain);
+  evaluate_expression (expr.get ());
 }
 
 static void
@@ -1676,7 +1663,6 @@ address_info (char *exp, int from_tty)
 static void
 x_command (char *exp, int from_tty)
 {
-  struct expression *expr;
   struct format_data fmt;
   struct cleanup *old_chain;
   struct value *val;
@@ -1698,14 +1684,13 @@ x_command (char *exp, int from_tty)
 
   if (exp != 0 && *exp != 0)
     {
-      expr = parse_expression (exp);
+      expression_up expr = parse_expression (exp);
       /* Cause expression not to be there any more if this command is
          repeated with Newline.  But don't clobber a user-defined
          command's definition.  */
       if (from_tty)
 	*exp = 0;
-      old_chain = make_cleanup (free_current_contents, &expr);
-      val = evaluate_expression (expr);
+      val = evaluate_expression (expr.get ());
       if (TYPE_CODE (value_type (val)) == TYPE_CODE_REF)
 	val = coerce_ref (val);
       /* In rvalue contexts, such as this, functions are coerced into
@@ -1718,7 +1703,6 @@ x_command (char *exp, int from_tty)
 	next_address = value_as_address (val);
 
       next_gdbarch = expr->gdbarch;
-      do_cleanups (old_chain);
     }
 
   if (!next_gdbarch)
@@ -1764,7 +1748,6 @@ static void
 display_command (char *arg, int from_tty)
 {
   struct format_data fmt;
-  struct expression *expr;
   struct display *newobj;
   const char *exp = arg;
 
@@ -1792,12 +1775,12 @@ display_command (char *arg, int from_tty)
     }
 
   innermost_block = NULL;
-  expr = parse_expression (exp);
+  expression_up expr = parse_expression (exp);
 
-  newobj = XNEW (struct display);
+  newobj = new display ();
 
   newobj->exp_string = xstrdup (exp);
-  newobj->exp = expr;
+  newobj->exp = gdb::move (expr);
   newobj->block = innermost_block;
   newobj->pspace = current_program_space;
   newobj->number = ++display_number;
@@ -1826,8 +1809,7 @@ static void
 free_display (struct display *d)
 {
   xfree (d->exp_string);
-  xfree (d->exp);
-  xfree (d);
+  delete d;
 }
 
 /* Clear out the display_chain.  Done when new symtabs are loaded,
@@ -1951,8 +1933,7 @@ do_one_display (struct display *d)
      expression if the current architecture has changed.  */
   if (d->exp != NULL && d->exp->gdbarch != get_current_arch ())
     {
-      xfree (d->exp);
-      d->exp = NULL;
+      d->exp.reset ();
       d->block = NULL;
     }
 
@@ -2025,7 +2006,7 @@ do_one_display (struct display *d)
 	  struct value *val;
 	  CORE_ADDR addr;
 
-	  val = evaluate_expression (d->exp);
+	  val = evaluate_expression (d->exp.get ());
 	  addr = value_as_address (val);
 	  if (d->format.format == 'i')
 	    addr = gdbarch_addr_bits_remove (d->exp->gdbarch, addr);
@@ -2062,7 +2043,7 @@ do_one_display (struct display *d)
         {
 	  struct value *val;
 
-	  val = evaluate_expression (d->exp);
+	  val = evaluate_expression (d->exp.get ());
 	  print_formatted (val, d->format.size, &opts, gdb_stdout);
 	}
       CATCH (ex, RETURN_MASK_ERROR)
@@ -2222,10 +2203,9 @@ clear_dangling_display_expressions (struct objfile *objfile)
 	continue;
 
       if (lookup_objfile_from_block (d->block) == objfile
-	  || (d->exp && exp_uses_objfile (d->exp, objfile)))
+	  || (d->exp != NULL && exp_uses_objfile (d->exp.get (), objfile)))
       {
-	xfree (d->exp);
-	d->exp = NULL;
+	d->exp.reset ();
 	d->block = NULL;
       }
     }
diff --git a/gdb/remote.c b/gdb/remote.c
index 6c40f02..7fb32ac 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -12354,7 +12354,7 @@ 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);
+	  aexpr = gen_eval_for_expr (tpaddr, loc->cond.get ());
 	  aexpr_chain = make_cleanup_free_agent_expr (aexpr);
 	  xsnprintf (buf + strlen (buf), BUF_SIZE - strlen (buf), ":X%x,",
 		     aexpr->len);
diff --git a/gdb/stack.c b/gdb/stack.c
index 823433d..9b4e356 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -2433,13 +2433,12 @@ return_command (char *retval_exp, int from_tty)
      message.  */
   if (retval_exp)
     {
-      struct expression *retval_expr = parse_expression (retval_exp);
-      struct cleanup *old_chain = make_cleanup (xfree, retval_expr);
+      expression_up retval_expr = parse_expression (retval_exp);
       struct type *return_type = NULL;
 
       /* Compute the return value.  Should the computation fail, this
          call throws an error.  */
-      return_value = evaluate_expression (retval_expr);
+      return_value = evaluate_expression (retval_expr.get ());
 
       /* Cast return value to the return type of the function.  Should
          the cast fail, this call throws an error.  */
@@ -2454,7 +2453,6 @@ return_command (char *retval_exp, int from_tty)
 		     "Please use an explicit cast of the value to return."));
 	  return_type = value_type (return_value);
 	}
-      do_cleanups (old_chain);
       return_type = check_typedef (return_type);
       return_value = value_cast (return_type, return_value);
 
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index e5c353c..e0ef6e8 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -701,7 +701,6 @@ void
 validate_actionline (const char *line, struct breakpoint *b)
 {
   struct cmd_list_element *c;
-  struct expression *exp = NULL;
   struct cleanup *old_chain = NULL;
   const char *tmp_p;
   const char *p;
@@ -755,9 +754,8 @@ validate_actionline (const char *line, struct breakpoint *b)
 	  for (loc = t->base.loc; loc; loc = loc->next)
 	    {
 	      p = tmp_p;
-	      exp = parse_exp_1 (&p, loc->address,
-				 block_for_pc (loc->address), 1);
-	      old_chain = make_cleanup (free_current_contents, &exp);
+	      expression_up exp = parse_exp_1 (&p, loc->address,
+					       block_for_pc (loc->address), 1);
 
 	      if (exp->elts[0].opcode == OP_VAR_VALUE)
 		{
@@ -780,8 +778,8 @@ 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, trace_string);
-	      make_cleanup_free_agent_expr (aexpr);
+	      aexpr = gen_trace_for_expr (loc->address, exp.get (), trace_string);
+	      old_chain = make_cleanup_free_agent_expr (aexpr);
 
 	      if (aexpr->len > MAX_AGENT_EXPR_LEN)
 		error (_("Expression is too complicated."));
@@ -809,15 +807,14 @@ validate_actionline (const char *line, struct breakpoint *b)
 	      p = tmp_p;
 
 	      /* Only expressions are allowed for this action.  */
-	      exp = parse_exp_1 (&p, loc->address,
-				 block_for_pc (loc->address), 1);
-	      old_chain = make_cleanup (free_current_contents, &exp);
+	      expression_up exp = parse_exp_1 (&p, loc->address,
+					       block_for_pc (loc->address), 1);
 
 	      /* 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);
-	      make_cleanup_free_agent_expr (aexpr);
+	      aexpr = gen_eval_for_expr (loc->address, exp.get ());
+	      old_chain = make_cleanup_free_agent_expr (aexpr);
 
 	      if (aexpr->len > MAX_AGENT_EXPR_LEN)
 		error (_("Expression is too complicated."));
@@ -1399,7 +1396,6 @@ encode_actions_1 (struct command_line *action,
 		  struct collection_list *stepping_list)
 {
   const char *action_exp;
-  struct expression *exp = NULL;
   int i;
   struct value *tempval;
   struct cmd_list_element *cmd;
@@ -1501,12 +1497,11 @@ encode_actions_1 (struct command_line *action,
 	      else
 		{
 		  unsigned long addr;
-		  struct cleanup *old_chain = NULL;
 		  struct cleanup *old_chain1 = NULL;
 
-		  exp = parse_exp_1 (&action_exp, tloc->address,
-				     block_for_pc (tloc->address), 1);
-		  old_chain = make_cleanup (free_current_contents, &exp);
+		  expression_up exp = parse_exp_1 (&action_exp, tloc->address,
+						   block_for_pc (tloc->address),
+						   1);
 
 		  switch (exp->elts[0].opcode)
 		    {
@@ -1528,13 +1523,13 @@ encode_actions_1 (struct command_line *action,
 
 		    case UNOP_MEMVAL:
 		      /* Safe because we know it's a simple expression.  */
-		      tempval = evaluate_expression (exp);
+		      tempval = evaluate_expression (exp.get ());
 		      addr = value_address (tempval);
 		      /* Initialize the TYPE_LENGTH if it is a typedef.  */
 		      check_typedef (exp->elts[1].type);
 		      add_memrange (collect, memrange_absolute, addr,
 				    TYPE_LENGTH (exp->elts[1].type));
-		      append_exp (exp, &collect->computed);
+		      append_exp (exp.get (), &collect->computed);
 		      break;
 
 		    case OP_VAR_VALUE:
@@ -1556,7 +1551,7 @@ encode_actions_1 (struct command_line *action,
 		      break;
 
 		    default:	/* Full-fledged expression.  */
-		      aexpr = gen_trace_for_expr (tloc->address, exp,
+		      aexpr = gen_trace_for_expr (tloc->address, exp.get (),
 						  trace_string);
 
 		      old_chain1 = make_cleanup_free_agent_expr (aexpr);
@@ -1589,10 +1584,9 @@ encode_actions_1 (struct command_line *action,
 			    }
 			}
 
-		      append_exp (exp, &collect->computed);
+		      append_exp (exp.get (), &collect->computed);
 		      break;
 		    }		/* switch */
-		  do_cleanups (old_chain);
 		}		/* do */
 	    }
 	  while (action_exp && *action_exp++ == ',');
@@ -1605,14 +1599,13 @@ encode_actions_1 (struct command_line *action,
 	      action_exp = skip_spaces_const (action_exp);
 
 		{
-		  struct cleanup *old_chain = NULL;
 		  struct cleanup *old_chain1 = NULL;
 
-		  exp = parse_exp_1 (&action_exp, tloc->address,
-				     block_for_pc (tloc->address), 1);
-		  old_chain = make_cleanup (free_current_contents, &exp);
+		  expression_up exp = parse_exp_1 (&action_exp, tloc->address,
+						   block_for_pc (tloc->address),
+						   1);
 
-		  aexpr = gen_eval_for_expr (tloc->address, exp);
+		  aexpr = gen_eval_for_expr (tloc->address, exp.get ());
 		  old_chain1 = make_cleanup_free_agent_expr (aexpr);
 
 		  ax_reqs (aexpr);
@@ -1622,8 +1615,6 @@ encode_actions_1 (struct command_line *action,
 		  /* Even though we're not officially collecting, add
 		     to the collect list anyway.  */
 		  add_aexpr (collect, aexpr);
-
-		  do_cleanups (old_chain);
 		}		/* do */
 	    }
 	  while (action_exp && *action_exp++ == ',');
diff --git a/gdb/typeprint.c b/gdb/typeprint.c
index e77513e..6851a70 100644
--- a/gdb/typeprint.c
+++ b/gdb/typeprint.c
@@ -400,7 +400,6 @@ type_to_string (struct type *type)
 static void
 whatis_exp (char *exp, int show)
 {
-  struct expression *expr;
   struct value *val;
   struct cleanup *old_chain;
   struct type *real_type = NULL;
@@ -451,9 +450,8 @@ whatis_exp (char *exp, int show)
 	  exp = skip_spaces (exp);
 	}
 
-      expr = parse_expression (exp);
-      make_cleanup (free_current_contents, &expr);
-      val = evaluate_type (expr);
+      expression_up expr = parse_expression (exp);
+      val = evaluate_type (expr.get ());
     }
   else
     val = access_value_history (0);
@@ -600,13 +598,10 @@ maintenance_print_type (char *type_name, int from_tty)
 {
   struct value *val;
   struct type *type;
-  struct cleanup *old_chain;
-  struct expression *expr;
 
   if (type_name != NULL)
     {
-      expr = parse_expression (type_name);
-      old_chain = make_cleanup (free_current_contents, &expr);
+      expression_up expr = parse_expression (type_name);
       if (expr->elts[0].opcode == OP_TYPE)
 	{
 	  /* The user expression names a type directly, just use that type.  */
@@ -616,14 +611,13 @@ maintenance_print_type (char *type_name, int from_tty)
 	{
 	  /* The user expression may name a type indirectly by naming an
 	     object of that type.  Find that indirectly named type.  */
-	  val = evaluate_type (expr);
+	  val = evaluate_type (expr.get ());
 	  type = value_type (val);
 	}
       if (type != NULL)
 	{
 	  recursive_dump_type (type, 0);
 	}
-      do_cleanups (old_chain);
     }
 }
 
diff --git a/gdb/value.c b/gdb/value.c
index b825aec..62c5e37 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -2113,9 +2113,7 @@ init_if_undefined_command (char* args, int from_tty)
   struct internalvar* intvar;
 
   /* Parse the expression - this is taken from set_command().  */
-  struct expression *expr = parse_expression (args);
-  register struct cleanup *old_chain =
-    make_cleanup (free_current_contents, &expr);
+  expression_up expr = parse_expression (args);
 
   /* Validate the expression.
      Was the expression an assignment?
@@ -2133,9 +2131,7 @@ init_if_undefined_command (char* args, int from_tty)
   /* Only evaluate the expression if the lvalue is void.
      This may still fail if the expresssion is invalid.  */
   if (intvar->kind == INTERNALVAR_VOID)
-    evaluate_expression (expr);
-
-  do_cleanups (old_chain);
+    evaluate_expression (expr.get ());
 }
 
 
diff --git a/gdb/varobj.c b/gdb/varobj.c
index fb1349a..81e897f 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -64,12 +64,12 @@ varobj_enable_pretty_printing (void)
 /* Data structures */
 
 /* Every root variable has one of these structures saved in its
-   varobj.  Members which must be free'd are noted.  */
+   varobj.  */
 struct varobj_root
 {
 
-  /* Alloc'd expression for this parent.  */
-  struct expression *exp;
+  /* The expression for this parent.  */
+  expression_up exp;
 
   /* Block for which this expression is valid.  */
   const struct block *valid_block;
@@ -379,13 +379,13 @@ varobj_create (char *objname,
          But if it fails, we still go on with a call to evaluate_type().  */
       TRY
 	{
-	  value = evaluate_expression (var->root->exp);
+	  value = evaluate_expression (var->root->exp.get ());
 	}
       CATCH (except, RETURN_MASK_ERROR)
 	{
 	  /* Error getting the value.  Try to at least get the
 	     right type.  */
-	  struct value *type_only_value = evaluate_type (var->root->exp);
+	  struct value *type_only_value = evaluate_type (var->root->exp.get ());
 
 	  var->type = value_type (type_only_value);
 	}
@@ -1051,7 +1051,6 @@ varobj_set_value (struct varobj *var, char *expression)
   /* The argument "expression" contains the variable's new value.
      We need to first construct a legal expression for this -- ugh!  */
   /* Does this cover all the bases?  */
-  struct expression *exp;
   struct value *value = NULL; /* Initialize to keep gcc happy.  */
   int saved_input_radix = input_radix;
   const char *s = expression;
@@ -1059,16 +1058,15 @@ varobj_set_value (struct varobj *var, char *expression)
   gdb_assert (varobj_editable_p (var));
 
   input_radix = 10;		/* ALWAYS reset to decimal temporarily.  */
-  exp = parse_exp_1 (&s, 0, 0, 0);
+  expression_up exp = parse_exp_1 (&s, 0, 0, 0);
   TRY
     {
-      value = evaluate_expression (exp);
+      value = evaluate_expression (exp.get ());
     }
 
   CATCH (except, RETURN_MASK_ERROR)
     {
       /* We cannot proceed without a valid expression.  */
-      xfree (exp);
       return 0;
     }
   END_CATCH
@@ -2095,7 +2093,7 @@ new_root_variable (void)
 {
   struct varobj *var = new_variable ();
 
-  var->root = XNEW (struct varobj_root);
+  var->root = new varobj_root ();
   var->root->lang_ops = NULL;
   var->root->exp = NULL;
   var->root->valid_block = NULL;
@@ -2126,12 +2124,8 @@ free_variable (struct varobj *var)
   varobj_clear_saved_item (var->dynamic);
   value_free (var->value);
 
-  /* Free the expression if this is a root variable.  */
   if (is_root_p (var))
-    {
-      xfree (var->root->exp);
-      xfree (var->root);
-    }
+    delete var->root;
 
   xfree (var->name);
   xfree (var->obj_name);
@@ -2296,7 +2290,7 @@ value_of_root_1 (struct varobj **var_handle)
          expression fails we want to just return NULL.  */
       TRY
 	{
-	  new_val = evaluate_expression (var->root->exp);
+	  new_val = evaluate_expression (var->root->exp.get ());
 	}
       CATCH (except, RETURN_MASK_ERROR)
 	{
@@ -2355,10 +2349,7 @@ value_of_root (struct varobj **var_handle, int *type_changed)
 	     button, for example).  Naturally, those locations are not
 	     correct in other frames, so update the expression.  */
 
-         struct expression *tmp_exp = var->root->exp;
-
-         var->root->exp = tmp_var->root->exp;
-         tmp_var->root->exp = tmp_exp;
+	  std::swap (var->root->exp, tmp_var->root->exp);
 
 	  varobj_delete (tmp_var, 0);
 	  *type_changed = 0;