Re: Dynamic shared memory areas

[ blast-from-the-past department ]

Robert Haas <robertmhaas@gmail.com> writes:
> On Thu, Dec 1, 2016 at 6:33 AM, Thomas Munro
> <thomas.munro@enterprisedb.com> wrote:
>> Please find attached dsa-v8.patch, and also a small test module for
>> running random allocate/free exercises and dumping the internal
>> allocator state.

> OK, I've committed the main patch.

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?

			regards, tom lane