Re: Possible NULL dereferencing null pointer (src/backend/executor/nodeIncrementalSort.c)

On Fri, Oct 09, 2020 at 12:24:16PM -0300, Ranier Vilela wrote:
>I think that TupIsNull macro is no longer appropriate, to protect
>ExecCopySlot.
>
>See at tuptable.h:
>#define TupIsNull(slot) \
>((slot) == NULL || TTS_EMPTY(slot))
>
>If var node->group_pivot is NULL, ExecCopySlot will
>dereference a null pointer (first arg).
>

No. The C standard says there's a "sequence point" [1] between the left
and right arguments of the || operator, and that the expressions are
evaluated from left to right. So the program will do the first check,
and if the pointer really is NULL it won't do the second one (because
that is not necessary for determining the result). Similarly for the &&
operator, of course.

Had this been wrong, surely some of the the other places TupIsNull would
be wrong too (and there are hundreds of them).

>Maybe, this can be related to a bug reported in the btree deduplication.
>

Not sure which bug you mean, but this piece of code is pretty unrelated
to btree in general, so I don't see any link.

regards

--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services