Fix two off-by-one errors in bufmgr.c.

In 4b4b680c I passed a buffer index number (starting from 0) instead of
a proper Buffer id (which start from 1 for shared buffers) in two
places.

This wasn't noticed so far as one of those locations isn't compiled at
all (PrintPinnedBufs) and the other one (InvalidBuffer) requires a
unlikely, but possible, set of circumstances to trigger a symptom.

To reduce the likelihood of such incidents a bit also convert existing
open coded mappings from buffer descriptors to buffer ids with
BufferDescriptorGetBuffer().

Author: Qingqing Zhou
Reported-By: Qingqing Zhou
Discussion: CAJjS0u2ai9ooUisKtkV8cuVUtEkMTsbK8c7juNAjv8K11zeCQg@mail.gmail.com
Backpatch: 9.5 where the private ref count infrastructure was introduced

1 files changed, 19 insertions, 17 deletions

diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index e4b25587e98..cd3aaad6106 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -1273,7 +1273,7 @@ retry:
 		UnlockBufHdr(buf);
 		LWLockRelease(oldPartitionLock);
 		/* safety check: should definitely not be our *own* pin */
-		if (GetPrivateRefCount(buf->buf_id) > 0)
+		if (GetPrivateRefCount(BufferDescriptorGetBuffer(buf)) > 0)
 			elog(ERROR, "buffer is pinned in InvalidateBuffer");
 		WaitIO(buf);
 		goto retry;
@@ -1426,16 +1426,16 @@ ReleaseAndReadBuffer(Buffer buffer,
 static bool
 PinBuffer(volatile BufferDesc *buf, BufferAccessStrategy strategy)
 {
-	int			b = buf->buf_id;
+	Buffer		b = BufferDescriptorGetBuffer(buf);
 	bool		result;
 	PrivateRefCountEntry *ref;
 
-	ref = GetPrivateRefCountEntry(b + 1, true);
+	ref = GetPrivateRefCountEntry(b, true);
 
 	if (ref == NULL)
 	{
 		ReservePrivateRefCountEntry();
-		ref = NewPrivateRefCountEntry(b + 1);
+		ref = NewPrivateRefCountEntry(b);
 
 		LockBufHdr(buf);
 		buf->refcount++;
@@ -1460,8 +1460,7 @@ PinBuffer(volatile BufferDesc *buf, BufferAccessStrategy strategy)
 
 	ref->refcount++;
 	Assert(ref->refcount > 0);
-	ResourceOwnerRememberBuffer(CurrentResourceOwner,
-								BufferDescriptorGetBuffer(buf));
+	ResourceOwnerRememberBuffer(CurrentResourceOwner, b);
 	return result;
 }
 
@@ -1489,23 +1488,24 @@ PinBuffer(volatile BufferDesc *buf, BufferAccessStrategy strategy)
 static void
 PinBuffer_Locked(volatile BufferDesc *buf)
 {
-	int			b = buf->buf_id;
+	Buffer		b;
 	PrivateRefCountEntry *ref;
 
 	/*
 	 * As explained, We don't expect any preexisting pins. That allows us to
 	 * manipulate the PrivateRefCount after releasing the spinlock
 	 */
-	Assert(GetPrivateRefCountEntry(b + 1, false) == NULL);
+	Assert(GetPrivateRefCountEntry(BufferDescriptorGetBuffer(buf), false) == NULL);
 
 	buf->refcount++;
 	UnlockBufHdr(buf);
 
-	ref = NewPrivateRefCountEntry(b + 1);
+	b = BufferDescriptorGetBuffer(buf);
+
+	ref = NewPrivateRefCountEntry(b);
 	ref->refcount++;
 
-	ResourceOwnerRememberBuffer(CurrentResourceOwner,
-								BufferDescriptorGetBuffer(buf));
+	ResourceOwnerRememberBuffer(CurrentResourceOwner, b);
 }
 
 /*
@@ -1520,14 +1520,14 @@ static void
 UnpinBuffer(volatile BufferDesc *buf, bool fixOwner)
 {
 	PrivateRefCountEntry *ref;
+	Buffer		b = BufferDescriptorGetBuffer(buf);
 
 	/* not moving as we're likely deleting it soon anyway */
-	ref = GetPrivateRefCountEntry(buf->buf_id + 1, false);
+	ref = GetPrivateRefCountEntry(b, false);
 	Assert(ref != NULL);
 
 	if (fixOwner)
-		ResourceOwnerForgetBuffer(CurrentResourceOwner,
-								  BufferDescriptorGetBuffer(buf));
+		ResourceOwnerForgetBuffer(CurrentResourceOwner, b);
 
 	Assert(ref->refcount > 0);
 	ref->refcount--;
@@ -2739,6 +2739,7 @@ PrintBufferDescs(void)
 	for (i = 0; i < NBuffers; ++i)
 	{
 		volatile BufferDesc *buf = GetBufferDescriptor(i);
+		Buffer		b = BufferDescriptorGetBuffer(buf);
 
 		/* theoretically we should lock the bufhdr here */
 		elog(LOG,
@@ -2747,7 +2748,7 @@ PrintBufferDescs(void)
 			 i, buf->freeNext,
 		  relpathbackend(buf->tag.rnode, InvalidBackendId, buf->tag.forkNum),
 			 buf->tag.blockNum, buf->flags,
-			 buf->refcount, GetPrivateRefCount(i));
+			 buf->refcount, GetPrivateRefCount(b));
 	}
 }
 #endif
@@ -2761,8 +2762,9 @@ PrintPinnedBufs(void)
 	for (i = 0; i < NBuffers; ++i)
 	{
 		volatile BufferDesc *buf = GetBufferDescriptor(i);
+		Buffer		b = BufferDescriptorGetBuffer(buf);
 
-		if (GetPrivateRefCount(i + 1) > 0)
+		if (GetPrivateRefCount(b) > 0)
 		{
 			/* theoretically we should lock the bufhdr here */
 			elog(LOG,
@@ -2771,7 +2773,7 @@ PrintPinnedBufs(void)
 				 i, buf->freeNext,
 				 relpathperm(buf->tag.rnode, buf->tag.forkNum),
 				 buf->tag.blockNum, buf->flags,
-				 buf->refcount, GetPrivateRefCount(i + 1));
+				 buf->refcount, GetPrivateRefCount(b));
 		}
 	}
 }