Re: Fix htab lookup bug in reregister_specialization (issue5190046)

[Jakub Jelinek <jakub at redhat dot com>]

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