Re: pgsql: Fix another instance of unsafe coding for shm_toc_lookup failure

On Sat, Feb 3, 2018 at 12:32 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Fix another instance of unsafe coding for shm_toc_lookup failure.
>
> One or another author of commit 5bcf389ec seems to have thought that
> computing an offset from a NULL pointer would yield another NULL pointer.
> There may possibly be architectures where that works, but common machines
> don't work like that. Per a quick code review of places calling
> shm_toc_lookup and not using noError = false.

Hmm... That was me. FWIW I certainly didn't think that about pointer
arithmetic... I think I must have got confused about the sense of that
flag. ExecHashInitializeWorker() should always find a TOC entry using
plan_node_id, because ExecHashInitializeDSM() always inserts one, so
my mistake was actually to put noError = true there when noError =
false was called for. However, it's not surprising that you drew the
opposite conclusion (ie that it might in fact not be in the TOC),
since the shm space is really only necessary for EXPLAIN ANALYZE.
Perhaps I should make it skip setting up this shm stuff if
!node->ss.ps.instrument, just like the equivalent Sort node code. I
will look into that on Monday.

--
Thomas Munro
http://www.enterprisedb.com