Date: Fri, 19 Jul 2024 13:45:49 -0500
From: Nathan Bossart <nathandbossart@gmail.com>
To: Joseph Koshakow <koshy44@gmail.com>
Cc: jian he <jian.universality@gmail.com>,
	Alexander Lakhin <exclusion@gmail.com>,
	Tom Lane <tgl@sss.pgh.pa.us>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
	Andres Freund <andres@anarazel.de>
Subject: Re: Remove dependence on integer wrapping
Message-ID: <Zpq0XUQ9neudWYgC@nathan>
References: 
 <CAAvxfHcDQEziyQ-ehv0Bg05OovGPGO95VoO7-DyxJx6Tkq85vA@mail.gmail.com>
 <ZpFeopUrhdB5GgNN@nathan>
 <CAAvxfHdX-t03LvxROxbyodBrtzBzZQEKdKB0FCpEr+p1JnFxsA@mail.gmail.com>
 <ZpVAvLbtUPnK1JKe@nathan>
 <CAAvxfHdYnTB7BjkRSPO8__wxB3880TiYD-TVU2uW8q1KbzymPg@mail.gmail.com>
 <Zpa0m_OVw_iWYicV@nathan>
 <CAAvxfHeFDyNZj+rjcuMkh+VTSYmhb_ZPpbdqZUJcjnT=X3ampA@mail.gmail.com>
 <CACJufxFkTACY6MprpewMncdOpL9dpZBcAGtc6WLfnScLknS3DA@mail.gmail.com>
 <CAAvxfHfeUmVMXU6883in2H-jJ+Vn2=mcSgVpsNDRTTK=OYDjoA@mail.gmail.com>
 <ZpqesU46GIKUvtmS@nathan>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ZpqesU46GIKUvtmS@nathan>
Archived-At: <https://www.postgresql.org/message-id/Zpq0XUQ9neudWYgC%40nathan>
Precedence: bulk

I took a look at 0003.

+            /* dim[i] = 1 + upperIndx[i] - lowerIndx[i]; */
+            if (pg_add_s32_overflow(1, upperIndx[i], &dim[i]))
+                ereport(ERROR,
+                        (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
+                         errmsg("array upper bound is too large: %d",
+                                upperIndx[i])));
+            if (pg_sub_s32_overflow(dim[i], lowerIndx[i], &dim[i]))
+                ereport(ERROR,
+                        (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
+                         errmsg("array size exceeds the maximum allowed (%d)",
+                                (int) MaxArraySize)));

I think the problem with fixing it this way is that it prohibits more than
is necessary.  For example, doing the subtraction first might prevent the
addition from overflowing, and doing the addition first can prevent the
subtraction from overflowing.  Granted, this is probably not really worth
worrying about too much, but we're already dealing with "absurd slice
ranges," so we might as well set an example for elsewhere.

An easy way to deal with this problem is to first perform the calculation
with everything cast to an int64.  Before setting dim[i], you'd check that
the result is in [PG_INT32_MIN, PG_INT32_MAX] and fail if needed.

	int64 newdim;

	...

	newdim = (int64) 1 + (int64) upperIndx[i] - (int64) lowerIndx[i];
	if (unlikely(newdim < PG_INT32_MIN || newdim > PG_INT32_MAX))
		ereport(ERROR,
				...
	dim[i] = (int32) newdim;

-- 
nathan