OpenE2K/binutils-gdb
12
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Introduce a gdb_ref_ptr specialization for struct value

Browse Source 
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.
This commit is contained in:
Tom Tromey 2018-04-03 17:45:21 -06:00
parent 7f8a5d38ed
commit 22bc8444e6
19 changed files with 136 additions and 114 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

47
gdb/ChangeLog
View File

@ -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

2
gdb/alpha-tdep.c
View File

@ -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;
}

28
gdb/breakpoint.c
View File

@ -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;
}

9
gdb/dwarf2loc.c
View File

@ -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);

1
gdb/dwarf2read.c
View File

@ -10974,7 +10974,6 @@ dwarf2_compute_name (const char *name,
opts.raw = 1;
value_print (v, &buf, &opts);
release_value (v);
value_free (v);
}
}

1
gdb/findvar.c
View File

@ -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;
}

6
gdb/frame.c
View File

@ -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;

5
gdb/guile/scm-value.c
View File

@ -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;

2
gdb/m68hc11-tdep.c
View File

@ -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);

1
gdb/m88k-tdep.c
View File

@ -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;

3
gdb/mep-tdep.c
View File

@ -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)

2
gdb/mi/mi-main.c
View File

@ -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;
}

2
gdb/opencl-lang.c
View File

@ -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);
}

4
gdb/printcmd.c
View File

@ -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);

8
gdb/python/py-value.c
View File

@ -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;

1
gdb/stack.c
View File

@ -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. */

68
gdb/value.c
View File

@ -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:

52
gdb/value.h
View File

@ -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);

8
gdb/varobj.c
View File

@ -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;