libpq fails to build with TSAN
Hi, so libpq has this line in its Makefile
https://github.com/postgres/postgres/blob/6ee26c6a4bafabbd22a85f575d2446fd5ec6ad0d/src/interfaces/libpq/Makefile#L116
which checks that libpq does not use any "exit" functions. With
ThreadSanitizer it triggers on function `__tsan_func_exit` which is
used to record returns from functions. Should it be added to exclusion
list here?
Error looks like this, just in case:
...
rm -f libpq.so
ln -s libpq.so.5.15 libpq.so
libpq.so.5.15: U __tsan_func_exit
libpq must not be calling any function which invokes exit
make: *** [Makefile:121: libpq-refs-stamp] Error 1
On 31 Jan 2024, at 04:21, Roman Lozko <lozko.roma@gmail.com> wrote:
Hi, so libpq has this line in its Makefile
https://github.com/postgres/postgres/blob/6ee26c6a4bafabbd22a85f575d2446fd5ec6ad0d/src/interfaces/libpq/Makefile#L116
which checks that libpq does not use any "exit" functions. With
ThreadSanitizer it triggers on function `__tsan_func_exit` which is
used to record returns from functions. Should it be added to exclusion
list here?
I think it should, the idea of that check is to catch calls to actual exits,
while this is instrumentation which has nothing to do with exit(2). The
attached diff should be enough to handle this.
--
Daniel Gustafsson
Attachments:
tsan_exit_exclusion.diffapplication/octet-stream; name=tsan_exit_exclusion.diff; x-unix-mode=0644Download
diff --git a/src/interfaces/libpq/Makefile b/src/interfaces/libpq/Makefile
index bfcc7cdde9..083ca6f4cc 100644
--- a/src/interfaces/libpq/Makefile
+++ b/src/interfaces/libpq/Makefile
@@ -105,7 +105,9 @@ backend_src = $(top_srcdir)/src/backend
# build toolchains insert abort() calls, e.g. to implement assert().)
# If nm doesn't exist or doesn't work on shlibs, this test will do nothing,
# which is fine. The exclusion of __cxa_atexit is necessary on OpenBSD,
-# which seems to insert references to that even in pure C code.
+# which seems to insert references to that even in pure C code. Excluding
+# __tsan_func_exit is necessary when using ThreadSanitizer data race detector
+# which use this function for instrumentation of function exit.
# Skip the test when profiling, as gcc may insert exit() calls for that.
# Also skip the test on platforms where libpq infrastructure may be provided
# by statically-linked libraries, as we can't expect them to honor this
@@ -113,7 +115,7 @@ backend_src = $(top_srcdir)/src/backend
libpq-refs-stamp: $(shlib)
ifneq ($(enable_coverage), yes)
ifeq (,$(filter aix solaris,$(PORTNAME)))
- @if nm -A -u $< 2>/dev/null | grep -v __cxa_atexit | grep exit; then \
+ @if nm -A -u $< 2>/dev/null | grep -v -e __cxa_atexit -e __tsan_func_exit | grep exit; then \
echo 'libpq must not be calling any function which invokes exit'; exit 1; \
fi
endif
Daniel Gustafsson <daniel@yesql.se> writes:
I think it should, the idea of that check is to catch calls to actual exits,
while this is instrumentation which has nothing to do with exit(2). The
attached diff should be enough to handle this.
+1
regards, tom lane
On 31 Jan 2024, at 16:39, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Daniel Gustafsson <daniel@yesql.se> writes:
I think it should, the idea of that check is to catch calls to actual exits,
while this is instrumentation which has nothing to do with exit(2). The
attached diff should be enough to handle this.+1
Pushed. It can be argued that it should be backpatched, but for now I've only
pushed it to master. If there are strong opinions on backpatching I'd be happy
to fix that.
--
Daniel Gustafsson