Re: Remove dependence on integer wrapping

Hello Nathan,

21.08.2024 00:21, Nathan Bossart wrote:
> I've combined all the current proposed changes into one patch. I've also
> introduced signed versions of the negation functions into int.h to avoid
> relying on multiplication.
>

Thank you for taking care of this!

I'd like to add some info to show how big the iceberg is.

Beside other trap-triggered places in date/time conversion functions, I
also discovered:
1)
CREATE TABLE jt(j jsonb); INSERT INTO jt VALUES('[]'::jsonb);
UPDATE jt SET j[0][-2147483648] = '0';

#4  0x00007f15ab00d7f3 in __GI_abort () at ./stdlib/abort.c:79
#5  0x00005570113b2591 in __addvsi3 ()
#6  0x00005570111d55a0 in push_null_elements (ps=0x7fff37385fb8, num=-2147483648) at jsonfuncs.c:1707
#7  0x00005570111d5749 in push_path (st=0x7fff37385fb8, level=0, path_elems=0x55701300c880, path_nulls=0x55701300d520,
    path_len=2, newval=0x7fff37386030) at jsonfuncs.c:1770

The "problematic" code:
        while (num-- > 0)
                *ps = 0;
looks innocent to me, but is not for good enough for -ftrapv.
I think there could be other similar places and this raises two questions:
can they be reached with INT_MIN and what to do if so?

By the way, the same can be seen with CC=clang CPPFLAGS="-ftrapv". Please
look at the code produced by both compilers for x86_64:
https://godbolt.org/z/vjszjf4b3
(clang generates ud1, while gcc uses call __addvsi3)

The aside question is: should jsonb subscripting accept negative indexes
when the target array is not initialized yet?

Compare:
CREATE TABLE jt(j jsonb); INSERT INTO jt VALUES('[]'::jsonb);
UPDATE jt SET j[0][-1] = '0';
SELECT * FROM jt;
   j
-------
 [[0]]

with
CREATE TABLE jt(j jsonb); INSERT INTO jt VALUES('[[]]'::jsonb);
UPDATE jt SET j[0][-1] = '0';
ERROR:  path element at position 2 is out of range: -1

2)
SELECT x, lag(x, -2147483648) OVER (ORDER BY x) FROM (SELECT 1) x;

#4  0x00007fa7d00f47f3 in __GI_abort () at ./stdlib/abort.c:79
#5  0x00005623a7336851 in __negvsi2 ()
#6  0x00005623a726ae35 in leadlag_common (fcinfo=0x7ffd59cca950, forward=false, withoffset=true, withdefault=false)
    at windowfuncs.c:551
#7  0x00005623a726af19 in window_lag_with_offset (fcinfo=0x7ffd59cca950) at windowfuncs.c:594

As to 32-bit Debian, I wrote about before, I use gcc (Debian 12.2.0-14).
Please look at the demo code (and it's assembly, produced with
gcc -S -ftrapv t.c) attached:
gcc -Wall -Wextra -fsanitize=signed-integer-overflow -Wstrict-overflow=5 \
 -O0 -ftrapv t.c -o t && ./t
Aborted (core dumped)

#4  0xb762226a in __GI_abort () at ./stdlib/abort.c:79
#5  0x00495077 in __mulvdi3.cold ()
#6  0x00495347 in pg_mul_s64_overflow ()

(It looks like -Wstrict-overflow can't help with the static analysis
desired in such cases.)

Moreover, I got `make check` failed with -ftrapv on aarch64 (using gcc 8.3)
as follows:
#1  0x0000007e1edc48e8 in __GI_abort () at abort.c:79
#2  0x0000005ee66b71cc in __subvdi3 ()
#3  0x0000005ee6560e24 in int8gcd_internal (arg1=-9223372036854775808, arg2=1) at int8.c:623
#4  0x0000005ee62f576c in ExecInterpExpr (state=0x5eeaba9d18, econtext=0x5eeaba95f0, isnull=<optimized out>)
    at execExprInterp.c:770
...
#13 0x0000005ee64e5d84 in exec_simple_query (
    query_string=query_string(at)entry=0x5eeaac7500 "SELECT a, b, gcd(a, b), gcd(a, -b), gcd(b, a), gcd(-b, a)\nFROM
(VALUES (0::int8, 0::int8),\n", ' ' <repeats 13 times>, "(0::int8, 29893644334::int8),\n", ' ' <repeats 13 times>,
"(288484263558::int8, 29893644334::int8),\n", ' ' <repeats 12 times>...) at postgres.c:1284

So I wonder whether enabling -ftrapv can really help us prepare the code
for -fno-wrapv?

Best regards,
Alexander