On Thu, 5 Jan 2023 at 20:06, Richard Guo <guofenglinux@gmail.com> 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?