Closed andypost closed 1 month ago
The first one is related to #15152.
Here, for the SHA-NI related errors, setting some additional CFLAGS like -march=native
or -msha -mssse3
resolves the error.
For example:
./configure CFLAGS="-msha -msse3 -g -O2"
But we probably need to add that also to the build system files somewhere appropriately. Basically, only hash_sha_ni.c file needs additional compile options (was quickly tested in CMake and works ok).
As for the 2nd error on 32-bit build, attributes are not defined properly in zend_portability.h. This check is a bit basic if you ask me:
#if defined(__GNUC__) && ZEND_GCC_VERSION >= 3004 && defined(__i386__)
# define ZEND_FASTCALL __attribute__((fastcall))
#elif defined(_MSC_VER) && defined(_M_IX86) && _MSC_VER == 1700
# define ZEND_FASTCALL __fastcall
#elif defined(_MSC_VER) && _MSC_VER >= 1800 && !defined(__clang__)
# define ZEND_FASTCALL __vectorcall
#else
# define ZEND_FASTCALL
#endif
Here, for the SHA-NI related errors, setting some additional CFLAGS like
-march=native
or-msha -mssse3
resolves the error.
Yeah, but dynamic detection of the CPU features is supposed to be supported. Might be able to solve this with #15312 (when finished).
As for the 2nd error on 32-bit build, attributes are not defined properly in zend_portability.h.
I don't think FASTCALL makes sense for variadic functions. See #15389.
I don't think FASTCALL makes sense for variadic functions. See https://github.com/php/php-src/pull/15389.
Ok. I see. Better, yes. Thanks.
@andypost, can you please retry the x86 build with latest master (having 65c6d723199355e27151afd73dc312c3b82cb30d)?
I confirm the problem on Alpine. Unfortunately, https://github.com/php/php-src/commit/65c6d723199355e27151afd73dc312c3b82cb30d doesn't solve the first error.
It seems the issue is that PHP_HASH_INTRIN_SHA_RESOLVER
is not defined so we don't declare SHA256_Transform_shani
with __attribute__((target("ssse3,sha")))
in https://github.com/php/php-src/blob/bf1b0eb8ea02c94ee6540e8783d82939359db431/ext/hash/hash_sha_ni.c#L34-L36
PHP_HASH_INTRIN_SHA_RESOLVER
is not defined because HAVE_FUNC_ATTRIBUTE_TARGET
is not defined, because we explicitly exclude musl in https://github.com/php/php-src/blob/bf1b0eb8ea02c94ee6540e8783d82939359db431/configure.ac#L524-L527
Removing the musl exclusion fixes the problem for me (after https://github.com/php/php-src/commit/65c6d723199355e27151afd73dc312c3b82cb30d). I don't know if ifuncs are well supported by musl, but I assume that __attribute__((target))
doesn't rely on the libc, so we may check it on all hosts?
I think we should not compile SHA256_Transform_shani
at all when PHP_HASH_INTRIN_SHA_RESOLVER
is not defined.
@arnaud-lb, please see https://github.com/php/php-src/pull/15312, which is work in progress. Originally meant to support dynamic SHANI detection for MSVC, but may be extended. Especially see https://github.com/php/php-src/pull/15312#discussion_r1712023803 what might solve the problem (I hadn't had time to address that yet).
PS: oh, and feel free to commit fixes to that PR. :)
Oh nice. https://github.com/php/php-src/pull/15312 fixes the Alpine issue for me :+1: I will take a closer look tomorrow
Used patch with variadics and gonna add #15312 next, meantime build using gcc14 fails on x86 as well
Build passed, thank you a lot for quick fixes!
btw x86 (32-bit) has only 3 warnings
In file included from /builds/alpine/aports/testing/php84/src/php-8.4.0beta1/ext/posix/posix.c:47:
/builds/alpine/aports/testing/php84/src/php-8.4.0beta1/ext/posix/posix_arginfo.h:414:50: warning: implicit conversion from 'unsigned long long' to 'zend_long' (aka 'int') changes value from 18446744073709551615 to -1 [-Wconstant-conversion]
414 | REGISTER_LONG_CONSTANT("POSIX_RLIMIT_INFINITY", RLIM_INFINITY, CONST_PERSISTENT);
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/sys/resource.h:72:24: note: expanded from macro 'RLIM_INFINITY'
72 | #define RLIM_INFINITY (~0ULL)
| ^~~~~
/builds/alpine/aports/testing/php84/src/php-8.4.0beta1/Zend/zend_constants.h:51:105: note: expanded from macro 'REGISTER_LONG_CONSTANT'
51 | #define REGISTER_LONG_CONSTANT(name, lval, flags) zend_register_long_constant((name), sizeof(name)-1, (lval), (flags), module_number)
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^~~~
/builds/alpine/aports/testing/php84/src/php-8.4.0beta1/ext/sodium/libsodium.c:1840:52: warning: result of comparison of constant 68719476704 with expression of type 'unsigned int' is always false [-Wtautological-constant-out-of-range-compare]
1840 | if (ciphertext_len - crypto_aead_aes256gcm_ABYTES > 16ULL * ((1ULL << 32) - 2ULL)) {
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /builds/alpine/aports/testing/php84/src/php-8.4.0beta1/ext/opcache/jit/ir/ir_emit.c:418:
/builds/alpine/aports/testing/php84/src/php-8.4.0beta1/ext/opcache/jit/ir/ir_x86.dasc:8353:13: warning: variable 'offset' set but not used [-Wunused-but-set-variable]
8353 | int64_t offset = -min.i64;
| ^
I think we should not compile
SHA256_Transform_shani
at all whenPHP_HASH_INTRIN_SHA_RESOLVER
is not defined.
Indeed. I have done this in #15404 as the minimal patch to fix the Alpine build issue. I've confirmed it against the Alpine Docker Image.
I wonder how many users may be affected by this, and whether we should release beta2 this week (with an appropriate fix cherry-picked)? /cc @NattyNarwhal
FWIW I've been using clang on Mac, but breaking on x86 is obviously pretty bad.
If things are in a good state right now, I'll build a beta2.
FWIW I've been using clang on Mac, but breaking on x86 is obviously pretty bad.
To clarify: This is not a clang issue, but a non-glibc issue.
If things are in a good state right now, I'll build a beta2.
It's safe to tag a beta2, yes, with what is in master branch. Not ideal like a final release, but a nice beta2 state.
@NattyNarwhal can you just recheck if correct patches are in the current beta 2 tag? Because it seems that this one isn't added yet.
Indeed. It appears that the php-8.4.0beta2
tag effectively is a copy of the php-8.4.0beta1
tag.
Edit: Not quite a copy, because of the commit Ilija snuck in during the release procedure, but definitely not up to date.
Ugh, I had a git incident and it branched from the wrong point. I can push beta2 again, or would it be better to just call it beta3?
Tag replacement is a no-no, because folks might already have the broken tag. It (unfortunately) should be a Beta 3.
Oh, yep, looks beta3 is better option
I'm pushing beta3, which should have the actual fix. Sorry for the embarrassing git mistake.
Thank you! Awaiting announce to start fixing Drupal for compatibility) Meantime all arches passed on Alpinelinux
I've just remembered and sorry about polluting this PR here. Should the API numbers like ZEND_MODULE_API_NO, PHP_API_VERSION, ZEND_VERSION and similar be also bumped or does this happen in RC phase?
EDIT: Ah, ok! That is done in the RC1 (noted in the release-process.md file). All good then.
Yeah, I have to keep double-checking, but:
We update Zend/zend.h only when preparing RC and GA releases. We do not update ZEND_VERSION for alpha or beta releases.
Description
Attempted to build for Alpine fresh beta and it fails on x86 and x86_64
As I can find it was fixed earlier in https://github.com/php/php-src/pull/2975
Logs x86_64
and x86
PHP Version
PHP 8.4.0beta1
Operating System
Alpinelinux