From 22bc8444e6d377fd016253926c2a2597ff944842 Mon Sep 17 00:00:00 2001
From: Tom Tromey <tom@tromey.com>
Date: Tue, 3 Apr 2018 17:45:21 -0600
Subject: [PATCH] Introduce a gdb_ref_ptr specialization for struct value

struct value is internally reference counted and so, while it also has
some ownership rules unique to it, it makes sense to use a gdb_ref_ptr
when managing it automatically.

This patch removes the existing unique_ptr specialization in favor of
a reference-counted pointer.  It also introduces two other
clarifications:

1. Rename value_free to value_decref, which I think is more in line
   with what the function actually does; and

2. Change release_value to return a gdb_ref_ptr.  This change allows
   us to remove the confusing release_value_or_incref function,
   primarily by making it much simpler to reason about the result of
   release_value.

gdb/ChangeLog
2018-04-06  Tom Tromey  <tom@tromey.com>

	* varobj.c (varobj_clear_saved_item)
	(update_dynamic_varobj_children, install_new_value, ~varobj):
	Update.
	* value.h (value_incref): Move declaration earlier.
	(value_decref): Rename from value_free.
	(struct value_ref_policy): New.
	(value_ref_ptr): New typedef.
	(struct value_deleter): Remove.
	(gdb_value_up): Remove typedef.
	(release_value): Change return type.
	(release_value_or_incref): Remove.
	* value.c (set_value_parent): Update.
	(value_incref): Change return type.
	(value_decref): Rename from value_free.
	(value_free_to_mark, free_all_values, free_value_chain): Update.
	(release_value): Return value_ref_ptr.
	(release_value_or_incref): Remove.
	(record_latest_value, set_internalvar, clear_internalvar):
	Update.
	* stack.c (info_frame_command): Don't call value_free.
	* python/py-value.c (valpy_dealloc, valpy_new)
	(value_to_value_object): Update.
	* printcmd.c (do_examine): Update.
	* opencl-lang.c (lval_func_free_closure): Update.
	* mi/mi-main.c (register_changed_p): Don't call value_free.
	* mep-tdep.c (mep_frame_prev_register): Don't call value_free.
	* m88k-tdep.c (m88k_frame_prev_register): Don't call value_free.
	* m68hc11-tdep.c (m68hc11_frame_prev_register): Don't call
	value_free.
	* guile/scm-value.c (vlscm_free_value_smob)
	(vlscm_scm_from_value): Update.
	* frame.c (frame_register_unwind, frame_unwind_register_signed)
	(frame_unwind_register_unsigned, get_frame_register_bytes)
	(put_frame_register_bytes): Don't call value_free.
	* findvar.c (address_from_register): Don't call value_free.
	* dwarf2read.c (dwarf2_compute_name): Don't call value_free.
	* dwarf2loc.c (entry_data_value_free_closure)
	(value_of_dwarf_reg_entry, free_pieced_value_closure)
	(dwarf2_evaluate_loc_desc_full): Update.
	* breakpoint.c (update_watchpoint, breakpoint_init_inferior)
	(~bpstats, bpstats, bpstat_clear_actions, watchpoint_check)
	(~watchpoint, watch_command_1)
	(invalidate_bp_value_on_memory_change): Update.
	* alpha-tdep.c (alpha_register_to_value): Don't call value_free.
---
 gdb/ChangeLog         | 47 ++++++++++++++++++++++++++++++
 gdb/alpha-tdep.c      |  2 --
 gdb/breakpoint.c      | 28 ++++++++----------
 gdb/dwarf2loc.c       |  9 +++---
 gdb/dwarf2read.c      |  1 -
 gdb/findvar.c         |  1 -
 gdb/frame.c           |  6 ----
 gdb/guile/scm-value.c |  5 ++--
 gdb/m68hc11-tdep.c    |  2 --
 gdb/m88k-tdep.c       |  1 -
 gdb/mep-tdep.c        |  3 --
 gdb/mi/mi-main.c      |  2 --
 gdb/opencl-lang.c     |  2 +-
 gdb/printcmd.c        |  4 +--
 gdb/python/py-value.c |  8 ++---
 gdb/stack.c           |  1 -
 gdb/value.c           | 68 ++++++++++++++++++++-----------------------
 gdb/value.h           | 52 +++++++++++++++++++--------------
 gdb/varobj.c          |  8 ++---
 19 files changed, 136 insertions(+), 114 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 8b986c67031..543dafcda6f 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,50 @@
