On Wed, Oct 05, 2011 at 03:49:38PM -0400, Diego Novillo wrote:
> Jason, I don't think this is affecting any bugs in trunk, but it looks
> worth fixing. OK for trunk?
Well, it can cause problems. Consider 3 entries in the hash table
with the same hash. (1) inserted first, then (2), then (3), then (2)
gets htab_remove_elt (decl_specializations has deletions on it too),
so (2) in the hash table is overwritten with HTAB_DELETED_ENTRY.
Next reregister_specialization is called with the same hash.
It will find the slot (where (2) used to be stored), overwrites
the HTAB_DELETED_ENTRY with HTAB_EMPTY_ENTRY (aka NULL) and return
that slot, but as reregister_specialization doesn't store anything there,
afterwards htab_find won't be able to find (3).
BTW, register_specialization has the same problems. If slot != NULL and fn
== NULL, it can still return without storing non-NULL into *slot
(when optimize_specialization_lookup_p (tmpl) returns non-zero).
You can do else if (slot != NULL && fn == NULL) htab_clear_slot
(decl_specializations, slot);
And, IMHO slot should be void **, otherwise there is aliasing violation.
> --- a/gcc/cp/pt.c
> +++ b/gcc/cp/pt.c
> @@ -1648,8 +1648,8 @@ reregister_specialization (tree spec, tree tinfo, tree
> new_spec)
> elt.args = TI_ARGS (tinfo);
> elt.spec = NULL_TREE;
>
> - slot = (spec_entry **) htab_find_slot (decl_specializations, &elt, INSERT);
> - if (*slot)
> + slot = (spec_entry **) htab_find_slot (decl_specializations, &elt,
> NO_INSERT);
> + if (slot && *slot)
> {
> gcc_assert ((*slot)->spec == spec || (*slot)->spec == new_spec);
> gcc_assert (new_spec != NULL_TREE);
If you never insert, it should be htab_find only (with spec_entry * instead
of spec_entry ** variable).
Jakub
