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

From: Ranier Vilela <ranier(dot)vf(at)gmail(dot)com>
To: David Zhang <david(dot)zhang(at)highgo(dot)ca>
Cc: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, Mahendra Singh Thalor <mahi6run(at)gmail(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Fix uninitialized variable access (src/backend/utils/mmgr/freepage.c)
Date: 2021-10-01 20:03:04
Message-ID: CAEudQAoOGkfAczUjtHGU0=3z_2+F76zh6_NVvsVngRKbzqJiQg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Em sex., 1 de out. de 2021 às 16:24, David Zhang <david(dot)zhang(at)highgo(dot)ca>
escreveu:

> 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);
>
For me the assertion remains valid and usable.

regards,
Ranier Vilela

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Melanie Plageman 2021-10-01 20:05:31 Re: pg_stat_bgwriter.buffers_backend is pretty meaningless (and more?)
Previous Message Jeremy Schneider 2021-10-01 19:49:34 Re: [BUG] Logical replication failure "ERROR: could not map filenode "base/13237/442428" to relation OID" with catalog modifying txns