Thread: When is int32 not an int32?
Hell Hackers, long time no email! I got a bug report for the semver extension: https://github.com/theory/pg-semver/issues/58 It claims that a test unexpected passes. That is, Test #31 is expected to fail, because it intentionally tests a versionin which its parts overflow the int32[3] they’re stored in, with the expectation that one day we can refactor thetype to handle larger version parts. I can’t imagine there would be any circumstance under which int32 would somehow be larger than a signed 32-bit integer, butperhaps there is? Scroll to the bottom of these pages to see the unexpected passes on i386 and armhf: https://ci.debian.net/data/autopkgtest/unstable/i386/p/postgresql-semver/15208658/log.gz https://ci.debian.net/data/autopkgtest/unstable/armhf/p/postgresql-semver/15208657/log.gz Here’s the Postgres build output for those two platforms, as well, though nothing jumps out at me: https://buildd.debian.org/status/fetch.php?pkg=postgresql-13&arch=i386&ver=13.4-3&stamp=1630408269&raw=0 https://buildd.debian.org/status/fetch.php?pkg=postgresql-13&arch=armhf&ver=13.4-3&stamp=1630412028&raw=0 Thanks, David
Attachment
"David E. Wheeler" <david@justatheory.com> writes:
> It claims that a test unexpected passes. That is, Test #31 is expected to fail, because it intentionally tests a
versionin which its parts overflow the int32[3] they’re stored in, with the expectation that one day we can refactor
thetype to handle larger version parts.
> I can’t imagine there would be any circumstance under which int32 would somehow be larger than a signed 32-bit
integer,but perhaps there is?
I'd bet more along the lines of "your overflow check is less portable than
you thought".
regards, tom lane
On Sun, Sep 26, 2021 at 05:32:11PM -0400, David E. Wheeler wrote: > Hell Hackers, long time no email! > > I got a bug report for the semver extension: > > https://github.com/theory/pg-semver/issues/58 > > It claims that a test unexpected passes. That is, Test #31 is expected to fail, because it intentionally tests a versionin which its parts overflow the int32[3] they’re stored in, with the expectation that one day we can refactor thetype to handle larger version parts. > > I can’t imagine there would be any circumstance under which int32 would somehow be larger than a signed 32-bit integer,but perhaps there is? > > Scroll to the bottom of these pages to see the unexpected passes on i386 and armhf: > > https://ci.debian.net/data/autopkgtest/unstable/i386/p/postgresql-semver/15208658/log.gz > https://ci.debian.net/data/autopkgtest/unstable/armhf/p/postgresql-semver/15208657/log.gz > > Here’s the Postgres build output for those two platforms, as well, though nothing jumps out at me: > > https://buildd.debian.org/status/fetch.php?pkg=postgresql-13&arch=i386&ver=13.4-3&stamp=1630408269&raw=0 > https://buildd.debian.org/status/fetch.php?pkg=postgresql-13&arch=armhf&ver=13.4-3&stamp=1630412028&raw=0 I noticed that in i386, configure finds none of (int8, uint8, int64, uint64), and I wonder whether we're actually testing whatever alternative we provide when we don't have them. I also noticed that the first of the long sequences of 9s doesn't even fit inside a uint64. The other two fit inside an int64, so if promotion were somehow happening, that wouldn't be a great test. 99999999999999999999999.999999999999999999.99999999999999999 over 2^72 over 2^59 over 2^56 These two observations taken together, get me to my first guess is that the machinery we provide when we see non-working 64-bit integers is totally broken. If that's right, we should at least discuss reversing our claim that we support such systems, seeing as it doesn't appear that people will be deploying new versions of PostgreSQL on them. Best, David. -- David Fetter <david(at)fetter(dot)org> http://fetter.org/ Phone: +1 415 235 3778 Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate
On Sep 26, 2021, at 18:31, Tom Lane <tgl@sss.pgh.pa.us> wrote: > I'd bet more along the lines of "your overflow check is less portable than > you thought”. Oh well now that you mention it and I look past things, I see we’re using INT_MAX, but should probably use INT32_MAX. https://github.com/theory/pg-semver/blob/87cc30cbe80aa3992a4af6f19a35a9441111a86c/src/semver.c#L145-L149 And also that the destination value we’re storing it in is an int parts[], not int32 parts[]. Which we do so we can parsenumbers up to int size. But to Fetter’s point, we’re not properly handling something greater than int (usually int64,presumably). Not sure what changes are required to improve memory safety over and above using INT32_MAX instead ofINT_MAX. Thanks, Daavid
Attachment
"David E. Wheeler" <david@justatheory.com> writes:
> On Sep 26, 2021, at 18:31, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I'd bet more along the lines of "your overflow check is less portable than
>> you thought”.
> Oh well now that you mention it and I look past things, I see we’re using INT_MAX, but should probably use INT32_MAX.
More to the point, you should be checking whether strtol reports overflow.
Having now seen your code, I'll opine that the failing platforms have
32-bit long.
regards, tom lane
On Sep 26, 2021, at 19:25, Tom Lane <tgl@sss.pgh.pa.us> wrote: > More to the point, you should be checking whether strtol reports overflow. > Having now seen your code, I'll opine that the failing platforms have > 32-bit long. Thanks for the pointer, Tom. I believe this fixes that particular issue. https://github.com/theory/pg-semver/commit/4d79dcc Best, David