v7-0002-Adding-checks-of-invalid-combinations-of-hint-bit.patch

From a611cd647372231c5fb3c54c8a2fa18d84ef7d2a Mon Sep 17 00:00:00 2001
From: Mark Dilger <mark.dilger@enterprisedb.com>
Date: Fri, 12 Jun 2020 13:26:14 -0700
Subject: [PATCH v7 2/2] Adding checks of invalid combinations of hint bits

Per code review by Dilip Kumar, adding a corruption check for
HEAP_XMAX_LOCK_ONLY and HEAP_KEYS_UPDATED being set simultaneously
in an on-disk tuple.

While doing that, I noticed that the heap/README.tuplock file
documents that HEAP_XMAX_COMMITTED and HEAP_XMAX_IS_MULTI are never
set simultaneously, so adding a corruption check for that, too.

Since some clever hacker in the future may notice that these
combinations never occur on disk and use them to mean some new
thing, adding an Assert for each of them in RelationPutHeapTuple
with comments about amcheck's expectations.
---
 contrib/amcheck/verify_heapam.c           | 12 ++++++++++
 contrib/pg_amcheck/t/004_verify_heapam.pl | 27 ++++++++++++++++++++---
 src/backend/access/heap/hio.c             | 11 +++++++++
 3 files changed, 47 insertions(+), 3 deletions(-)

diff --git a/contrib/amcheck/verify_heapam.c b/contrib/amcheck/verify_heapam.c
index 1bddff7fc6..f5e68906b2 100644
--- a/contrib/amcheck/verify_heapam.c
+++ b/contrib/amcheck/verify_heapam.c
@@ -954,6 +954,18 @@ check_tuple(HeapCheckContext * ctx)
 							  ctx->tuphdr->t_hoff));
 		fatal = true;
 	}
