Re: Fix some ubsan/asan related issues - Mailing list pgsql-hackers

From Andres Freund
Subject Re: Fix some ubsan/asan related issues
Date
Msg-id 20240130215812.thwd3wprzp3im2ej@awork3.anarazel.de
Whole thread Raw
In response to Re: Fix some ubsan/asan related issues  ("Tristan Partin" <tristan@neon.tech>)
Responses Re: Fix some ubsan/asan related issues
List pgsql-hackers
Hi,

On 2024-01-30 09:59:25 -0600, Tristan Partin wrote:
> From 331cec1c9db6ff60dcc6d9ba62a9c8be4e5e95ed Mon Sep 17 00:00:00 2001
> From: Tristan Partin <tristan@neon.tech>
> Date: Mon, 29 Jan 2024 18:03:39 -0600
> Subject: [PATCH v1 1/3] Refuse to register message in LogLogicalMessage if
>  NULL

> If this occurs, the memcpy of rdata_data in CopyXLogRecordToWAL breaks
> the API contract of memcpy in glibc. The two pointer arguments are
> marked as nonnull, even in the event the amount to copy is 0 bytes.

It seems pretty odd to call LogLogicalMessage() with a NULL argument. Why is
that something useful?


> From dc9488f3fdee69b981b52c985fb77106d7d301ff Mon Sep 17 00:00:00 2001
> From: Tristan Partin <tristan@neon.tech>
> Date: Wed, 24 Jan 2024 17:07:01 -0600
> Subject: [PATCH v1 2/3] meson: Support compiling with -Db_sanitize=address
> 
> The ecpg is parser is extremely leaky, so we need to silence leak
> detection.

This does stuff beyond epcg...


> +if get_option('b_sanitize').contains('address')
> +  cdata.set('USE_ADDRESS_SANITIZER', 1)
> +endif
>  
>  ###############################################################
>  # NLS / Gettext
> diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
> index ac409b0006..e18e716d9c 100644
> --- a/src/bin/initdb/initdb.c
> +++ b/src/bin/initdb/initdb.c
> @@ -338,6 +338,17 @@ do { \
>          output_failed = true, output_errno = errno; \
>  } while (0)
>  
> +#ifdef USE_ADDRESS_SANITIZER

When asan is used  __SANITIZE_ADDRESS__ is defined, so we don't need to
implement this ourselves.


> +const char *__asan_default_options(void);
> +
> +const char *__asan_default_options(void)
> +{
> +    return "detect_leaks=0";
> +}
> +
> +#endif

Wonder if we should move this into some static library and link it into all
binaries that don't want leak detection? It doesn't seem great to have to
adjust this in a bunch of files if we want to adjust the options...

Greetings,

Andres Freund



pgsql-hackers by date:

Previous
From: Andrew Dunstan
Date:
Subject: Re: [PATCH] Add native windows on arm64 support
Next
From: Magnus Hagander
Date:
Subject: Re: Possibility to disable `ALTER SYSTEM`