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.

diff --git a/src/backend/utils/adt/array_userfuncs.c b/src/backend/utils/adt/array_userfuncs.c
index be2068ac5dc7476d8ffebe2d5234f5ff689a8623..ffb071ea72bb7a71b72427b1e095f34780a04658 100644 (file)
--- 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 81f8e35a67d8cb5255bc786abe095d1bde9d14a7..110a7412b6b2e5bd2446801d7fbde2580f78054f 100644 (file)
--- a/src/backend/utils/adt/arrayfuncs.c
+++ b/src/backend/utils/adt/arrayfuncs.c
@@ -1274,7 +1274,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;
@@ -1874,7 +1874,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;
@@ -2127,7 +2127,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;
@@ -2457,7 +2457,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;
@@ -2716,7 +2716,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;
@@ -2852,7 +2852,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;
@@ -2876,7 +2876,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 21c079784595c63c0145b9a5219bf55e356f3fde..29080aed4fbdf8717d31e74bf1b310736898aadb 100644 (file)
--- a/src/backend/utils/adt/geo_ops.c
+++ b/src/backend/utils/adt/geo_ops.c
@@ -1476,6 +1476,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++)
        {
@@ -4090,6 +4092,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++)
        {
@@ -4323,6 +4327,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++)
        {