From 871dd92dccc4ba90977c2af647d103d50a9e5319 Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Wed, 6 Nov 2024 22:05:13 +0000 Subject: [PATCH] Use auxv to check for CRC32 instructions on ARM. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously we probed for CRC32 instructions by testing if they raised SIGILL. Some have expressed doubts about that technique, and it's not exactly beautiful. Now that several operating systems expose the contents of the feature register to user space (something like x86's CPUID, but not accessible without kernel help on ARM), let's use that instead. This is expected to work on Linux, FreeBSD and OpenBSD. For macOS we don't use this code at all, because its compiler already assumes a higher -march level by default and our configure tests detect that and use CRC32 builtins directly without probing first; therefore we don't bother to implement the macOS way of probing. For NetBSD, CRC32 instruction probing is lost with this commit when using a conservative -march setting. Patches are welcome, but if NetBSD ever adopts the same API as FreeBSD and OpenBSD this change should hopefully just work there too. Suggested-by: Bastien Roucariès Discussion: https://postgr.es/m/4496616.iHFcN1HehY%40portable-bastien --- configure | 2 +- configure.ac | 2 ++ meson.build | 2 ++ src/include/pg_config.h.in | 6 ++++ src/port/pg_crc32c_armv8_choose.c | 54 +++++++------------------------ 5 files changed, 22 insertions(+), 44 deletions(-) diff --git a/configure b/configure index 6e256b417b9..1e4c6f3d5cf 100755 --- a/configure +++ b/configure @@ -15044,7 +15044,7 @@ fi LIBS_including_readline="$LIBS" LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'` -for ac_func in backtrace_symbols copyfile copy_file_range getifaddrs getpeerucred inet_pton kqueue mbstowcs_l memset_s posix_fallocate ppoll pthread_is_threaded_np setproctitle setproctitle_fast strchrnul strsignal syncfs sync_file_range uselocale wcstombs_l +for ac_func in backtrace_symbols copyfile copy_file_range elf_aux_info getauxval getifaddrs getpeerucred inet_pton kqueue mbstowcs_l memset_s posix_fallocate ppoll pthread_is_threaded_np setproctitle setproctitle_fast strchrnul strsignal syncfs sync_file_range uselocale wcstombs_l do : as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh` ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var" diff --git a/configure.ac b/configure.ac index 3992694dacc..1bc19af7dff 100644 --- a/configure.ac +++ b/configure.ac @@ -1717,6 +1717,8 @@ AC_CHECK_FUNCS(m4_normalize([ backtrace_symbols copyfile copy_file_range + elf_aux_info + getauxval getifaddrs getpeerucred inet_pton diff --git a/meson.build b/meson.build index 9a98f0c86a0..d3ac84d72fd 100644 --- a/meson.build +++ b/meson.build @@ -2628,7 +2628,9 @@ func_checks = [ # when enabling asan the dlopen check doesn't notice that -ldl is actually # required. Just checking for dlsym() ought to suffice. ['dlsym', {'dependencies': [dl_dep], 'define': false}], + ['elf_aux_info'], ['explicit_bzero'], + ['getauxval'], ['getifaddrs'], ['getopt', {'dependencies': [getopt_dep, gnugetopt_dep], 'skip': always_replace_getopt}], ['getopt_long', {'dependencies': [getopt_dep, gnugetopt_dep], 'skip': always_replace_getopt_long}], diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in index cdd9a6e9355..a903c60a3a0 100644 --- a/src/include/pg_config.h.in +++ b/src/include/pg_config.h.in @@ -125,6 +125,9 @@ /* Define to 1 if you have the header file. */ #undef HAVE_EDITLINE_READLINE_H +/* Define to 1 if you have the `elf_aux_info' function. */ +#undef HAVE_ELF_AUX_INFO + /* Define to 1 if you have the header file. */ #undef HAVE_EXECINFO_H @@ -154,6 +157,9 @@ */ #undef HAVE_GCC__SYNC_INT64_CAS +/* Define to 1 if you have the `getauxval' function. */ +#undef HAVE_GETAUXVAL + /* Define to 1 if you have the `getifaddrs' function. */ #undef HAVE_GETIFADDRS diff --git a/src/port/pg_crc32c_armv8_choose.c b/src/port/pg_crc32c_armv8_choose.c index eaf39cf3e81..fff926b13a7 100644 --- a/src/port/pg_crc32c_armv8_choose.c +++ b/src/port/pg_crc32c_armv8_choose.c @@ -24,57 +24,25 @@ #include "postgres_fe.h" #endif -#include -#include +#if defined(HAVE_ELF_AUX_INFO) || defined(HAVE_GETAUXVAL) +#include +#endif #include "port/pg_crc32c.h" - -static sigjmp_buf illegal_instruction_jump; - -/* - * Probe by trying to execute pg_comp_crc32c_armv8(). If the instruction - * isn't available, we expect to get SIGILL, which we can trap. - */ -static void -illegal_instruction_handler(SIGNAL_ARGS) -{ - siglongjmp(illegal_instruction_jump, 1); -} - static bool pg_crc32c_armv8_available(void) { - uint64 data = 42; - int result; - - /* - * Be careful not to do anything that might throw an error while we have - * the SIGILL handler set to a nonstandard value. - */ - pqsignal(SIGILL, illegal_instruction_handler); - if (sigsetjmp(illegal_instruction_jump, 1) == 0) - { - /* Rather than hard-wiring an expected result, compare to SB8 code */ - result = (pg_comp_crc32c_armv8(0, &data, sizeof(data)) == - pg_comp_crc32c_sb8(0, &data, sizeof(data))); - } - else - { - /* We got the SIGILL trap */ - result = -1; - } - pqsignal(SIGILL, SIG_DFL); +#if defined(HAVE_ELF_AUX_INFO) + unsigned long value; -#ifndef FRONTEND - /* We don't expect this case, so complain loudly */ - if (result == 0) - elog(ERROR, "crc32 hardware and software results disagree"); - - elog(DEBUG1, "using armv8 crc32 hardware = %d", (result > 0)); + return elf_aux_info(AT_HWCAP, &value, sizeof(value)) == 0 && + (value & HWCAP_CRC32) != 0; +#elif defined(HAVE_GETAUXVAL) + return (getauxval(AT_HWCAP) & HWCAP_CRC32) != 0; +#else + return false; #endif - - return (result > 0); } /* -- 2.46.2