Building 8.4beta1 fail with clang on x86(64) about php-src HOT 25 CLOSED

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.

from php-src.

I'm pushing beta3, which should have the actual fix. Sorry for the embarrassing git mistake.

from php-src.

I don't think FASTCALL makes sense for variadic functions. See #15389.

Ok. I see. Better, yes. Thanks.

from php-src.

Oh nice. #15312 fixes the Alpine issue for me 👍 I will take a closer look tomorrow

from php-src.

I think we should not compile SHA256_Transform_shani at all when PHP_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.

from php-src.

Thank you! Awaiting announce to start fixing Drupal for compatibility) Meantime all arches passed on Alpinelinux

from php-src.

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.

from php-src.

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.

from php-src.

The first one is related to #15152.

from php-src.

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

from php-src.

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.

from php-src.

@andypost, can you please retry the x86 build with latest master (having 65c6d72)?

from php-src.

I confirm the problem on Alpine. Unfortunately, 65c6d72 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

PHP_HASH_INTRIN_SHA_RESOLVER is not defined because HAVE_FUNC_ATTRIBUTE_TARGET is not defined, because we explicitly exclude musl in

Removing the musl exclusion fixes the problem for me (after 65c6d72). 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.

from php-src.

@arnaud-lb, please see #15312, which is work in progress. Originally meant to support dynamic SHANI detection for MSVC, but may be extended. Especially see #15312 (comment) 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. :)

from php-src.

Used patch with variadics and gonna add #15312 next, meantime build using gcc14 fails on x86 as well

from php-src.

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;
      |                                         ^

from php-src.

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

from php-src.

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.

from php-src.

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.

from php-src.

Confirm, that 2 patches in enough to pass for 8 arches

from php-src.

@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.

from php-src.

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.

from php-src.

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?

from php-src.

Tag replacement is a no-no, because folks might already have the broken tag. It (unfortunately) should be a Beta 3.

from php-src.

Oh, yep, looks beta3 is better option

from php-src.