Re: Possible api miuse bms_next_member

On Wed, 9 Apr 2025 at 15:01, Ranier Vilela <ranier.vf@gmail.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.[showdetails] 
>
> 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
dueto 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").

Or did I miss something?

Kind regards,

Matthias van de Meent
Neon (https://neon.tech)