Fix uninitialized memory propagation mistakes

Valgrind complains that some uninitialized bytes are being passed around
by the extended statistics code since commit 7b504eb282ca2f, as reported
by Andres Freund.  Silence it.

Tomas Vondra submitted a patch which he verified to fix the complaints
in his machine; however I messed with it a bit before pushing, so any
remaining problems are likely my (Álvaro's) fault.

Author: Tomas Vondra
Discussion: https://postgr.es/m/20170325211031.4xxoptigqxm2emn2@alap3.anarazel.de

2 files changed, 61 insertions, 35 deletions

diff --git a/src/backend/statistics/mvdistinct.c b/src/backend/statistics/mvdistinct.c
index 5df4e29eec3..6082ff01a9a 100644
--- a/src/backend/statistics/mvdistinct.c
+++ b/src/backend/statistics/mvdistinct.c
@@ -161,10 +161,10 @@ statext_ndistinct_serialize(MVNDistinct *ndistinct)
 	Assert(ndistinct->type == STATS_NDISTINCT_TYPE_BASIC);
 
 	/*
-	 * Base size is base struct size, plus one base struct for each items,
-	 * including number of items for each.
+	 * Base size is size of scalar fields in the struct, plus one base struct
+	 * for each item, including number of items for each.
 	 */
-	len = VARHDRSZ + offsetof(MVNDistinct, items) +
+	len = VARHDRSZ + SizeOfMVNDistinct +
 		ndistinct->nitems * (offsetof(MVNDistinctItem, attrs) + sizeof(int));
 
 	/* and also include space for the actual attribute numbers */
@@ -182,9 +182,13 @@ statext_ndistinct_serialize(MVNDistinct *ndistinct)
 
 	tmp = VARDATA(output);
 
-	/* Store the base struct values */
-	memcpy(tmp, ndistinct, offsetof(MVNDistinct, items));
-	tmp += offsetof(MVNDistinct, items);
+	/* Store the base struct values (magic, type, nitems) */
+	memcpy(tmp, &ndistinct->magic, sizeof(uint32));
+	tmp += sizeof(uint32);
+	memcpy(tmp, &ndistinct->type, sizeof(uint32));
+	tmp += sizeof(uint32);
+	memcpy(tmp, &ndistinct->nitems, sizeof(uint32));
+	tmp += sizeof(uint32);
 
 	/*
 	 * store number of attributes and attribute numbers for each ndistinct
@@ -224,49 +228,64 @@ MVNDistinct *
 statext_ndistinct_deserialize(bytea *data)
 {
 	int			i;
-	Size		expected_size;
+	Size		minimum_size;
+	MVNDistinct	ndist;
 	MVNDistinct *ndistinct;
 	char	   *tmp;
 
 	if (data == NULL)
 		return NULL;
 
-	if (VARSIZE_ANY_EXHDR(data) < offsetof(MVNDistinct, items))
+	/* we expect at least the basic fields of MVNDistinct struct */
+	if (VARSIZE_ANY_EXHDR(data) < SizeOfMVNDistinct)
 		elog(ERROR, "invalid MVNDistinct size %ld (expected at least %ld)",
-			 VARSIZE_ANY_EXHDR(data), offsetof(MVNDistinct, items));
-
-	/* read the MVNDistinct header */
-	ndistinct = (MVNDistinct *) palloc(sizeof(MVNDistinct));
+			 VARSIZE_ANY_EXHDR(data), SizeOfMVNDistinct);
 
 	/* initialize pointer to the data part (skip the varlena header) */
 	tmp = VARDATA_ANY(data);
 
