On Fri, 6 Aug 2021 at 03:58, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Dean Rasheed <dean.a.rasheed@gmail.com> writes:
> > On Thu, 5 Aug 2021 at 17:04, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> It looks like castoroides is not happy with this patch:
> >> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=castoroides&dt=2021-08-01%2008%3A52%3A43
>
> > Hmm, there's something very weird going on there.
>
> Yeah. I tried to reproduce this on the gcc compile farm's Solaris 10
> machine, but the test passed fine for me. The only obvious configuration
> difference I can find is that that machine has
>
> $ cc -V
> cc: Sun C 5.10 SunOS_sparc Patch 141861-10 2012/11/07
>
> whereas castorides' compiler seems to be a few years older. So this
> does seem like it could be a compiler bug.
>
Ah, so the latest test results from castoroides confirm my previous
hypothesis, that it isn't even reaching the new code in power_var():
0.9999999999 ^ 23300000000000 returned 1.0199545627709647
0.9999999999 ^ 70000000000000 returned 0.9396000441558118
which are actually the results you'd get if you just cast the exponent
to an int32, throwing away the top 32 bits and compute the results:
0.9999999999 ^ -197580800 = 1.0199545627709647
0.9999999999 ^ 623009792 = 0.9396000441558118
So the "test for overflow by reverse-conversion" obviously isn't
working as intended, and it's going through power_var_int() when it
shouldn't. I don't think there's anything wrong with that code, so I
think this is a compiler bug.
I guess the best thing to do is just test the value against
PG_INT32_MIN/MAX, which is what int84() does. There are 2 other places
in numeric.c that use similar code to check for int16/32 overflow, so
it's possible that they're broken in the same way on that platform,
but they aren't covered by the regression tests, so it's also possible
that they're OK. Anyway, something like the attached seems likely to
be safer.
Regards,
Dean