Re: Dynamic shared memory areas - Mailing list pgsql-hackers

From Tom Lane
Subject Re: Dynamic shared memory areas
Date
Msg-id 799984.1761150474@sss.pgh.pa.us
Whole thread Raw
In response to Re: Dynamic shared memory areas  (Robert Haas <robertmhaas@gmail.com>)
List pgsql-hackers
[ 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



pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: Confine vacuum skip logic to lazy_scan_skip
Next
From: Konstantin Knizhnik
Date:
Subject: Bug in amcheck?