-	/* get the header and perform basic sanity checks */
-	memcpy(ndistinct, tmp, offsetof(MVNDistinct, items));
-	tmp += offsetof(MVNDistinct, items);
-
-	if (ndistinct->magic != STATS_NDISTINCT_MAGIC)
-		elog(ERROR, "invalid ndistinct magic %d (expected %d)",
-			 ndistinct->magic, STATS_NDISTINCT_MAGIC);
-
-	if (ndistinct->type != STATS_NDISTINCT_TYPE_BASIC)
-		elog(ERROR, "invalid ndistinct type %d (expected %d)",
-			 ndistinct->type, STATS_NDISTINCT_TYPE_BASIC);
-
-	Assert(ndistinct->nitems > 0);
+	/* read the header fields and perform basic sanity checks */
+	memcpy(&ndist.magic, tmp, sizeof(uint32));
+	tmp += sizeof(uint32);
+	memcpy(&ndist.type, tmp, sizeof(uint32));
+	tmp += sizeof(uint32);
+	memcpy(&ndist.nitems, tmp, sizeof(uint32));
+	tmp += sizeof(uint32);
+
+	if (ndist.magic != STATS_NDISTINCT_MAGIC)
+		ereport(ERROR,
+				(errcode(ERRCODE_DATA_CORRUPTED),
+				 errmsg("invalid ndistinct magic %08x (expected %08x)",
+						ndist.magic, STATS_NDISTINCT_MAGIC)));
+	if (ndist.type != STATS_NDISTINCT_TYPE_BASIC)
+		ereport(ERROR,
+				(errcode(ERRCODE_DATA_CORRUPTED),
+				 errmsg("invalid ndistinct type %d (expected %d)",
+						ndist.type, STATS_NDISTINCT_TYPE_BASIC)));
+	if (ndist.nitems == 0)
+		ereport(ERROR,
+				(errcode(ERRCODE_DATA_CORRUPTED),
+				 errmsg("invalid zero-length item array in MVNDistinct")));
 
 	/* what minimum bytea size do we expect for those parameters */
-	expected_size = offsetof(MVNDistinct, items) +
-		ndistinct->nitems * (offsetof(MVNDistinctItem, attrs) +
-							 sizeof(AttrNumber) * 2);
+	minimum_size = (SizeOfMVNDistinct +
+					ndist.nitems * (SizeOfMVNDistinctItem +
+									sizeof(AttrNumber) * 2));
+	if (VARSIZE_ANY_EXHDR(data) < minimum_size)
+		ereport(ERROR,
+				(errcode(ERRCODE_DATA_CORRUPTED),
+				 errmsg("invalid MVNDistinct size %ld (expected at least %ld)",
+						VARSIZE_ANY_EXHDR(data), minimum_size)));
 
-	if (VARSIZE_ANY_EXHDR(data) < expected_size)
-		elog(ERROR, "invalid dependencies size %ld (expected at least %ld)",
-			 VARSIZE_ANY_EXHDR(data), expected_size);
-
-	/* allocate space for the ndistinct items */
-	ndistinct = repalloc(ndistinct, offsetof(MVNDistinct, items) +
-						 (ndistinct->nitems * sizeof(MVNDistinctItem)));
+	/*
+	 * Allocate space for the ndistinct items (no space for each item's attnos:
+	 * those live in bitmapsets allocated separately)
+	 */
+	ndistinct = palloc0(MAXALIGN(SizeOfMVNDistinct) +
+						(ndist.nitems * sizeof(MVNDistinctItem)));
+	ndistinct->magic = ndist.magic;
+	ndistinct->type = ndist.type;
+	ndistinct->nitems = ndist.nitems;
 
 	for (i = 0; i < ndistinct->nitems; i++)
 	{
diff --git a/src/include/statistics/statistics.h b/src/include/statistics/statistics.h
index a15e39e1a30..91645bff4b0 100644
--- a/src/include/statistics/statistics.h
+++ b/src/include/statistics/statistics.h
@@ -27,6 +27,9 @@ typedef struct MVNDistinctItem
 	double		ndistinct;		/* ndistinct value for this combination */
 	Bitmapset  *attrs;			/* attr numbers of items */
 } MVNDistinctItem;
+/* size of the struct, excluding attribute list */
+#define SizeOfMVNDistinctItem \
+	(offsetof(MVNDistinctItem, ndistinct) + sizeof(double))
 
 /* A MVNDistinct object, comprising all possible combinations of columns */
 typedef struct MVNDistinct
@@ -37,6 +40,10 @@ typedef struct MVNDistinct
 	MVNDistinctItem items[FLEXIBLE_ARRAY_MEMBER];
 } MVNDistinct;
 
+/* size of the struct excluding the items array */
+#define SizeOfMVNDistinct	(offsetof(MVNDistinct, nitems) + sizeof(uint32))
+
+
 extern MVNDistinct *statext_ndistinct_load(Oid mvoid);
 
 extern void BuildRelationExtStatistics(Relation onerel, double totalrows,