Dear 7b4ac19 authors, Field ps_snapshot_data usually receives four-byte alignment within ParallelIndexScanDescData, but it contains the eight-byte whenTaken field. The select_parallel test dies with SIGBUS on "Oracle Solaris 10 1/13 s10s_u11wos_24a SPARC", building with gcc 4.9.2. Some credible fixes:
1. Move the SerializedSnapshotData declaration from snapmgr.c to snapmgr.h and declare the ps_snapshot_data field to be of type SerializedSnapshotData. Probably also add a field "TransactionId xids[FLEXIBLE_ARRAY_MEMBER]" to SerializedSnapshotData, to assert the variable-length nature. 2. Change "char ps_snapshot_data[FLEXIBLE_ARRAY_MEMBER]" to "int64 ...". I have attached this in SerializedSnapshot-int64-v1.patch. 3. Change no declarations, and make snapmgr.c memcpy() the SerializedSnapshotData through a local buffer. I have attached this as SerializedSnapshot-memcpy-v1.patch. I like (2) well enough, but I don't see that technique used elsewhere in the tree. (1) is more typical of PostgreSQL, though I personally like it when structs can stay private to a file. (3) is also well-attested, particularly in xlog replay code. I am leaning toward (2). Other opinions? Field phs_snapshot_data happens to receive eight-byte alignment within ParallelHeapScanDescData; otherwise, such scans would fail the same way.
diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index f232c84..a46a73e 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -2013,7 +2013,7 @@ EstimateSnapshotSpace(Snapshot snap) * memory location at start_address. */ void -SerializeSnapshot(Snapshot snapshot, char *start_address) +SerializeSnapshot(Snapshot snapshot, int64 *start_address) { SerializedSnapshotData *serialized_snapshot; @@ -2069,7 +2069,7 @@ SerializeSnapshot(Snapshot snapshot, char *start_address) * to 0. The returned snapshot has the copied flag set. */ Snapshot -RestoreSnapshot(char *start_address) +RestoreSnapshot(int64 *start_address) { SerializedSnapshotData *serialized_snapshot; Size size; diff --git a/src/include/access/relscan.h b/src/include/access/relscan.h index ce3ca8d..3a5c5c9 100644 --- a/src/include/access/relscan.h +++ b/src/include/access/relscan.h @@ -38,7 +38,7 @@ typedef struct ParallelHeapScanDescData slock_t phs_mutex; /* mutual exclusion for block number fields */ BlockNumber phs_startblock; /* starting block number */ BlockNumber phs_cblock; /* current block number */ - char phs_snapshot_data[FLEXIBLE_ARRAY_MEMBER]; + int64 phs_snapshot_data[FLEXIBLE_ARRAY_MEMBER]; } ParallelHeapScanDescData; typedef struct HeapScanDescData @@ -138,7 +138,7 @@ typedef struct ParallelIndexScanDescData Oid ps_relid; Oid ps_indexid; Size ps_offset; /* Offset in bytes of am specific structure */ - char ps_snapshot_data[FLEXIBLE_ARRAY_MEMBER]; + int64 ps_snapshot_data[FLEXIBLE_ARRAY_MEMBER]; } ParallelIndexScanDescData; /* Struct for heap-or-index scans of system tables */ diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h index ab95366..4b5feb7 100644 --- a/src/include/utils/snapmgr.h +++ b/src/include/utils/snapmgr.h @@ -106,8 +106,8 @@ extern void TeardownHistoricSnapshot(bool is_error); extern bool HistoricSnapshotActive(void); extern Size EstimateSnapshotSpace(Snapshot snapshot); -extern void SerializeSnapshot(Snapshot snapshot, char *start_address); -extern Snapshot RestoreSnapshot(char *start_address); +extern void SerializeSnapshot(Snapshot snapshot, int64 *start_address); +extern Snapshot RestoreSnapshot(int64 *start_address); extern void RestoreTransactionSnapshot(Snapshot snapshot, void *master_pgproc); #endif /* SNAPMGR_H */
diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c index f232c84..e6d7a19 100644 --- a/src/backend/utils/time/snapmgr.c +++ b/src/backend/utils/time/snapmgr.c @@ -2015,34 +2015,35 @@ EstimateSnapshotSpace(Snapshot snap) void SerializeSnapshot(Snapshot snapshot, char *start_address) { - SerializedSnapshotData *serialized_snapshot; + SerializedSnapshotData serialized_snapshot; Assert(snapshot->subxcnt >= 0); - serialized_snapshot = (SerializedSnapshotData *) start_address; - /* Copy all required fields */ - serialized_snapshot->xmin = snapshot->xmin; - serialized_snapshot->xmax = snapshot->xmax; - serialized_snapshot->xcnt = snapshot->xcnt; - serialized_snapshot->subxcnt = snapshot->subxcnt; - serialized_snapshot->suboverflowed = snapshot->suboverflowed; - serialized_snapshot->takenDuringRecovery = snapshot->takenDuringRecovery; - serialized_snapshot->curcid = snapshot->curcid; - serialized_snapshot->whenTaken = snapshot->whenTaken; - serialized_snapshot->lsn = snapshot->lsn; + serialized_snapshot.xmin = snapshot->xmin; + serialized_snapshot.xmax = snapshot->xmax; + serialized_snapshot.xcnt = snapshot->xcnt; + serialized_snapshot.subxcnt = snapshot->subxcnt; + serialized_snapshot.suboverflowed = snapshot->suboverflowed; + serialized_snapshot.takenDuringRecovery = snapshot->takenDuringRecovery; + serialized_snapshot.curcid = snapshot->curcid; + serialized_snapshot.whenTaken = snapshot->whenTaken; + serialized_snapshot.lsn = snapshot->lsn; /* * Ignore the SubXID array if it has overflowed, unless the snapshot was * taken during recovey - in that case, top-level XIDs are in subxip as * well, and we mustn't lose them. */ - if (serialized_snapshot->suboverflowed && !snapshot->takenDuringRecovery) - serialized_snapshot->subxcnt = 0; + if (serialized_snapshot.suboverflowed && !snapshot->takenDuringRecovery) + serialized_snapshot.subxcnt = 0; + + /* Copy struct to possibly-unaligned buffer */ + memcpy(start_address, &serialized_snapshot, sizeof(SerializedSnapshotData)); /* Copy XID array */ if (snapshot->xcnt > 0) - memcpy((TransactionId *) (serialized_snapshot + 1), + memcpy((TransactionId *) (start_address + sizeof(SerializedSnapshotData)), snapshot->xip, snapshot->xcnt * sizeof(TransactionId)); /* @@ -2051,12 +2052,12 @@ SerializeSnapshot(Snapshot snapshot, char *start_address) * snapshot taken during recovery; all the top-level XIDs are in subxip as * well in that case, so we mustn't lose them. */ - if (serialized_snapshot->subxcnt > 0) + if (serialized_snapshot.subxcnt > 0) { Size subxipoff = sizeof(SerializedSnapshotData) + snapshot->xcnt * sizeof(TransactionId); - memcpy((TransactionId *) ((char *) serialized_snapshot + subxipoff), + memcpy((TransactionId *) (start_address + subxipoff), snapshot->subxip, snapshot->subxcnt * sizeof(TransactionId)); } } @@ -2071,50 +2072,51 @@ SerializeSnapshot(Snapshot snapshot, char *start_address) Snapshot RestoreSnapshot(char *start_address) { - SerializedSnapshotData *serialized_snapshot; + SerializedSnapshotData serialized_snapshot; Size size; Snapshot snapshot; TransactionId *serialized_xids; - serialized_snapshot = (SerializedSnapshotData *) start_address; + memcpy(&serialized_snapshot, start_address, + sizeof(SerializedSnapshotData)); serialized_xids = (TransactionId *) (start_address + sizeof(SerializedSnapshotData)); /* We allocate any XID arrays needed in the same palloc block. */ size = sizeof(SnapshotData) - + serialized_snapshot->xcnt * sizeof(TransactionId) - + serialized_snapshot->subxcnt * sizeof(TransactionId); + + serialized_snapshot.xcnt * sizeof(TransactionId) + + serialized_snapshot.subxcnt * sizeof(TransactionId); /* Copy all required fields */ snapshot = (Snapshot) MemoryContextAlloc(TopTransactionContext, size); snapshot->satisfies = HeapTupleSatisfiesMVCC; - snapshot->xmin = serialized_snapshot->xmin; - snapshot->xmax = serialized_snapshot->xmax; + snapshot->xmin = serialized_snapshot.xmin; + snapshot->xmax = serialized_snapshot.xmax; snapshot->xip = NULL; - snapshot->xcnt = serialized_snapshot->xcnt; + snapshot->xcnt = serialized_snapshot.xcnt; snapshot->subxip = NULL; - snapshot->subxcnt = serialized_snapshot->subxcnt; - snapshot->suboverflowed = serialized_snapshot->suboverflowed; - snapshot->takenDuringRecovery = serialized_snapshot->takenDuringRecovery; - snapshot->curcid = serialized_snapshot->curcid; - snapshot->whenTaken = serialized_snapshot->whenTaken; - snapshot->lsn = serialized_snapshot->lsn; + snapshot->subxcnt = serialized_snapshot.subxcnt; + snapshot->suboverflowed = serialized_snapshot.suboverflowed; + snapshot->takenDuringRecovery = serialized_snapshot.takenDuringRecovery; + snapshot->curcid = serialized_snapshot.curcid; + snapshot->whenTaken = serialized_snapshot.whenTaken; + snapshot->lsn = serialized_snapshot.lsn; /* Copy XIDs, if present. */ - if (serialized_snapshot->xcnt > 0) + if (serialized_snapshot.xcnt > 0) { snapshot->xip = (TransactionId *) (snapshot + 1); memcpy(snapshot->xip, serialized_xids, - serialized_snapshot->xcnt * sizeof(TransactionId)); + serialized_snapshot.xcnt * sizeof(TransactionId)); } /* Copy SubXIDs, if present. */ - if (serialized_snapshot->subxcnt > 0) + if (serialized_snapshot.subxcnt > 0) { snapshot->subxip = ((TransactionId *) (snapshot + 1)) + - serialized_snapshot->xcnt; - memcpy(snapshot->subxip, serialized_xids + serialized_snapshot->xcnt, - serialized_snapshot->subxcnt * sizeof(TransactionId)); + serialized_snapshot.xcnt; + memcpy(snapshot->subxip, serialized_xids + serialized_snapshot.xcnt, + serialized_snapshot.subxcnt * sizeof(TransactionId)); } /* Set the copied flag so that the caller will set refcounts correctly. */
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers