Horton

Re: ubsan

Andres Freund <andres@anarazel.de>

View thread View on postgr.es
From: Andres Freund <andres@anarazel.de>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-hackers@postgresql.org
Date: 2022-03-23T18:21:37Z
Lists: pgsql-hackers 
Hi,

On 2022-03-23 13:54:50 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > I tried to run postgres with ubsan to debug something.
> 
> For 0001, could we just replace configure's dlopen check with the
> dlsym check?  Or are you afraid of reverse-case failures?

Yea, I was worried about that. But now that I think more about it, it's hard
to believe something could provide / intercept dlsym but not dlopen. I guess
we can try and see?


> 0002: ugh, but my only real complaint is that __ubsan_default_options
> needs more than zero comment.

Yea, definitely. I am still hoping that somebody could see a better approach
than that ugly hack.

Haven't yet checked, but probably should also verify asan either doesn't have
the same problem or provide the same hack for ASAN_OPTIONS.


> Also, it's not "our" getenv is it?

Not really. "libc's getenv()"?


> 0003: OK.  Interesting though that we haven't seen these before.

I assume it's a question of library version and configure flags.

Looks like the fwrite nonnull case isn't actually due to the nonnull
attribute, but just fwrite() getting intercepted by the sanitizer
library. Looks like that was added starting in gcc 9 [1]

And the guc.c case presumably requires --enable-nls and a version of gettext
using the nonnull attribute?


Wonder if there's a few functions we should add nonnull to ourselves. Probably
would help "everyday compiler warnings", static analyzers, and ubsan.

Greetings,

Andres Freund

[1]
5d3805fca3e9 (Jakub Jelinek     2017-10-19 13:23:59 +0200  1151) #if SANITIZER_INTERCEPT_FWRITE
5d3805fca3e9 (Jakub Jelinek     2017-10-19 13:23:59 +0200  1152) INTERCEPTOR(SIZE_T, fwrite, const void *p, uptr size, uptr nmemb, void *file) {
5d3805fca3e9 (Jakub Jelinek     2017-10-19 13:23:59 +0200  1153)   // libc file streams can call user-supplied functions, see fopencookie.
5d3805fca3e9 (Jakub Jelinek     2017-10-19 13:23:59 +0200  1154)   void *ctx;
5d3805fca3e9 (Jakub Jelinek     2017-10-19 13:23:59 +0200  1155)   COMMON_INTERCEPTOR_ENTER(ctx, fwrite, p, size, nmemb, file);
5d3805fca3e9 (Jakub Jelinek     2017-10-19 13:23:59 +0200  1156)   SIZE_T res = REAL(fwrite)(p, size, nmemb, file);
5d3805fca3e9 (Jakub Jelinek     2017-10-19 13:23:59 +0200  1157)   if (res > 0) COMMON_INTERCEPTOR_READ_RANGE(ctx, p, res * size);
5d3805fca3e9 (Jakub Jelinek     2017-10-19 13:23:59 +0200  1158)   return res;
5d3805fca3e9 (Jakub Jelinek     2017-10-19 13:23:59 +0200  1159) }

$ git describe --tags 5d3805fca3e9
basepoints/gcc-8-3961-g5d3805fca3e

Commits

  1. Avoid possible crash within libsanitizer.

  2. ci: Use -fsanitize=undefined,alignment,address in linux tasks

  3. Add workaround to make ubsan and ps_status.c compatible

  4. Don't try to translate NULL in GetConfigOptionByNum().

  5. Don't call fwrite() with len == 0 when writing out relcache init file.

  6. configure: check for dlsym instead of dlopen.

  7. Clean up assorted failures under clang's -fsanitize=undefined checks.