From 433853e9269ee81627400c7edb1e4f9a001d3816 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Wed, 27 Apr 2011 13:58:44 -0400 Subject: [PATCH] Fix array- and path-creating functions to ensure padding bytes are zeroes. Per recent discussion, it's important for all computed datums (not only the results of input functions) to not contain any ill-defined (uninitialized) bits. Failing to ensure that can result in equal() reporting that semantically indistinguishable Consts are not equal, which in turn leads to bizarre and undesirable planner behavior, such as in a recent example from David Johnston. We might eventually try to fix this in a general manner by allowing datatypes to define identity-testing functions, but for now the path of least resistance is to expect datatypes to force all unused bits into consistent states. Per some testing by Noah Misch, array and path functions seem to be the only ones presenting risks at the moment, so I looked through all the functions in adt/array*.c and geo_ops.c and fixed them as necessary. In the array functions, the easiest/safest fix is to allocate result arrays with palloc0 instead of palloc. Possibly in future someone will want to look into whether we can just zero the padding bytes, but that looks too complex for a back-patchable fix. In the path functions, we already had a precedent in path_in for just zeroing the one known pad field, so duplicate that code as needed. Back-patch to all supported branches. --- src/backend/utils/adt/array_userfuncs.c | 2 +- src/backend/utils/adt/arrayfuncs.c | 14 +++++++------- src/backend/utils/adt/geo_ops.c | 6 ++++++ 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/backend/utils/adt/array_userfuncs.c b/src/backend/utils/adt/array_userfuncs.c index bca0b89442..159353598f 100644 --- a/src/backend/utils/adt/array_userfuncs.c +++ b/src/backend/utils/adt/array_userfuncs.c @@ -375,7 +375,7 @@ array_cat(PG_FUNCTION_ARGS) dataoffset = 0; /* marker for no null bitmap */ nbytes = ndatabytes + ARR_OVERHEAD_NONULLS(ndims); } - result = (ArrayType *) palloc(nbytes); + result = (ArrayType *) palloc0(nbytes); SET_VARSIZE(result, nbytes); result->ndim = ndims; result->dataoffset = dataoffset; diff --git a/src/backend/utils/adt/arrayfuncs.c b/src/backend/utils/adt/arrayfuncs.c index df7190dbfc..add2a55e5d 100644 --- a/src/backend/utils/adt/arrayfuncs.c +++ b/src/backend/utils/adt/arrayfuncs.c @@ -1314,7 +1314,7 @@ array_recv(PG_FUNCTION_ARGS) dataoffset = 0; /* marker for no null bitmap */ nbytes += ARR_OVERHEAD_NONULLS(ndim); } - retval = (ArrayType *) palloc(nbytes); + retval = (ArrayType *) palloc0(nbytes); SET_VARSIZE(retval, nbytes); retval->ndim = ndim; retval->dataoffset = dataoffset; @@ -1952,7 +1952,7 @@ array_get_slice(ArrayType *array, bytes += ARR_OVERHEAD_NONULLS(ndim); } - newarray = (ArrayType *) palloc(bytes); + newarray = (ArrayType *) palloc0(bytes); SET_VARSIZE(newarray, bytes); newarray->ndim = ndim; newarray->dataoffset = dataoffset; @@ -2205,7 +2205,7 @@ array_set(ArrayType *array, /* * OK, create the new array and fill in header/dimensions */ - newarray = (ArrayType *) palloc(newsize); + newarray = (ArrayType *) palloc0(newsize); SET_VARSIZE(newarray, newsize); newarray->ndim = ndim; newarray->dataoffset = newhasnulls ? overheadlen : 0; @@ -2535,7 +2535,7 @@ array_set_slice(ArrayType *array, newsize = overheadlen + olddatasize - olditemsize + newitemsize; - newarray = (ArrayType *) palloc(newsize); + newarray = (ArrayType *) palloc0(newsize); SET_VARSIZE(newarray, newsize); newarray->ndim = ndim; newarray->dataoffset = newhasnulls ? overheadlen : 0; @@ -2794,7 +2794,7 @@ array_map(FunctionCallInfo fcinfo, Oid inpType, Oid retType, dataoffset = 0; /* marker for no null bitmap */ nbytes += ARR_OVERHEAD_NONULLS(ndim); } - result = (ArrayType *) palloc(nbytes); + result = (ArrayType *) palloc0(nbytes); SET_VARSIZE(result, nbytes); result->ndim = ndim; result->dataoffset = dataoffset; @@ -2930,7 +2930,7 @@ construct_md_array(Datum *elems, dataoffset = 0; /* marker for no null bitmap */ nbytes += ARR_OVERHEAD_NONULLS(ndims); } - result = (ArrayType *) palloc(nbytes); + result = (ArrayType *) palloc0(nbytes); SET_VARSIZE(result, nbytes); result->ndim = ndims; result->dataoffset = dataoffset; @@ -2954,7 +2954,7 @@ construct_empty_array(Oid elmtype) { ArrayType *result; - result = (ArrayType *) palloc(sizeof(ArrayType)); + result = (ArrayType *) palloc0(sizeof(ArrayType)); SET_VARSIZE(result, sizeof(ArrayType)); result->ndim = 0; result->dataoffset = 0; diff --git a/src/backend/utils/adt/geo_ops.c b/src/backend/utils/adt/geo_ops.c index 3e93bac282..9fe9cef204 100644 --- a/src/backend/utils/adt/geo_ops.c +++ b/src/backend/utils/adt/geo_ops.c @@ -1478,6 +1478,8 @@ path_recv(PG_FUNCTION_ARGS) SET_VARSIZE(path, size); path->npts = npts; path->closed = (closed ? 1 : 0); + /* prevent instability in unused pad bytes */ + path->dummy = 0; for (i = 0; i < npts; i++) { @@ -4253,6 +4255,8 @@ path_add(PG_FUNCTION_ARGS) SET_VARSIZE(result, size); result->npts = (p1->npts + p2->npts); result->closed = p1->closed; + /* prevent instability in unused pad bytes */ + result->dummy = 0; for (i = 0; i < p1->npts; i++) { @@ -4486,6 +4490,8 @@ poly_path(PG_FUNCTION_ARGS) SET_VARSIZE(path, size); path->npts = poly->npts; path->closed = TRUE; + /* prevent instability in unused pad bytes */ + path->dummy = 0; for (i = 0; i < poly->npts; i++) {