Re: Fix some ubsan/asan related issues

From: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
To: Tristan Partin <tristan(at)neon(dot)tech>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Fix some ubsan/asan related issues
Date: 2024-01-30 20:05:28
Message-ID: 729b1382-6e2a-4626-8b52-dce0097d8421@iki.fi
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 30/01/2024 17:57, Tristan Partin wrote:
> Patch 1:
>
> Passing NULL as a second argument to memcpy breaks ubsan, and there
> didn't seem to be anything preventing that in the LogLogicalMessage()
> codepath. Here is a preventative measure in LogLogicalMessage() and an
> Assert() in CopyXLogRecordToWAL().

For the audience: We ran into this one with the neon extension. The
solution is to call LogLogicalMessage("", 0, ...) instead of
LogLogicalMessage(NULL, 0, ...). . But it's true that it's pointlessfor
LogLogicalMessage to call XLogRegisterData() if the message is empty. If
we do this, we should check for 'size == 0' rather than 'message == NULL'.

> Patch 2:
>
> Support building with -Db_sanitize=address in Meson. Various executables
> are leaky which can cause the builds to fail and tests to fail, when we
> are fine leaking this memory.
>
> Personally, I am a big stickler for always freeing my memory in
> executables even if it gets released at program exit because it makes
> the leak sanitizer much more effective. However (!), I am not here to
> change the philosophy of memory management in one-off executables, so
> I have just silenced memory leaks in various executables for now.
>
> Patch 3:
>
> THIS DOESN'T WORK. DO NOT COMMIT. PROPOSING FOR DISCUSSION!
>
> In my effort to try to see if the test suite would pass with asan
> enabled, I ran into a max_stack_depth issue. I tried maxing it out
> (hence, the patch), but that still didn't remedy my issue. I tried to
> look on the list for any relevant emails, but nothing turned up. Maybe
> others have not attempted this before? Seems doubtful.
>
> Not entirely sure how to fix this issue. I personally find asan
> extremely effective, even more than valgrind, so it would be great if
> I could run Postgres with asan enabled to catch various stupid C issues
> I might create. On my system, ulimit -a returns 8192 kbytes, so Postgres
> just lets me set 7680 kbytes as the max. Whatever asan is doing doesn't
> seem to leave enough stack space for Postgres.

I'm a bit confused by these. We already run with sanitizer in the cirrus
CI. What does this enable that we're not already doing?

--
Heikki Linnakangas
Neon (https://neon.tech)

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jeff Davis 2024-01-30 20:45:39 Re: [17] CREATE SUBSCRIPTION ... SERVER
Previous Message Pavel Stehule 2024-01-30 19:23:50 Re: Schema variables - new implementation for Postgres 15