From ab53efd7767a48e0d48de985d435c5dc0d977c42 Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Wed, 6 Nov 2024 22:05:13 +0000 Subject: [PATCH v2] 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, the Linux documentation advises not to do that, and it's not exactly beautiful. Now that more operating systems expose CPU features to userspace via the ELF loader following the Linux conventions, 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 adopts the same API as FreeBSD and OpenBSD this change should hopefully work there too without change. 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 | 66 +++++++++++-------------------- 5 files changed, 34 insertions(+), 44 deletions(-) diff --git a/configure b/configure index 4b01b682b15..4d08dd59990 100755 --- a/configure +++ b/configure @@ -15041,7 +15041,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 e7f4f0fc22f..b0c3366d5e7 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 5b0510cef78..b64d253fe41 100644 --- a/meson.build +++ b/meson.build @@ -2623,7 +2623,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..147c6533e90 100644 --- a/src/port/pg_crc32c_armv8_choose.c +++ b/src/port/pg_crc32c_armv8_choose.c @@ -24,57 +24,37 @@ #include "postgres_fe.h" #endif -#include -#include +#if defined(HAVE_ELF_AUX_INFO) || defined(HAVE_GETAUXVAL) +#include +#if defined(__linux__) && !defined(__aarch64__) +#include +#endif +#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; +#if defined(HAVE_ELF_AUX_INFO) + unsigned long value; - /* - * 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); - -#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)); +#ifdef __aarch64__ + return elf_aux_info(AT_HWCAP, &value, sizeof(value)) == 0 && + (value & HWCAP_CRC32) != 0; +#else + return elf_aux_info(AT_HWCAP2, &value, sizeof(value)) == 0 && + (value & HWCAP2_CRC32) != 0; +#endif +#elif defined(HAVE_GETAUXVAL) +#ifdef __aarch64__ + return (getauxval(AT_HWCAP) & HWCAP_CRC32) != 0; +#else + return (getauxval(AT_HWCAP2) & HWCAP2_CRC32) != 0; +#endif +#else + return false; #endif - - return (result > 0); } /* -- 2.39.5 (Apple Git-154)