Re: Small and unlikely overflow hazard in bms_next_member()

On Thu, 2 Apr 2026 at 19:39, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
> Both the return value of bms_next_member() and the parameter “prevbit" are defined as int, which seems to imply that a bitmap can hold at most INT32_MAX elements. So I wonder if a cleaner fix would be to just add a range guard, like this:
>
> ```
> diff --git a/src/backend/nodes/bitmapset.c b/src/backend/nodes/bitmapset.c
> index 786f343b3c9..7f79f81f278 100644
> --- a/src/backend/nodes/bitmapset.c
> +++ b/src/backend/nodes/bitmapset.c
> @@ -1294,7 +1294,7 @@ bms_next_member(const Bitmapset *a, int prevbit)
>
> Assert(bms_is_valid_set(a));
>
> - if (a == NULL)
> + if (a == NULL || prevbit == INT32_MAX)

I don't think that's a nice way at all. Adding a special case plus
extra instructions, including a new jump instruction for the boolean
short-circuiting. More instruction decoding, more L1i space needed,
more branches to predict and more space in the branch predictor tables
to overwrite other useful branches. Adds run-time overhead.

I was aiming for low overhead and no special cases. 2a600a93c means we
don't care about the performance on 32-bit hardware anymore, so that
can't be used as a counterargument.

David