Hi, On 2019-01-13 19:52:32 -0500, Tom Lane wrote: > Thomas Munro <thomas.mu...@enterprisedb.com> writes: > > On Thu, Aug 9, 2018 at 12:59 PM Asim R P <aprav...@pivotal.io> wrote: > >> On Tue, Aug 7, 2018 at 7:00 PM, Peter Geoghegan <p...@bowt.ie> wrote: > >>> I wonder if it would be a better idea to enable Valgrind's memcheck to > >>> mark buffers as read-only or read-write. > > >> Basic question: how do you mark buffers as read-only using memcheck > >> tool? Running installcheck with valgrind didn't uncover any errors: > > > Presumably with VALGRIND_xxx macros, but is there a way to make that > > work for shared memory? > > > I like the mprotect() patch. This could be enabled on a build farm > > animal. > > I think this is a cute idea and potentially useful as an alternative > to valgrind, but I don't like the patch much. It'd be better to > set things up so that the patch adds support for catching bad accesses > with either valgrind or mprotect, according to compile options. Also, > this sort of thing is gratitously ugly: > > +#ifdef MPROTECT_BUFFERS > + BufferMProtect(buf, PROT_NONE); > +#endif > > The right way IMO is to just have macro calls like > > ProtectBuffer(buf, NO_ACCESS); > > which expand to nothing at all if the feature isn't enabled by #ifdefs, > and otherwise to whatever we need it to do. (The access-type symbols > then need to be something that can be defined correctly for either > implementation.)
As this has not been addressed since, and the CF has ended, I'm marking this patch as returned with feedback. Please resubmit once that's addressed. > > I guess it would either fail explicitly or behave incorrectly > > for VM page size > BLCKSZ depending on OS, but at least on Linux/amd64 > > you have to go out of your way to get pages > 4KB so that seems OK for > > a debugging feature. > > What worries me more is that I don't think we try hard to ensure that > buffers are aligned on system page boundaries. It's probably worthwhile to just always force that, to reduce unnecessary page misses. Greetings, Andres Freund