aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTomas Vondra <tomas.vondra@postgresql.org>2021-03-26 16:33:19 +0100
committerTomas Vondra <tomas.vondra@postgresql.org>2021-03-26 16:48:36 +0100
commit73b96bad4af8fd113a36e4633dd3312001c115dc (patch)
treee9f90f1d8ede39a5af974209ddb30c5091bac9c5
parentab596105b55f1d7fbd5a66b66f65227d210b047d (diff)
downloadpostgresql-73b96bad4af8fd113a36e4633dd3312001c115dc.tar.gz
postgresql-73b96bad4af8fd113a36e4633dd3312001c115dc.zip
Fix alignment in BRIN minmax-multi deserialization
The deserialization failed to ensure correct alignment, as it assumed it can simply point into the serialized value. The serialization however ignores alignment and copies just the significant bytes in order to make the result as small as possible. This caused failures on systems that are sensitive to mialigned addresses, like sparc, or with address sanitizer enabled. Fixed by copying the serialized data to ensure proper alignment. While at it, fix an issue with serialization on big endian machines, using the same store_att_byval/fetch_att trick as extended statistics. Discussion: https://postgr.es/0c8c3304-d3dd-5e29-d5ac-b50589a23c8c%40enterprisedb.com
-rw-r--r--src/backend/access/brin/brin_minmax_multi.c87
1 files changed, 74 insertions, 13 deletions
diff --git a/src/backend/access/brin/brin_minmax_multi.c b/src/backend/access/brin/brin_minmax_multi.c
index 1ca86b7fb7c..70109960e83 100644
--- a/src/backend/access/brin/brin_minmax_multi.c
+++ b/src/backend/access/brin/brin_minmax_multi.c
@@ -665,7 +665,20 @@ range_serialize(Ranges *range)
{
if (typbyval) /* simple by-value data types */
{
- memcpy(ptr, &range->values[i], typlen);
+ Datum tmp;
+
+ /*
+ * For values passed by value, we need to copy just the
+ * significant bytes - we can't use memcpy directly, as that
+ * assumes little endian behavior. store_att_byval does
+ * almost what we need, but it requires properly aligned
+ * buffer - the output buffer does not guarantee that. So we
+ * simply use a local Datum variable (which guarantees proper
+ * alignment), and then copy the value from it.
+ */
+ store_att_byval(&tmp, range->values[i], typlen);
+
+ memcpy(ptr, &tmp, typlen);
ptr += typlen;
}
else if (typlen > 0) /* fixed-length by-ref types */
@@ -710,9 +723,11 @@ range_deserialize(int maxvalues, SerializedRanges *serialized)
{
int i,
nvalues;
- char *ptr;
+ char *ptr,
+ *dataptr;
bool typbyval;
int typlen;
+ Size datalen;
Ranges *range;
@@ -740,9 +755,42 @@ range_deserialize(int maxvalues, SerializedRanges *serialized)
typlen = get_typlen(serialized->typid);
/*
- * And now deconstruct the values into Datum array. We don't need to copy
- * the values and will instead just point the values to the serialized
- * varlena value (assuming it will be kept around).
+ * And now deconstruct the values into Datum array. We have to copy the
+ * data because the serialized representation ignores alignment, and we
+ * don't want to rely it will be kept around anyway.
+ */
+ ptr = serialized->data;
+
+ /*
+ * We don't want to allocate many pieces, so we just allocate everything
+ * in one chunk. How much space will we need?
+ *
+ * XXX We don't need to copy simple by-value data types.
+ */
+ datalen = 0;
+ dataptr = NULL;
+ for (i = 0; (i < nvalues) && (!typbyval); i++)
+ {
+ if (typlen > 0) /* fixed-length by-ref types */
+ datalen += MAXALIGN(typlen);
+ else if (typlen == -1) /* varlena */
+ {
+ datalen += MAXALIGN(VARSIZE_ANY(DatumGetPointer(ptr)));
+ ptr += VARSIZE_ANY(DatumGetPointer(ptr));
+ }
+ else if (typlen == -2) /* cstring */
+ {
+ datalen += MAXALIGN(strlen(DatumGetPointer(ptr)) + 1);
+ ptr += strlen(DatumGetPointer(ptr)) + 1;
+ }
+ }
+
+ if (datalen > 0)
+ dataptr = palloc(datalen);
+
+ /*
+ * Restore the source pointer (might have been modified when calculating
+ * the space we need to allocate).
*/
ptr = serialized->data;
@@ -750,24 +798,38 @@ range_deserialize(int maxvalues, SerializedRanges *serialized)
{
if (typbyval) /* simple by-value data types */
{
- memcpy(&range->values[i], ptr, typlen);
+ Datum v = 0;
+
+ memcpy(&v, ptr, typlen);
+
+ range->values[i] = fetch_att(&v, true, typlen);
ptr += typlen;
}
else if (typlen > 0) /* fixed-length by-ref types */
{
- /* no copy, just set the value to the pointer */
- range->values[i] = PointerGetDatum(ptr);
+ range->values[i] = PointerGetDatum(dataptr);
+
+ memcpy(dataptr, ptr, typlen);
+ dataptr += MAXALIGN(typlen);
+
ptr += typlen;
}
else if (typlen == -1) /* varlena */
{
- range->values[i] = PointerGetDatum(ptr);
- ptr += VARSIZE_ANY(DatumGetPointer(range->values[i]));
+ range->values[i] = PointerGetDatum(dataptr);
+
+ memcpy(dataptr, ptr, VARSIZE_ANY(ptr));
+ dataptr += MAXALIGN(VARSIZE_ANY(ptr));
+ ptr += VARSIZE_ANY(ptr);
}
else if (typlen == -2) /* cstring */
{
- range->values[i] = PointerGetDatum(ptr);
- ptr += strlen(DatumGetPointer(range->values[i])) + 1;
+ Size slen = strlen(ptr) + 1;
+ range->values[i] = PointerGetDatum(dataptr);
+
+ memcpy(dataptr, ptr, slen);
+ dataptr += MAXALIGN(slen);
+ ptr += (slen);
}
/* make sure we haven't overflown the buffer end */
@@ -2823,7 +2885,6 @@ minmax_multi_get_strategy_procinfo(BrinDesc *bdesc, uint16 attno, Oid subtype,
ObjectIdGetDatum(attr->atttypid),
ObjectIdGetDatum(subtype),
Int16GetDatum(strategynum));
-
if (!HeapTupleIsValid(tuple))
elog(ERROR, "missing operator %d(%u,%u) in opfamily %u",
strategynum, attr->atttypid, subtype, opfamily);