On 03/26/2017 08:47 PM, Andres Freund wrote:
On 2017-03-26 20:38:52 +0200, Tomas Vondra wrote:
...
Hmmm, so I have a theory about what's going on, but no matter what I do I
can't trigger these valgrind failures. What switches are you using to start
valgrind? I'm using this:
valgrind --leak-check=no --undef-value-errors=yes \
--expensive-definedness-checks=yes --track-origins=yes \
--read-var-info=yes --gen-suppressions=all \
--suppressions=src/tools/valgrind.supp --time-stamp=yes \
--log-file=$HOME/pg-valgrind/%p.log --trace-children=yes \
postgres --log_line_prefix="%m %p " --log_statement=all \
--shared_buffers=64MB -D /home/user/tmp/data-valgrind 2>&1
--quiet \
--error-exitcode=55 \
--suppressions=${PG_SRC_DIR}/src/tools/valgrind.supp \
--trace-children=yes --track-origins=yes --read-var-info=yes \
--num-callers=20 \
--leak-check=no \
--gen-suppressions=all \
the error-exitcode makes it a whole lot easier to see an error, because
it triggers a crash-restart cycle at backend exit ;)
OK, this did the trick. And just as I suspected, it seems to be due to
doing memcpy+offsetof when serializing the statistic into a bytea. The
attached patch fixes that for me. Can you test it on your build?
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
diff --git a/src/backend/statistics/mvdistinct.c b/src/backend/statistics/mvdistinct.c
index 5df4e29..acc2d7e 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 + (3 * sizeof(uint32)) +
ndistinct->nitems * (offsetof(MVNDistinctItem, attrs) + sizeof(int));
/* and also include space for the actual attribute numbers */
@@ -182,9 +182,15 @@ 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
@@ -231,9 +237,10 @@ statext_ndistinct_deserialize(bytea *data)
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) < (3 * sizeof(uint32)))
elog(ERROR, "invalid MVNDistinct size %ld (expected at least %ld)",
- VARSIZE_ANY_EXHDR(data), offsetof(MVNDistinct, items));
+ VARSIZE_ANY_EXHDR(data), 3 * sizeof(uint32));
/* read the MVNDistinct header */
ndistinct = (MVNDistinct *) palloc(sizeof(MVNDistinct));
@@ -241,18 +248,24 @@ statext_ndistinct_deserialize(bytea *data)
/* 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);
+ /* get the header fields and perform basic sanity checks */
+ memcpy(&ndistinct->magic, tmp, sizeof(uint32));
+ tmp += sizeof(uint32);
if (ndistinct->magic != STATS_NDISTINCT_MAGIC)
elog(ERROR, "invalid ndistinct magic %d (expected %d)",
ndistinct->magic, STATS_NDISTINCT_MAGIC);
+ memcpy(&ndistinct->type, tmp, sizeof(uint32));
+ tmp += sizeof(uint32);
+
if (ndistinct->type != STATS_NDISTINCT_TYPE_BASIC)
elog(ERROR, "invalid ndistinct type %d (expected %d)",
ndistinct->type, STATS_NDISTINCT_TYPE_BASIC);
+ memcpy(&ndistinct->nitems, tmp, sizeof(uint32));
+ tmp += sizeof(uint32);
+
Assert(ndistinct->nitems > 0);
/* what minimum bytea size do we expect for those parameters */
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
