On Tue, Nov 14, 2017 at 04:46:01PM -0700, Martin Sebor wrote:
> How about at least detecting the problem then? The attached patch
> catches the bug while running the Wstringop-truncation tests and
> passes x86_64 bootstrap.
Well, IMHO then the extra argument should be there only #if CHECKING_P,
so that you don't grow or slow down --enable-checking=release,
and more importantly,
> template<typename Descriptor, template<typename Type> class Allocator>
> void
> -hash_table<Descriptor, Allocator>::expand ()
> +hash_table<Descriptor, Allocator>::expand (const void *ptr /* = NULL */)
> {
> value_type *oentries = m_entries;
> unsigned int oindex = m_size_prime_index;
> @@ -718,6 +721,15 @@ hash_table<Descriptor, Allocator>::expand ()
> value_type *olimit = oentries + osize;
> size_t elts = elements ();
>
> +#if CHECKING_P
> + /* Verify that the pointer doesn't point into the table to detect
> + insertions of existing elements. */
> + uintptr_t iptr = (uintptr_t)ptr;
> + uintptr_t ibeg = (uintptr_t)oentries;
> + uintptr_t iend = (uintptr_t)olimit;
> + gcc_checking_assert (iptr < ibeg || iend < iptr);
> +#endif
> +
This is the wrong spot to check it.
> /* Resize only when table after removal of unused elements is either
> too full or too empty. */
> unsigned int nindex;
> @@ -866,17 +878,22 @@ hash_table<Descriptor, Allocator>
> HASH. To delete an entry, call this with insert=NO_INSERT, then
> call clear_slot on the slot returned (possibly after doing some
> checks). To insert an entry, call this with insert=INSERT, then
> - write the value you want into the returned slot. When inserting an
> - entry, NULL may be returned if memory allocation fails. */
> + write the value you want into the returned slot. When inserting
> + an entry, NULL may be returned if memory allocation fails.
> + If PTR points into an element already in the table and the table
> + is expanded, the function aborts. This makes it possible to
> + detect insertions of elements that are already in the table and
> + references to which would be invalidated by the reallocation that
> + results from the insertion. */
>
> template<typename Descriptor, template<typename Type> class Allocator>
> typename hash_table<Descriptor, Allocator>::value_type *
> hash_table<Descriptor, Allocator>
> ::find_slot_with_hash (const compare_type &comparable, hashval_t hash,
> - enum insert_option insert)
> + enum insert_option insert, const void *ptr)
> {
> if (insert == INSERT && m_size * 3 <= m_n_elements * 4)
> - expand ();
> + expand (ptr);
>
> m_searches++;
It should be checked here.
if (insert == INSERT)
{
#if CHECKING_P
...
#endif
if (m_size * 3 <= m_n_elements * 4)
expand ();
}
because otherwise you'll find the problem only if you are unlucky enough
that the hash table is expanded, while checking it this way would ensure
that there are no such potential problems.
Jakub
