From d4b538ea367de43b2f2b939621272682417cd290 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Thu, 21 May 2015 17:21:46 -0400
Subject: [PATCH] Improve packing/alignment annotation for ItemPointerData.
We want this struct to be exactly a series of 3 int16 words, no more
and no less. Historically, at least, some ARM compilers preferred to
pad it to 8 bytes unless coerced. Our old way of doing that was just
to use __attribute__((packed)), but as pointed out by Piotr Stefaniak,
that does too much: it also licenses the compiler to give the struct
only byte-alignment. We don't want that because it adds access overhead,
possibly quite significant overhead. According to the GCC manual, what
we want requires also specifying __attribute__((align(2))). It's not
entirely clear if all the relevant compilers accept this pragma as well,
but we can hope the buildfarm will tell us if not. We can also add a
static assertion that should fire if the compiler padded the struct.
Since the combination of these pragmas should define exactly what we
want on any compiler that accepts them, let's try using them wherever
we think they exist, not only for __arm__. (This is likely to expose
that the conditional definitions in c.h are inadequate, but finding
that out would be a good thing.)
The immediate motivation for this is that the current definition of
ExecRowMark allows its curCtid field to be misaligned. It is not clear
whether there are any other uses of ItemPointerData with a similar hazard.
We could change the definition of ExecRowMark if this doesn't work, but
it would be far better to have a future-proof fix.
Piotr Stefaniak, some further hacking by me
---
src/backend/storage/page/itemptr.c | 7 +++++++
src/include/storage/itemptr.h | 7 ++++---
2 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/src/backend/storage/page/itemptr.c b/src/backend/storage/page/itemptr.c
index c9918289691..244d8fba2f6 100644
--- a/src/backend/storage/page/itemptr.c
+++ b/src/backend/storage/page/itemptr.c
@@ -28,6 +28,13 @@
bool
ItemPointerEquals(ItemPointer pointer1, ItemPointer pointer2)
{
+ /*
+ * We really want ItemPointerData to be exactly 6 bytes. This is rather a
+ * random place to check, but there is no better place.
+ */
+ StaticAssertStmt(sizeof(ItemPointerData) == 3 * sizeof(uint16),
+ "ItemPointerData struct is improperly padded");
+
if (ItemPointerGetBlockNumber(pointer1) ==
ItemPointerGetBlockNumber(pointer2) &&
ItemPointerGetOffsetNumber(pointer1) ==
diff --git a/src/include/storage/itemptr.h b/src/include/storage/itemptr.h
index eb06c794d2b..60e76e9ffa0 100644
--- a/src/include/storage/itemptr.h
+++ b/src/include/storage/itemptr.h
@@ -39,9 +39,10 @@ typedef struct ItemPointerData
BlockIdData ip_blkid;
OffsetNumber ip_posid;
}
-
-#ifdef __arm__
-pg_attribute_packed() /* Appropriate whack upside the head for ARM */
+/* If compiler understands packed and aligned pragmas, use those */
+#if defined(pg_attribute_packed) && defined(pg_attribute_aligned)
+pg_attribute_packed()
+pg_attribute_aligned(2)
#endif
ItemPointerData;
--
GitLab