Improving asan/ubsan support

[ this thread separated from [1] as the discussion focus shifted ]

H Andres,

29.11.2023 22:39, Andres Freund wrote:
>> 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.

I think that such explicit expression of the project policy regarding
sanitizer checks is for good, but I see some obstacles on this way.

First, I'm not sure what to do with new useful options/maybe new option
values, that will appear in sanitizers eventually. Should the only options,
that are supported by all sanitizers' versions, be specified, or we may
expect that unsupported options/values would be ignored by old versions?

Second, what to do with other binaries, that need detect_leaks=0, for
example, that same ecpg?

> 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.

I've tried adding:
  bool
+__attribute__((no_sanitize("address")))
  stack_is_too_deep(void)

and it does work got me with clang 15, 18: `make check-world` passes with
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=1
UBSAN_OPTIONS=abort_on_error=1:print_stacktrace=1

(with a fix for pg_bsd_indent applied [2])

But with gcc 11, 12, 13 I get an assertion failure during `make check`:
#4  0x00007fabadcd67f3 in __GI_abort () at ./stdlib/abort.c:79
#5  0x0000557f35260382 in ExceptionalCondition (conditionName=0x557f35ca51a0 "(uintptr_t) buffer == 
TYPEALIGN(PG_IO_ALIGN_SIZE, buffer)", fileName=0x557f35ca4fc0 "md.c", lineNumber=471) at assert.c:66
#6  0x0000557f34a3b2bc in mdextend (reln=0x6250000375c8, forknum=MAIN_FORKNUM, blocknum=18, buffer=0x7fabaa800020, 
skipFsync=true) at md.c:471
#7  0x0000557f34a45a6f in smgrextend (reln=0x6250000375c8, forknum=MAIN_FORKNUM, blocknum=18, buffer=0x7fabaa800020, 
skipFsync=true) at smgr.c:501
#8  0x0000557f349139ed in RelationCopyStorageUsingBuffer (srclocator=..., dstlocator=..., forkNum=MAIN_FORKNUM, 
permanent=true) at bufmgr.c:4386

The buffer (buf) declared as follows:
static void
RelationCopyStorageUsingBuffer(RelFileLocator srclocator,
                                RelFileLocator dstlocator,
                                ForkNumber forkNum, bool permanent)
{
...
     PGIOAlignedBlock buf;
...

But as we can see, the buffer address is really not 4k-aligned, and that
offset 0x20 added in run-time only when the server started with
detect_stack_use_after_return=1.
So it looks like the asan feature detect_stack_use_after_return implemented
in gcc allows itself to add some data on stack, that breaks our alignment
expectations. With all three such Asserts in md.c removed,
`make check-world` passes for me.

> 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.

Yes, so maybe it's reasonable to support only basic/common features (such
as detect_leaks), leaving advanced ones for ad-hoc usage till they prove
their worthiness.

Best regards,
Alexander

[1] https://www.postgresql.org/message-id/flat/CWTLB2WWVJJ2.2YV6ERNOL1WVF%40neon.tech
[2] https://www.postgresql.org/message-id/591971ce-25c1-90f3-0526-5f54e3ebb32e%40gmail.com