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 ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
