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

Commit 73b96ba

Browse files
committed
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
1 parent ab59610 commit 73b96ba

File tree

1 file changed

+74
-13
lines changed

1 file changed

+74
-13
lines changed

src/backend/access/brin/brin_minmax_multi.c

+74-13
Original file line numberDiff line numberDiff line change
@@ -665,7 +665,20 @@ range_serialize(Ranges *range)
665665
{
666666
if (typbyval) /* simple by-value data types */
667667
{
668-
memcpy(ptr, &range->values[i], typlen);
668+
Datum tmp;
669+
670+
/*
671+
* For values passed by value, we need to copy just the
672+
* significant bytes - we can't use memcpy directly, as that
673+
* assumes little endian behavior. store_att_byval does
674+
* almost what we need, but it requires properly aligned
675+
* buffer - the output buffer does not guarantee that. So we
676+
* simply use a local Datum variable (which guarantees proper
677+
* alignment), and then copy the value from it.
678+
*/
679+
store_att_byval(&tmp, range->values[i], typlen);
680+
681+
memcpy(ptr, &tmp, typlen);
669682
ptr += typlen;
670683
}
671684
else if (typlen > 0) /* fixed-length by-ref types */
@@ -710,9 +723,11 @@ range_deserialize(int maxvalues, SerializedRanges *serialized)
710723
{
711724
int i,
712725
nvalues;
713-
char *ptr;
726+
char *ptr,
727+
*dataptr;
714728
bool typbyval;
715729
int typlen;
730+
Size datalen;
716731

717732
Ranges *range;
718733

@@ -740,34 +755,81 @@ range_deserialize(int maxvalues, SerializedRanges *serialized)
740755
typlen = get_typlen(serialized->typid);
741756

742757
/*
743-
* And now deconstruct the values into Datum array. We don't need to copy
744-
* the values and will instead just point the values to the serialized
745-
* varlena value (assuming it will be kept around).
758+
* And now deconstruct the values into Datum array. We have to copy the
759+
* data because the serialized representation ignores alignment, and we
760+
* don't want to rely it will be kept around anyway.
761+
*/
762+
ptr = serialized->data;
763+
764+
/*
765+
* We don't want to allocate many pieces, so we just allocate everything
766+
* in one chunk. How much space will we need?
767+
*
768+
* XXX We don't need to copy simple by-value data types.
769+
*/
770+
datalen = 0;
771+
dataptr = NULL;
772+
for (i = 0; (i < nvalues) && (!typbyval); i++)
773+
{
774+
if (typlen > 0) /* fixed-length by-ref types */
775+
datalen += MAXALIGN(typlen);
776+
else if (typlen == -1) /* varlena */
777+
{
778+
datalen += MAXALIGN(VARSIZE_ANY(DatumGetPointer(ptr)));
779+
ptr += VARSIZE_ANY(DatumGetPointer(ptr));
780+
}
781+
else if (typlen == -2) /* cstring */
782+
{
783+
datalen += MAXALIGN(strlen(DatumGetPointer(ptr)) + 1);
784+
ptr += strlen(DatumGetPointer(ptr)) + 1;
785+
}
786+
}
787+
788+
if (datalen > 0)
789+
dataptr = palloc(datalen);
790+
791+
/*
792+
* Restore the source pointer (might have been modified when calculating
793+
* the space we need to allocate).
746794
*/
747795
ptr = serialized->data;
748796

749797
for (i = 0; i < nvalues; i++)
750798
{
751799
if (typbyval) /* simple by-value data types */
752800
{
753-
memcpy(&range->values[i], ptr, typlen);
801+
Datum v = 0;
802+
803+
memcpy(&v, ptr, typlen);
804+
805+
range->values[i] = fetch_att(&v, true, typlen);
754806
ptr += typlen;
755807
}
756808
else if (typlen > 0) /* fixed-length by-ref types */
757809
{
758-
/* no copy, just set the value to the pointer */
759-
range->values[i] = PointerGetDatum(ptr);
810+
range->values[i] = PointerGetDatum(dataptr);
811+
812+
memcpy(dataptr, ptr, typlen);
813+
dataptr += MAXALIGN(typlen);
814+
760815
ptr += typlen;
761816
}
762817
else if (typlen == -1) /* varlena */
763818
{
764-
range->values[i] = PointerGetDatum(ptr);
765-
ptr += VARSIZE_ANY(DatumGetPointer(range->values[i]));
819+
range->values[i] = PointerGetDatum(dataptr);
820+
821+
memcpy(dataptr, ptr, VARSIZE_ANY(ptr));
822+
dataptr += MAXALIGN(VARSIZE_ANY(ptr));
823+
ptr += VARSIZE_ANY(ptr);
766824
}
767825
else if (typlen == -2) /* cstring */
768826
{
769-
range->values[i] = PointerGetDatum(ptr);
770-
ptr += strlen(DatumGetPointer(range->values[i])) + 1;
827+
Size slen = strlen(ptr) + 1;
828+
range->values[i] = PointerGetDatum(dataptr);
829+
830+
memcpy(dataptr, ptr, slen);
831+
dataptr += MAXALIGN(slen);
832+
ptr += (slen);
771833
}
772834

773835
/* make sure we haven't overflown the buffer end */
@@ -2823,7 +2885,6 @@ minmax_multi_get_strategy_procinfo(BrinDesc *bdesc, uint16 attno, Oid subtype,
28232885
ObjectIdGetDatum(attr->atttypid),
28242886
ObjectIdGetDatum(subtype),
28252887
Int16GetDatum(strategynum));
2826-
28272888
if (!HeapTupleIsValid(tuple))
28282889
elog(ERROR, "missing operator %d(%u,%u) in opfamily %u",
28292890
strategynum, attr->atttypid, subtype, opfamily);

0 commit comments

Comments
 (0)