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

Em ter., 26 de set. de 2023 às 07:34, Ashutosh Bapat <
ashutosh(dot)bapat(dot)oss(at)gmail(dot)com> escreveu:

> On Tue, Sep 26, 2023 at 3:32 PM David Rowley <dgrowleyml(at)gmail(dot)com> wrote:
> >
> > 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]
>
> That's a good suggestion.
>
> I am fine with find_base_rel() as it is today as well. But
> future-proofing it seems to be fine too.
>
> >
> > 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.
>
> I am curious, is the overhead in find_base_rel() impacting overall
> performance?
>
It seems to me that it adds a LEA instruction.
https://godbolt.org/z/b4jK3PErE

Although it doesn't seem like much,
I believe the solution (casting to unsigned) seems better.
So +1.

best regards,
Ranier Vilela