+2018-04-06  Tom Tromey  <tom@tromey.com>
+
+	* varobj.c (varobj_clear_saved_item)
+	(update_dynamic_varobj_children, install_new_value, ~varobj):
+	Update.
+	* value.h (value_incref): Move declaration earlier.
+	(value_decref): Rename from value_free.
+	(struct value_ref_policy): New.
+	(value_ref_ptr): New typedef.
+	(struct value_deleter): Remove.
+	(gdb_value_up): Remove typedef.
+	(release_value): Change return type.
+	(release_value_or_incref): Remove.
+	* value.c (set_value_parent): Update.
+	(value_incref): Change return type.
+	(value_decref): Rename from value_free.
+	(value_free_to_mark, free_all_values, free_value_chain): Update.
+	(release_value): Return value_ref_ptr.
+	(release_value_or_incref): Remove.
+	(record_latest_value, set_internalvar, clear_internalvar):
+	Update.
+	* stack.c (info_frame_command): Don't call value_free.
+	* python/py-value.c (valpy_dealloc, valpy_new)
+	(value_to_value_object): Update.
+	* printcmd.c (do_examine): Update.
+	* opencl-lang.c (lval_func_free_closure): Update.
+	* mi/mi-main.c (register_changed_p): Don't call value_free.
+	* mep-tdep.c (mep_frame_prev_register): Don't call value_free.
+	* m88k-tdep.c (m88k_frame_prev_register): Don't call value_free.
+	* m68hc11-tdep.c (m68hc11_frame_prev_register): Don't call
+	value_free.
+	* guile/scm-value.c (vlscm_free_value_smob)
+	(vlscm_scm_from_value): Update.
+	* frame.c (frame_register_unwind, frame_unwind_register_signed)
+	(frame_unwind_register_unsigned, get_frame_register_bytes)
+	(put_frame_register_bytes): Don't call value_free.
+	* findvar.c (address_from_register): Don't call value_free.
+	* dwarf2read.c (dwarf2_compute_name): Don't call value_free.
+	* dwarf2loc.c (entry_data_value_free_closure)
+	(value_of_dwarf_reg_entry, free_pieced_value_closure)
+	(dwarf2_evaluate_loc_desc_full): Update.
+	* breakpoint.c (update_watchpoint, breakpoint_init_inferior)
+	(~bpstats, bpstats, bpstat_clear_actions, watchpoint_check)
+	(~watchpoint, watch_command_1)
+	(invalidate_bp_value_on_memory_change): Update.
+	* alpha-tdep.c (alpha_register_to_value): Don't call value_free.
+
 2018-04-06  Simon Marchi  <simon.marchi@polymtl.ca>
 
 	PR gdb/23022
diff --git a/gdb/alpha-tdep.c b/gdb/alpha-tdep.c
index 52a46d96eaf..78422faa6c0 100644
--- a/gdb/alpha-tdep.c
+++ b/gdb/alpha-tdep.c
@@ -252,7 +252,6 @@ alpha_register_to_value (struct frame_info *frame, int regnum,
   if (*optimizedp || *unavailablep)
     {
       release_value (value);
-      value_free (value);
       return 0;
     }
 
@@ -262,7 +261,6 @@ alpha_register_to_value (struct frame_info *frame, int regnum,
   alpha_sts (gdbarch, out, value_contents_all (value));
 
   release_value (value);
-  value_free (value);
   return 1;
 }
 
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index f84fef2beaa..68292626d37 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -1740,7 +1740,7 @@ update_watchpoint (struct watchpoint *b, int reparse)
 	 no longer relevant.  We don't want to report a watchpoint hit
 	 to the user when the old value and the new value may actually
 	 be completely different objects.  */
-      value_free (b->val);
+      value_decref (b->val);
       b->val = NULL;
       b->val_valid = 0;
 
@@ -1795,7 +1795,7 @@ update_watchpoint (struct watchpoint *b, int reparse)
 	    {
 	      v = extract_bitfield_from_watchpoint_value (b, v);
 	      if (v != NULL)
-		release_value (v);
+		release_value (v).release ();
 	    }
 	  b->val = v;
 	  b->val_valid = 1;
@@ -1971,7 +1971,7 @@ update_watchpoint (struct watchpoint *b, int reparse)
 	{
 	  next = value_next (v);
 	  if (v != b->val)
-	    value_free (v);
+	    value_decref (v);
 	}
 
       /* If a software watchpoint is not watching any memory, then the
@@ -3952,7 +3952,7 @@ breakpoint_init_inferior (enum inf_context context)
 		  /* Reset val field to force reread of starting value in
 		     insert_breakpoints.  */
 		  if (w->val)
-		    value_free (w->val);
+		    value_decref (w->val);
 		  w->val = NULL;
 		  w->val_valid = 0;
 		}
