Avoid using potentially-under-aligned page buffers.

There's a project policy against using plain "char buf[BLCKSZ]" local
or static variables as page buffers; preferred style is to palloc or
malloc each buffer to ensure it is MAXALIGN'd.  However, that policy's
been ignored in an increasing number of places.  We've apparently got
away with it so far, probably because (a) relatively few people use
platforms on which misalignment causes core dumps and/or (b) the
variables chance to be sufficiently aligned anyway.  But this is not
something to rely on.  Moreover, even if we don't get a core dump,
we might be paying a lot of cycles for misaligned accesses.

To fix, invent new union types PGAlignedBlock and PGAlignedXLogBlock
that the compiler must allocate with sufficient alignment, and use
those in place of plain char arrays.

I used these types even for variables where there's no risk of a
misaligned access, since ensuring proper alignment should make
kernel data transfers faster.  I also changed some places where
we had been palloc'ing short-lived buffers, for coding style
uniformity and to save palloc/pfree overhead.

Since this seems to be a live portability hazard (despite the lack
of field reports), back-patch to all supported versions.

Patch by me; thanks to Michael Paquier for review.

Discussion: https://postgr.es/m/1535618100.1286.3.camel@credativ.de

6 files changed, 34 insertions, 51 deletions

diff --git a/src/bin/pg_basebackup/walmethods.c b/src/bin/pg_basebackup/walmethods.c
index 42e3f3023a0..68ef7fa6d77 100644
--- a/src/bin/pg_basebackup/walmethods.c
+++ b/src/bin/pg_basebackup/walmethods.c
@@ -116,18 +116,17 @@ dir_open_for_write(const char *pathname, const char *temp_suffix, size_t pad_to_
 	/* Do pre-padding on non-compressed files */
 	if (pad_to_size && dir_data->compression == 0)
 	{
-		char	   *zerobuf;
+		PGAlignedXLogBlock zerobuf;
 		int			bytes;
 
-		zerobuf = pg_malloc0(XLOG_BLCKSZ);
+		memset(zerobuf.data, 0, XLOG_BLCKSZ);
 		for (bytes = 0; bytes < pad_to_size; bytes += XLOG_BLCKSZ)
 		{
 			errno = 0;
-			if (write(fd, zerobuf, XLOG_BLCKSZ) != XLOG_BLCKSZ)
+			if (write(fd, zerobuf.data, XLOG_BLCKSZ) != XLOG_BLCKSZ)
 			{
 				int			save_errno = errno;
 
-				pg_free(zerobuf);
 				close(fd);
 
 				/*
@@ -137,7 +136,6 @@ dir_open_for_write(const char *pathname, const char *temp_suffix, size_t pad_to_
 				return NULL;
 			}
 		}
-		pg_free(zerobuf);
 
 		if (lseek(fd, 0, SEEK_SET) != 0)
 		{
@@ -513,24 +511,20 @@ tar_write(Walfile f, const void *buf, size_t count)
 static bool
 tar_write_padding_data(TarMethodFile *f, size_t bytes)
 {
-	char	   *zerobuf = pg_malloc0(XLOG_BLCKSZ);
+	PGAlignedXLogBlock zerobuf;
 	size_t		bytesleft = bytes;
 
+	memset(zerobuf.data, 0, XLOG_BLCKSZ);
 	while (bytesleft)
 	{
-		size_t		bytestowrite = bytesleft > XLOG_BLCKSZ ? XLOG_BLCKSZ : bytesleft;
-
-		ssize_t		r = tar_write(f, zerobuf, bytestowrite);
+		size_t		bytestowrite = Min(bytesleft, XLOG_BLCKSZ);
+		ssize_t		r = tar_write(f, zerobuf.data, bytestowrite);
 
 		if (r < 0)
-		{
-			pg_free(zerobuf);
 			return false;
-		}
 		bytesleft -= r;
 	}
 
-	pg_free(zerobuf);
 	return true;
 }
 
diff --git a/src/bin/pg_resetwal/pg_resetwal.c b/src/bin/pg_resetwal/pg_resetwal.c
index d63a3a27f60..6fb403a5a8a 100644
--- a/src/bin/pg_resetwal/pg_resetwal.c
+++ b/src/bin/pg_resetwal/pg_resetwal.c
@@ -1209,7 +1209,7 @@ KillExistingArchiveStatus(void)
 static void
 WriteEmptyXLOG(void)
 {
-	char	   *buffer;
+	PGAlignedXLogBlock buffer;
 	XLogPageHeader page;
 	XLogLongPageHeader longpage;
 	XLogRecord *record;
@@ -1219,12 +1219,10 @@ WriteEmptyXLOG(void)
 	int			nbytes;
 	char	   *recptr;
 
-	/* Use malloc() to ensure buffer is MAXALIGNED */
-	buffer = (char *) pg_malloc(XLOG_BLCKSZ);
-	page = (XLogPageHeader) buffer;
-	memset(buffer, 0, XLOG_BLCKSZ);
+	memset(buffer.data, 0, XLOG_BLCKSZ);
 
 	/* Set up the XLOG page header */
+	page = (XLogPageHeader) buffer.data;
 	page->xlp_magic = XLOG_PAGE_MAGIC;
 	page->xlp_info = XLP_LONG_HEADER;
 	page->xlp_tli = ControlFile.checkPointCopy.ThisTimeLineID;
@@ -1271,7 +1269,7 @@ WriteEmptyXLOG(void)
 	}
 
 	errno = 0;
-	if (write(fd, buffer, XLOG_BLCKSZ) != XLOG_BLCKSZ)
+	if (write(fd, buffer.data, XLOG_BLCKSZ) != XLOG_BLCKSZ)
 	{
 		/* if write didn't set errno, assume problem is no disk space */
 		if (errno == 0)
@@ -1282,11 +1280,11 @@ WriteEmptyXLOG(void)
 	}
 
 	/* Fill the rest of the file with zeroes */
-	memset(buffer, 0, XLOG_BLCKSZ);
+	memset(buffer.data, 0, XLOG_BLCKSZ);
 	for (nbytes = XLOG_BLCKSZ; nbytes < WalSegSz; nbytes += XLOG_BLCKSZ)
 	{
 		errno = 0;
-		if (write(fd, buffer, XLOG_BLCKSZ) != XLOG_BLCKSZ)
+		if (write(fd, buffer.data, XLOG_BLCKSZ) != XLOG_BLCKSZ)
 		{
 			if (errno == 0)
 				errno = ENOSPC;
diff --git a/src/bin/pg_rewind/copy_fetch.c b/src/bin/pg_rewind/copy_fetch.c
index 160a9128477..36d48899f74 100644
--- a/src/bin/pg_rewind/copy_fetch.c
+++ b/src/bin/pg_rewind/copy_fetch.c
@@ -156,7 +156,7 @@ recurse_dir(const char *datadir, const char *parentpath,
 static void
 rewind_copy_file_range(const char *path, off_t begin, off_t end, bool trunc)
 {
-	char		buf[BLCKSZ];
+	PGAlignedBlock buf;
 	char		srcpath[MAXPGPATH];
 	int			srcfd;
 
@@ -182,7 +182,7 @@ rewind_copy_file_range(const char *path, off_t begin, off_t end, bool trunc)
 		else
 			len = end - begin;
 
-		readlen = read(srcfd, buf, len);
+		readlen = read(srcfd, buf.data, len);
 
 		if (readlen < 0)
 			pg_fatal("could not read file \"%s\": %s\n",
@@ -190,7 +190,7 @@ rewind_copy_file_range(const char *path, off_t begin, off_t end, bool trunc)
 		else if (readlen == 0)
 			pg_fatal("unexpected EOF while reading file \"%s\"\n", srcpath);
 
-		write_target_range(buf, begin, readlen);
+		write_target_range(buf.data, begin, readlen);
 		begin += readlen;
 	}
 
diff --git a/src/bin/pg_upgrade/file.c b/src/bin/pg_upgrade/file.c
index f68211aa20a..c27cc93dc2e 100644
--- a/src/bin/pg_upgrade/file.c
+++ b/src/bin/pg_upgrade/file.c
@@ -132,8 +132,8 @@ rewriteVisibilityMap(const char *fromfile, const char *tofile,
 {
 	int			src_fd;
 	int			dst_fd;
-	char	   *buffer;
-	char	   *new_vmbuf;
+	PGAlignedBlock buffer;
+	PGAlignedBlock new_vmbuf;
 	ssize_t		totalBytesRead = 0;
 	ssize_t		src_filesize;
 	int			rewriteVmBytesPerPage;
@@ -160,13 +160,6 @@ rewriteVisibilityMap(const char *fromfile, const char *tofile,
 	src_filesize = statbuf.st_size;
 
 	/*
-	 * Malloc the work buffers, rather than making them local arrays, to
-	 * ensure adequate alignment.
-	 */
-	buffer = (char *) pg_malloc(BLCKSZ);
-	new_vmbuf = (char *) pg_malloc(BLCKSZ);
-
-	/*
 	 * Turn each visibility map page into 2 pages one by one. Each new page
 	 * has the same page header as the old one.  If the last section of the
 	 * last page is empty, we skip it, mostly to avoid turning one-page
@@ -181,7 +174,7 @@ rewriteVisibilityMap(const char *fromfile, const char *tofile,
 		PageHeaderData pageheader;
 		bool		old_lastblk;
 
-		if ((bytesRead = read(src_fd, buffer, BLCKSZ)) != BLCKSZ)
+		if ((bytesRead = read(src_fd, buffer.data, BLCKSZ)) != BLCKSZ)
 		{
 			if (bytesRead < 0)
 				pg_fatal("error while copying relation \"%s.%s\": could not read file \"%s\": %s\n",
@@ -195,7 +188,7 @@ rewriteVisibilityMap(const char *fromfile, const char *tofile,
 		old_lastblk = (totalBytesRead == src_filesize);
 
 		/* Save the page header data */
-		memcpy(&pageheader, buffer, SizeOfPageHeaderData);
+		memcpy(&pageheader, buffer.data, SizeOfPageHeaderData);
 
 		/*
 		 * These old_* variables point to old visibility map page. old_cur
@@ -203,8 +196,8 @@ rewriteVisibilityMap(const char *fromfile, const char *tofile,
 		 * old block.  old_break is the end+1 position on the old page for the
 		 * data that will be transferred to the current new page.
 		 */
-		old_cur = buffer + SizeOfPageHeaderData;
-		old_blkend = buffer + bytesRead;
+		old_cur = buffer.data + SizeOfPageHeaderData;
+		old_blkend = buffer.data + bytesRead;
 		old_break = old_cur + rewriteVmBytesPerPage;
 
 		while (old_break <= old_blkend)
@@ -214,12 +207,12 @@ rewriteVisibilityMap(const char *fromfile, const char *tofile,
 			bool		old_lastpart;
 
 			/* First, copy old page header to new page */
-			memcpy(new_vmbuf, &pageheader, SizeOfPageHeaderData);
+			memcpy(new_vmbuf.data, &pageheader, SizeOfPageHeaderData);
 
 			/* Rewriting the last part of the last old page? */
 			old_lastpart = old_lastblk && (old_break == old_blkend);
 
-			new_cur = new_vmbuf + SizeOfPageHeaderData;
+			new_cur = new_vmbuf.data + SizeOfPageHeaderData;
 
 			/* Process old page bytes one by one, and turn it into new page. */
 			while (old_cur < old_break)
@@ -253,11 +246,11 @@ rewriteVisibilityMap(const char *fromfile, const char *tofile,
 
 			/* Set new checksum for visibility map page, if enabled */
 			if (new_cluster.controldata.data_checksum_version != 0)
-				((PageHeader) new_vmbuf)->pd_checksum =
-					pg_checksum_page(new_vmbuf, new_blkno);
+				((PageHeader) new_vmbuf.data)->pd_checksum =
+					pg_checksum_page(new_vmbuf.data, new_blkno);
 
 			errno = 0;
-			if (write(dst_fd, new_vmbuf, BLCKSZ) != BLCKSZ)
+			if (write(dst_fd, new_vmbuf.data, BLCKSZ) != BLCKSZ)
 			{
 				/* if write didn't set errno, assume problem is no disk space */
 				if (errno == 0)
@@ -273,8 +266,6 @@ rewriteVisibilityMap(const char *fromfile, const char *tofile,
 	}
 
 	/* Clean up */
-	pg_free(buffer);
-	pg_free(new_vmbuf);
 	close(dst_fd);
 	close(src_fd);
 }
diff --git a/src/bin/pg_verify_checksums/pg_verify_checksums.c b/src/bin/pg_verify_checksums/pg_verify_checksums.c
index bf7feedf346..d46bac2cd65 100644
--- a/src/bin/pg_verify_checksums/pg_verify_checksums.c
+++ b/src/bin/pg_verify_checksums/pg_verify_checksums.c
@@ -75,8 +75,8 @@ skipfile(const char *fn)
 static void
 scan_file(const char *fn, BlockNumber segmentno)
 {
-	char		buf[BLCKSZ];
-	PageHeader	header = (PageHeader) buf;
+	PGAlignedBlock buf;
+	PageHeader	header = (PageHeader) buf.data;
 	int			f;
 	BlockNumber blockno;
 
@@ -93,7 +93,7 @@ scan_file(const char *fn, BlockNumber segmentno)
 	for (blockno = 0;; blockno++)
 	{
 		uint16		csum;
-		int			r = read(f, buf, BLCKSZ);
+		int			r = read(f, buf.data, BLCKSZ);
 
 		if (r == 0)
 			break;
@@ -109,7 +109,7 @@ scan_file(const char *fn, BlockNumber segmentno)
 		if (PageIsNew(header))
 			continue;
 
-		csum = pg_checksum_page(buf, blockno + segmentno * RELSEG_SIZE);
+		csum = pg_checksum_page(buf.data, blockno + segmentno * RELSEG_SIZE);
 		if (csum != header->pd_checksum)
 		{
 			if (ControlFile->data_checksum_version == PG_DATA_CHECKSUM_VERSION)
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index 2a557b37e5d..ad28333a1e9 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -209,13 +209,13 @@ search_directory(const char *directory, const char *fname)
 	/* set WalSegSz if file is successfully opened */
 	if (fd >= 0)
 	{
-		char		buf[XLOG_BLCKSZ];
+		PGAlignedXLogBlock buf;
 		int			r;
 
-		r = read(fd, buf, XLOG_BLCKSZ);
+		r = read(fd, buf.data, XLOG_BLCKSZ);
 		if (r == XLOG_BLCKSZ)
 		{
-			XLogLongPageHeader longhdr = (XLogLongPageHeader) buf;
+			XLogLongPageHeader longhdr = (XLogLongPageHeader) buf.data;
 
 			WalSegSz = longhdr->xlp_seg_size;