Content-Type: multipart/mixed; boundary="------------yCTvCVx2b4vN4NNBF0Fgwyhl"
Message-ID: <9f18f27f-8023-e72d-4c25-95a62117d9fa@gmail.com>
Date: Wed, 21 Aug 2024 10:00:00 +0300
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.2
Subject: Re: Remove dependence on integer wrapping
Content-Language: en-US
To: Nathan Bossart <nathandbossart@gmail.com>
Cc: Joseph Koshakow <koshy44@gmail.com>, jian he
 <jian.universality@gmail.com>, Heikki Linnakangas <hlinnaka@iki.fi>,
 Matthew Kim <matthewkmkim@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>,
 PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
 Andres Freund <andres@anarazel.de>
References: <Zr5M3KxOZlEZRZIN@nathan> <Zr50ZpSyrBx0EHfR@nathan>
 <CAAvxfHfkHS1n0+wMbtKwj6_GE6+aSJ9JjnsCOOdZPhTKRs-Yhw@mail.gmail.com>
 <Zr-DwBxep-IaayQF@nathan> <bfff906f-300d-81ea-83b7-f2c93845e7f2@gmail.com>
 <Zr-b1c6x-Yf31TCZ@nathan> <Zr-yiLatqDlen3oH@nathan>
 <CAAvxfHd43nT6Va20wgftJ9XQuNPpmYSjR4WRc1jxTv=U4qOZgg@mail.gmail.com>
 <CAAvxfHeWeETpiwxn11NvWkWd4o0curubg4cTXuk6yOGtixzAdg@mail.gmail.com>
 <e5d3f013-e206-835c-0c08-53c786b098f2@gmail.com> <ZsUI3QJdUOIpPkZ5@nathan>
From: Alexander Lakhin <exclusion@gmail.com>
In-Reply-To: <ZsUI3QJdUOIpPkZ5@nathan>
Archived-At: 
 <https://www.postgresql.org/message-id/9f18f27f-8023-e72d-4c25-95a62117d9fa%40gmail.com>
Precedence: bulk

This is a multi-part message in MIME format.
--------------yCTvCVx2b4vN4NNBF0Fgwyhl
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit

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@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
--------------yCTvCVx2b4vN4NNBF0Fgwyhl
Content-Type: text/x-csrc; charset=UTF-8; name="t.c"
Content-Disposition: attachment; filename="t.c"
Content-Transfer-Encoding: base64

I2luY2x1ZGUgPHN0ZGlvLmg+Cgp0eXBlZGVmIGNoYXIgYm9vbDsKdHlwZWRlZiBsb25nIGxv
bmcgaW50IGludDY0OwoKc3RhdGljIGlubGluZSBib29sCnBnX211bF9zNjRfb3ZlcmZsb3co
aW50NjQgYSwgaW50NjQgYiwgaW50NjQgKnJlc3VsdCkKewoJcmV0dXJuIF9fYnVpbHRpbl9t
dWxfb3ZlcmZsb3coYSwgYiwgcmVzdWx0KTsKfQoKaW50IG1haW4oKQp7CglpbnQ2NCBhID0g
OTIyMzM3MjAzNjg1NDc3NTgwN0w7CglpbnQ2NCBiID0gMkw7CglpbnQ2NCBjID0gMDsKCWJv
b2wgcjsKCXIgPSBwZ19tdWxfczY0X292ZXJmbG93KGEsIGIsICZjKTsKCXByaW50Zigicjog
JWQsIGM6ICVsbGRcbiIsIHIsIGMpOwp9Cg==
--------------yCTvCVx2b4vN4NNBF0Fgwyhl
Content-Type: text/plain; charset=UTF-8; name="t.x86.s"
Content-Disposition: attachment; filename="t.x86.s"
Content-Transfer-Encoding: base64

