Re: Dereference before NULL check (src/backend/storage/ipc/latch.c)

Em seg., 2 de nov. de 2020 às 05:25, Kyotaro Horiguchi <
horikyota(dot)ntt(at)gmail(dot)com> escreveu:

> At Mon, 02 Nov 2020 14:33:40 +0900 (JST), Kyotaro Horiguchi <
> horikyota(dot)ntt(at)gmail(dot)com> wrote in
> > At Mon, 2 Nov 2020 16:22:09 +1300, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
> wrote in
> > > On Mon, Nov 2, 2020 at 1:49 PM Kyotaro Horiguchi
> > > <horikyota(dot)ntt(at)gmail(dot)com> wrote:
> > > > At Sat, 31 Oct 2020 11:40:53 -0300, Ranier Vilela <
> ranier(dot)vf(at)gmail(dot)com> wrote in
> > > > > Per Coverity.
> > > > >
> > > > > If test set->latch against NULL, is why it can be NULL.
> > > > > ResetEvent can dereference NULL.
> > > >
> > > > If the returned event is WL_LATCH_SET, set->latch cannot be NULL. We
> > > > shouldn't inadvertently ignore the unexpected or broken situation.We
> > > > could put Assert instead, but I think that we don't need do something
> > > > here at all since SIGSEGV would be raised at the right location.
> > >
> > > Hmm. I changed that to support set->latch == NULL, so that you can
> > > use the long lived WES in the rare code paths that call WaitLatch()
> > > without a latch (for example the code I proposed at [1]). The Windows
> >
> > Ooo. We don't update epoll events in that case. Ok, I understand
> > WL_LATCH_SET can fire while set->latch == NULL.
> >
> > (I was confused by WaitEventAdjust* asserts set->latch != NULL for
> > WL_LATCH_SET. Isn't it better if we moved the check on latch from
> > ModifyWaitEvent() to WaitEventAdjust*()?))
> >
> > > version leaves the event handle of the most recently used latch in
> > > set->handles[n] (because AFAICS there is no way to have a "hole" in
> > > the handles array). The event can fire while you are waiting on "no
> > > latch". Perhaps it should be changed to
> > > ResetEvent(set->handles[cur_event->pos + 1])?
> > >
> > > [1]
> https://www.postgresql.org/message-id/flat/CA%2BhUKGK1607VmtrDUHQXrsooU%3Dap4g4R2yaoByWOOA3m8xevUQ%40mail.gmail.com
> >
> > Seems right. Just doing that *seems* to work fine, but somehow I
> > cannot build on Windows for now...
>
> That was caused by a leftover change on config_default.pl I made when
> I tried to enable NLS.
>
> I called SetLatch() during WaitLatch(NULL, ) but that doesn't fire
> WL_LATCH_SET event for me on Windows. (I got it fired on Linux..) On
> Windows, the latch is detected after exiting the WaitLatch()
> call. Seems like MyLatch of waiter is different from
> peerPGPROC->procLatch. And... an update for Visual Studio broke my
> environment... I will investigate this further but everything feel
> cumbersome on Windows...
>
I can build.
vc_regress is it enough to test it?

regards,
Ranier Vilela