Re: Possible api miuse bms_next_member

Em qua., 9 de abr. de 2025 às 10:27, Matthias van de Meent <
boekewurm+postgres(at)gmail(dot)com> escreveu:

> On Wed, 9 Apr 2025 at 15:01, Ranier Vilela <ranier(dot)vf(at)gmail(dot)com> wrote:
> >
> > Hi.
> >
> > Per Coverity.
> >
> > CID 1608872: (#1 of 1): Improper use of negative value (NEGATIVE_RETURNS)
> > 32. negative_returns: bms_next_member(child_joinrel->relids, -1) is
> passed to a parameter that cannot be negative.[show details]
> >
> > CID 1608871: (#1 of 1): Out-of-bounds access (OVERRUN)
> > 32. overrun-buffer-arg: Calling add_child_eq_member with
> cur_ec->ec_childmembers and bms_next_member(child_joinrel->relids, -1) is
> suspicious because of the very large index, 4294967294. The index may be
> due to a negative parameter being interpreted as unsigned.
> >
> > Coverity has two new reports about use of the function *bms_next_member*.
> > I think that he is right.
> >
> > The function bms_next_member can return NEGATIVE.
> > So it is necessary to validate the function's return.
>
> I don't know much about the planner, but I would expect a RelOptInfo's
> relids field to always contain at least one relid when it's not
> currently being constructed; thus guaranteeing a non-negative result
> when looking for the first bit (as indicated by "next bit after -1").
>
I think it is worth the effort to prevent this.
In this particular case, the function *add_child_join_rel_equivalences*,
has the following comment:
"Note that this function won't be called at all unless we have at least some
* reason to believe that the EC members it generates will be useful."
So I believe the function is not critical.

best regards,
Ranier Vilela