Re: Fix uninitialized variable access (src/backend/utils/mmgr/freepage.c)

On 2021-08-18 1:29 a.m., Kyotaro Horiguchi wrote:
> At Tue, 17 Aug 2021 17:04:44 +0900, Michael Paquier <michael(at)paquier(dot)xyz> wrote in
>> On Fri, Jul 02, 2021 at 06:22:56PM -0300, Ranier Vilela wrote:
>>> Em qui., 1 de jul. de 2021 às 17:20, Mahendra Singh Thalor <
>>> mahi6run(at)gmail(dot)com> escreveu:
>>>> Please can we try to hit this rare condition by any test case. If you have
>>>> any test cases, please share.
>> Yeah, this needs to be proved. Are you sure that this change is
>> actually right? The bottom of FreePageManagerPutInternal() has
>> assumptions that a page may not be found during a btree search, with
>> an index value used.
> By a quick look, FreePageBtreeSearch is called only from
> FreePageManagerPutInternal at three points. The first one assumes that
> result.found == true, at the rest points are passed only when
> fpm->btree_depth > 0, i.e, fpm->btree_root is non-NULL.
>
> In short FreePageBtreeSeach is never called when fpm->btree_root is
> NULL. I don't think we need to fill-in other members since the
> contract of the function looks fine.
>
> It might be simpler to turn 'if (btp == NULL)' to an assertion.
After added the initialization of split_pages in patch
fix_unitialized_var_index_freepage-v1.patch,

+        result->split_pages = 0;

it actually changed the assertion condition after the second time
function call of FreePageBtreeSearch.
            FreePageBtreeSearch(fpm, first_page, &result);

            /*
             * The act of allocating pages for use in constructing our
btree
             * should never cause any page to become more full, so the new
             * split depth should be no greater than the old one, and
perhaps
             * less if we fortuitously allocated a chunk that freed up
a slot
             * on the page we need to update.
             */
            Assert(result.split_pages <= fpm->btree_recycle_count);

Should we consider adding some test cases to make sure this assertion
will still function properly?

>
> regards.
>
--
David

Software Engineer
Highgo Software Inc. (Canada)
www.highgo.ca