Re: BUG #14344: string_agg(DISTINCT ..) crash

On Thu, Sep 29, 2016 at 1:10 PM, Peter Geoghegan <pg(at)heroku(dot)com> wrote:
> I have been able to reproduce the problem, in a fashion, so can now
> probably debug it easily enough. Specifically, I can get Valgrind to
> complain about Regina's test case.

I see what the problem is. This use-after-free bug occurs because, for
some reason, tuplesort_gettupleslot()'s contract was not updated by me
alongside analogous routines like tuplesort_getindextuple() as part of
the batch memory commit. This meant that at least one particular case
(process_ordered_aggregate_multi() calls to tuplesort_gettupleslot(),
with distinct columns) felt entitled to reuse some slot's tuple, a
tuple located in memory managed by tuplesort.c as batch memory. The
reuse was across calls to tuplesort_gettupleslot().

Attached patch fixes the bug, and updates the contract of
tuplesort_gettupleslot() so it's no longer the odd one out. However,
I'm not recommending this be committed without further discussion,
because the performance overhead could be a concern. Also, it looks
like hypothetical_dense_rank_final() requires similar treatment, which
isn't addressed by this patch. I did audit other callers of
tuplesort_gettupleslot() and analogous routines for other classes of
caller-tuple (e.g., tuplesort_getindextuple() callers were examined
too). Look like it's just these two tuplesort callers that are
affected.

How much could it hurt performance to fix the bug in this way? Does
anyone see an alternative? Pushing knowledge of this special case into
tuplesort.c seems like an unappealing alternative.

--
Peter Geoghegan