Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 18c0b4e

Browse files
committed
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.
1 parent 348c10e commit 18c0b4e

File tree

3 files changed

+14
-8
lines changed

3 files changed

+14
-8
lines changed

src/backend/utils/adt/array_userfuncs.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -375,7 +375,7 @@ array_cat(PG_FUNCTION_ARGS)
375375
dataoffset = 0; /* marker for no null bitmap */
376376
nbytes = ndatabytes + ARR_OVERHEAD_NONULLS(ndims);
377377
}
378-
result = (ArrayType *) palloc(nbytes);
378+
result = (ArrayType *) palloc0(nbytes);
379379
SET_VARSIZE(result, nbytes);
380380
result->ndim = ndims;
381381
result->dataoffset = dataoffset;

src/backend/utils/adt/arrayfuncs.c

+7-7
Original file line numberDiff line numberDiff line change
@@ -1339,7 +1339,7 @@ array_recv(PG_FUNCTION_ARGS)
13391339
dataoffset = 0; /* marker for no null bitmap */
13401340
nbytes += ARR_OVERHEAD_NONULLS(ndim);
13411341
}
1342-
retval = (ArrayType *) palloc(nbytes);
1342+
retval = (ArrayType *) palloc0(nbytes);
13431343
SET_VARSIZE(retval, nbytes);
13441344
retval->ndim = ndim;
13451345
retval->dataoffset = dataoffset;
@@ -1977,7 +1977,7 @@ array_get_slice(ArrayType *array,
19771977
bytes += ARR_OVERHEAD_NONULLS(ndim);
19781978
}
19791979

1980-
newarray = (ArrayType *) palloc(bytes);
1980+
newarray = (ArrayType *) palloc0(bytes);
19811981
SET_VARSIZE(newarray, bytes);
19821982
newarray->ndim = ndim;
19831983
newarray->dataoffset = dataoffset;
@@ -2230,7 +2230,7 @@ array_set(ArrayType *array,
22302230
/*
22312231
* OK, create the new array and fill in header/dimensions
22322232
*/
2233-
newarray = (ArrayType *) palloc(newsize);
2233+
newarray = (ArrayType *) palloc0(newsize);
22342234
SET_VARSIZE(newarray, newsize);
22352235
newarray->ndim = ndim;
22362236
newarray->dataoffset = newhasnulls ? overheadlen : 0;
@@ -2560,7 +2560,7 @@ array_set_slice(ArrayType *array,
25602560

25612561
newsize = overheadlen + olddatasize - olditemsize + newitemsize;
25622562

2563-
newarray = (ArrayType *) palloc(newsize);
2563+
newarray = (ArrayType *) palloc0(newsize);
25642564
SET_VARSIZE(newarray, newsize);
25652565
newarray->ndim = ndim;
25662566
newarray->dataoffset = newhasnulls ? overheadlen : 0;
@@ -2819,7 +2819,7 @@ array_map(FunctionCallInfo fcinfo, Oid inpType, Oid retType,
28192819
dataoffset = 0; /* marker for no null bitmap */
28202820
nbytes += ARR_OVERHEAD_NONULLS(ndim);
28212821
}
2822-
result = (ArrayType *) palloc(nbytes);
2822+
result = (ArrayType *) palloc0(nbytes);
28232823
SET_VARSIZE(result, nbytes);
28242824
result->ndim = ndim;
28252825
result->dataoffset = dataoffset;
@@ -2955,7 +2955,7 @@ construct_md_array(Datum *elems,
29552955
dataoffset = 0; /* marker for no null bitmap */
29562956
nbytes += ARR_OVERHEAD_NONULLS(ndims);
29572957
}
2958-
result = (ArrayType *) palloc(nbytes);
2958+
result = (ArrayType *) palloc0(nbytes);
29592959
SET_VARSIZE(result, nbytes);
29602960
result->ndim = ndims;
29612961
result->dataoffset = dataoffset;
@@ -2979,7 +2979,7 @@ construct_empty_array(Oid elmtype)
29792979
{
29802980
ArrayType *result;
29812981

2982-
result = (ArrayType *) palloc(sizeof(ArrayType));
2982+
result = (ArrayType *) palloc0(sizeof(ArrayType));
29832983
SET_VARSIZE(result, sizeof(ArrayType));
29842984
result->ndim = 0;
29852985
result->dataoffset = 0;

src/backend/utils/adt/geo_ops.c

+6
Original file line numberDiff line numberDiff line change
@@ -1478,6 +1478,8 @@ path_recv(PG_FUNCTION_ARGS)
14781478
SET_VARSIZE(path, size);
14791479
path->npts = npts;
14801480
path->closed = (closed ? 1 : 0);
1481+
/* prevent instability in unused pad bytes */
1482+
path->dummy = 0;
14811483

14821484
for (i = 0; i < npts; i++)
14831485
{
@@ -4253,6 +4255,8 @@ path_add(PG_FUNCTION_ARGS)
42534255
SET_VARSIZE(result, size);
42544256
result->npts = (p1->npts + p2->npts);
42554257
result->closed = p1->closed;
4258+
/* prevent instability in unused pad bytes */
4259+
result->dummy = 0;
42564260

42574261
for (i = 0; i < p1->npts; i++)
42584262
{
@@ -4486,6 +4490,8 @@ poly_path(PG_FUNCTION_ARGS)
44864490
SET_VARSIZE(path, size);
44874491
path->npts = poly->npts;
44884492
path->closed = TRUE;
4493+
/* prevent instability in unused pad bytes */
4494+
path->dummy = 0;
44894495

44904496
for (i = 0; i < poly->npts; i++)
44914497
{

0 commit comments

Comments
 (0)