On Tue, Jul 16, 2024 at 1:57 PM Nathan Bossart <nathandboss...@gmail.com>
wrote:
>>
>> On Mon, Jul 15, 2024 at 07:55:22PM -0400, Joseph Koshakow wrote:
>> On Mon, Jul 15, 2024 at 11:31 AM Nathan Bossart <nathandboss...@gmail.com
>
>> wrote:
>>> I'm curious why you aren't using float8_mul/float8_div here, i.e.,
>>>
>>> fresult = rint(float8_mul((float8) c, f));
>>> fresult = rint(float8_div((float8) c, f));
>>
>> I wrongly assumed that it was only meant to be used to implement
>> multiplication and division for the built-in float types. I've updated
>> the patch to use these functions.
>
> The reason I suggested this is so that we could omit all the prerequisite
> isinf(), isnan(), etc. checks in the cash_mul_float8() and friends. The
> checks are slighly different, but from a quick glance it just looks like
we
> might end up relying on the FLOAT8_FITS_IN_INT64 check in more cases.
I don't think we can omit the prerequisite isnan() checks. Neither
float8_mul() nor float8_div() reject nan inputs/result, and
FLOAT8_FITS_IN_INT64 has the following to say about inf and nan
These macros will do the right thing for Inf, but not necessarily for
NaN,
so check isnan(num) first if that's a possibility.
Though, I think you're right that we can remove the isinf() check from
cash_mul_float8(). That check is fully covered by FLOAT8_FITS_IN_INT64,
since all infinite inputs will result in an infinite output. That also
makes the infinite result check in float8_mul() redundant.
Additionally, I believe that the underflow check in float8_mul() is
unnecessary. val1 is an int64 casted to a float8, so it can never be
-1 < val < 1, so it can never cause an underflow to 0. So I went ahead
and removed float8_mul() since all of its checks are redundant.
For cash_div_float8() we have a choice. The isinf() input check
protects against the following, which is not rejected by any of
the other checks.
test=# SELECT '5'::money / 'inf'::float8;
?column?
----------
$0.00
(1 row)
For now, I've kept the isinf() input check to reject the above query,
let me know if you think we should allow this.
The infinite check in float8_div() is redundant because it's covered
by FLOAT8_FITS_IN_INT64. Also, the underflow check in float8_div() is
unnecessary for similar reasons to float8_mul(). So if we continue to
have a divide by zero check in cash_div_float8(), then we can remove
float8_div() as well.
>>> and "cash_sub_int64"
>>
>> Did you mean "cash_div_int64"? There's only a single function that
>> subtracts cash and an integer, but there's multiple functions that
>> divide cash by an integer. I've added a "cash_div_int64" in the updated
>> patch.
>
> My personal preference would be to add helper functions for each of these
> so that all the overflow, etc. checks are centralized in one place and
> don't clutter the calling code. Plus, it might help ensure error
> handling/messages remain consistent.
Ah, OK. I've added helpers for both subtraction and addition then.
> +static Cash
> +cash_mul_float8(Cash c, float8 f)
>
> nitpick: Can you mark these "inline"? I imagine most compilers inline
them
> without any prompting, but we might as well make our intent clear.
Updated in the attached patch.
Once again, the other patches, 0001, 0003, and 0004 are unchanged but
have their version number incremented.
Thanks,
Joe Koshakow
From 176c3b4c7b6f3cf184390828f1141b26107b6fb2 Mon Sep 17 00:00:00 2001
From: Joseph Koshakow <kosh...@gmail.com>
Date: Sat, 6 Jul 2024 14:35:00 -0400
Subject: [PATCH 3/4] Remove overflow from array_set_slice
This commit removes an overflow from array_set_slice that allows seting
absurd slice ranges.
---
src/backend/utils/adt/arrayfuncs.c | 8 +++++++-
src/test/regress/expected/arrays.out | 8 ++++++++
src/test/regress/sql/arrays.sql | 6 ++++++
3 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/src/backend/utils/adt/arrayfuncs.c b/src/backend/utils/adt/arrayfuncs.c
index d6641b570d..95e027e9ea 100644
--- a/src/backend/utils/adt/arrayfuncs.c
+++ b/src/backend/utils/adt/arrayfuncs.c
@@ -2887,7 +2887,13 @@ array_set_slice(Datum arraydatum,
errdetail("When assigning to a slice of an empty array value,"
" slice boundaries must be fully specified.")));
- dim[i] = 1 + upperIndx[i] - lowerIndx[i];
+ /* dim[i] = 1 + upperIndx[i] - lowerIndx[i]; */
+ if (pg_add_s32_overflow(1, upperIndx[i], &dim[i]) ||
+ 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)));
lb[i] = lowerIndx[i];
}
diff --git a/src/test/regress/expected/arrays.out b/src/test/regress/expected/arrays.out
index 23404982f7..a2382387e2 100644
--- a/src/test/regress/expected/arrays.out
+++ b/src/test/regress/expected/arrays.out
@@ -2699,3 +2699,11 @@ SELECT array_sample('{1,2,3,4,5,6}'::int[], -1); -- fail
ERROR: sample size must be between 0 and 6
SELECT array_sample('{1,2,3,4,5,6}'::int[], 7); --fail
ERROR: sample size must be between 0 and 6
+-- Test for overflow in array slicing
+CREATE temp table arroverflowtest (i int[]);
+INSERT INTO arroverflowtest(i[-2147483648:2147483647]) VALUES ('{}');
+ERROR: array size exceeds the maximum allowed (134217727)
+INSERT INTO arroverflowtest(i[1:2147483647]) VALUES ('{}');
+ERROR: array size exceeds the maximum allowed (134217727)
+INSERT INTO arroverflowtest(i[2147483647:2147483647]) VALUES ('{}');
+ERROR: array size exceeds the maximum allowed (134217727)
diff --git a/src/test/regress/sql/arrays.sql b/src/test/regress/sql/arrays.sql
index 50aa539fdc..e9d6737117 100644
--- a/src/test/regress/sql/arrays.sql
+++ b/src/test/regress/sql/arrays.sql
@@ -825,3 +825,9 @@ SELECT array_dims(array_sample('[-1:2][2:3]={{1,2},{3,NULL},{5,6},{7,8}}'::int[]
SELECT array_dims(array_sample('{{{1,2},{3,NULL}},{{5,6},{7,8}},{{9,10},{11,12}}}'::int[], 2));
SELECT array_sample('{1,2,3,4,5,6}'::int[], -1); -- fail
SELECT array_sample('{1,2,3,4,5,6}'::int[], 7); --fail
+
+-- Test for overflow in array slicing
+CREATE temp table arroverflowtest (i int[]);
+INSERT INTO arroverflowtest(i[-2147483648:2147483647]) VALUES ('{}');
+INSERT INTO arroverflowtest(i[1:2147483647]) VALUES ('{}');
+INSERT INTO arroverflowtest(i[2147483647:2147483647]) VALUES ('{}');
--
2.34.1
From 9758bc3b8c4bee0851d734a36ab4398d2cda9541 Mon Sep 17 00:00:00 2001
From: Joseph Koshakow <kosh...@gmail.com>
Date: Sat, 6 Jul 2024 15:41:09 -0400
Subject: [PATCH 4/4] Remove dependence on integer wrapping for jsonb
This commit updates various jsonb operators and functions to no longer
rely on integer wrapping for correctness. Not all compilers support
-fwrapv, so it's best not to rely on it.
---
src/backend/utils/adt/jsonfuncs.c | 4 ++--
src/test/regress/expected/jsonb.out | 12 ++++++++++++
src/test/regress/sql/jsonb.sql | 2 ++
3 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index 48c3f88140..8783c57303 100644
--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -946,7 +946,7 @@ jsonb_array_element(PG_FUNCTION_ARGS)
{
uint32 nelements = JB_ROOT_COUNT(jb);
- if (-element > nelements)
+ if (element == PG_INT32_MIN || -element > nelements)
PG_RETURN_NULL();
else
element += nelements;
@@ -5425,7 +5425,7 @@ setPathArray(JsonbIterator **it, Datum *path_elems, bool *path_nulls,
if (idx < 0)
{
- if (-idx > nelems)
+ if (idx == INT_MIN || -idx > nelems)
{
/*
* If asked to keep elements position consistent, it's not allowed
diff --git a/src/test/regress/expected/jsonb.out b/src/test/regress/expected/jsonb.out
index e66d760189..a9d93052fc 100644
--- a/src/test/regress/expected/jsonb.out
+++ b/src/test/regress/expected/jsonb.out
@@ -680,6 +680,18 @@ select '"foo"'::jsonb -> 'z';
(1 row)
+select '[]'::jsonb -> -2147483648;
+ ?column?
+----------
+
+(1 row)
+
+select jsonb_delete_path('{"a":[]}', '{"a",-2147483648}');
+ jsonb_delete_path
+-------------------
+ {"a": []}
+(1 row)
+
select '{"a": [{"b": "c"}, {"b": "cc"}]}'::jsonb ->> null::text;
?column?
----------
diff --git a/src/test/regress/sql/jsonb.sql b/src/test/regress/sql/jsonb.sql
index 97bc2242a1..6a18577ead 100644
--- a/src/test/regress/sql/jsonb.sql
+++ b/src/test/regress/sql/jsonb.sql
@@ -204,6 +204,8 @@ select '[{"b": "c"}, {"b": "cc"}]'::jsonb -> 'z';
select '{"a": "c", "b": null}'::jsonb -> 'b';
select '"foo"'::jsonb -> 1;
select '"foo"'::jsonb -> 'z';
+select '[]'::jsonb -> -2147483648;
+select jsonb_delete_path('{"a":[]}', '{"a",-2147483648}');
select '{"a": [{"b": "c"}, {"b": "cc"}]}'::jsonb ->> null::text;
select '{"a": [{"b": "c"}, {"b": "cc"}]}'::jsonb ->> null::int;
--
2.34.1
From 9a36354ff9fa94a2d43d0b53008e8e29bb01954f Mon Sep 17 00:00:00 2001
From: Joseph Koshakow <kosh...@gmail.com>
Date: Thu, 13 Jun 2024 22:39:25 -0400
Subject: [PATCH 2/4] Handle overflow in money arithmetic
---
src/backend/utils/adt/cash.c | 162 ++++++++++++++++++++--------
src/test/regress/expected/money.out | 90 ++++++++++++++++
src/test/regress/sql/money.sql | 47 ++++++++
3 files changed, 253 insertions(+), 46 deletions(-)
diff --git a/src/backend/utils/adt/cash.c b/src/backend/utils/adt/cash.c
index f6f095a57b..195b365fb4 100644
--- a/src/backend/utils/adt/cash.c
+++ b/src/backend/utils/adt/cash.c
@@ -86,6 +86,105 @@ num_word(Cash value)
return buf;
} /* num_word() */
+static inline Cash
+cash_pl_cash(Cash c1, Cash c2)
+{
+ Cash result;
+
+ if (pg_add_s64_overflow(c1, c2, &result))
+ ereport(ERROR,
+ (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+ errmsg("money out of range")));
+
+ return result;
+}
+
+static inline Cash
+cash_mi_cash(Cash c1, Cash c2)
+{
+ Cash result;
+
+ if (pg_sub_s64_overflow(c1, c2, &result))
+ ereport(ERROR,
+ (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+ errmsg("money out of range")));
+
+ return result;
+}
+
+static inline Cash
+cash_mul_float8(Cash c, float8 f)
+{
+ float8 fresult;
+
+ if (unlikely(isnan(f)))
+ ereport(ERROR,
+ (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+ errmsg("invalid float value")));
+
+ fresult = rint((float8) c * f);
+
+ if (unlikely(!FLOAT8_FITS_IN_INT64(fresult)))
+ ereport(ERROR,
+ (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+ errmsg("money out of range")));
+
+ return (Cash) fresult;
+}
+
+static inline Cash
+cash_div_float8(Cash c, float8 f)
+{
+ float8 fresult;
+
+ if (unlikely(f == 0.0))
+ ereport(ERROR,
+ (errcode(ERRCODE_DIVISION_BY_ZERO),
+ errmsg("division by zero")));
+ if (unlikely(isinf(f) || isnan(f)))
+ ereport(ERROR,
+ (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+ errmsg("invalid float value")));
+
+ fresult = rint(float8_div((float8) c, f));
+
+ if (unlikely(!FLOAT8_FITS_IN_INT64(fresult)))
+ ereport(ERROR,
+ (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+ errmsg("money out of range")));
+
+ return (Cash) fresult;
+}
+
+static inline Cash
+cash_mul_int64(Cash c, int64 i)
+{
+ Cash result;
+
+ if (pg_mul_s64_overflow(c, i, &result))
+ ereport(ERROR,
+ (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+ errmsg("money out of range")));
+
+ return result;
+}
+
+static inline Cash
+cash_div_int64(Cash c, int64 i)
+{
+ Cash result;
+
+ if (i == 0)
+ ereport(ERROR,
+ (errcode(ERRCODE_DIVISION_BY_ZERO),
+ errmsg("division by zero")));
+
+ result = c / i;
+
+ return result;
+}
+
+
/* cash_in()
* Convert a string to a cash data type.
* Format is [$]###[,]###[.##]
@@ -617,8 +716,7 @@ cash_pl(PG_FUNCTION_ARGS)
Cash c2 = PG_GETARG_CASH(1);
Cash result;
- result = c1 + c2;
-
+ result = cash_pl_cash(c1, c2);
PG_RETURN_CASH(result);
}
@@ -633,8 +731,7 @@ cash_mi(PG_FUNCTION_ARGS)
Cash c2 = PG_GETARG_CASH(1);
Cash result;
- result = c1 - c2;
-
+ result = cash_mi_cash(c1, c2);
PG_RETURN_CASH(result);
}
@@ -669,7 +766,7 @@ cash_mul_flt8(PG_FUNCTION_ARGS)
float8 f = PG_GETARG_FLOAT8(1);
Cash result;
- result = rint(c * f);
+ result = cash_mul_float8(c, f);
PG_RETURN_CASH(result);
}
@@ -684,7 +781,7 @@ flt8_mul_cash(PG_FUNCTION_ARGS)
Cash c = PG_GETARG_CASH(1);
Cash result;
- result = rint(f * c);
+ result = cash_mul_float8(c, f);
PG_RETURN_CASH(result);
}
@@ -699,12 +796,7 @@ cash_div_flt8(PG_FUNCTION_ARGS)
float8 f = PG_GETARG_FLOAT8(1);
Cash result;
- if (f == 0.0)
- ereport(ERROR,
- (errcode(ERRCODE_DIVISION_BY_ZERO),
- errmsg("division by zero")));
-
- result = rint(c / f);
+ result = cash_div_float8(c, (float8) f);
PG_RETURN_CASH(result);
}
@@ -719,7 +811,7 @@ cash_mul_flt4(PG_FUNCTION_ARGS)
float4 f = PG_GETARG_FLOAT4(1);
Cash result;
- result = rint(c * (float8) f);
+ result = cash_mul_float8(c, f);
PG_RETURN_CASH(result);
}
@@ -734,7 +826,7 @@ flt4_mul_cash(PG_FUNCTION_ARGS)
Cash c = PG_GETARG_CASH(1);
Cash result;
- result = rint((float8) f * c);
+ result = cash_mul_float8(c, f);
PG_RETURN_CASH(result);
}
@@ -750,12 +842,7 @@ cash_div_flt4(PG_FUNCTION_ARGS)
float4 f = PG_GETARG_FLOAT4(1);
Cash result;
- if (f == 0.0)
- ereport(ERROR,
- (errcode(ERRCODE_DIVISION_BY_ZERO),
- errmsg("division by zero")));
-
- result = rint(c / (float8) f);
+ result = cash_div_float8(c, (float8) f);
PG_RETURN_CASH(result);
}
@@ -770,7 +857,7 @@ cash_mul_int8(PG_FUNCTION_ARGS)
int64 i = PG_GETARG_INT64(1);
Cash result;
- result = c * i;
+ result = cash_mul_int64(c, i);
PG_RETURN_CASH(result);
}
@@ -785,7 +872,7 @@ int8_mul_cash(PG_FUNCTION_ARGS)
Cash c = PG_GETARG_CASH(1);
Cash result;
- result = i * c;
+ result = cash_mul_int64(c, i);
PG_RETURN_CASH(result);
}
@@ -799,13 +886,7 @@ cash_div_int8(PG_FUNCTION_ARGS)
int64 i = PG_GETARG_INT64(1);
Cash result;
- if (i == 0)
- ereport(ERROR,
- (errcode(ERRCODE_DIVISION_BY_ZERO),
- errmsg("division by zero")));
-
- result = c / i;
-
+ result = cash_div_int64(c, i);
PG_RETURN_CASH(result);
}
@@ -820,7 +901,7 @@ cash_mul_int4(PG_FUNCTION_ARGS)
int32 i = PG_GETARG_INT32(1);
Cash result;
- result = c * i;
+ result = cash_mul_int64(c, (int64) i);
PG_RETURN_CASH(result);
}
@@ -835,7 +916,7 @@ int4_mul_cash(PG_FUNCTION_ARGS)
Cash c = PG_GETARG_CASH(1);
Cash result;
- result = i * c;
+ result = cash_mul_int64(c, (int64) i);
PG_RETURN_CASH(result);
}
@@ -851,13 +932,7 @@ cash_div_int4(PG_FUNCTION_ARGS)
int32 i = PG_GETARG_INT32(1);
Cash result;
- if (i == 0)
- ereport(ERROR,
- (errcode(ERRCODE_DIVISION_BY_ZERO),
- errmsg("division by zero")));
-
- result = c / i;
-
+ result = cash_div_int64(c, (int64) i);
PG_RETURN_CASH(result);
}
@@ -872,7 +947,7 @@ cash_mul_int2(PG_FUNCTION_ARGS)
int16 s = PG_GETARG_INT16(1);
Cash result;
- result = c * s;
+ result = cash_mul_int64(c, (int64) s);
PG_RETURN_CASH(result);
}
@@ -886,7 +961,7 @@ int2_mul_cash(PG_FUNCTION_ARGS)
Cash c = PG_GETARG_CASH(1);
Cash result;
- result = s * c;
+ result = cash_mul_int64(c, (int64) s);
PG_RETURN_CASH(result);
}
@@ -901,12 +976,7 @@ cash_div_int2(PG_FUNCTION_ARGS)
int16 s = PG_GETARG_INT16(1);
Cash result;
- if (s == 0)
- ereport(ERROR,
- (errcode(ERRCODE_DIVISION_BY_ZERO),
- errmsg("division by zero")));
-
- result = c / s;
+ result = cash_div_int64(c, (int64) s);
PG_RETURN_CASH(result);
}
diff --git a/src/test/regress/expected/money.out b/src/test/regress/expected/money.out
index 7fd4e31804..f9e707dce8 100644
--- a/src/test/regress/expected/money.out
+++ b/src/test/regress/expected/money.out
@@ -528,3 +528,93 @@ SELECT '-92233720368547758.08'::money::numeric;
-92233720368547758.08
(1 row)
+-- Test overflow checks
+SELECT '92233720368547758.07'::money + '0.01'::money;
+ERROR: money out of range
+SELECT '-92233720368547758.08'::money - '0.01'::money;
+ERROR: money out of range
+SELECT '92233720368547758.07'::money * 2::int8;
+ERROR: money out of range
+SELECT '-92233720368547758.08'::money * 2::int8;
+ERROR: money out of range
+SELECT 2::int8 * '92233720368547758.07'::money ;
+ERROR: money out of range
+SELECT 2::int8 * '-92233720368547758.08'::money;
+ERROR: money out of range
+SELECT '92233720368547758.07'::money * 2::int4;
+ERROR: money out of range
+SELECT '-92233720368547758.08'::money * 2::int4;
+ERROR: money out of range
+SELECT 2::int4 * '92233720368547758.07'::money ;
+ERROR: money out of range
+SELECT 2::int4 * '-92233720368547758.08'::money;
+ERROR: money out of range
+SELECT '92233720368547758.07'::money * 2::int2;
+ERROR: money out of range
+SELECT '-92233720368547758.08'::money * 2::int2;
+ERROR: money out of range
+SELECT 2::int2 * '92233720368547758.07'::money ;
+ERROR: money out of range
+SELECT 2::int2 * '-92233720368547758.08'::money;
+ERROR: money out of range
+SELECT '92233720368547758.07'::money * 2::float8;
+ERROR: money out of range
+SELECT '-92233720368547758.08'::money * 2::float8;
+ERROR: money out of range
+SELECT 2::float8 * '92233720368547758.07'::money ;
+ERROR: money out of range
+SELECT 2::float8 * '-92233720368547758.08'::money;
+ERROR: money out of range
+SELECT '92233720368547758.07'::money * 2::float4;
+ERROR: money out of range
+SELECT '-92233720368547758.08'::money * 2::float4;
+ERROR: money out of range
+SELECT 2::float4 * '92233720368547758.07'::money ;
+ERROR: money out of range
+SELECT 2::float4 * '-92233720368547758.08'::money;
+ERROR: money out of range
+SELECT '1'::money / 1.175494e-38::float8;
+ERROR: money out of range
+SELECT '-1'::money / 1.175494e-38::float8;
+ERROR: money out of range
+SELECT '1'::money / 1.175494e-38::float4;
+ERROR: money out of range
+SELECT '-1'::money / 1.175494e-38::float4;
+ERROR: money out of range
+-- Test invalid multipliers/divisors
+SELECT '42'::money * 'inf'::float8;
+ERROR: money out of range
+SELECT '42'::money * '-inf'::float8;
+ERROR: money out of range
+SELECT '42'::money * 'nan'::float8;
+ERROR: invalid float value
+SELECT 'inf'::float8 * '42'::money;
+ERROR: money out of range
+SELECT '-inf'::float8 * '42'::money;
+ERROR: money out of range
+SELECT 'nan'::float8 * '42'::money;
+ERROR: invalid float value
+SELECT '42'::money / 'inf'::float8;
+ERROR: invalid float value
+SELECT '42'::money / '-inf'::float8;
+ERROR: invalid float value
+SELECT '42'::money / 'nan'::float8;
+ERROR: invalid float value
+SELECT '42'::money * 'inf'::float4;
+ERROR: money out of range
+SELECT '42'::money * '-inf'::float4;
+ERROR: money out of range
+SELECT '42'::money * 'nan'::float4;
+ERROR: invalid float value
+SELECT 'inf'::float4 * '42'::money;
+ERROR: money out of range
+SELECT '-inf'::float4 * '42'::money;
+ERROR: money out of range
+SELECT 'nan'::float4 * '42'::money;
+ERROR: invalid float value
+SELECT '42'::money / 'inf'::float4;
+ERROR: invalid float value
+SELECT '42'::money / '-inf'::float4;
+ERROR: invalid float value
+SELECT '42'::money / 'nan'::float4;
+ERROR: invalid float value
diff --git a/src/test/regress/sql/money.sql b/src/test/regress/sql/money.sql
index 81c92dd960..4422d2816b 100644
--- a/src/test/regress/sql/money.sql
+++ b/src/test/regress/sql/money.sql
@@ -135,3 +135,50 @@ SELECT '12345678901234567'::money::numeric;
SELECT '-12345678901234567'::money::numeric;
SELECT '92233720368547758.07'::money::numeric;
SELECT '-92233720368547758.08'::money::numeric;
+
+-- Test overflow checks
+SELECT '92233720368547758.07'::money + '0.01'::money;
+SELECT '-92233720368547758.08'::money - '0.01'::money;
+SELECT '92233720368547758.07'::money * 2::int8;
+SELECT '-92233720368547758.08'::money * 2::int8;
+SELECT 2::int8 * '92233720368547758.07'::money ;
+SELECT 2::int8 * '-92233720368547758.08'::money;
+SELECT '92233720368547758.07'::money * 2::int4;
+SELECT '-92233720368547758.08'::money * 2::int4;
+SELECT 2::int4 * '92233720368547758.07'::money ;
+SELECT 2::int4 * '-92233720368547758.08'::money;
+SELECT '92233720368547758.07'::money * 2::int2;
+SELECT '-92233720368547758.08'::money * 2::int2;
+SELECT 2::int2 * '92233720368547758.07'::money ;
+SELECT 2::int2 * '-92233720368547758.08'::money;
+SELECT '92233720368547758.07'::money * 2::float8;
+SELECT '-92233720368547758.08'::money * 2::float8;
+SELECT 2::float8 * '92233720368547758.07'::money ;
+SELECT 2::float8 * '-92233720368547758.08'::money;
+SELECT '92233720368547758.07'::money * 2::float4;
+SELECT '-92233720368547758.08'::money * 2::float4;
+SELECT 2::float4 * '92233720368547758.07'::money ;
+SELECT 2::float4 * '-92233720368547758.08'::money;
+SELECT '1'::money / 1.175494e-38::float8;
+SELECT '-1'::money / 1.175494e-38::float8;
+SELECT '1'::money / 1.175494e-38::float4;
+SELECT '-1'::money / 1.175494e-38::float4;
+-- Test invalid multipliers/divisors
+SELECT '42'::money * 'inf'::float8;
+SELECT '42'::money * '-inf'::float8;
+SELECT '42'::money * 'nan'::float8;
+SELECT 'inf'::float8 * '42'::money;
+SELECT '-inf'::float8 * '42'::money;
+SELECT 'nan'::float8 * '42'::money;
+SELECT '42'::money / 'inf'::float8;
+SELECT '42'::money / '-inf'::float8;
+SELECT '42'::money / 'nan'::float8;
+SELECT '42'::money * 'inf'::float4;
+SELECT '42'::money * '-inf'::float4;
+SELECT '42'::money * 'nan'::float4;
+SELECT 'inf'::float4 * '42'::money;
+SELECT '-inf'::float4 * '42'::money;
+SELECT 'nan'::float4 * '42'::money;
+SELECT '42'::money / 'inf'::float4;
+SELECT '42'::money / '-inf'::float4;
+SELECT '42'::money / 'nan'::float4;
--
2.34.1
From 950da2c4ce85632f6085680c3ed7b75fb1f780f7 Mon Sep 17 00:00:00 2001
From: Joseph Koshakow <kosh...@gmail.com>
Date: Sat, 8 Jun 2024 22:16:46 -0400
Subject: [PATCH 1/4] Remove dependence on integer wrapping
This commit updates various parts of the code to no longer rely on
integer wrapping for correctness. Not all compilers support -fwrapv, so
it's best not to rely on it.
---
src/backend/utils/adt/cash.c | 7 +-
src/backend/utils/adt/numeric.c | 5 +-
src/backend/utils/adt/numutils.c | 34 ++++---
src/backend/utils/adt/timestamp.c | 28 +-----
src/include/common/int.h | 105 +++++++++++++++++++++
src/interfaces/ecpg/pgtypeslib/timestamp.c | 11 +--
src/test/regress/expected/timestamp.out | 13 +++
src/test/regress/expected/timestamptz.out | 13 +++
src/test/regress/sql/timestamp.sql | 4 +
src/test/regress/sql/timestamptz.sql | 4 +
10 files changed, 169 insertions(+), 55 deletions(-)
diff --git a/src/backend/utils/adt/cash.c b/src/backend/utils/adt/cash.c
index 32fbad2f57..f6f095a57b 100644
--- a/src/backend/utils/adt/cash.c
+++ b/src/backend/utils/adt/cash.c
@@ -352,8 +352,11 @@ cash_out(PG_FUNCTION_ARGS)
if (value < 0)
{
- /* make the amount positive for digit-reconstruction loop */
- value = -value;
+ /*
+ * make the amount positive for digit-reconstruction loop, we can
+ * leave INT64_MIN unchanged
+ */
+ pg_neg_s64_overflow(value, &value);
/* set up formatting data */
signsymbol = (*lconvert->negative_sign != '\0') ? lconvert->negative_sign : "-";
sign_posn = lconvert->n_sign_posn;
diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c
index d0f0923710..38965b4023 100644
--- a/src/backend/utils/adt/numeric.c
+++ b/src/backend/utils/adt/numeric.c
@@ -8114,15 +8114,14 @@ int64_to_numericvar(int64 val, NumericVar *var)
/* int64 can require at most 19 decimal digits; add one for safety */
alloc_var(var, 20 / DEC_DIGITS);
+ uval = pg_abs_s64(val);
if (val < 0)
{
var->sign = NUMERIC_NEG;
- uval = -val;
}
else
{
var->sign = NUMERIC_POS;
- uval = val;
}
var->dscale = 0;
if (val == 0)
@@ -11443,7 +11442,7 @@ power_var_int(const NumericVar *base, int exp, int exp_dscale,
* Now we can proceed with the multiplications.
*/
neg = (exp < 0);
- mask = abs(exp);
+ mask = pg_abs_s32(exp);
init_var(&base_prod);
set_var_from_var(base, &base_prod);
diff --git a/src/backend/utils/adt/numutils.c b/src/backend/utils/adt/numutils.c
index adc1e8a4cb..a3d7d6bf01 100644
--- a/src/backend/utils/adt/numutils.c
+++ b/src/backend/utils/adt/numutils.c
@@ -18,6 +18,7 @@
#include <limits.h>
#include <ctype.h>
+#include "common/int.h"
#include "port/pg_bitutils.h"
#include "utils/builtins.h"
@@ -131,6 +132,7 @@ pg_strtoint16_safe(const char *s, Node *escontext)
uint16 tmp = 0;
bool neg = false;
unsigned char digit;
+ int16 result;
/*
* The majority of cases are likely to be base-10 digits without any
@@ -190,10 +192,9 @@ pg_strtoint16_safe(const char *s, Node *escontext)
if (neg)
{
- /* check the negative equivalent will fit without overflowing */
- if (unlikely(tmp > (uint16) (-(PG_INT16_MIN + 1)) + 1))
+ if (pg_neg_u16_overflow(tmp, &result))
goto out_of_range;
- return -((int16) tmp);
+ return result;
}
if (unlikely(tmp > PG_INT16_MAX))
@@ -333,10 +334,9 @@ slow:
if (neg)
{
- /* check the negative equivalent will fit without overflowing */
- if (tmp > (uint16) (-(PG_INT16_MIN + 1)) + 1)
+ if (pg_neg_u16_overflow(tmp, &result))
goto out_of_range;
- return -((int16) tmp);
+ return result;
}
if (tmp > PG_INT16_MAX)
@@ -393,6 +393,7 @@ pg_strtoint32_safe(const char *s, Node *escontext)
uint32 tmp = 0;
bool neg = false;
unsigned char digit;
+ int32 result;
/*
* The majority of cases are likely to be base-10 digits without any
@@ -452,10 +453,9 @@ pg_strtoint32_safe(const char *s, Node *escontext)
if (neg)
{
- /* check the negative equivalent will fit without overflowing */
- if (unlikely(tmp > (uint32) (-(PG_INT32_MIN + 1)) + 1))
+ if (pg_neg_u32_overflow(tmp, &result))
goto out_of_range;
- return -((int32) tmp);
+ return result;
}
if (unlikely(tmp > PG_INT32_MAX))
@@ -595,10 +595,9 @@ slow:
if (neg)
{
- /* check the negative equivalent will fit without overflowing */
- if (tmp > (uint32) (-(PG_INT32_MIN + 1)) + 1)
+ if (pg_neg_u32_overflow(tmp, &result))
goto out_of_range;
- return -((int32) tmp);
+ return result;
}
if (tmp > PG_INT32_MAX)
@@ -655,6 +654,7 @@ pg_strtoint64_safe(const char *s, Node *escontext)
uint64 tmp = 0;
bool neg = false;
unsigned char digit;
+ int64 result;
/*
* The majority of cases are likely to be base-10 digits without any
@@ -714,10 +714,9 @@ pg_strtoint64_safe(const char *s, Node *escontext)
if (neg)
{
- /* check the negative equivalent will fit without overflowing */
- if (unlikely(tmp > (uint64) (-(PG_INT64_MIN + 1)) + 1))
+ if (pg_neg_u64_overflow(tmp, &result))
goto out_of_range;
- return -((int64) tmp);
+ return result;
}
if (unlikely(tmp > PG_INT64_MAX))
@@ -857,10 +856,9 @@ slow:
if (neg)
{
- /* check the negative equivalent will fit without overflowing */
- if (tmp > (uint64) (-(PG_INT64_MIN + 1)) + 1)
+ if (pg_neg_u64_overflow(tmp, &result))
goto out_of_range;
- return -((int64) tmp);
+ return result;
}
if (tmp > PG_INT64_MAX)
diff --git a/src/backend/utils/adt/timestamp.c b/src/backend/utils/adt/timestamp.c
index 69fe7860ed..c76793f72d 100644
--- a/src/backend/utils/adt/timestamp.c
+++ b/src/backend/utils/adt/timestamp.c
@@ -618,19 +618,8 @@ make_timestamp_internal(int year, int month, int day,
time = (((hour * MINS_PER_HOUR + min) * SECS_PER_MINUTE)
* USECS_PER_SEC) + (int64) rint(sec * USECS_PER_SEC);
- result = date * USECS_PER_DAY + time;
- /* check for major overflow */
- if ((result - time) / USECS_PER_DAY != date)
- ereport(ERROR,
- (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
- errmsg("timestamp out of range: %d-%02d-%02d %d:%02d:%02g",
- year, month, day,
- hour, min, sec)));
-
- /* check for just-barely overflow (okay except time-of-day wraps) */
- /* caution: we want to allow 1999-12-31 24:00:00 */
- if ((result < 0 && date > 0) ||
- (result > 0 && date < -1))
+ if (pg_mul_s64_overflow(date, USECS_PER_DAY, &result) ||
+ pg_add_s64_overflow(result, time, &result))
ereport(ERROR,
(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
errmsg("timestamp out of range: %d-%02d-%02d %d:%02d:%02g",
@@ -2010,17 +1999,8 @@ tm2timestamp(struct pg_tm *tm, fsec_t fsec, int *tzp, Timestamp *result)
date = date2j(tm->tm_year, tm->tm_mon, tm->tm_mday) - POSTGRES_EPOCH_JDATE;
time = time2t(tm->tm_hour, tm->tm_min, tm->tm_sec, fsec);
- *result = date * USECS_PER_DAY + time;
- /* check for major overflow */
- if ((*result - time) / USECS_PER_DAY != date)
- {
- *result = 0; /* keep compiler quiet */
- return -1;
- }
- /* check for just-barely overflow (okay except time-of-day wraps) */
- /* caution: we want to allow 1999-12-31 24:00:00 */
- if ((*result < 0 && date > 0) ||
- (*result > 0 && date < -1))
+ if (pg_mul_s64_overflow(date, USECS_PER_DAY, result) ||
+ pg_add_s64_overflow(*result, time, result))
{
*result = 0; /* keep compiler quiet */
return -1;
diff --git a/src/include/common/int.h b/src/include/common/int.h
index 7fc046e78a..3065d586d4 100644
--- a/src/include/common/int.h
+++ b/src/include/common/int.h
@@ -154,6 +154,23 @@ pg_mul_s32_overflow(int32 a, int32 b, int32 *result)
#endif
}
+static inline uint32
+pg_abs_s32(int32 a)
+{
+ if (unlikely(a == PG_INT32_MIN))
+ {
+ return ((uint32) PG_INT32_MAX) + 1;
+ }
+ else if (a < 0)
+ {
+ return (uint32) -a;
+ }
+ else
+ {
+ return (uint32) a;
+ }
+}
+
/*
* INT64
*/
@@ -258,6 +275,37 @@ pg_mul_s64_overflow(int64 a, int64 b, int64 *result)
#endif
}
+static inline bool
+pg_neg_s64_overflow(int64 a, int64 *result)
+{
+ if (unlikely(a == PG_INT64_MIN))
+ {
+ return true;
+ }
+ else
+ {
+ *result = -a;
+ return false;
+ }
+}
+
+static inline uint64
+pg_abs_s64(int64 a)
+{
+ if (unlikely(a == PG_INT64_MIN))
+ {
+ return ((uint64) PG_INT64_MAX) + 1;
+ }
+ else if (a < 0)
+ {
+ return (uint64) -a;
+ }
+ else
+ {
+ return (uint64) a;
+ }
+}
+
/*------------------------------------------------------------------------
* Overflow routines for unsigned integers
*------------------------------------------------------------------------
@@ -318,6 +366,25 @@ pg_mul_u16_overflow(uint16 a, uint16 b, uint16 *result)
#endif
}
+static inline bool
+pg_neg_u16_overflow(uint16 a, int16 *result)
+{
+ if (unlikely(a > ((uint16) PG_INT16_MAX) + 1))
+ {
+ return true;
+ }
+ else if (unlikely(a == ((uint16) PG_INT16_MAX) + 1))
+ {
+ *result = PG_INT16_MIN;
+ return false;
+ }
+ else
+ {
+ *result = -((int16) a);
+ return false;
+ }
+}
+
/*
* INT32
*/
@@ -373,6 +440,25 @@ pg_mul_u32_overflow(uint32 a, uint32 b, uint32 *result)
#endif
}
+static inline bool
+pg_neg_u32_overflow(uint32 a, int32 *result)
+{
+ if (unlikely(a > ((uint32) PG_INT32_MAX) + 1))
+ {
+ return true;
+ }
+ else if (unlikely(a == ((uint32) PG_INT32_MAX) + 1))
+ {
+ *result = PG_INT32_MIN;
+ return false;
+ }
+ else
+ {
+ *result = -((int32) a);
+ return false;
+ }
+}
+
/*
* UINT64
*/
@@ -438,6 +524,25 @@ pg_mul_u64_overflow(uint64 a, uint64 b, uint64 *result)
#endif
}
+static inline bool
+pg_neg_u64_overflow(uint64 a, int64 *result)
+{
+ if (unlikely(a > ((uint64) PG_INT64_MAX) + 1))
+ {
+ return true;
+ }
+ else if (unlikely(a == ((uint64) PG_INT64_MAX) + 1))
+ {
+ *result = PG_INT64_MIN;
+ return false;
+ }
+ else
+ {
+ *result = -((int64) a);
+ return false;
+ }
+}
+
/*------------------------------------------------------------------------
*
* Comparison routines for integer types.
diff --git a/src/interfaces/ecpg/pgtypeslib/timestamp.c b/src/interfaces/ecpg/pgtypeslib/timestamp.c
index f1b143fbd2..93d4cc323d 100644
--- a/src/interfaces/ecpg/pgtypeslib/timestamp.c
+++ b/src/interfaces/ecpg/pgtypeslib/timestamp.c
@@ -11,6 +11,7 @@
#error -ffast-math is known to break this code
#endif
+#include "common/int.h"
#include "dt.h"
#include "pgtypes_date.h"
#include "pgtypes_timestamp.h"
@@ -48,14 +49,8 @@ tm2timestamp(struct tm *tm, fsec_t fsec, int *tzp, timestamp * result)
dDate = date2j(tm->tm_year, tm->tm_mon, tm->tm_mday) - date2j(2000, 1, 1);
time = time2t(tm->tm_hour, tm->tm_min, tm->tm_sec, fsec);
- *result = (dDate * USECS_PER_DAY) + time;
- /* check for major overflow */
- if ((*result - time) / USECS_PER_DAY != dDate)
- return -1;
- /* check for just-barely overflow (okay except time-of-day wraps) */
- /* caution: we want to allow 1999-12-31 24:00:00 */
- if ((*result < 0 && dDate > 0) ||
- (*result > 0 && dDate < -1))
+ if (pg_mul_s64_overflow(dDate, USECS_PER_DAY, result) ||
+ pg_add_s64_overflow(*result, time, result))
return -1;
if (tzp != NULL)
*result = dt2local(*result, -(*tzp));
diff --git a/src/test/regress/expected/timestamp.out b/src/test/regress/expected/timestamp.out
index cf337da517..e287260051 100644
--- a/src/test/regress/expected/timestamp.out
+++ b/src/test/regress/expected/timestamp.out
@@ -2201,3 +2201,16 @@ select age(timestamp '-infinity', timestamp 'infinity');
select age(timestamp '-infinity', timestamp '-infinity');
ERROR: interval out of range
+-- test timestamp near POSTGRES_EPOCH_JDATE
+select timestamp '1999-12-31 24:00:00';
+ timestamp
+--------------------------
+ Sat Jan 01 00:00:00 2000
+(1 row)
+
+select make_timestamp(1999, 12, 31, 24, 0, 0);
+ make_timestamp
+--------------------------
+ Sat Jan 01 00:00:00 2000
+(1 row)
+
diff --git a/src/test/regress/expected/timestamptz.out b/src/test/regress/expected/timestamptz.out
index bfb3825ff6..d01d174983 100644
--- a/src/test/regress/expected/timestamptz.out
+++ b/src/test/regress/expected/timestamptz.out
@@ -3286,3 +3286,16 @@ SELECT age(timestamptz '-infinity', timestamptz 'infinity');
SELECT age(timestamptz '-infinity', timestamptz '-infinity');
ERROR: interval out of range
+-- test timestamp near POSTGRES_EPOCH_JDATE
+select timestamptz '1999-12-31 24:00:00';
+ timestamptz
+------------------------------
+ Sat Jan 01 00:00:00 2000 PST
+(1 row)
+
+select make_timestamptz(1999, 12, 31, 24, 0, 0);
+ make_timestamptz
+------------------------------
+ Sat Jan 01 00:00:00 2000 PST
+(1 row)
+
diff --git a/src/test/regress/sql/timestamp.sql b/src/test/regress/sql/timestamp.sql
index 820ef7752a..748469576d 100644
--- a/src/test/regress/sql/timestamp.sql
+++ b/src/test/regress/sql/timestamp.sql
@@ -424,3 +424,7 @@ select age(timestamp 'infinity', timestamp 'infinity');
select age(timestamp 'infinity', timestamp '-infinity');
select age(timestamp '-infinity', timestamp 'infinity');
select age(timestamp '-infinity', timestamp '-infinity');
+
+-- test timestamp near POSTGRES_EPOCH_JDATE
+select timestamp '1999-12-31 24:00:00';
+select make_timestamp(1999, 12, 31, 24, 0, 0);
diff --git a/src/test/regress/sql/timestamptz.sql b/src/test/regress/sql/timestamptz.sql
index ccfd90d646..c71d5489b4 100644
--- a/src/test/regress/sql/timestamptz.sql
+++ b/src/test/regress/sql/timestamptz.sql
@@ -668,3 +668,7 @@ SELECT age(timestamptz 'infinity', timestamptz 'infinity');
SELECT age(timestamptz 'infinity', timestamptz '-infinity');
SELECT age(timestamptz '-infinity', timestamptz 'infinity');
SELECT age(timestamptz '-infinity', timestamptz '-infinity');
+
+-- test timestamp near POSTGRES_EPOCH_JDATE
+select timestamptz '1999-12-31 24:00:00';
+select make_timestamptz(1999, 12, 31, 24, 0, 0);
--
2.34.1
