Horton

Thread

Commits

  1. Adjust Valgrind macro usage to protect chunk headers

  1. Protecting allocator headers with Valgrind

    David Rowley <dgrowleyml@gmail.com> — 2023-04-11T13:28:08Z

    Over on [1], Tom mentioned that we might want to rethink the decision
to not protect chunk headers with Valgrind.  That thread fixed a bug
that was accessing array element -1, which effectively was reading the
MemoryChunk at the start of the allocated chunk as an array element.

I wrote a patch to adjust the Valgrind macros to mark the MemoryChunks
as NOACCESS and that finds the bug reported on that thread (with the
fix for it reverted).

I didn't quite get a clear run at committing the changes during the
v16 cycle, but wondering since they're really just Valgrind macro
changes if anyone would object to doing it now?

I know there are a few people out there running sqlsmith and/or
sqlancer under Valgrind. It would be good to have this in so we could
address any new issues the attached patch might help them highlight.

Any objections?

(Copying in Tom and Richard same as original thread.  Reposting for
more visibility of this change)

David

  2. Re: Protecting allocator headers with Valgrind

    Richard Guo <guofenglinux@gmail.com> — 2023-04-12T01:59:22Z

    On Tue, Apr 11, 2023 at 9:28 PM David Rowley <dgrowleyml@gmail.com> wrote:

> Over on [1], Tom mentioned that we might want to rethink the decision
> to not protect chunk headers with Valgrind.  That thread fixed a bug
> that was accessing array element -1, which effectively was reading the
> MemoryChunk at the start of the allocated chunk as an array element.


Seems the link to the original thread is not pasted.  Here it is.

[1] https://www.postgresql.org/message-id/1650235.1672694719%40sss.pgh.pa.us

Thanks
Richard

  3. Re: Protecting allocator headers with Valgrind

    David Rowley <dgrowleyml@gmail.com> — 2023-04-13T12:24:20Z

    On Wed, 12 Apr 2023 at 01:28, David Rowley <dgrowleyml@gmail.com> wrote:
> Any objections?

It seems there are none. I'll have another look at the patch tomorrow
with the aim to get it in.

(Unless someone objects to me doing that before then)

David

  4. Re: Protecting allocator headers with Valgrind

    Noah Misch <noah@leadboat.com> — 2023-04-15T15:25:58Z

    On Wed, Apr 12, 2023 at 01:28:08AM +1200, David Rowley wrote:
> Any objections?

Not objecting.  I think the original Valgrind integration refrained from this
because it would have added enough Valgrind client requests to greatly slow
Valgrind runs.  Valgrind reduced the cost of client requests in later years,
so this new conclusion is reasonable.

  5. Re: Protecting allocator headers with Valgrind

    David Rowley <dgrowleyml@gmail.com> — 2023-04-16T05:29:34Z

    On Sun, 16 Apr 2023 at 03:26, Noah Misch <noah@leadboat.com> wrote:
> Not objecting.  I think the original Valgrind integration refrained from this
> because it would have added enough Valgrind client requests to greatly slow
> Valgrind runs.  Valgrind reduced the cost of client requests in later years,
> so this new conclusion is reasonable.

I tested that. It's not much slowdown:

time make installcheck

Unpatched: real    79m36.458s
Patched: real    81m31.589s

I forgot to mention, I pushed the patch yesterday.

David