From fd679ecb827f3a38e5580dfee58697bbaec977be Mon Sep 17 00:00:00 2001 From: Cong Ma Date: Wed, 24 Mar 2021 15:03:37 +0200 Subject: [PATCH] Properly implement Py_UNREACHABLE macro using autoconf. The problem ----------- Definition of the ``Py_UNREACHABLE()`` macro relied on testing compiler versions in preprocessor directives. This is unreliable chiefly because compilers regularly masquerade as each other nowadays. The current implementation tests for GCC version >= 4.5 using the ``__GNUC__`` and ``__GNUC_MINOR__`` macros. However, the Clang compiler defines these macros too and can cause confusion. For example, Clang 11 pretends to be GCC 4.2.1 in its predefined macros. As a result, the current implementation will miss the opportunity to use ``__builtin_unreachable()`` even if it's supported, because the apparent GCC version shown by Clang is lower than 4.5. This can hardly be the intent of the original implementation (see commits eebaa9bf, 24ba3b0d). The solution ------------ Test the presence of the compiler-builtin ``__builtin_unreachable()`` at configure-time, and conditionally define the ``Py_UNREACHABLE()`` macro depending on the configuration. In the case of an MS compiler, fall back to the current implementation. If options are exhausted, fall back to the current alternative implemented with ``Py_FatalError()``. The idea is based on the ``ax_gcc_builtin.m4`` code [0] by Gabriele Svelto. Alternative ideas ----------------- Recent versions of Clang and GCC support the ``__has_builtin()`` macro. However, Clang documentation says that "[p]rior to Clang 10, ``__has_builtin`` could not be used to detect most builtin pseudo-functions" [1], while GCC support is only available as of GCC 10 and its semantics may not be compatible with Clang's [2]. Therefore ``__has_builtin()`` may not be as useful as it seems. We may attempt to improve the accuracy of version checking in ``#if`` directives, but this could be brittle and difficult to explain, verify, or maintain. Links ----- [0] https://www.gnu.org/software/autoconf-archive/ax_gcc_builtin.html [1] https://clang.llvm.org/docs/LanguageExtensions.html#has-builtin [2] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66970#c24 --- Include/pymacro.h | 4 +--- configure.ac | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/Include/pymacro.h b/Include/pymacro.h index 202b936d96..d83c0d1b03 100644 --- a/Include/pymacro.h +++ b/Include/pymacro.h @@ -118,9 +118,7 @@ "We've reached an unreachable state. Anything is possible.\n" \ "The limits were in our heads all along. Follow your dreams.\n" \ "https://xkcd.com/2200") -#elif defined(__GNUC__) && (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 5)) -# define Py_UNREACHABLE() __builtin_unreachable() -#elif defined(__clang__) || defined(__INTEL_COMPILER) +#elif HAVE_BUILTIN_UNREACHABLE == 1 # define Py_UNREACHABLE() __builtin_unreachable() #elif defined(_MSC_VER) # define Py_UNREACHABLE() __assume(0) diff --git a/configure.ac b/configure.ac index e43b7733ae..8899d15eb7 100644 --- a/configure.ac +++ b/configure.ac @@ -5659,6 +5659,26 @@ if test "$have_builtin_atomic" = yes; then AC_DEFINE(HAVE_BUILTIN_ATOMIC, 1, [Has builtin __atomic_load_n() and __atomic_store_n() functions]) fi +# Check for __builtin_unreachable() compiler builtin to support the definition +# of C macro Py_UNREACHABLE. Define the config macro HAVE_BUILTIN_UNREACHABLE +# if present. +# The idea is based on the "ax_gcc_builtin.m4" file with the following terms: +# ------ +# Copyright (c) 2013 Gabriele Svelto +# Copying and distribution of this file, with or without modification, are +# permitted in any medium without royalty provided the copyright notice and +# this notice are preserved. This file is offered as-is, without any warranty. +# ------ +# +AC_MSG_CHECKING(for __builtin_unreachable) +AC_LINK_IFELSE([AC_LANG_PROGRAM([], [__builtin_unreachable()])], + [have_builtin_unreachable=yes], + [have_builtin_unreachable=no]) +AC_MSG_RESULT($have_builtin_unreachable) +if test "$have_builtin_unreachable" = yes; then + AC_DEFINE(HAVE_BUILTIN_UNREACHABLE, 1, [Has the compiler-builtin __builtin_unreachable() function]) +fi + # ensurepip option AC_MSG_CHECKING(for ensurepip) AC_ARG_WITH(ensurepip, -- 2.31.0