@@ -4200,7 +4200,7 @@ is_catchpoint (struct breakpoint *ep)
 bpstats::~bpstats ()
 {
   if (old_val != NULL)
-    value_free (old_val);
+    value_decref (old_val);
   if (bp_location_at != NULL)
     decref_bp_location (&bp_location_at);
 }
@@ -4237,10 +4237,7 @@ bpstats::bpstats (const bpstats &other)
     print_it (other.print_it)
 {
   if (old_val != NULL)
-    {
-      old_val = value_copy (old_val);
-      release_value (old_val);
-    }
+    old_val = release_value (value_copy (old_val)).release ();
   incref_bp_location (bp_location_at);
 }
 
@@ -4364,7 +4361,7 @@ bpstat_clear_actions (void)
 
       if (bs->old_val != NULL)
 	{
-	  value_free (bs->old_val);
+	  value_decref (bs->old_val);
 	  bs->old_val = NULL;
 	}
     }
@@ -4942,7 +4939,7 @@ watchpoint_check (bpstat bs)
 	{
 	  if (new_val != NULL)
 	    {
-	      release_value (new_val);
+	      release_value (new_val).release ();
 	      value_free_to_mark (mark);
 	    }
 	  bs->old_val = b->val;
@@ -10102,7 +10099,7 @@ watchpoint::~watchpoint ()
 {
   xfree (this->exp_string);
   xfree (this->exp_string_reparse);
-  value_free (this->val);
+  value_decref (this->val);
 }
 
 /* Implement the "re_set" breakpoint_ops method for watchpoints.  */
@@ -10725,8 +10722,7 @@ watch_command_1 (const char *arg, int accessflag, int from_tty,
       int ret;
 
       exp_valid_block = NULL;
-      val = value_addr (result);
-      release_value (val);
+      val = release_value (value_addr (result)).release ();
       value_free_to_mark (mark);
 
       if (use_mask)
@@ -10740,7 +10736,7 @@ watch_command_1 (const char *arg, int accessflag, int from_tty,
 	}
     }
   else if (val != NULL)
-    release_value (val);
+    release_value (val).release ();
 
   tok = skip_spaces (arg);
   end_tok = skip_to_space (tok);
@@ -14546,7 +14542,7 @@ invalidate_bp_value_on_memory_change (struct inferior *inferior,
 		  && loc->address + loc->length > addr
 		  && addr + len > loc->address)
 		{
-		  value_free (wp->val);
+		  value_decref (wp->val);
 		  wp->val = NULL;
 		  wp->val_valid = 0;
 		}
diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index 51f133f1b5e..6c84e4ad7e8 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -1376,7 +1376,7 @@ entry_data_value_free_closure (struct value *v)
 {
   struct value *target_val = (struct value *) value_computed_closure (v);
 
-  value_free (target_val);
+  value_decref (target_val);
 }
 
 /* Vector for methods for an entry value reference where the referenced value
@@ -1434,7 +1434,7 @@ value_of_dwarf_reg_entry (struct type *type, struct frame_info *frame,
 					       target_type, caller_frame,
 					       caller_per_cu);
 
-  release_value (target_val);
+  release_value (target_val).release ();
   val = allocate_computed_value (type, &entry_data_value_funcs,
 				 target_val /* closure */);
 
@@ -2299,7 +2299,7 @@ free_pieced_value_closure (struct value *v)
     {
       for (dwarf_expr_piece &p : c->pieces)
 	if (p.location == DWARF_VALUE_STACK)
-	  value_free (p.v.value);
+	  value_decref (p.v.value);
 
       delete c;
     }
