Re: Remove dependence on integer wrapping

Hi,

I wanted to take this opportunity to provide a brief summary of
outstanding work.

> Also there are several trap-producing cases with date types:
> SELECT to_date('100000000', 'CC');
> SELECT to_timestamp('1000000000,999', 'Y,YYY');
> SELECT make_date(-2147483648, 1, 1);

This is resolved with Matthew's patches, which I've rebased, squashed
and attached to this email. They still require a review.

----

> SET temp_buffers TO 1000000000;
>
> CREATE TEMP TABLE t(i int PRIMARY KEY);
> INSERT INTO t VALUES(1);
>
> #4 0x00007f385cdd37f3 in __GI_abort () at ./stdlib/abort.c:79
> #5 0x00005620071c4f51 in __addvsi3 ()
> #6 0x0000562007143f3c in init_htab (hashp=0x562008facb20,
nelem=610070812) at dynahash.c:720
>
> (gdb) f 6
> #6 0x0000560915207f3c in init_htab (hashp=0x560916039930,
nelem=1000000000) at dynahash.c:720
> 720 hctl->high_mask = (nbuckets << 1) - 1;
> (gdb) p nbuckets
> $1 = 1073741824

I've taken a look at this and my current proposal is to convert
`nbuckets` to 64 bit integer which would prevent the overflow. I'm
hoping to look into if this is feasible soon.

----

> CREATE FUNCTION check_foreign_key () RETURNS trigger AS .../refint.so'
LANGUAGE C;
> CREATE TABLE t (i int4 NOT NULL);
> CREATE TRIGGER check_fkey BEFORE DELETE ON t FOR EACH ROW EXECUTE
PROCEDURE
> check_foreign_key (2147483647, 'cascade', 'i', "ft", "i");
> INSERT INTO t VALUES (1);
> DELETE FROM t;
>
> #4 0x00007f57f0bef7f3 in __GI_abort () at ./stdlib/abort.c:79
> #5 0x00007f57f1671351 in __addvsi3 () from .../src/test/regress/refint.so
> #6 0x00007f57f1670234 in check_foreign_key (fcinfo=0x7ffebf523650) at
refint.c:321
>
> (gdb) f 6
> #6 0x00007f3400ef9234 in check_foreign_key (fcinfo=0x7ffd6e16a600) at
refint.c:321
> 321 nkeys = (nargs - nrefs) / (nrefs + 1);
> (gdb) p nargs
> $1 = 3
> (gdb) p nrefs
> $2 = 2147483647

I have not looked into this yet, though I was unable to reproduce it
immediately.

test=# CREATE FUNCTION check_foreign_key () RETURNS trigger AS
'.../refint.so' LANGUAGE C;
ERROR: could not access file ".../refint.so": No such file or directory

I think I just have to play around with the path.

----

>> Moreover, I tried to use "-ftrapv" on 32-bit Debian and came across
>> another failure:
>> select '9223372036854775807'::int8 * 2147483648::int8;
>> server closed the connection unexpectedly
>> ...
>> #4 0xb722226a in __GI_abort () at ./stdlib/abort.c:79
>> #5 0x004cb2e1 in __mulvdi3.cold ()
>> #6 0x00abe7ab in pg_mul_s64_overflow (a=9223372036854775807,
b=2147483648, result=0xbff1da68)
>> at ../../../../src/include/common/int.h:264
>> #7 0x00abfbff in int8mul (fcinfo=0x14d9d04) at int8.c:496
>> #8 0x00782675 in ExecInterpExpr (state=0x14d9c4c, econtext=0x14da15c,
isnull=0xbff1dc3f) at execExprInterp.c:765
>
> Hm. It looks like that is pointing to __builtin_mul_overflow(), which
> seems strange.

Agreed that this looks strange. The docs [0] seem to indicate that this
shouldn't happen.

> These built-in functions promote the first two operands into infinite
> precision signed type and perform addition on those promoted
> operands.
...
> As the addition is performed in infinite signed precision, these
> built-in functions have fully defined behavior for all argument
> values.
...
> The first built-in function allows arbitrary integral types for
> operands and the result type must be pointer to some integral type
> other than enumerated or boolean type

The docs for the mul functions say that they behave the same as
addition. Alexander, is it possible that you're compiling with
something other than GCC?

----

>>> #6 0x00005576cf627c68 in bms_singleton_member (a=0x5576d09f7fb0) at
bitmapset.c:691
>>> 691 if (result >= 0 || HAS_MULTIPLE_ONES(w))
>>
>> At a glance, this appears to be caused by the RIGHTMOST_ONE macro:
>>
>> #define RIGHTMOST_ONE(x) ((signedbitmapword) (x) &
-((signedbitmapword) (x)))
>
> I believe hand-rolling the two's complement calculation should be
> sufficient to avoid depending on -fwrapv here. godbolt.org indicates that
> it produces roughly the same code, too.
>
> diff --git a/src/backend/nodes/bitmapset.c b/src/backend/nodes/bitmapset.c
> index cd05c642b0..d37a997c0e 100644
> --- a/src/backend/nodes/bitmapset.c
> +++ b/src/backend/nodes/bitmapset.c
> @@ -67,7 +67,7 @@
> * we get zero.
> *----------
> */
> -#define RIGHTMOST_ONE(x) ((signedbitmapword) (x) & -((signedbitmapword)
(x)))
> +#define RIGHTMOST_ONE(x) ((bitmapword) (x) & (~((bitmapword) (x)) + 1))

This approach seems to resolve the issue locally for me, and I think it
falls out cleanly from the comment in the code above.

Thanks,
Joseph Koshakow

[0] https://gcc.gnu.org/onlinedocs/gcc/Integer-Overflow-Builtins.html