RE: [EXTERNAL] Re: [PATCH] Report the query string that caused a memory error under Valgrind

From: Onur Tirtir <Onur(dot)Tirtir(at)microsoft(dot)com>
To: "peter(dot)eisentraut(at)enterprisedb(dot)com" <peter(dot)eisentraut(at)enterprisedb(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: RE: [EXTERNAL] Re: [PATCH] Report the query string that caused a memory error under Valgrind
Date: 2023-03-23 17:11:32
Message-ID: AM9PR83MB0498C9DFFA9804AA13325E84E9879@AM9PR83MB0498.EURPRD83.prod.outlook.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hey Peter,

Thank you for reviewing the patch and for your feedback. I believe the v2 patch should be able to handle other protocol messages too.

-----Original Message-----
From: Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>
Sent: Wednesday, March 22, 2023 7:00 PM
To: Onur Tirtir <Onur(dot)Tirtir(at)microsoft(dot)com>; pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: [EXTERNAL] Re: [PATCH] Report the query string that caused a memory error under Valgrind

[You don't often get email from peter(dot)eisentraut(at)enterprisedb(dot)com(dot) Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]

On 31.01.23 15:00, Onur Tirtir wrote:
> We use Valgrind --together with the suppression file provided in
> Postgres repo-- to test Citus extension against memory errors.
>
> We replace /bin/postgres executable with a simple bash script that
> executes the original postgres executable under Valgrind and then we
> run our usual regression tests.
>
> However, it is quite hard to understand which query caused a memory
> error in the stack traces that has been dumped into valgrind logfile.
>
> For this reason, we propose the attached patch to allow Valgrind to
> report the query string that caused a memory error right after the
> relevant stack trace.
>
> I belive this would not only be useful for Citus but also for Postgres
> and other extensions in their valgrind-testing process.

I can see how this could be useful. But this only handles queries using the simple protocol. At least the extended protocol should be handled as well. Maybe it would be better to move this up to PostgresMain() and handle all protocol messages?

Attachment Content-Type Size
v2-0001-Report-the-query-string-that-caused-a-memory-erro.patch application/octet-stream 2.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jeff Davis 2023-03-23 17:16:45 Re: ICU locale validation / canonicalization
Previous Message Tomas Vondra 2023-03-23 17:10:27 Re: Add LZ4 compression in pg_dump