@@ -2482,9 +2482,8 @@ dwarf2_evaluate_loc_desc_full (struct type *type, struct frame_info *frame,
 	    /* Preserve VALUE because we are going to free values back
 	       to the mark, but we still need the value contents
 	       below.  */
-	    value_incref (value);
+	    value_ref_ptr value_holder (value_incref (value));
 	    free_values.free_to_mark ();
-	    gdb_value_up value_holder (value);
 
 	    retval = allocate_value (subobj_type);
 
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index fd544a7f9b9..2eaec2d2a46 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -10974,7 +10974,6 @@ dwarf2_compute_name (const char *name,
 		      opts.raw = 1;
 		      value_print (v, &buf, &opts);
 		      release_value (v);
-		      value_free (v);
 		    }
 		}
 
diff --git a/gdb/findvar.c b/gdb/findvar.c
index ee8f57159d0..8ad5e25cb27 100644
--- a/gdb/findvar.c
+++ b/gdb/findvar.c
@@ -1004,7 +1004,6 @@ address_from_register (int regnum, struct frame_info *frame)
 
   result = value_as_address (value);
   release_value (value);
-  value_free (value);
 
   return result;
 }
diff --git a/gdb/frame.c b/gdb/frame.c
index c792d1f4e12..90d0ac7b870 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -1117,7 +1117,6 @@ frame_register_unwind (struct frame_info *frame, int regnum,
   /* Dispose of the new value.  This prevents watchpoints from
      trying to watch the saved frame pointer.  */
   release_value (value);
-  value_free (value);
 }
 
 void
@@ -1264,7 +1263,6 @@ frame_unwind_register_signed (struct frame_info *frame, int regnum)
 				      byte_order);
 
   release_value (value);
-  value_free (value);
   return r;
 }
 
@@ -1299,7 +1297,6 @@ frame_unwind_register_unsigned (struct frame_info *frame, int regnum)
 					 byte_order);
 
   release_value (value);
-  value_free (value);
   return r;
 }
 
@@ -1446,12 +1443,10 @@ get_frame_register_bytes (struct frame_info *frame, int regnum,
 	  if (*optimizedp || *unavailablep)
 	    {
 	      release_value (value);
-	      value_free (value);
 	      return 0;
 	    }
 	  memcpy (myaddr, value_contents_all (value) + offset, curr_len);
 	  release_value (value);
-	  value_free (value);
 	}
 
       myaddr += curr_len;
@@ -1500,7 +1495,6 @@ put_frame_register_bytes (struct frame_info *frame, int regnum,
 		  curr_len);
 	  put_frame_register (frame, regnum, value_contents_raw (value));
 	  release_value (value);
-	  value_free (value);
 	}
 
       myaddr += curr_len;
diff --git a/gdb/guile/scm-value.c b/gdb/guile/scm-value.c
index 58162593302..5a28d4d0ba0 100644
--- a/gdb/guile/scm-value.c
+++ b/gdb/guile/scm-value.c
@@ -131,7 +131,7 @@ vlscm_free_value_smob (SCM self)
   value_smob *v_smob = (value_smob *) SCM_SMOB_DATA (self);
 
   vlscm_forget_value_smob (v_smob);
-  value_free (v_smob->value);
+  value_decref (v_smob->value);
 
   return 0;
 }
@@ -253,8 +253,7 @@ vlscm_scm_from_value (struct value *value)
   SCM v_scm = vlscm_make_value_smob ();
   value_smob *v_smob = (value_smob *) SCM_SMOB_DATA (v_scm);
 
-  v_smob->value = value;
-  release_value_or_incref (value);
+  v_smob->value = release_value (value).release ();
   vlscm_remember_scheme_value (v_smob);
 
   return v_scm;
diff --git a/gdb/m68hc11-tdep.c b/gdb/m68hc11-tdep.c
index 58ef4a32927..d6f3593f6e1 100644
--- a/gdb/m68hc11-tdep.c
+++ b/gdb/m68hc11-tdep.c
@@ -919,13 +919,11 @@ m68hc11_frame_prev_register (struct frame_info *this_frame,
           CORE_ADDR page;
 
 	  release_value (value);
-	  value_free (value);
 
 	  value = trad_frame_get_prev_register (this_frame, info->saved_regs,
 						HARD_PAGE_REGNUM);
 	  page = value_as_long (value);
 	  release_value (value);
-	  value_free (value);
 
           pc -= 0x08000;
           pc += ((page & 0x0ff) << 14);
diff --git a/gdb/m88k-tdep.c b/gdb/m88k-tdep.c
index 6a50126548d..dd84350c47b 100644
--- a/gdb/m88k-tdep.c
+++ b/gdb/m88k-tdep.c
@@ -726,7 +726,6 @@ m88k_frame_prev_register (struct frame_info *this_frame,
 					    M88K_SXIP_REGNUM);
       pc = value_as_long (value);
       release_value (value);
-      value_free (value);
 
       if (regnum == M88K_SFIP_REGNUM)
 	pc += 4;
diff --git a/gdb/mep-tdep.c b/gdb/mep-tdep.c
index 1cda2b35af9..0e8c3f97e22 100644
--- a/gdb/mep-tdep.c
+++ b/gdb/mep-tdep.c
@@ -2005,7 +2005,6 @@ mep_frame_prev_register (struct frame_info *this_frame,
 				       MEP_LP_REGNUM);
       lp = value_as_long (value);
       release_value (value);
-      value_free (value);
 
       return frame_unwind_got_constant (this_frame, regnum, lp & ~1);
     }
