public inbox for [email protected]  

help / color / mirror / Atom feed
From: Richard Guo <[email protected]>
To: David Rowley <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Subject: Re: An oversight in ExecInitAgg for grouping sets
Date: Mon, 20 Mar 2023 14:18:15 +0800
Message-ID: <CAMbWs4-oPinvc+50L+fwd9e+3Z8YcTs7D3GpL-558StyLY-WLA@mail.gmail.com> (raw)
In-Reply-To: <CAApHDvoHNUC8iw-S8Y3_1MTzEwHZR8TGAynUuvJpf9So0N6oKw@mail.gmail.com>
References: <CAMbWs4-vZuuPOZsKOYnSAaPYGKhmacxhki+vpOKk0O7rymccXQ@mail.gmail.com>
	<CAMbWs49nMd1up4BOq=gXutTJintTUpe1u3qxvhRhPj9=9h5U=g@mail.gmail.com>
	<[email protected]>
	<CAApHDvqOnG67BS8SuqT55MvfNmZicc1_aPfBk+Nr-Sw5WcPLWA@mail.gmail.com>
	<CAMbWs4-aqpZ0t_jbGWnY9VbgAkfk8qitPeDjRC-_mjdQ_40kYA@mail.gmail.com>
	<CAApHDvoHNUC8iw-S8Y3_1MTzEwHZR8TGAynUuvJpf9So0N6oKw@mail.gmail.com>

On Mon, Jan 9, 2023 at 5:21 PM David Rowley <[email protected]> wrote:

> On Thu, 5 Jan 2023 at 20:06, Richard Guo <[email protected]> wrote:
> > I reviewed this patch and have some comments.
>
> Thanks for looking at this. I think I've fixed all the issues you
> mentioned.
>
> One extra thing I noticed was that I had to add a new
> VALGRIND_MAKE_MEM_DEFINED in AllocSetAlloc when grabbing an item off
> the freelist. I didn't quite manage to figure out why that's needed as
> when we do AllocSetFree() we don't mark the pfree'd memory with
> NOACCESS, and it also looks like AllocSetReset() sets the keeper
> block's memory to NOACCESS, but that function also clears the
> freelists too, so the freelist chunk is not coming from a recently
> reset context.
>
> I might need to spend a bit more time on this to see if I can figure
> out why this is happening.  On the other hand, maybe we should just
> mark pfree'd memory as NOACCESS as that might find another class of
> issues.


It occurred to me that this hasn't been applied.  Should we add it to
the CF to not lose track of it?

Thanks
Richard


view thread (6+ messages)  latest in thread

reply
Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: An oversight in ExecInitAgg for grouping sets
  In-Reply-To: <CAMbWs4-oPinvc+50L+fwd9e+3Z8YcTs7D3GpL-558StyLY-WLA@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox