From: Tom Lane Date: Mon, 21 Jul 2008 04:47:00 +0000 (+0000) Subject: Code review for array_fill patch: fix inadequate check for array size overflow X-Git-Tag: REL8_4_BETA1~1139 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=5618ece82b49c06c6ee9509f07760b8e5b283973;p=postgresql Code review for array_fill patch: fix inadequate check for array size overflow and bogus documentation (dimension arrays are int[] not anyarray). Also the errhint() messages seem to be really errdetail(), since there is nothing heuristic about them. Some other trivial cosmetic improvements. --- diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index fb77ae43ea..15162ed5ed 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -1,4 +1,4 @@ - + Functions and Operators @@ -9186,17 +9186,16 @@ SELECT NULLIF(value, '(none)') ... - Array Functions and Operators shows the operators - available for array types. + available for array types. - <type>array</type> Operators + Array Operators @@ -9326,7 +9325,7 @@ SELECT NULLIF(value, '(none)') ...
- <type>array</type> Functions + Array Functions @@ -9374,13 +9373,13 @@ SELECT NULLIF(value, '(none)') ... - array_fill(anyelement, anyarray, - , anyarray) + array_fill(anyelement, int[], + , int[]) anyarray - returns an array initialized with supplied value, - dimensions, and lower bounds + returns an array initialized with supplied value and + dimensions, optionally with lower bounds other than 1 array_fill(7, ARRAY[3], ARRAY[2]) [2:4]={7,7,7} diff --git a/src/backend/utils/adt/arrayfuncs.c b/src/backend/utils/adt/arrayfuncs.c index 6c810025e5..5f3356f560 100644 --- a/src/backend/utils/adt/arrayfuncs.c +++ b/src/backend/utils/adt/arrayfuncs.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/utils/adt/arrayfuncs.c,v 1.146 2008/07/16 00:48:53 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/utils/adt/arrayfuncs.c,v 1.147 2008/07/21 04:47:00 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -97,9 +97,9 @@ static void array_insert_slice(ArrayType *destArray, ArrayType *origArray, static int array_cmp(FunctionCallInfo fcinfo); static ArrayType *create_array_envelope(int ndims, int *dimv, int *lbv, int nbytes, Oid elmtype, int dataoffset); -static ArrayType *array_fill_internal(ArrayType *dims, ArrayType *lbs, Datum value, - Oid elmtype, bool isnull, - FunctionCallInfo fcinfo); +static ArrayType *array_fill_internal(ArrayType *dims, ArrayType *lbs, + Datum value, bool isnull, Oid elmtype, + FunctionCallInfo fcinfo); /* @@ -4245,7 +4245,7 @@ typedef struct generate_subscripts_fctx bool reverse; } generate_subscripts_fctx; -/* +/* * generate_subscripts(array anyarray, dim int [, reverse bool]) * Returns all subscripts of the array for any dimension */ @@ -4335,7 +4335,7 @@ array_fill_with_lower_bounds(PG_FUNCTION_ARGS) bool isnull; if (PG_ARGISNULL(1) || PG_ARGISNULL(2)) - ereport(ERROR, + ereport(ERROR, (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED), errmsg("dimension array or low bound array cannot be NULL"))); @@ -4353,11 +4353,11 @@ array_fill_with_lower_bounds(PG_FUNCTION_ARGS) isnull = true; } - elmtype = get_fn_expr_argtype(fcinfo->flinfo, 0); - if (!OidIsValid(elmtype)) - elog(ERROR, "could not determine data type of input"); + elmtype = get_fn_expr_argtype(fcinfo->flinfo, 0); + if (!OidIsValid(elmtype)) + elog(ERROR, "could not determine data type of input"); - result = array_fill_internal(dims, lbs, value, elmtype, isnull, fcinfo); + result = array_fill_internal(dims, lbs, value, isnull, elmtype, fcinfo); PG_RETURN_ARRAYTYPE_P(result); } @@ -4375,7 +4375,7 @@ array_fill(PG_FUNCTION_ARGS) bool isnull; if (PG_ARGISNULL(1)) - ereport(ERROR, + ereport(ERROR, (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED), errmsg("dimension array or low bound array cannot be NULL"))); @@ -4392,17 +4392,17 @@ array_fill(PG_FUNCTION_ARGS) isnull = true; } - elmtype = get_fn_expr_argtype(fcinfo->flinfo, 0); - if (!OidIsValid(elmtype)) - elog(ERROR, "could not determine data type of input"); + elmtype = get_fn_expr_argtype(fcinfo->flinfo, 0); + if (!OidIsValid(elmtype)) + elog(ERROR, "could not determine data type of input"); - result = array_fill_internal(dims, NULL, value, elmtype, isnull, fcinfo); + result = array_fill_internal(dims, NULL, value, isnull, elmtype, fcinfo); PG_RETURN_ARRAYTYPE_P(result); } static ArrayType * create_array_envelope(int ndims, int *dimv, int *lbsv, int nbytes, - Oid elmtype, int dataoffset) + Oid elmtype, int dataoffset) { ArrayType *result; @@ -4418,9 +4418,9 @@ create_array_envelope(int ndims, int *dimv, int *lbsv, int nbytes, } static ArrayType * -array_fill_internal(ArrayType *dims, ArrayType *lbs, Datum value, - Oid elmtype, bool isnull, - FunctionCallInfo fcinfo) +array_fill_internal(ArrayType *dims, ArrayType *lbs, + Datum value, bool isnull, Oid elmtype, + FunctionCallInfo fcinfo) { ArrayType *result; int *dimv; @@ -4428,34 +4428,34 @@ array_fill_internal(ArrayType *dims, ArrayType *lbs, Datum value, int ndims; int nitems; int deflbs[MAXDIM]; - int16 elmlen; - bool elmbyval; + int16 elmlen; + bool elmbyval; char elmalign; ArrayMetaState *my_extra; - /* + /* * Params checks */ if (ARR_NDIM(dims) != 1) ereport(ERROR, (errcode(ERRCODE_ARRAY_SUBSCRIPT_ERROR), errmsg("wrong number of array subscripts"), - errhint("Dimension array must be one dimensional."))); + errdetail("Dimension array must be one dimensional."))); if (ARR_LBOUND(dims)[0] != 1) ereport(ERROR, (errcode(ERRCODE_ARRAY_SUBSCRIPT_ERROR), errmsg("wrong range of array_subscripts"), - errhint("Lower bound of dimension array must be one."))); - + errdetail("Lower bound of dimension array must be one."))); + if (ARR_HASNULL(dims)) - ereport(ERROR, + ereport(ERROR, (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED), errmsg("dimension values cannot be null"))); dimv = (int *) ARR_DATA_PTR(dims); ndims = ARR_DIMS(dims)[0]; - + if (ndims < 0) /* we do allow zero-dimension arrays */ ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), @@ -4465,23 +4465,23 @@ array_fill_internal(ArrayType *dims, ArrayType *lbs, Datum value, (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), errmsg("number of array dimensions (%d) exceeds the maximum allowed (%d)", ndims, MAXDIM))); - + if (lbs != NULL) { if (ARR_NDIM(lbs) != 1) ereport(ERROR, (errcode(ERRCODE_ARRAY_SUBSCRIPT_ERROR), errmsg("wrong number of array subscripts"), - errhint("Dimension array must be one dimensional."))); + errdetail("Dimension array must be one dimensional."))); if (ARR_LBOUND(lbs)[0] != 1) ereport(ERROR, (errcode(ERRCODE_ARRAY_SUBSCRIPT_ERROR), errmsg("wrong range of array_subscripts"), - errhint("Lower bound of dimension array must be one."))); - + errdetail("Lower bound of dimension array must be one."))); + if (ARR_HASNULL(lbs)) - ereport(ERROR, + ereport(ERROR, (errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED), errmsg("dimension values cannot be null"))); @@ -4489,14 +4489,14 @@ array_fill_internal(ArrayType *dims, ArrayType *lbs, Datum value, ereport(ERROR, (errcode(ERRCODE_ARRAY_SUBSCRIPT_ERROR), errmsg("wrong number of array_subscripts"), - errhint("Low bound array has different size than dimensions array."))); - + errdetail("Low bound array has different size than dimensions array."))); + lbsv = (int *) ARR_DATA_PTR(lbs); } - else + else { int i; - + for (i = 0; i < MAXDIM; i++) deflbs[i] = 1; @@ -4506,9 +4506,8 @@ array_fill_internal(ArrayType *dims, ArrayType *lbs, Datum value, /* fast track for empty array */ if (ndims == 0) return construct_empty_array(elmtype); - - nitems = ArrayGetNItems(ndims, dimv); + nitems = ArrayGetNItems(ndims, dimv); /* * We arrange to look up info about element type only once per series of @@ -4543,7 +4542,7 @@ array_fill_internal(ArrayType *dims, ArrayType *lbs, Datum value, int i; char *p; int nbytes; - Datum aux_value = value; + int totbytes; /* make sure data is not toasted */ if (elmlen == -1) @@ -4551,40 +4550,44 @@ array_fill_internal(ArrayType *dims, ArrayType *lbs, Datum value, nbytes = att_addlength_datum(0, elmlen, value); nbytes = att_align_nominal(nbytes, elmalign); + Assert(nbytes > 0); - nbytes *= nitems; - /* check for overflow of total request */ - if (!AllocSizeIsValid(nbytes)) + totbytes = nbytes * nitems; + + /* check for overflow of multiplication or total request */ + if (totbytes / nbytes != nitems || + !AllocSizeIsValid(totbytes)) ereport(ERROR, (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED), errmsg("array size exceeds the maximum allowed (%d)", (int) MaxAllocSize))); - nbytes += ARR_OVERHEAD_NONULLS(ndims); - result = create_array_envelope(ndims, dimv, lbsv, nbytes, - elmtype, 0); + /* + * This addition can't overflow, but it might cause us to go past + * MaxAllocSize. We leave it to palloc to complain in that case. + */ + totbytes += ARR_OVERHEAD_NONULLS(ndims); + + result = create_array_envelope(ndims, dimv, lbsv, totbytes, + elmtype, 0); + p = ARR_DATA_PTR(result); for (i = 0; i < nitems; i++) p += ArrayCastAndSet(value, elmlen, elmbyval, elmalign, p); - - /* cleaning up detoasted copies of datum */ - if (aux_value != value) - pfree((Pointer) value); } else { int nbytes; int dataoffset; - bits8 *bitmap; dataoffset = ARR_OVERHEAD_WITHNULLS(ndims, nitems); nbytes = dataoffset; result = create_array_envelope(ndims, dimv, lbsv, nbytes, - elmtype, dataoffset); - bitmap = ARR_NULLBITMAP(result); - MemSet(bitmap, 0, (nitems + 7) / 8); + elmtype, dataoffset); + + /* create_array_envelope already zeroed the bitmap, so we're done */ } - + return result; } diff --git a/src/test/regress/expected/arrays.out b/src/test/regress/expected/arrays.out index 7b7a01694a..bcb451e808 100644 --- a/src/test/regress/expected/arrays.out +++ b/src/test/regress/expected/arrays.out @@ -988,6 +988,6 @@ select array_fill(1, array[2,2], null); ERROR: dimension array or low bound array cannot be NULL select array_fill(1, array[3,3], array[1,1,1]); ERROR: wrong number of array_subscripts -HINT: Low bound array has different size than dimensions array. +DETAIL: Low bound array has different size than dimensions array. select array_fill(1, array[1,2,null]); ERROR: dimension values cannot be null