@@ -2036,13 +2035,11 @@ mep_frame_prev_register (struct frame_info *this_frame,
 
 	  psw = value_as_long (value);
 	  release_value (value);
-	  value_free (value);
 
           /* Get the LP's value, too.  */
 	  value = get_frame_register_value (this_frame, MEP_LP_REGNUM);
 	  lp = value_as_long (value);
 	  release_value (value);
-	  value_free (value);
 
           /* If LP.LTOM is set, then toggle PSW.OM.  */
 	  if (lp & 0x1)
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 9c4e44ba6b4..deb96b4036b 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1017,8 +1017,6 @@ register_changed_p (int regnum, readonly_detached_regcache *prev_regs,
 
   release_value (prev_value);
   release_value (this_value);
-  value_free (prev_value);
-  value_free (this_value);
   return ret;
 }
 
diff --git a/gdb/opencl-lang.c b/gdb/opencl-lang.c
index 9c5b90cc55a..8af63f76206 100644
--- a/gdb/opencl-lang.c
+++ b/gdb/opencl-lang.c
@@ -292,7 +292,7 @@ lval_func_free_closure (struct value *v)
 
   if (c->refc == 0)
     {
-      value_free (c->val); /* Decrement the reference counter of the value.  */
+      value_decref (c->val); /* Decrement the reference counter of the value.  */
       xfree (c->indices);
       xfree (c);
     }
diff --git a/gdb/printcmd.c b/gdb/printcmd.c
index 58bdac61921..8822ae12e92 100644
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -1094,7 +1094,7 @@ do_examine (struct format_data fmt, struct gdbarch *gdbarch, CORE_ADDR addr)
 	  last_examine_address = next_address;
 
 	  if (last_examine_value)
-	    value_free (last_examine_value);
+	    value_decref (last_examine_value);
 
 	  /* The value to be displayed is not fetched greedily.
 	     Instead, to avoid the possibility of a fetched value not
@@ -1108,7 +1108,7 @@ do_examine (struct format_data fmt, struct gdbarch *gdbarch, CORE_ADDR addr)
 	  last_examine_value = value_at_lazy (val_type, next_address);
 
 	  if (last_examine_value)
-	    release_value (last_examine_value);
+	    release_value (last_examine_value).release ();
 
 	  print_formatted (last_examine_value, size, &opts, gdb_stdout);
 
diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
index 1673fa41b02..bba6d0b8a48 100644
--- a/gdb/python/py-value.c
+++ b/gdb/python/py-value.c
@@ -88,7 +88,7 @@ valpy_dealloc (PyObject *obj)
   if (self->next)
     self->next->prev = self->prev;
 
-  value_free (self->value);
+  value_decref (self->value);
 
   if (self->address)
     /* Use braces to appease gcc warning.  *sigh*  */
@@ -147,8 +147,7 @@ valpy_new (PyTypeObject *subtype, PyObject *args, PyObject *keywords)
       return NULL;
     }
 
-  value_obj->value = value;
-  release_value_or_incref (value);
+  value_obj->value = release_value (value).release ();
   value_obj->address = NULL;
   value_obj->type = NULL;
   value_obj->dynamic_type = NULL;
@@ -1583,8 +1582,7 @@ value_to_value_object (struct value *val)
   val_obj = PyObject_New (value_object, &value_object_type);
   if (val_obj != NULL)
     {
-      val_obj->value = val;
-      release_value_or_incref (val);
+      val_obj->value = release_value (val).release ();
       val_obj->address = NULL;
       val_obj->type = NULL;
       val_obj->dynamic_type = NULL;
diff --git a/gdb/stack.c b/gdb/stack.c
index 9fdc9eece28..ecf1ee83793 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -1645,7 +1645,6 @@ info_frame_command (const char *addr_exp, int from_tty)
 	      }
 
 	    release_value (value);
