On Wed, Jul 14, 2021 at 4:08 PM Thomas Munro <thomas.mu...@gmail.com> wrote: > On Wed, Jul 14, 2021 at 6:17 AM Tom Lane <t...@sss.pgh.pa.us> wrote: > > (I suppose a hacky solution might be to never define USE_SIGWAIT > > on Solaris.) > > Yeah I was thinking about that. I've just got a shell on an illumos > VM and will try a couple of ideas out if I can remember how to drive > this thing... more soon.
I decided to try to make it work properly on that OS instead of the hacky solution now that I have access. I took your last patch, moved -D_POSIX_PTHREAD_SEMANTICS into CFLAGS in src/template/solaris, removed the special flags and libs from the new configure check where you had them, and then used HAVE_POSIX_SIGWAIT directly without the c.h change. I can't find any evidence on the 'net that any other OS cares about _POSIX_PTHREAD_SEMANTICS (every reference says it's a Solaris thing) so this should just remove some useless clutter, and it's not like we want POSIX draft 6 behaviour anywhere. (It's a bit weird to define a macro that has PTHREAD in the name when building things that aren't multi-threaded, but that was their choice). Do you see any problems with this approach? The main thing I can think of is that people who are using a custom CFLAGS on that OS will need to update the value they're using, but that seems OK (they'll likely see the new error and be alerted to the change). Another thing I noticed is that config/ax_pthread.m4 (something from autoconf that we don't want to modify) will add another copy of -D_POSIX_PTHREAD_SEMANTICS, but that's already the case.
From 899621ea9131ad5819dbf86c363f51b6f60fb332 Mon Sep 17 00:00:00 2001 From: Thomas Munro <thomas.mu...@gmail.com> Date: Wed, 14 Jul 2021 10:32:15 +0000 Subject: [PATCH v3 1/2] Portability fixes for sigwait. Build farm animals running ancient HPUX and Solaris have a non-standard sigwait() from draft versions of POSIX, so they didn't like commit 7c09d279. To avoid the problem in general, only try to use sigwait() if it's declared by <signal.h> and matches the expected declaration. To select the modern declaration on Solaris (even in non-threaded programs), move -D_POSIX_PTHREAD_SEMANTICS from PTHREAD_CFLAGS into CFLAGS on Solaris only. Also fix the error checking. Modern sigwait() doesn't set errno. Thanks to Tom Lane for help with this. Discussion: https://postgr.es/m/3187588.1626136248%40sss.pgh.pa.us --- config/thread_test.c | 4 --- configure | 70 ++++++++++++++++++++++++++++++++++---- configure.ac | 33 ++++++++++++++++-- src/bin/psql/command.c | 13 +++---- src/bin/psql/startup.c | 4 +-- src/include/pg_config.h.in | 7 ++++ src/template/solaris | 5 ++- src/tools/msvc/Solution.pm | 2 ++ 8 files changed, 116 insertions(+), 22 deletions(-) diff --git a/config/thread_test.c b/config/thread_test.c index 784f4fe8ce..e2a9e62f49 100644 --- a/config/thread_test.c +++ b/config/thread_test.c @@ -43,10 +43,6 @@ #include <winsock2.h> #endif -/* Test for POSIX.1c 2-arg sigwait() and fail on single-arg version */ -#include <signal.h> -int sigwait(const sigset_t *set, int *sig); - #define TEMP_FILENAME_1 "thread_test.1" #define TEMP_FILENAME_2 "thread_test.2" diff --git a/configure b/configure index 1ea28a0d67..e6c85d48e3 100755 --- a/configure +++ b/configure @@ -11296,9 +11296,8 @@ ac_compiler_gnu=$ac_cv_c_compiler_gnu # set thread flags # Some platforms use these, so just define them. They can't hurt if they -# are not supported. For example, on Solaris -D_POSIX_PTHREAD_SEMANTICS -# enables 5-arg getpwuid_r, among other things. -PTHREAD_CFLAGS="$PTHREAD_CFLAGS -D_REENTRANT -D_THREAD_SAFE -D_POSIX_PTHREAD_SEMANTICS" +# are not supported. +PTHREAD_CFLAGS="$PTHREAD_CFLAGS -D_REENTRANT -D_THREAD_SAFE" # Check for *_r functions _CFLAGS="$CFLAGS" @@ -15861,9 +15860,11 @@ $as_echo "#define HAVE_FSEEKO 1" >>confdefs.h fi -# posix_fadvise() is a no-op on Solaris, so don't incur function overhead -# by calling it, 2009-04-02 -# http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/lib/libc/port/gen/posix_fadvise.c +# Make sure there's a declaration for sigwait(), then make sure +# that it conforms to the POSIX standard (there seem to still be +# some platforms out there with pre-POSIX sigwait()). On Solaris, +# _POSIX_PTHREAD_SEMANTICS affects the result, but we defined it +# in CFLAGS. # The Clang compiler raises a warning for an undeclared identifier that matches # a compiler builtin function. All extant Clang versions are affected, as of # Clang 3.6.0. Test a builtin known to every version. This problem affects the @@ -15952,6 +15953,63 @@ case $ac_cv_c_decl_report in *) ac_c_decl_warn_flag= ;; esac +ac_fn_c_check_decl "$LINENO" "sigwait" "ac_cv_have_decl_sigwait" "#include <signal.h> +" +if test "x$ac_cv_have_decl_sigwait" = xyes; then : + ac_have_decl=1 +else + ac_have_decl=0 +fi + +cat >>confdefs.h <<_ACEOF +#define HAVE_DECL_SIGWAIT $ac_have_decl +_ACEOF + +if test "x$ac_cv_have_decl_sigwait" = xyes; then + { $as_echo "$as_me:${as_lineno-$LINENO}: checking for POSIX-conforming sigwait declaration" >&5 +$as_echo_n "checking for POSIX-conforming sigwait declaration... " >&6; } +if ${pgac_cv_have_posix_decl_sigwait+:} false; then : + $as_echo_n "(cached) " >&6 +else + cat confdefs.h - <<_ACEOF >conftest.$ac_ext +/* end confdefs.h. */ + + #include <signal.h> + int sigwait(const sigset_t *set, int *sig); + +int +main () +{ + + ; + return 0; +} +_ACEOF +if ac_fn_c_try_compile "$LINENO"; then : + pgac_cv_have_posix_decl_sigwait=yes +else + pgac_cv_have_posix_decl_sigwait=no +fi +rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext + +fi +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_have_posix_decl_sigwait" >&5 +$as_echo "$pgac_cv_have_posix_decl_sigwait" >&6; } +fi +if test "x$pgac_cv_have_posix_decl_sigwait" = xyes; then + +$as_echo "#define HAVE_POSIX_DECL_SIGWAIT 1" >>confdefs.h + +else + # On non-Windows, libpq requires POSIX sigwait() for thread safety. + if test "$enable_thread_safety" = yes -a "$PORTNAME" != "win32"; then + as_fn_error $? "POSIX-conforming sigwait is required to enable thread safety." "$LINENO" 5 + fi +fi + +# posix_fadvise() is a no-op on Solaris, so don't incur function overhead +# by calling it, 2009-04-02 +# http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/lib/libc/port/gen/posix_fadvise.c if test "$PORTNAME" != "solaris"; then : for ac_func in posix_fadvise diff --git a/configure.ac b/configure.ac index 57336e1fb6..dad2bbf0d8 100644 --- a/configure.ac +++ b/configure.ac @@ -1122,9 +1122,8 @@ AS_IF([test "$enable_thread_safety" = yes -a "$PORTNAME" != "win32"], AX_PTHREAD # set thread flags # Some platforms use these, so just define them. They can't hurt if they -# are not supported. For example, on Solaris -D_POSIX_PTHREAD_SEMANTICS -# enables 5-arg getpwuid_r, among other things. -PTHREAD_CFLAGS="$PTHREAD_CFLAGS -D_REENTRANT -D_THREAD_SAFE -D_POSIX_PTHREAD_SEMANTICS" +# are not supported. +PTHREAD_CFLAGS="$PTHREAD_CFLAGS -D_REENTRANT -D_THREAD_SAFE" # Check for *_r functions _CFLAGS="$CFLAGS" @@ -1741,6 +1740,34 @@ PGAC_CHECK_BUILTIN_FUNC([__builtin_popcount], [unsigned int x]) # in case it finds that _LARGEFILE_SOURCE has to be #define'd for that. AC_FUNC_FSEEKO +# Make sure there's a declaration for sigwait(), then make sure +# that it conforms to the POSIX standard (there seem to still be +# some platforms out there with pre-POSIX sigwait()). On Solaris, +# _POSIX_PTHREAD_SEMANTICS affects the result, but we defined it +# in CFLAGS. +AC_CHECK_DECLS(sigwait, [], [], [#include <signal.h>]) +if test "x$ac_cv_have_decl_sigwait" = xyes; then + AC_CACHE_CHECK([for POSIX-conforming sigwait declaration], + [pgac_cv_have_posix_decl_sigwait], + [AC_COMPILE_IFELSE([AC_LANG_PROGRAM([ + #include <signal.h> + int sigwait(const sigset_t *set, int *sig); + ], + [])], + [pgac_cv_have_posix_decl_sigwait=yes], + [pgac_cv_have_posix_decl_sigwait=no]) + ]) +fi +if test "x$pgac_cv_have_posix_decl_sigwait" = xyes; then + AC_DEFINE(HAVE_POSIX_DECL_SIGWAIT, 1, + [Define to 1 if you have a POSIX-conforming sigwait declaration.]) +else + # On non-Windows, libpq requires POSIX sigwait() for thread safety. + if test "$enable_thread_safety" = yes -a "$PORTNAME" != "win32"; then + AC_MSG_ERROR([POSIX-conforming sigwait is required to enable thread safety.]) + fi +fi + # posix_fadvise() is a no-op on Solaris, so don't incur function overhead # by calling it, 2009-04-02 # http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/lib/libc/port/gen/posix_fadvise.c diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c index d704c4220c..49d4c0e3ce 100644 --- a/src/bin/psql/command.c +++ b/src/bin/psql/command.c @@ -4899,7 +4899,7 @@ do_watch(PQExpBuffer query_buf, double sleep) FILE *pagerpipe = NULL; int title_len; int res = 0; -#ifndef WIN32 +#ifdef HAVE_POSIX_DECL_SIGWAIT sigset_t sigalrm_sigchld_sigint; sigset_t sigalrm_sigchld; sigset_t sigint; @@ -4913,7 +4913,7 @@ do_watch(PQExpBuffer query_buf, double sleep) return false; } -#ifndef WIN32 +#ifdef HAVE_POSIX_DECL_SIGWAIT sigemptyset(&sigalrm_sigchld_sigint); sigaddset(&sigalrm_sigchld_sigint, SIGCHLD); sigaddset(&sigalrm_sigchld_sigint, SIGALRM); @@ -4952,7 +4952,7 @@ do_watch(PQExpBuffer query_buf, double sleep) * PAGER environment variables, because traditional pagers probably won't * be very useful for showing a stream of results. */ -#ifndef WIN32 +#ifdef HAVE_POSIX_DECL_SIGWAIT pagerprog = getenv("PSQL_WATCH_PAGER"); #endif if (pagerprog && myopt.topt.pager) @@ -5023,7 +5023,7 @@ do_watch(PQExpBuffer query_buf, double sleep) if (pagerpipe && ferror(pagerpipe)) break; -#ifdef WIN32 +#ifndef HAVE_POSIX_DECL_SIGWAIT /* * Set up cancellation of 'watch' via SIGINT. We redo this each time @@ -5059,7 +5059,8 @@ do_watch(PQExpBuffer query_buf, double sleep) { int signal_received; - if (sigwait(&sigalrm_sigchld_sigint, &signal_received) < 0) + errno = sigwait(&sigalrm_sigchld_sigint, &signal_received); + if (errno != 0) { /* Some other signal arrived? */ if (errno == EINTR) @@ -5091,7 +5092,7 @@ do_watch(PQExpBuffer query_buf, double sleep) restore_sigpipe_trap(); } -#ifndef WIN32 +#ifdef HAVE_POSIX_DECL_SIGWAIT /* Disable the interval timer. */ memset(&interval, 0, sizeof(interval)); setitimer(ITIMER_REAL, &interval, NULL); diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c index 5f36f0d1c6..2931530f33 100644 --- a/src/bin/psql/startup.c +++ b/src/bin/psql/startup.c @@ -110,7 +110,7 @@ log_locus_callback(const char **filename, uint64 *lineno) } } -#ifndef WIN32 +#ifdef HAVE_POSIX_DECL_SIGWAIT static void empty_signal_handler(SIGNAL_ARGS) { @@ -309,7 +309,7 @@ main(int argc, char *argv[]) psql_setup_cancel_handler(); -#ifndef WIN32 +#ifdef HAVE_POSIX_DECL_SIGWAIT /* * do_watch() needs signal handlers installed (otherwise sigwait() will diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in index d69d461ff2..15ffdd895a 100644 --- a/src/include/pg_config.h.in +++ b/src/include/pg_config.h.in @@ -158,6 +158,10 @@ don't. */ #undef HAVE_DECL_RTLD_NOW +/* Define to 1 if you have the declaration of `sigwait', and to 0 if you + don't. */ +#undef HAVE_DECL_SIGWAIT + /* Define to 1 if you have the declaration of `strlcat', and to 0 if you don't. */ #undef HAVE_DECL_STRLCAT @@ -414,6 +418,9 @@ /* Define to 1 if you have the <poll.h> header file. */ #undef HAVE_POLL_H +/* Define to 1 if you have a POSIX-conforming sigwait declaration. */ +#undef HAVE_POSIX_DECL_SIGWAIT + /* Define to 1 if you have the `posix_fadvise' function. */ #undef HAVE_POSIX_FADVISE diff --git a/src/template/solaris b/src/template/solaris index f88b1cdad3..28a16ba8a9 100644 --- a/src/template/solaris +++ b/src/template/solaris @@ -7,9 +7,12 @@ else CFLAGS_SL="-KPIC" fi +# Select conforming versions of many interfaces (sigwait, getpwuid_r, ...). +CFLAGS="-D_POSIX_PTHREAD_SEMANTICS" + if test "$SUN_STUDIO_CC" = yes ; then CC="$CC -Xa" # relaxed ISO C mode - CFLAGS="-v" # -v is like gcc -Wall + CFLAGS="$CFLAGS -v" # -v is like gcc -Wall if test "$enable_debug" != yes; then CFLAGS="$CFLAGS -O" # any optimization breaks debug fi diff --git a/src/tools/msvc/Solution.pm b/src/tools/msvc/Solution.pm index 294b968dcd..c967743467 100644 --- a/src/tools/msvc/Solution.pm +++ b/src/tools/msvc/Solution.pm @@ -249,6 +249,7 @@ sub GenerateFiles HAVE_DECL_PWRITEV => 0, HAVE_DECL_RTLD_GLOBAL => 0, HAVE_DECL_RTLD_NOW => 0, + HAVE_DECL_SIGWAIT => 0, HAVE_DECL_STRLCAT => undef, HAVE_DECL_STRLCPY => undef, HAVE_DECL_STRNLEN => 1, @@ -332,6 +333,7 @@ sub GenerateFiles HAVE_PAM_PAM_APPL_H => undef, HAVE_POLL => undef, HAVE_POLL_H => undef, + HAVE_POSIX_DECL_SIGWAIT => undef, HAVE_POSIX_FADVISE => undef, HAVE_POSIX_FALLOCATE => undef, HAVE_PPC_LWARX_MUTEX_HINT => undef, -- 2.30.2
