When is int32 not an int32?

"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 version in which its parts overflow the int32[3] they’re stored in, with the expectation that one day we can refactor the type 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 version in which its parts overflow the int32[3] they’re stored in, with the expectation that one day we can refactor the type 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&amp;arch=i386&amp;ver=13.4-3&amp;stamp=1630408269&amp;raw=0
https://buildd.debian.org/status/fetch.php?pkg=postgresql-13&amp;arch=armhf&amp;ver=13.4-3&amp;stamp=1630412028&amp;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 parse numbers 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 of INT_MAX.

Thanks,

Daavid

"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