-	    value_free (value);
 	    need_nl = 0;
 	  }
 	/* else keep quiet.  */
diff --git a/gdb/value.c b/gdb/value.c
index 110d16dcf5b..002270f6344 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -1151,7 +1151,7 @@ set_value_parent (struct value *value, struct value *parent)
   value->parent = parent;
   if (parent != NULL)
     value_incref (parent);
-  value_free (old);
+  value_decref (old);
 }
 
 gdb_byte *
@@ -1594,10 +1594,11 @@ value_mark (void)
 /* Take a reference to VAL.  VAL will not be deallocated until all
    references are released.  */
 
-void
+struct value *
 value_incref (struct value *val)
 {
   val->reference_count++;
+  return val;
 }
 
 /* Release a reference to VAL, which was acquired with value_incref.
@@ -1605,7 +1606,7 @@ value_incref (struct value *val)
    chain.  */
 
 void
-value_free (struct value *val)
+value_decref (struct value *val)
 {
   if (val)
     {
@@ -1617,7 +1618,7 @@ value_free (struct value *val)
       /* If there's an associated parent value, drop our reference to
 	 it.  */
       if (val->parent != NULL)
-	value_free (val->parent);
+	value_decref (val->parent);
 
       if (VALUE_LVAL (val) == lval_computed)
 	{
@@ -1647,7 +1648,7 @@ value_free_to_mark (const struct value *mark)
     {
       next = val->next;
       val->released = 1;
-      value_free (val);
+      value_decref (val);
     }
   all_values = val;
 }
@@ -1666,7 +1667,7 @@ free_all_values (void)
     {
       next = val->next;
       val->released = 1;
-      value_free (val);
+      value_decref (val);
     }
 
   all_values = 0;
@@ -1682,50 +1683,48 @@ free_value_chain (struct value *v)
   for (; v; v = next)
     {
       next = value_next (v);
-      value_free (v);
+      value_decref (v);
     }
 }
 
 /* Remove VAL from the chain all_values
    so it will not be freed automatically.  */
 
-void
+value_ref_ptr
 release_value (struct value *val)
 {
   struct value *v;
+  bool released = false;
 
   if (all_values == val)
     {
       all_values = val->next;
       val->next = NULL;
-      val->released = 1;
-      return;
+      released = true;
     }
-
-  for (v = all_values; v; v = v->next)
+  else
     {
-      if (v->next == val)
+      for (v = all_values; v; v = v->next)
 	{
-	  v->next = val->next;
-	  val->next = NULL;
-	  val->released = 1;
-	  break;
+	  if (v->next == val)
+	    {
+	      v->next = val->next;
+	      val->next = NULL;
+	      released = true;
+	      break;
+	    }
 	}
     }
-}
 
-/* If the value is not already released, release it.
-   If the value is already released, increment its reference count.
-   That is, this function ensures that the value is released from the
-   value chain and that the caller owns a reference to it.  */
+  if (!released)
+    {
+      /* We must always return an owned reference.  Normally this
+	 happens because we transfer the reference from the value
+	 chain, but in this case the value was not on the chain.  */
+      value_incref (val);
+    }
 
-void
-release_value_or_incref (struct value *val)
-{
-  if (val->released)
-    value_incref (val);
-  else
-    release_value (val);
+  return value_ref_ptr (val);
 }
 
 /* Release all values up to mark  */
@@ -1896,11 +1895,6 @@ record_latest_value (struct value *val)
      but the current contents of that location.  c'est la vie...  */
   val->modifiable = 0;
 
-  /* The value may have already been released, in which case we're adding a
-     new reference for its entry in the history.  That is why we call
-     release_value_or_incref here instead of release_value.  */
-  release_value_or_incref (val);
-
   /* Here we treat value_history_count as origin-zero
      and applying to the value being stored now.  */
 
@@ -1913,7 +1907,7 @@ record_latest_value (struct value *val)
       value_history_chain = newobj;
     }
 
-  value_history_chain->values[i] = val;
+  value_history_chain->values[i] = release_value (val).release ();
 
   /* Now we regard value_history_count as origin-one
      and applying to the value just stored.  */
