From: Tom Lane <tgl@sss.pgh.pa.us>
To: Robins Tharakan <tharakan@gmail.com>
cc: John Naylor <john.naylor@enterprisedb.com>,
        pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #17725: Sefault when seg_in() called with a large argument
In-reply-to: 
 <CAEP4nAyECwMeeuzLW=RL4XHwCgxiQgMnfnG4ky-LP1LN82kKaw@mail.gmail.com>
References: <17725-0a09313b67fbe86e@postgresql.org>
 <CAFBsxsEn0HgkvmX4J3mipZn_gdoF19dj8g-q9WdS76G4QO2cYA@mail.gmail.com>
 <CAEP4nAyECwMeeuzLW=RL4XHwCgxiQgMnfnG4ky-LP1LN82kKaw@mail.gmail.com>
Comments: In-reply-to Robins Tharakan <tharakan@gmail.com>
	message dated "Tue, 20 Dec 2022 23:18:19 +1030"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <1180063.1671555286.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable
Date: Tue, 20 Dec 2022 11:54:46 -0500
Message-ID: <1180064.1671555286@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/1180064.1671555286%40sss.pgh.pa.us>
Precedence: bulk

Robins Tharakan <tharakan@gmail.com> writes:
> On Tue, 20 Dec 2022 at 20:44, John Naylor <john.naylor@enterprisedb.com>=
 wrote:
>> Neither query shows the reported problem in my environment on master (a=
s of today) or v14, so not sure

> After trying a few combinations, I see that passing
> CFLAGS=3D"-Wuninitialized" (default for my test setup) causes this failu=
re.
> Removing the flag gives the error you mention, and possibly why this
> may not be easy to reproduce on a production system (unsure).

I don't see a crash either, but I can't help observing that this
input leads to a "seg" struct with "-46" significant digits:

(gdb) p *seg
$3 =3D {lower =3D 31, upper =3D 31, l_sigd =3D -46 '\322', u_sigd =3D -46 =
'\322', =

  l_ext =3D 0 '\000', u_ext =3D 0 '\000'}

So we're invoking sprintf with a fairly insane precision spec:

939             sprintf(result, "%.*e", n - 1, val);
(gdb) p n
$4 =3D -46
(gdb) p val
$5 =3D 31

POSIX says "a negative precision is taken as if the precision were
omitted", and our code seems to do that, but I wonder if this is
managing to overrun the output buffer on your platform.

IMO:

1. The seg grammar needs to constrain the result of significant_digits()
to something that will fit in the allocated "char" field width.
It looks like some code paths there have clamps, but not all.

2. Because we might already have stored "seg" values with bogus
sigd values, restore() had better clamp the "n" value it's given
to something sane.  I see it clamps large positive values, but
it's not worrying about zero-or-negative.

			regards, tom lane