CS5maWxlCSJ0LmMiCgkudGV4dAoJLmdsb2JsCV9fbXVsdmRpMwoJLnR5cGUJcGdfbXVsX3M2
NF9vdmVyZmxvdywgQGZ1bmN0aW9uCnBnX211bF9zNjRfb3ZlcmZsb3c6Ci5MRkIwOgoJLmNm
aV9zdGFydHByb2MKCXB1c2hsCSVlYnAKCS5jZmlfZGVmX2NmYV9vZmZzZXQgOAoJLmNmaV9v
ZmZzZXQgNSwgLTgKCW1vdmwJJWVzcCwgJWVicAoJLmNmaV9kZWZfY2ZhX3JlZ2lzdGVyIDUK
CXB1c2hsCSVlZGkKCXB1c2hsCSVlc2kKCXB1c2hsCSVlYngKCXN1YmwJJDc2LCAlZXNwCgku
Y2ZpX29mZnNldCA3LCAtMTIKCS5jZmlfb2Zmc2V0IDYsIC0xNgoJLmNmaV9vZmZzZXQgMywg
LTIwCgljYWxsCV9feDg2LmdldF9wY190aHVuay5heAoJYWRkbAkkX0dMT0JBTF9PRkZTRVRf
VEFCTEVfLCAlZWF4Cgltb3ZsCSVlYXgsIC01MiglZWJwKQoJbW92bAk4KCVlYnApLCAlZWF4
Cgltb3ZsCSVlYXgsIC0zMiglZWJwKQoJbW92bAkxMiglZWJwKSwgJWVheAoJbW92bAklZWF4
LCAtMjgoJWVicCkKCW1vdmwJMTYoJWVicCksICVlYXgKCW1vdmwJJWVheCwgLTQwKCVlYnAp
Cgltb3ZsCTIwKCVlYnApLCAlZWF4Cgltb3ZsCSVlYXgsIC0zNiglZWJwKQoJbW92bAkkMCwg
LTY0KCVlYnApCgltb3ZsCSQwLCAtNjAoJWVicCkKCW1vdmwJLTMyKCVlYnApLCAlZWF4Cglt
b3ZsCS0yOCglZWJwKSwgJWVkeAoJbW92bAklZWR4LCAlZWF4Cgltb3ZsCSVlYXgsICVlZHgK
CXNhcmwJJDMxLCAlZWR4Cgltb3ZsCSVlYXgsIC00OCglZWJwKQoJbW92bAklZWR4LCAtNDQo
JWVicCkKCW1vdmwJLTMyKCVlYnApLCAlZWF4Cgltb3ZsCSVlYXgsICVlY3gKCXNhcmwJJDMx
LCAlZWN4Cgltb3ZsCS00MCglZWJwKSwgJWVheAoJbW92bAktMzYoJWVicCksICVlZHgKCW1v
dmwJJWVheCwgJWVzaQoJbW92bAklZWR4LCAlZWRpCgltb3ZsCSVlZGksICVlc2kKCW1vdmwJ
JWVzaSwgJWVkaQoJc2FybAkkMzEsICVlZGkKCW1vdmwJLTQwKCVlYnApLCAlZWF4CglzYXJs
CSQzMSwgJWVheAoJbW92bAktNDgoJWVicCksICVlYngKCWNtcGwJJWVieCwgJWVjeAoJam5l
CS5MNAoJY21wbAklZXNpLCAlZWF4CglqbmUJLkw1Cgltb3ZsCS00MCglZWJwKSwgJWVheAoJ
aW11bGwJLTMyKCVlYnApCglqbXAJLkwyCi5MNToKCW1vdmwJLTQwKCVlYnApLCAlZWF4Cglt
b3ZsCS0zNiglZWJwKSwgJWVkeAoJbW92bAklZWF4LCAtNzIoJWVicCkKCW1vdmwJJWVkeCwg
LTY4KCVlYnApCgltb3ZsCS0zMiglZWJwKSwgJWVheAoJbW92bAklZWF4LCAtNDgoJWVicCkK
CWptcAkuTDYKLkw0OgoJY21wbAklZXNpLCAlZWF4CglqbmUJLkw3Cgltb3ZsCS0zMiglZWJw
KSwgJWVheAoJbW92bAktMjgoJWVicCksICVlZHgKCW1vdmwJJWVheCwgLTcyKCVlYnApCglt
b3ZsCSVlZHgsIC02OCglZWJwKQoJbW92bAktNDgoJWVicCksICVlc2kKCW1vdmwJLTQwKCVl
YnApLCAlZWF4Cgltb3ZsCSVlYXgsIC00OCglZWJwKQouTDY6Cgltb3ZsCS00MCglZWJwKSwg
JWVkaQoJbW92bAklZWRpLCAlZWF4CgltdWxsCS0zMiglZWJwKQoJbW92bAklZWF4LCAtODAo
JWVicCkKCW1vdmwJJWVkeCwgLTc2KCVlYnApCgltb3ZsCS00OCglZWJwKSwgJWVkaQoJbW92
bAklZWRpLCAlZWF4CgltdWxsCSVlc2kKCW1vdmwJJWVheCwgJWVjeAoJbW92bAklZWR4LCAl
ZWJ4Cgl0ZXN0bAklZXNpLCAlZXNpCglqbnMJLkw4Cgltb3ZsCSVlZGksICVlYXgKCW1vdmwJ
JDAsICVlZHgKCW1vdmwJJWVheCwgJWVkeAoJbW92bAkkMCwgJWVheAoJbW92bAklZWF4LCAl
ZXNpCgltb3ZsCSVlZHgsICVlZGkKCW1vdmwJJWVjeCwgJWVheAoJbW92bAklZWJ4LCAlZWR4
CglzdWJsCSVlc2ksICVlYXgKCXNiYmwJJWVkaSwgJWVkeAoJbW92bAklZWF4LCAlZWN4Cglt
b3ZsCSVlZHgsICVlYngKLkw4OgoJY21wbAkkMCwgLTQ4KCVlYnApCglqbnMJLkw5Cgltb3Zs
CSVlY3gsICVlYXgKCW1vdmwJJWVieCwgJWVkeAoJc3VibAktNzIoJWVicCksICVlYXgKCXNi
YmwJLTY4KCVlYnApLCAlZWR4Cgltb3ZsCSVlYXgsICVlY3gKCW1vdmwJJWVkeCwgJWVieAou
TDk6Cgltb3ZsCS04MCglZWJwKSwgJWVheAoJbW92bAktNzYoJWVicCksICVlZHgKCW1vdmwJ
JWVkeCwgJWVheAoJeG9ybAklZWR4LCAlZWR4CglhZGRsCSVlY3gsICVlYXgKCWFkY2wJJWVi
eCwgJWVkeAoJbW92bAklZWF4LCAlZWN4Cgltb3ZsCSVlZHgsICVlYngKCW1vdmwJJWVjeCwg
JWVheAoJbW92bAklZWJ4LCAlZWR4Cgltb3ZsCSVlZHgsICVlYXgKCW1vdmwJJWVheCwgJWVk
eAoJc2FybAkkMzEsICVlZHgKCW1vdmwJJWVjeCwgJWVzaQoJc2FybAkkMzEsICVlc2kKCWNt
cGwJJWVheCwgJWVzaQoJam5lCS5MMTAKCW1vdmwJJWVjeCwgJWVieAoJbW92bAkkMCwgJWVj
eAoJbW92bAklZWN4LCAlZWF4Cgltb3ZsCSVlYngsICVlZHgKCW1vdmwJLTgwKCVlYnApLCAl
ZWN4Cgltb3ZsCSQwLCAlZWJ4CglvcmwJJWVjeCwgJWVheAoJb3JsCSVlYngsICVlZHgKCWpt
cAkuTDIKLkw3OgoJcHVzaGwJLTM2KCVlYnApCglwdXNobAktNDAoJWVicCkKCXB1c2hsCS0y
OCglZWJwKQoJcHVzaGwJLTMyKCVlYnApCgltb3ZsCS01MiglZWJwKSwgJWVieAoJY2FsbAlf
X211bHZkaTNAUExUCglhZGRsCSQxNiwgJWVzcAoJbW92bAktNDgoJWVicCksICVlYngKCWxl
YWwJMSglZWJ4KSwgJWVjeAoJY21wbAkkMSwgJWVjeAoJamEJLkwzCglsZWFsCTEoJWVzaSks
ICVlY3gKCWNtcGwJJDEsICVlY3gKCWphCS5MMwoJY21wbAklZXNpLCAtNDgoJWVicCkKCWpu
ZQkuTDExCgltb3ZsCSQwLCAlZWJ4Cgltb3ZsCSQwLCAlZWN4CgljbXBsCSVlYXgsICVlYngK
CXNiYmwJJWVkeCwgJWVjeAoJamdlCS5MMwoJam1wCS5MMgouTDExOgoJdGVzdGwJJWVkeCwg
JWVkeAoJam5zCS5MMwoJam1wCS5MMgouTDEwOgoJcHVzaGwJLTM2KCVlYnApCglwdXNobAkt
NDAoJWVicCkKCXB1c2hsCS0yOCglZWJwKQoJcHVzaGwJLTMyKCVlYnApCgltb3ZsCS01Migl
ZWJwKSwgJWVieAoJY2FsbAlfX211bHZkaTNAUExUCglhZGRsCSQxNiwgJWVzcAouTDM6Cglt
b3ZsCSQxLCAtNjQoJWVicCkKCW1vdmwJJDAsIC02MCglZWJwKQouTDI6Cgltb3ZsCTI0KCVl
YnApLCAlZWN4Cgltb3ZsCSVlYXgsICglZWN4KQoJbW92bAklZWR4LCA0KCVlY3gpCgltb3Zs
CS02NCglZWJwKSwgJWVheAoJbW92bAktNjAoJWVicCksICVlZHgKCWFuZGwJJDEsICVlYXgK
CWxlYWwJLTEyKCVlYnApLCAlZXNwCglwb3BsCSVlYngKCS5jZmlfcmVzdG9yZSAzCglwb3Bs
CSVlc2kKCS5jZmlfcmVzdG9yZSA2Cglwb3BsCSVlZGkKCS5jZmlfcmVzdG9yZSA3Cglwb3Bs
CSVlYnAKCS5jZmlfcmVzdG9yZSA1CgkuY2ZpX2RlZl9jZmEgNCwgNAoJcmV0CgkuY2ZpX2Vu
ZHByb2MKLkxGRTA6Cgkuc2l6ZQlwZ19tdWxfczY0X292ZXJmbG93LCAuLXBnX211bF9zNjRf
b3ZlcmZsb3cKCS5zZWN0aW9uCS5yb2RhdGEKLkxDMDoKCS5zdHJpbmcJInI6ICVkLCBjOiAl
bGxkXG4iCgkudGV4dAoJLmdsb2JsCW1haW4KCS50eXBlCW1haW4sIEBmdW5jdGlvbgptYWlu
OgouTEZCMToKCS5jZmlfc3RhcnRwcm9jCglsZWFsCTQoJWVzcCksICVlY3gKCS5jZmlfZGVm
X2NmYSAxLCAwCglhbmRsCSQtMTYsICVlc3AKCXB1c2hsCS00KCVlY3gpCglwdXNobAklZWJw
Cgltb3ZsCSVlc3AsICVlYnAKCS5jZmlfZXNjYXBlIDB4MTAsMHg1LDB4MiwweDc1LDAKCXB1
c2hsCSVlYngKCXB1c2hsCSVlY3gKCS5jZmlfZXNjYXBlIDB4ZiwweDMsMHg3NSwweDc4LDB4
NgoJLmNmaV9lc2NhcGUgMHgxMCwweDMsMHgyLDB4NzUsMHg3YwoJc3VibAkkMzIsICVlc3AK
CWNhbGwJX194ODYuZ2V0X3BjX3RodW5rLmJ4CglhZGRsCSRfR0xPQkFMX09GRlNFVF9UQUJM
RV8sICVlYngKCW1vdmwJJC0xLCAtMTYoJWVicCkKCW1vdmwJJDIxNDc0ODM2NDcsIC0xMigl
ZWJwKQoJbW92bAkkMiwgLTI0KCVlYnApCgltb3ZsCSQwLCAtMjAoJWVicCkKCW1vdmwJJDAs
IC00MCglZWJwKQoJbW92bAkkMCwgLTM2KCVlYnApCglzdWJsCSQxMiwgJWVzcAoJbGVhbAkt
NDAoJWVicCksICVlYXgKCXB1c2hsCSVlYXgKCXB1c2hsCS0yMCglZWJwKQoJcHVzaGwJLTI0
KCVlYnApCglwdXNobAktMTIoJWVicCkKCXB1c2hsCS0xNiglZWJwKQoJY2FsbAlwZ19tdWxf
czY0X292ZXJmbG93CglhZGRsCSQzMiwgJWVzcAoJbW92YgklYWwsIC0yNSglZWJwKQoJbW92
bAktNDAoJWVicCksICVlYXgKCW1vdmwJLTM2KCVlYnApLCAlZWR4Cgltb3ZzYmwJLTI1KCVl
YnApLCAlZWN4CglwdXNobAklZWR4CglwdXNobAklZWF4CglwdXNobAklZWN4CglsZWFsCS5M
QzBAR09UT0ZGKCVlYngpLCAlZWF4CglwdXNobAklZWF4CgljYWxsCXByaW50ZkBQTFQKCWFk
ZGwJJDE2LCAlZXNwCgltb3ZsCSQwLCAlZWF4CglsZWFsCS04KCVlYnApLCAlZXNwCglwb3Bs
CSVlY3gKCS5jZmlfcmVzdG9yZSAxCgkuY2ZpX2RlZl9jZmEgMSwgMAoJcG9wbAklZWJ4Cgku
Y2ZpX3Jlc3RvcmUgMwoJcG9wbAklZWJwCgkuY2ZpX3Jlc3RvcmUgNQoJbGVhbAktNCglZWN4
KSwgJWVzcAoJLmNmaV9kZWZfY2ZhIDQsIDQKCXJldAoJLmNmaV9lbmRwcm9jCi5MRkUxOgoJ
LnNpemUJbWFpbiwgLi1tYWluCgkuc2VjdGlvbgkudGV4dC5fX3g4Ni5nZXRfcGNfdGh1bmsu
YXgsImF4RyIsQHByb2diaXRzLF9feDg2LmdldF9wY190aHVuay5heCxjb21kYXQKCS5nbG9i
bAlfX3g4Ni5nZXRfcGNfdGh1bmsuYXgKCS5oaWRkZW4JX194ODYuZ2V0X3BjX3RodW5rLmF4
CgkudHlwZQlfX3g4Ni5nZXRfcGNfdGh1bmsuYXgsIEBmdW5jdGlvbgpfX3g4Ni5nZXRfcGNf
dGh1bmsuYXg6Ci5MRkIyOgoJLmNmaV9zdGFydHByb2MKCW1vdmwJKCVlc3ApLCAlZWF4Cgly
ZXQKCS5jZmlfZW5kcHJvYwouTEZFMjoKCS5zZWN0aW9uCS50ZXh0Ll9feDg2LmdldF9wY190
aHVuay5ieCwiYXhHIixAcHJvZ2JpdHMsX194ODYuZ2V0X3BjX3RodW5rLmJ4LGNvbWRhdAoJ
Lmdsb2JsCV9feDg2LmdldF9wY190aHVuay5ieAoJLmhpZGRlbglfX3g4Ni5nZXRfcGNfdGh1
bmsuYngKCS50eXBlCV9feDg2LmdldF9wY190aHVuay5ieCwgQGZ1bmN0aW9uCl9feDg2Lmdl
dF9wY190aHVuay5ieDoKLkxGQjM6CgkuY2ZpX3N0YXJ0cHJvYwoJbW92bAkoJWVzcCksICVl
YngKCXJldAoJLmNmaV9lbmRwcm9jCi5MRkUzOgoJLmlkZW50CSJHQ0M6IChEZWJpYW4gMTIu
Mi4wLTE0KSAxMi4yLjAiCgkuc2VjdGlvbgkubm90ZS5HTlUtc3RhY2ssIiIsQHByb2diaXRz
Cg==

--------------yCTvCVx2b4vN4NNBF0Fgwyhl--