+	if ((ctx->tuphdr->t_infomask & HEAP_XMAX_LOCK_ONLY) &&
+		(ctx->tuphdr->t_infomask2 & HEAP_KEYS_UPDATED))
+	{
+		confess(ctx,
+			psprintf("HEAP_XMAX_LOCK_ONLY and HEAP_KEYS_UPDATED both set"));
+	}
+	if ((ctx->tuphdr->t_infomask & HEAP_XMAX_COMMITTED) &&
+		(ctx->tuphdr->t_infomask & HEAP_XMAX_IS_MULTI))
+	{
+		confess(ctx,
+			psprintf("HEAP_XMAX_COMMITTED and HEAP_XMAX_IS_MULTI both set"));
+	}
 
 	/*
 	 * If the tuple has nulls, check that the implied length of the variable
diff --git a/contrib/pg_amcheck/t/004_verify_heapam.pl b/contrib/pg_amcheck/t/004_verify_heapam.pl
index a96b763886..b2c1f36928 100644
--- a/contrib/pg_amcheck/t/004_verify_heapam.pl
+++ b/contrib/pg_amcheck/t/004_verify_heapam.pl
@@ -4,7 +4,7 @@ use warnings;
 use PostgresNode;
 use TestLib;
 
-use Test::More tests => 36;
+use Test::More tests => 42;
 
 # This regression test demonstrates that the verify_heapam() function supplied
 # with the amcheck contrib module and depended upon by this pg_amcheck contrib
@@ -215,7 +215,7 @@ $node->safe_psql(
 my $rel = $node->safe_psql('postgres', qq(SELECT pg_relation_filepath('public.test')));
 my $relpath = "$pgdata/$rel";
 
-use constant ROWCOUNT => 12;
+use constant ROWCOUNT => 14;
 $node->safe_psql('postgres', qq(
 	INSERT INTO public.test (a, b, c)
 		VALUES (
@@ -250,10 +250,14 @@ $node->stop;
 
 # Some #define constants from access/htup_details.h for use while corrupting.
 use constant HEAP_HASNULL            => 0x0001;
+use constant HEAP_XMAX_LOCK_ONLY     => 0x0080;
 use constant HEAP_XMIN_COMMITTED     => 0x0100;
 use constant HEAP_XMIN_INVALID       => 0x0200;
+use constant HEAP_XMAX_COMMITTED     => 0x0400;
 use constant HEAP_XMAX_INVALID       => 0x0800;
 use constant HEAP_NATTS_MASK         => 0x07FF;
+use constant HEAP_XMAX_IS_MULTI      => 0x1000;
+use constant HEAP_KEYS_UPDATED       => 0x2000;
 
 # Corrupt the tuples, one type of corruption per tuple.  Some types of
 # corruption cause verify_heapam to skip to the next tuple without
@@ -350,6 +354,18 @@ for (my $tupidx = 0; $tupidx < ROWCOUNT; $tupidx++)
 		$tup->{c6} = 41;
 		$tup->{c7} = 41;
 	}
+	elsif ($tupidx == 12)
+	{
+		# Set both HEAP_XMAX_LOCK_ONLY and HEAP_KEYS_UPDATED
+		$tup->{t_infomask} |= HEAP_XMAX_LOCK_ONLY;
+		$tup->{t_infomask2} |= HEAP_KEYS_UPDATED;
+	}
+	elsif ($tupidx == 13)
+	{
+		# Set both HEAP_XMAX_COMMITTED and HEAP_XMAX_IS_MULTI
+		$tup->{t_infomask} |= HEAP_XMAX_COMMITTED;
+		$tup->{t_infomask} |= HEAP_XMAX_IS_MULTI;
+	}
 	write_tuple($file, $offset, $tup);
 }
 close($file);
@@ -374,7 +390,10 @@ is ($result,
 0|10|7552|1|58|||relation natts < tuple natts (3 < 67)
 0|11|7488|1|58|2||t_hoff + offset > lp_len (24 + 416847976 > 58)
 0|12|7424|1|58|2|0|final chunk number differs from expected (0 vs. 6)
-0|12|7424|1|58|2|0|toasted value missing from toast table",
+0|12|7424|1|58|2|0|toasted value missing from toast table
+0|13|7360|1|58|||HEAP_XMAX_LOCK_ONLY and HEAP_KEYS_UPDATED both set
+0|14|7296|1|58|||tuple xmax = 0 precedes relation relminmxid = 1
+0|14|7296|1|58|||HEAP_XMAX_COMMITTED and HEAP_XMAX_IS_MULTI both set",
 "Expected verify_heapam output");
 
 # Each table corruption message is returned with a standard header, and we can
@@ -396,6 +415,8 @@ my @corruption_re = (
 	qr/t_hoff \+ offset > lp_le/,
 	qr/final chunk number differs from expected/,
 	qr/toasted value missing from toast table/,
+	qr/HEAP_XMAX_LOCK_ONLY and HEAP_KEYS_UPDATED both set/,
+	qr/HEAP_XMAX_COMMITTED and HEAP_XMAX_IS_MULTI both set/,
 );
 
 $node->command_like(
diff --git a/src/backend/access/heap/hio.c b/src/backend/access/heap/hio.c
index aa3f14c019..00de10b7c9 100644
--- a/src/backend/access/heap/hio.c
+++ b/src/backend/access/heap/hio.c
@@ -47,6 +47,17 @@ RelationPutHeapTuple(Relation relation,
 	 */
 	Assert(!token || HeapTupleHeaderIsSpeculative(tuple->t_data));
 
+	/*
+	 * Do not allow tuples with invalid combinations of hint bits to be placed
+	 * on a page.  These combinations are detected as corruption by the
+	 * contrib/amcheck logic, so if you decide to disable one or more of these
+	 * assertions, make corresponding changes to contrib/amcheck.
+	 */
+	Assert(!((tuple->t_data->t_infomask & HEAP_XMAX_LOCK_ONLY) &&
+			 (tuple->t_data->t_infomask2 & HEAP_KEYS_UPDATED)));
+	Assert(!((tuple->t_data->t_infomask & HEAP_XMAX_COMMITTED) &&
+			 (tuple->t_data->t_infomask & HEAP_XMAX_IS_MULTI)));
+
 	/* Add the tuple to the page */
 	pageHeader = BufferGetPage(buffer);
 
-- 
2.21.1 (Apple Git-122.3)