Re: Dynamic shared memory areas

[ blast-from-the-past department ]

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Thu, Dec 1, 2016 at 6:33 AM, Thomas Munro
> <thomas(dot)munro(at)enterprisedb(dot)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