@@ -2416,7 +2410,7 @@ set_internalvar (struct internalvar *var, struct value *val)
 	 deleted by free_all_values.  From here on this function should not
 	 call error () until new_data is installed into the var->u to avoid
 	 leaking memory.  */
-      release_value (new_data.value);
+      release_value (new_data.value).release ();
 
       /* Internal variables which are created from values with a dynamic
          location don't need the location property of the origin anymore.
@@ -2478,7 +2472,7 @@ clear_internalvar (struct internalvar *var)
   switch (var->kind)
     {
     case INTERNALVAR_VALUE:
-      value_free (var->u.value);
+      value_decref (var->u.value);
       break;
 
     case INTERNALVAR_STRING:
diff --git a/gdb/value.h b/gdb/value.h
index 0bc51304ea5..f7e7387ff1c 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -22,6 +22,7 @@
 
 #include "frame.h"		/* For struct frame_id.  */
 #include "extension.h"
+#include "common/gdb_ref_ptr.h"
 
 struct block;
 struct expression;
@@ -87,6 +88,34 @@ struct value_print_options;
 
 struct value;
 
+/* Decrease VAL's reference count.  When the reference count drops to
+   0, VAL will be freed.  */
+
+extern struct value *value_incref (struct value *val);
+
+/* Increate VAL's reference count.  VAL is returned.  */
+
+extern void value_decref (struct value *val);
+
+/* A policy class to interface gdb::ref_ptr with struct value.  */
+
+struct value_ref_policy
+{
+  static void incref (struct value *ptr)
+  {
+    value_incref (ptr);
+  }
+
+  static void decref (struct value *ptr)
+  {
+    value_decref (ptr);
+  }
+};
+
+/* A gdb:;ref_ptr pointer to a struct value.  */
+
+typedef gdb::ref_ptr<struct value, value_ref_policy> value_ref_ptr;
+
 /* Values are stored in a chain, so that they can be deleted easily
    over calls to the inferior.  Values assigned to internal variables,
    put into the value history or exposed to Python are taken off this
@@ -1024,32 +1053,11 @@ extern int unop_user_defined_p (enum exp_opcode op, struct value *arg1);
 
 extern int destructor_name_p (const char *name, struct type *type);
 
-extern void value_incref (struct value *val);
-
-extern void value_free (struct value *val);
-
-/* A free policy class to interface std::unique_ptr with
-   value_free.  */
-
-struct value_deleter
-{
-  void operator() (struct value *value) const
-  {
-    value_free (value);
-  }
-};
-
-/* A unique pointer to a struct value.  */
-
-typedef std::unique_ptr<struct value, value_deleter> gdb_value_up;
-
 extern void free_all_values (void);
 
 extern void free_value_chain (struct value *v);
 
-extern void release_value (struct value *val);
-
-extern void release_value_or_incref (struct value *val);
+extern value_ref_ptr release_value (struct value *val);
 
 extern int record_latest_value (struct value *val);
 
diff --git a/gdb/varobj.c b/gdb/varobj.c
index f23243f3b72..58eb531927e 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -707,7 +707,7 @@ varobj_clear_saved_item (struct varobj_dynamic *var)
 {
   if (var->saved_item != NULL)
     {
-      value_free (var->saved_item->value);
+      value_decref (var->saved_item->value);
       delete var->saved_item;
       var->saved_item = NULL;
     }
@@ -761,7 +761,7 @@ update_dynamic_varobj_children (struct varobj *var,
 	  /* Release vitem->value so its lifetime is not bound to the
 	     execution of a command.  */
 	  if (item != NULL && item->value != NULL)
-	    release_value_or_incref (item->value);
+	    release_value (item->value).release ();
 	}
 
       if (item == NULL)
@@ -1393,7 +1393,7 @@ install_new_value (struct varobj *var, struct value *value, bool initial)
 
   /* We must always keep the new value, since children depend on it.  */
   if (var->value != NULL && var->value != value)
-    value_free (var->value);
+    value_decref (var->value);
   var->value = value;
   if (value && value_lazy (value) && intentionally_not_fetched)
     var->not_fetched = true;
@@ -1990,7 +1990,7 @@ varobj::~varobj ()
 
   varobj_iter_delete (var->dynamic->child_iter);
   varobj_clear_saved_item (var->dynamic);
-  value_free (var->value);
+  value_decref (var->value);
 
   if (is_root_p (var))
     delete var->root;
-- 
2.30.2