Re: Fix some memory leaks in ecpg.addons

Hi,

On 2023-11-08 22:00:00 +0300, Alexander Lakhin wrote:
> Hello Tristan,
>
> 08.11.2023 20:37, Tristan Partin wrote:
> > Are people using some suppression file or setting ASAN_OPTIONS to something?
> >
>
> I use the following:
> ASAN_OPTIONS=detect_leaks=0:abort_on_error=1:print_stacktrace=1:\
> disable_coredump=0:strict_string_checks=1:check_initialization_order=1:\
> strict_init_order=1:detect_stack_use_after_return=0

I wonder if we should add some of these options by default ourselves. We could
e.g. add something like the __ubsan_default_options() in
src/backend/main/main.c to src/port/... instead, and return a combination of
"our" options (like detect_leaks=0) and the ones from the environment.

> (You'll need to add detect_stack_use_after_return=0 with a newer clang
> (I use clang-18) to workaround an incompatibility of check_stack_depth()
> with that sanitizer feature enabled by default.)

I have been wondering if we should work on fixing that. There are a few ways:

- We can add a compiler parameter explicitly disabling the use-after-return
checks - however, the checks are quite useful, so that'd be somewhat of a
shame.

- We could exempt the stack depth checking functions from being validated with
asan, I think that should fix this issue. Looks like
__attribute__((no_sanitize("address")))
would work

- Asan has an interface for getting the real stack address. See
https://github.com/llvm/llvm-project/blob/main/compiler-rt/include/sanitizer/asan_interface.h#L322

ISTM that, if it actually works as I theorize it should, using
__attribute__((no_sanitize("address"))) would be the easiest approach
here. Something like

#if defined(__has_feature) && __has_feature(address_sanitizer)
#define pg_attribute_no_asan __attribute__((no_sanitize("address")))
#else
#define pg_attribute_no_asan
#endif

or such should work.

> So I would say that fixing ecpg won't make postgres sanitizer-friendly in
> a whole.

One thing that's been holding me back on trying to do something around this is
the basically non-existing documentation around all of this. I haven't even
found documentation referencing the fact that there are headers like
sanitizer/asan_interface.h, you just have to figure that out yourself. Compare
that to something like valgrind, which has documented this at least somewhat.

Greetings,

Andres Freund