Hi,
On 2022-03-23 13:54:50 -0400, Tom Lane wrote:
> 0002: ugh, but my only real complaint is that __ubsan_default_options
> needs more than zero comment. Also, it's not "our" getenv is it?
>
> 0004: no opinion
Attached is a rebased version of this patch. Hopefully with a reasonable
amount of comments? I kind of wanted to add a comment to reached_main, but it
just seems to end up restating the variable name...
Greetings,
Andres Freund
>From 641f77e53c86ad7eaa15138984ce39471689b1d9 Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Wed, 23 Mar 2022 09:57:19 -0700
Subject: [PATCH v2 1/3] Add a hacky workaround to make ubsan and ps_status.c
compatible
At least on linux set_ps_display() breaks /proc/$pid/environ. The sanitizer
library uses /proc/$pid/environ to implement getenv() because it wants to work
independent of libc. When just using undefined and alignment sanitizers, the
sanitizer library is only initialized when the first error occurs, by which
time we've often already called set_ps_display(), preventing the sanitizer
libraries from seeing the options.
We can work around that by defining __ubsan_default_options, a weak symbol
libsanitizer uses to get defaults from the application, and return
getenv("UBSAN_OPTIONS"). But only if main already was reached, so that we
don't end up relying on a not-yet-working getenv().
As it's just a function that won't get called when not running a sanitizer, it
doesn't seem necessary to make compilation of the function conditional.
Reviewed-by: Tom Lane <t...@sss.pgh.pa.us>
Discussion: https://postgr.es/m/20220323173537.ll7klrglnp4gn...@alap3.anarazel.de
---
src/backend/main/main.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/src/backend/main/main.c b/src/backend/main/main.c
index f5da4260a13..16bcf045ae6 100644
--- a/src/backend/main/main.c
+++ b/src/backend/main/main.c
@@ -43,6 +43,7 @@
const char *progname;
+static bool reached_main = false;
static void startup_hacks(const char *progname);
@@ -59,6 +60,8 @@ main(int argc, char *argv[])
{
bool do_check_root = true;
+ reached_main = true;
+
/*
* If supported on the current platform, set up a handler to be called if
* the backend/postmaster crashes with a fatal signal or exception.
@@ -421,3 +424,27 @@ check_root(const char *progname)
}
#endif /* WIN32 */
}
+
+/*
+ * At least on linux set_ps_display() breaks /proc/$pid/environ. The sanitizer
+ * library uses /proc/$pid/environ to implement getenv() because it wants to
+ * work independent of libc. When just using undefined and alignment
+ * sanitizers, the sanitizer library is only initialized when the first error
+ * occurs, by which time we've often already called set_ps_display(),
+ * preventing the sanitizer libraries from seeing the options.
+ *
+ * We can work around that by defining __ubsan_default_options, a weak symbol
+ * libsanitizer uses to get defaults from the application, and return
+ * getenv("UBSAN_OPTIONS"). But only if main already was reached, so that we
+ * don't end up relying on a not-yet-working getenv().
+ */
+const char *__ubsan_default_options(void);
+const char *
+__ubsan_default_options(void)
+{
+ /* don't call libc before it's initialized */
+ if (!reached_main)
+ return "";
+
+ return getenv("UBSAN_OPTIONS");
+}
--
2.37.3.542.gdd3f6c4cae
>From e081cd01fba0a860a9cb3c0e4c9525e1c4146e57 Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Thu, 29 Sep 2022 17:44:45 -0700
Subject: [PATCH v2 2/3] ci: use -fsanitize=undefined,alignment in linux tasks
---
.cirrus.yml | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/.cirrus.yml b/.cirrus.yml
index 7b5cb021027..c329a2befe4 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -169,6 +169,19 @@ task:
LINUX_CONFIGURE_FEATURES: *LINUX_CONFIGURE_FEATURES
LINUX_MESON_FEATURES: *LINUX_MESON_FEATURES
+ # Enable a reasonable set of sanitizers. Use the linux task for that, as
+ # it currently is the fastest task. Also several of the sanitizers work
+ # best on linux.
+ # The overhead of alignment sanitizer is low, undefined behaviour has
+ # moderate overhead. address sanitizer howerever is pretty expensive and
+ # thus not enabled.
+ #
+ # disable_coredump=0, abort_on_error=1: for useful backtraces in case of crashes
+ # print_stacktraces=1,verbosity=2, duh
+ # detect_leaks=0: too many uninteresting leak errors in short-lived binaries
+ UBSAN_OPTIONS: print_stacktrace=1:disable_coredump=0:abort_on_error=1:verbosity=2
+ ASAN_OPTIONS: print_stacktrace=1:disable_coredump=0:abort_on_error=1:detect_leaks=0
+
only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*linux.*'
compute_engine_instance:
@@ -230,6 +243,10 @@ task:
- name: Linux - Debian Bullseye - Meson
configure_script: |
+ SANITIZER_FLAGS="-fsanitize=alignment,undefined -fno-sanitize-recover=all -fno-common"
+ CFLAGS="$SANITIZER_FLAGS $CFLAGS"
+ LDFLAGS="$SANITIZER_FLAGS $LDFLAGS"
+
su postgres <<-EOF
meson setup \
--buildtype=debug \
--
2.37.3.542.gdd3f6c4cae
