Re: Dynamic shared memory areas

Robert Haas <robertmhaas@gmail.com>

From: Robert Haas <robertmhaas@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Thomas Munro <thomas.munro@gmail.com>, pgsql-hackers@lists.postgresql.org
Date: 2025-12-01T19:14:57Z
Lists: pgsql-hackers
On Wed, Oct 22, 2025 at 12:27 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Our shiny new version of Coverity kvetches about
> FreePageBtreeInsertInternal:
>
> *** CID 1667414:           (OVERRUN)
> /srv/coverity/git/pgsql-git/postgresql/src/backend/utils/mmgr/freepage.c: 908             in FreePageBtreeInsertInternal()
> 902     {
> 903         Assert(btp->hdr.magic == FREE_PAGE_INTERNAL_MAGIC);
> 904         Assert(btp->hdr.nused <= FPM_ITEMS_PER_INTERNAL_PAGE);
> 905         Assert(index <= btp->hdr.nused);
> 906         memmove(&btp->u.internal_key[index + 1], &btp->u.internal_key[index],
> 907                 sizeof(FreePageBtreeInternalKey) * (btp->hdr.nused - index));
> >>>     CID 1667414:           (OVERRUN)
> >>>     Overrunning array "btp->u.internal_key" of 254 16-byte elements at element index 254 (byte offset 4079) using index "index" (which evaluates to 254).
> 908         btp->u.internal_key[index].first_page = first_page;
> 909         relptr_store(base, btp->u.internal_key[index].child, child);
> 910         ++btp->hdr.nused;
> 911     }
>
> I believe the reason is that the second Assert is wrong, and it
> should instead be
>
> 904         Assert(btp->hdr.nused < FPM_ITEMS_PER_INTERNAL_PAGE);
>
> to assert that there is room for the item we are about to insert.
>
> The same thinko exists in FreePageBtreeInsertLeaf, although
> for some reason Coverity isn't whining about that.
>
> Thoughts?

I only just noticed this email. I see you've already fixed the issue.
I agree with your analysis, and thanks for taking care of it.

-- 
Robert Haas
EDB: http://www.enterprisedb.com



Commits

  1. Fix off-by-one Asserts in FreePageBtreeInsertInternal/Leaf.

  2. Reserve zero as an invalid DSM handle.