Re: Bug in huge simplehash

Hi,

On 2021-08-10 11:52:59 +0300, Yura Sokolov wrote:
> - sizemask is set only in SH_COMPUTE_PARAMETERS . And it is set in this way:
>
> /* now set size */
> tb->size = size;
>
> if (tb->size == SH_MAX_SIZE)
> tb->sizemask = 0;
> else
> tb->sizemask = tb->size - 1;
>
> that means, when we are resizing to SH_MAX_SIZE, sizemask becomes zero.

I think that was intended to be ~0.

> Ahh... ok, patch is updated to fix this as well.

Any chance you'd write a test for simplehash with such huge amount of
values? It'd require a small bit of trickery to be practical. On systems
with MAP_NORESERVE it should be feasible.

> static inline void
> -SH_COMPUTE_PARAMETERS(SH_TYPE * tb, uint32 newsize)
> +SH_COMPUTE_PARAMETERS(SH_TYPE * tb, uint64 newsize)
> {
> uint64 size;
>
> @@ -322,11 +322,7 @@ SH_COMPUTE_PARAMETERS(SH_TYPE * tb, uint32 newsize)
>
> /* now set size */
> tb->size = size;
> -
> - if (tb->size == SH_MAX_SIZE)
> - tb->sizemask = 0;
> - else
> - tb->sizemask = tb->size - 1;
> + tb->sizemask = (uint32)(size - 1);

ISTM using ~0 would be nicer here?

Greetings,

Andres Freund