Re: Avoid a possible out-of-bounds access (src/backend/optimizer/util/relnode.c)

On Tue, 26 Sept 2023 at 21:45, Ashutosh Bapat
<ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> wrote:
> However, I agree that changing find_base_rel() the way you have done
> in your patch is fine and mildly future-proof. +1 to that idea
> irrespective of what bitmapset functions do.

I'm not a fan of adding additional run-time overhead for this
theoretical problem.

find_base_rel() could be made more robust for free by just casting the
relid and simple_rel_array_size to uint32 while checking that relid <
root->simple_rel_array_size. The 0th element should be NULL anyway,
so "if (rel)" should let relid==0 calls through and allow that to
ERROR still. I see that just changes a "jle" to "jnb" vs adding an
additional jump for Ranier's version. [1]

It seems worth not making find_base_rel() more expensive than it is
today as commonly we just reference root->simple_rel_array[n] directly
anyway because it's cheaper. It would be nice if we didn't add further
overhead to find_base_rel() as this would make the case for using
PlannerInfo.simple_rel_array directly even stronger.

David

[1] https://godbolt.org/z/qrxKTbvva