[PATCH v2 1/1] Fix detection of pwritev support for OSX.
Fixes:
gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Werror=vla -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -Wno-unused-command-line-argument -O2 -I../../../../src/include -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.1.sdk -c -o fd.o fd.c
fd.c:3661:10: warning: 'pwritev' is only available on macOS 11.0 or newer [-Wunguarded-availability-new]
part = pg_pwritev(fd, iov, iovcnt, offset);
^~~~~~~~~~
../../../../src/include/port/pg_iovec.h:49:20: note: expanded from macro 'pg_pwritev'
^~~~~~~
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX11.1.sdk/usr/include/sys/uio.h:104:9: note: 'pwritev' has been marked as being introduced in macOS 11.0
here, but the deployment target is macOS 10.15.0
ssize_t pwritev(int, const struct iovec *, int, off_t) __DARWIN_NOCANCEL(pwritev) __API_AVAILABLE(macos(11.0), ios(14.0), watchos(7.0), tvos(14.0));
^
fd.c:3661:10: note: enclose 'pwritev' in a __builtin_available check to silence this warning
part = pg_pwritev(fd, iov, iovcnt, offset);
^~~~~~~~~~
../../../../src/include/port/pg_iovec.h:49:20: note: expanded from macro 'pg_pwritev'
^~~~~~~
1 warning generated.
This results in a runtime error:
running bootstrap script ... dyld: lazy symbol binding failed: Symbol not found: _pwritev
Referenced from: /usr/local/pgsql/bin/postgres
Expected in: /usr/lib/libSystem.B.dylib
dyld: Symbol not found: _pwritev
Referenced from: /usr/local/pgsql/bin/postgres
Expected in: /usr/lib/libSystem.B.dylib
child process was terminated by signal 6: Abort trap: 6
To fix this we set -Werror=unguarded-availability-new so that a compile
test for pwritev will fail if the symbol is unavailable on the requested
SDK version.
---
Changes v1 -> v2:
- Add AC_LIBOBJ(pwritev) when pwritev not available
- set -Werror=unguarded-availability-new for CXX flags as well
---
configure | 145 ++++++++++++++++++++++++++++++++++++++++++++++-----
configure.ac | 21 +++++++-
2 files changed, 152 insertions(+), 14 deletions(-)
diff --git a/configure b/configure
index 8af4b99021..662b0ae9ce 100755
--- a/configure
+++ b/configure
@@ -5373,6 +5373,98 @@ if test x"$pgac_cv_prog_CC_cflags__Werror_vla" = x"yes"; then
fi+ # Prevent usage of symbols marked as newer than our target.
+
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CC} supports -Werror=unguarded-availability-new, for CFLAGS" >&5
+$as_echo_n "checking whether ${CC} supports -Werror=unguarded-availability-new, for CFLAGS... " >&6; }
+if ${pgac_cv_prog_CC_cflags__Werror_unguarded_availability_new+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ pgac_save_CFLAGS=$CFLAGS
+pgac_save_CC=$CC
+CC=${CC}
+CFLAGS="${CFLAGS} -Werror=unguarded-availability-new"
+ac_save_c_werror_flag=$ac_c_werror_flag
+ac_c_werror_flag=yes
+cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h. */
+
+int
+main ()
+{
+
+ ;
+ return 0;
+}
+_ACEOF
+if ac_fn_c_try_compile "$LINENO"; then :
+ pgac_cv_prog_CC_cflags__Werror_unguarded_availability_new=yes
+else
+ pgac_cv_prog_CC_cflags__Werror_unguarded_availability_new=no
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+ac_c_werror_flag=$ac_save_c_werror_flag
+CFLAGS="$pgac_save_CFLAGS"
+CC="$pgac_save_CC"
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_prog_CC_cflags__Werror_unguarded_availability_new" >&5
+$as_echo "$pgac_cv_prog_CC_cflags__Werror_unguarded_availability_new" >&6; }
+if test x"$pgac_cv_prog_CC_cflags__Werror_unguarded_availability_new" = x"yes"; then
+ CFLAGS="${CFLAGS} -Werror=unguarded-availability-new"
+fi
+
+
+ { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CXX} supports -Werror=unguarded-availability-new, for CXXFLAGS" >&5
+$as_echo_n "checking whether ${CXX} supports -Werror=unguarded-availability-new, for CXXFLAGS... " >&6; }
+if ${pgac_cv_prog_CXX_cxxflags__Werror_unguarded_availability_new+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ pgac_save_CXXFLAGS=$CXXFLAGS
+pgac_save_CXX=$CXX
+CXX=${CXX}
+CXXFLAGS="${CXXFLAGS} -Werror=unguarded-availability-new"
+ac_save_cxx_werror_flag=$ac_cxx_werror_flag
+ac_cxx_werror_flag=yes
+ac_ext=cpp
+ac_cpp='$CXXCPP $CPPFLAGS'
+ac_compile='$CXX -c $CXXFLAGS $CPPFLAGS conftest.$ac_ext >&5'
+ac_link='$CXX -o conftest$ac_exeext $CXXFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS >&5'
+ac_compiler_gnu=$ac_cv_cxx_compiler_gnu
+
+cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h. */
+
+int
+main ()
+{
+
+ ;
+ return 0;
+}
+_ACEOF
+if ac_fn_cxx_try_compile "$LINENO"; then :
+ pgac_cv_prog_CXX_cxxflags__Werror_unguarded_availability_new=yes
+else
+ pgac_cv_prog_CXX_cxxflags__Werror_unguarded_availability_new=no
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+ac_ext=c
+ac_cpp='$CPP $CPPFLAGS'
+ac_compile='$CC -c $CFLAGS $CPPFLAGS conftest.$ac_ext >&5'
+ac_link='$CC -o conftest$ac_exeext $CFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS >&5'
+ac_compiler_gnu=$ac_cv_c_compiler_gnu
+
+ac_cxx_werror_flag=$ac_save_cxx_werror_flag
+CXXFLAGS="$pgac_save_CXXFLAGS"
+CXX="$pgac_save_CXX"
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_prog_CXX_cxxflags__Werror_unguarded_availability_new" >&5
+$as_echo "$pgac_cv_prog_CXX_cxxflags__Werror_unguarded_availability_new" >&6; }
+if test x"$pgac_cv_prog_CXX_cxxflags__Werror_unguarded_availability_new" = x"yes"; then
+ CXXFLAGS="${CXXFLAGS} -Werror=unguarded-availability-new"
+fi
+
+
# -Wvla is not applicable for C++ { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CC} supports -Wendif-labels, for CFLAGS" >&5
@@ -15715,6 +15807,46 @@ $as_echo "#define HAVE_PS_STRINGS 1" >>confdefs.h
fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for pwritev" >&5
+$as_echo_n "checking for pwritev... " >&6; }
+cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h. */
+#ifdef HAVE_SYS_TYPES_H
+#include <sys/types.h>
+#endif
+#ifdef HAVE_SYS_UIO_H
+#include <sys/uio.h>
+#endif
+int
+main ()
+{
+struct iovec *iov;
+off_t offset;
+offset = 0;
+pwritev(0, iov, 0, offset);
+
+ ;
+ return 0;
+}
+_ACEOF
+if ac_fn_c_try_compile "$LINENO"; then :
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: yes" >&5
+$as_echo "yes" >&6; }
+
+$as_echo "#define HAVE_PWRITEV 1" >>confdefs.h
+
+else
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
+$as_echo "no" >&6; }
+case " $LIBOBJS " in
+ *" pwritev.$ac_objext "* ) ;;
+ *) LIBOBJS="$LIBOBJS pwritev.$ac_objext"
+ ;;
+esac
+
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+
ac_fn_c_check_func "$LINENO" "dlopen" "ac_cv_func_dlopen"
if test "x$ac_cv_func_dlopen" = xyes; then :
$as_echo "#define HAVE_DLOPEN 1" >>confdefs.h
@@ -15871,19 +16003,6 @@ esacfi
-ac_fn_c_check_func "$LINENO" "pwritev" "ac_cv_func_pwritev"
-if test "x$ac_cv_func_pwritev" = xyes; then :
- $as_echo "#define HAVE_PWRITEV 1" >>confdefs.h
-
-else
- case " $LIBOBJS " in
- *" pwritev.$ac_objext "* ) ;;
- *) LIBOBJS="$LIBOBJS pwritev.$ac_objext"
- ;;
-esac
-
-fi
-
ac_fn_c_check_func "$LINENO" "random" "ac_cv_func_random"
if test "x$ac_cv_func_random" = xyes; then :
$as_echo "#define HAVE_RANDOM 1" >>confdefs.h
diff --git a/configure.ac b/configure.ac
index 868a94c9ba..724881a7f0 100644
--- a/configure.ac
+++ b/configure.ac
@@ -494,6 +494,9 @@ if test "$GCC" = yes -a "$ICC" = no; then
AC_SUBST(PERMIT_DECLARATION_AFTER_STATEMENT)
# Really don't want VLAs to be used in our dialect of C
PGAC_PROG_CC_CFLAGS_OPT([-Werror=vla])
+ # Prevent usage of symbols marked as newer than our target.
+ PGAC_PROG_CC_CFLAGS_OPT([-Werror=unguarded-availability-new])
+ PGAC_PROG_CXX_CFLAGS_OPT([-Werror=unguarded-availability-new])
# -Wvla is not applicable for C++
PGAC_PROG_CC_CFLAGS_OPT([-Wendif-labels])
PGAC_PROG_CXX_CFLAGS_OPT([-Wendif-labels])
@@ -1726,6 +1729,23 @@ if test "$pgac_cv_var_PS_STRINGS" = yes ; then
AC_DEFINE([HAVE_PS_STRINGS], 1, [Define to 1 if the PS_STRINGS thing exists.])
fi+AC_MSG_CHECKING([for pwritev])
+AC_COMPILE_IFELSE([AC_LANG_PROGRAM(
+[#ifdef HAVE_SYS_TYPES_H
+#include <sys/types.h>
+#endif
+#ifdef HAVE_SYS_UIO_H
+#include <sys/uio.h>
+#endif],
+[struct iovec *iov;
+off_t offset;
+offset = 0;
+pwritev(0, iov, 0, offset);
+])], [AC_MSG_RESULT(yes)
+AC_DEFINE([HAVE_PWRITEV], 1, [Define to 1 if you have the `pwritev' function.])],
+[AC_MSG_RESULT(no)
+AC_LIBOBJ(pwritev)])
+
AC_REPLACE_FUNCS(m4_normalize([
dlopen
explicit_bzero
@@ -1739,7 +1759,6 @@ AC_REPLACE_FUNCS(m4_normalize([
pread
preadv
pwrite
- pwritev
random
srandom
strlcat
--
2.30.0
James Hilliard <james.hilliard1@gmail.com> writes:
Fixes:
fd.c:3661:10: warning: 'pwritev' is only available on macOS 11.0 or newer [-Wunguarded-availability-new]
It's still missing preadv, and it still has nonzero chance of breaking
successful detection of pwritev on platforms other than yours, and it's
still really ugly.
But the main reason I don't want to go this way is that I don't think
it'll stop with preadv/pwritev. If we make it our job to build
successfully even when using the wrong SDK version for the target
platform, we're going to be in for more and more pain with other
kernel APIs.
We could, of course, do what Apple wants us to do and try to build
executables that work across versions. I do not intend to put up
with the sort of invasive, error-prone source-code-level runtime test
they recommend ... but given that there is weak linking involved here,
I wonder if there is a way to silently sub in src/port/pwritev.c
when executing on a pre-11 macOS, by dint of marking it a weak
symbol?
regards, tom lane
On Tue, Jan 19, 2021 at 10:29 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
James Hilliard <james.hilliard1@gmail.com> writes:
Fixes:
fd.c:3661:10: warning: 'pwritev' is only available on macOS 11.0 or newer [-Wunguarded-availability-new]It's still missing preadv, and it still has nonzero chance of breaking
successful detection of pwritev on platforms other than yours, and it's
still really ugly.
Setting -Werror=unguarded-availability-new should in theory always
ensure that configure checks fail if the symbol is unavailable or marked
as requiring a target newer than the MACOSX_DEPLOYMENT_TARGET.
But the main reason I don't want to go this way is that I don't think
it'll stop with preadv/pwritev. If we make it our job to build
successfully even when using the wrong SDK version for the target
platform, we're going to be in for more and more pain with other
kernel APIs.
This issue really has nothing to do with the SDK version at all, it's the
MACOSX_DEPLOYMENT_TARGET that matters which must be taken
into account during configure in some way, this is what my patch does
by triggering the pwritev compile test error by setting
-Werror=unguarded-availability-new.
It's expected that MACOSX_DEPLOYMENT_TARGET=10.15 with a
MacOSX11.1.sdk will produce a binary that can run on OSX 10.15.
The MacOSX11.1.sdk is not the wrong SDK for a 10.15 target and
is fully capable of producing 10.15 compatible binaries.
We could, of course, do what Apple wants us to do and try to build
executables that work across versions. I do not intend to put up
with the sort of invasive, error-prone source-code-level runtime test
they recommend ... but given that there is weak linking involved here,
I wonder if there is a way to silently sub in src/port/pwritev.c
when executing on a pre-11 macOS, by dint of marking it a weak
symbol?
The check I added is strictly a compile time check still, not runtime.
I also don't think this is a weak symbol.
From the header file it is not have __attribute__((weak_import)):
ssize_t pwritev(int, const struct iovec *, int, off_t)
__DARWIN_NOCANCEL(pwritev) __API_AVAILABLE(macos(11.0), ios(14.0),
watchos(7.0), tvos(14.0));
Show quoted text
regards, tom lane
James Hilliard <james.hilliard1@gmail.com> writes:
I also don't think this is a weak symbol.
From the header file it is not have __attribute__((weak_import)):
ssize_t pwritev(int, const struct iovec *, int, off_t)
__DARWIN_NOCANCEL(pwritev) __API_AVAILABLE(macos(11.0), ios(14.0),
watchos(7.0), tvos(14.0));
See the other thread. I found by looking at the asm output that
what __API_AVAILABLE actually does is cause the compiler to emit
a ".weak_reference" directive when calling a function it thinks
might not be available. So there's some sort of weak linking
going on, though it's certainly possible that it's not shaped
in a way that'd help us do this the way we'd prefer.
regards, tom lane