Bug #81124 [Opn]: Determination of asm goto capability should be made by phpize configure script

Edit report at https://bugs.php.net/bug.php?id=81124&edit=1 ID: 81124 User updated by: php-bugs-2021 at ryandesign dot com Reported by: php-bugs-2021 at ryandesign dot com Summary: Determination of asm goto capability should be made by phpize configure script Status: Open Type: Bug Package: *Compile Issues Operating System: macOS PHP Version: 8.0.7 Block user comment: N Private report: N New Comment: You have a typo where __clang_major should be __clang_major__. And to check whether you're dealing with clang, usually you would check whether __clang__ is defined. (If it is, then surely __clang_major__ is defined too.) But I would caution you that open source clang and Apple clang are two different compilers with different version numbering schemes. A feature that becomes available in open source clang 9 (released September 2019) is not necessarily available in Apple clang 9 (released in Xcode 9 in September 2017) and vice-versa. If you want to check the clang version, then you should first check whether __apple_build_version__ is defined. If it is, it's Apple clang and your comparisons of __clang_major__ should be about Apple clang major versions, otherwise they should be about open source clang major versions. If you need finer granularity for checking Apple clang versions, you could check the value of __apple_build_version__. For example, Apple clang 9 has an __apple_build_version__ greater than or equal to 9000000 and less than 10000000. We have a list of Apple clang version numbers in each Xcode version if that's helpful: https://trac.macports.org/wiki/XcodeVersionInfo I believe support for asm goto was introduced in open source clang 9 (https://github.com/llvm/llvm-project/commit/784929d0454c4df6a98ef6fbbd1d30a6f71f9c16) but I don't know in which version of Apple clang it was introduced. But support appears to be absent in Apple clang 11.0.3 (__apple_build_version__ 11030032 from Xcode 11.7) and present in Apple clang 12.0.5 (__apple_build_version__ 12050022 from Xcode 12.5). My guess would be that support was added in Apple clang 12.0.0 (Xcode 12.0) in which case what you want is: --- a/Zend/zend_operators.h +++ b/Zend/zend_operators.h @@ -500,7 +500,8 @@ ZEND_API void zend_update_current_locale(void); #define ZVAL_OFFSETOF_TYPE \ (offsetof(zval, u1.type_info) - offsetof(zval, value)) -#if defined(HAVE_ASM_GOTO) && !__has_feature(memory_sanitizer) +#if defined(HAVE_ASM_GOTO) && !__has_feature(memory_sanitizer) \ + && (!defined(__clang__) || (defined(__apple_build_version__) && __clang_major__ >= 12) || __clang_major__ >= 9) # define ZEND_USE_ASM_ARITHMETIC 1 #else # define ZEND_USE_ASM_ARITHMETIC 0 but I haven't tested this. And it seems a little troubling to me to bother having a HAVE_ASM_GOTO define if we're going to second-guess its value. Previous Comments: ------------------------------------------------------------------------ [2021-06-14 09:29:57] nikic@php.net I'm thinking something like this: diff --git a/Zend/zend_operators.h b/Zend/zend_operators.h index dad23bc4d8..a38b651ff8 100644 --- a/Zend/zend_operators.h +++ b/Zend/zend_operators.h @@ -500,7 +500,8 @@ ZEND_API void zend_update_current_locale(void); #define ZVAL_OFFSETOF_TYPE \ (offsetof(zval, u1.type_info) - offsetof(zval, value)) -#if defined(HAVE_ASM_GOTO) && !__has_feature(memory_sanitizer) +#if defined(HAVE_ASM_GOTO) && !__has_feature(memory_sanitizer) \ + && (!defined(__clang_major__) || __clang_major >= 9) # define ZEND_USE_ASM_ARITHMETIC 1 #else # define ZEND_USE_ASM_ARITHMETIC 0 ------------------------------------------------------------------------ [2021-06-10 15:40:09] nikic@php.net I agree that this is a bug, but I'm not sure whether phpize is prepared for this. In particular we'll end up including both the extension config.h (typically first, I think) and the main php_config.h and will get symbol clashes at that point. I think for stable releases we should just add an explicit exclude for old clang versions (i.e. set ZEND_USE_ASM_ARITHMETIC=0 if HAVE_ASM_GOTO=1 but compiler too old), because I think that's the main practical problem here. ------------------------------------------------------------------------ [2021-06-10 14:35:44] php-bugs-2021 at ryandesign dot com Description: ------------ php 7.3.? and later use asm goto in zend_operators.h if HAVE_ASM_GOTO is set in php_config.h. A problem arises if the compiler that was used to compile php supports asm goto but the compiler used to compile a separate php module (with phpize) does not support asm goto. The error message in that case is e.g.: /opt/local/include/php73/php/Zend/zend_operators.h:523:10: error: expected '(' after 'asm' __asm__ goto( ^ See https://trac.macports.org/ticket/62022 for the way that this affects php 7.3.x in MacPorts today, but note that as far as I know the potential for similar issues continues to exist in all later php versions too. A previous bug report about this issue #75951 was closed as not a bug because of the difference in compilers. While perhaps you would prefer that all modules be compiled with the same compiler that was used to compile php, that cannot always be assured. For example, maybe a user compiles php, then changes their compiler (on macOS, maybe the user changes their Xcode version -- maybe it is updated for them automatically by Apple Software Update), then compiles a php module. Or maybe the choice of compiler is dictated by deficiencies in or requirements of a particular module (e.g. some require a C++11-capable compiler and some don't). php modules compiled with phpize get their own configure scripts, so presumably it was already recognized that certain determinations must be made anew for each module. Would it be possible to move or copy the determination of whether the compiler supports asm goto into this phpize-generated configure script? ------------------------------------------------------------------------ -- Edit this bug report at https://bugs.php.net/bug.php?id=81124&edit=1