Re: BUG #17774: Assert triggered on brin_minmax_multi.c

Dmitry Dolgov <9erthalion6@gmail.com>

From: Dmitry Dolgov <9erthalion6@gmail.com>
To: Tomas Vondra <tomas.vondra@enterprisedb.com>
Cc: John Naylor <john.naylor@enterprisedb.com>, Tom Lane <tgl@sss.pgh.pa.us>, tharakan@gmail.com, pgsql-bugs@lists.postgresql.org
Date: 2023-03-17T20:20:25Z
Lists: pgsql-bugs
> On Sat, Feb 11, 2023 at 08:00:37PM +0100, Dmitry Dolgov wrote:
> > On Thu, Feb 09, 2023 at 05:18:47PM +0100, Tomas Vondra wrote:
> >
> > I believe the bug is pretty trivial - the code applies the netmask
> > incorrectly, so that with 127.0.0.1/0 it ends with 0.0.0.1, and because
> > it assumes 0.0.0.1 < 0.0.0.0 it ends with negative delta.
> >
> > In particular, the issue is that the code does this:
> >
> >     lena = ip_bits(ipa);     -- 0
> >     len = ip_addrsize(ipa);  -- 4
> >
> >     for (for (i = 0; i < len; i++)
> >     {
> >       nbits = lena - (i * 8);
> >       ...
> >       mask = (0xFF << (8 - nbits));
> >       ...
> >     }
> >
> > But for 127.0.0.1/0 we get lena=0, so for i>0 nbits gets negative, and
> > the shift is probably going to do something silly (not sure what
> > exactly, but AFAICS it's undefined behavior).
> >
> > Attached is a fixup that resolves this failure for me. I need to look a
> > bit closer if there are some other issues (e.g. with the float rounding
> > errors, etc.).
>
> Thanks, the fix looks good and solves the issue. With the patch applied
> after a quick round of testing I haven't found any failures so far,
> whether due to float arithmetic or something else.

It occurred to me this fix wasn't applied yet, right? Are there any
concerns about it?



Commits

  1. Fix netmask handling in inet_minmax_multi_ops