From 8c62d9d16f014565d5f427506878812219a01604 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 31 Aug 2018 12:26:20 -0400 Subject: [PATCH] Make checksum_impl.h safe to compile with -fstrict-aliasing. In general, Postgres requires -fno-strict-aliasing with compilers that implement C99 strict aliasing rules. There's little hope of getting rid of that overall. But it seems like it would be a good idea if storage/checksum_impl.h in particular didn't depend on it, because that header is explicitly intended to be included by external programs. We don't have a lot of control over the compiler switches that an external program might use, as shown by Michael Banck's report of failure in a privately-modified version of pg_verify_checksums. Hence, switch to using a union in place of willy-nilly pointer casting inside this file. I think this makes the code a bit more readable anyway. checksum_impl.h hasn't changed since it was introduced in 9.3, so back-patch all the way. Discussion: https://postgr.es/m/1535618100.1286.3.camel@credativ.de --- src/include/storage/checksum_impl.h | 38 +++++++++++++++++------------ 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/src/include/storage/checksum_impl.h b/src/include/storage/checksum_impl.h index 64d76229ad..a49d27febb 100644 --- a/src/include/storage/checksum_impl.h +++ b/src/include/storage/checksum_impl.h @@ -107,6 +107,13 @@ /* prime multiplier of FNV-1a hash */ #define FNV_PRIME 16777619 +/* Use a union so that this code is valid under strict aliasing */ +typedef union +{ + PageHeaderData phdr; + uint32 data[BLCKSZ / (sizeof(uint32) * N_SUMS)][N_SUMS]; +} PGChecksummablePage; + /* * Base offsets to initialize each of the parallel FNV hashes into a * different initial state. @@ -132,28 +139,27 @@ do { \ } while (0) /* - * Block checksum algorithm. The data argument must be aligned on a 4-byte - * boundary. + * Block checksum algorithm. The page must be adequately aligned + * (at least on 4-byte boundary). */ static uint32 -pg_checksum_block(char *data, uint32 size) +pg_checksum_block(const PGChecksummablePage *page) { uint32 sums[N_SUMS]; - uint32 (*dataArr)[N_SUMS] = (uint32 (*)[N_SUMS]) data; uint32 result = 0; uint32 i, j; /* ensure that the size is compatible with the algorithm */ - Assert((size % (sizeof(uint32) * N_SUMS)) == 0); + Assert(sizeof(PGChecksummablePage) == BLCKSZ); /* initialize partial checksums to their corresponding offsets */ memcpy(sums, checksumBaseOffsets, sizeof(checksumBaseOffsets)); /* main checksum calculation */ - for (i = 0; i < size / sizeof(uint32) / N_SUMS; i++) + for (i = 0; i < (uint32) (BLCKSZ / (sizeof(uint32) * N_SUMS)); i++) for (j = 0; j < N_SUMS; j++) - CHECKSUM_COMP(sums[j], dataArr[i][j]); + CHECKSUM_COMP(sums[j], page->data[i][j]); /* finally add in two rounds of zeroes for additional mixing */ for (i = 0; i < 2; i++) @@ -168,8 +174,10 @@ pg_checksum_block(char *data, uint32 size) } /* - * Compute the checksum for a Postgres page. The page must be aligned on a - * 4-byte boundary. + * Compute the checksum for a Postgres page. + * + * The page must be adequately aligned (at least on a 4-byte boundary). + * Beware also that the checksum field of the page is transiently zeroed. * * The checksum includes the block number (to detect the case where a page is * somehow moved to a different location), the page header (excluding the @@ -178,12 +186,12 @@ pg_checksum_block(char *data, uint32 size) uint16 pg_checksum_page(char *page, BlockNumber blkno) { - PageHeader phdr = (PageHeader) page; + PGChecksummablePage *cpage = (PGChecksummablePage *) page; uint16 save_checksum; uint32 checksum; /* We only calculate the checksum for properly-initialized pages */ - Assert(!PageIsNew(page)); + Assert(!PageIsNew(&cpage->phdr)); /* * Save pd_checksum and temporarily set it to zero, so that the checksum @@ -191,10 +199,10 @@ pg_checksum_page(char *page, BlockNumber blkno) * Restore it after, because actually updating the checksum is NOT part of * the API of this function. */ - save_checksum = phdr->pd_checksum; - phdr->pd_checksum = 0; - checksum = pg_checksum_block(page, BLCKSZ); - phdr->pd_checksum = save_checksum; + save_checksum = cpage->phdr.pd_checksum; + cpage->phdr.pd_checksum = 0; + checksum = pg_checksum_block(cpage); + cpage->phdr.pd_checksum = save_checksum; /* Mix in the block number to detect transposed pages */ checksum ^= blkno;