Building 8.4beta1 fail with clang on x86(64)

[andypost]

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 #2975

Logs x86_64

/builds/alpine/aports/testing/php84/src/php-8.4.0beta1/ext/hash/hash_sha_ni.c:149:2: error: always_inline function '_mm_sha256rnds2_epu32' requires target feature 'sha', but would be inlined into function 'SHA256_Transform_shani' that is compiled without support for 'sha'
/builds/alpine/aports/testing/php84/src/php-8.4.0beta1/ext/hash/hash_sha_ni.c:103:2: note: expanded from macro 'RNDMSG'
  103 |         RND4(S, W[i % 4], K0, K1, K2, K3);                      \
      |         ^
/builds/alpine/aports/testing/php84/src/php-8.4.0beta1/ext/hash/hash_sha_ni.c:90:9: note: expanded from macro 'RND4'
   90 |         S[0] = _mm_sha256rnds2_epu32(S[0], S[1], M);                            \
      |                ^
/builds/alpine/aports/testing/php84/src/php-8.4.0beta1/ext/hash/hash_sha_ni.c:149:2: error: always_inline function '_mm_sha256msg1_epu32' requires target feature 'sha', but would be inlined into function 'SHA256_Transform_shani' that is compiled without support for 'sha'
/builds/alpine/aports/testing/php84/src/php-8.4.0beta1/ext/hash/hash_sha_ni.c:105:3: note: expanded from macro 'RNDMSG'
  105 |                 MSG4(W, i + 4);                                 \
      |                 ^
/builds/alpine/aports/testing/php84/src/php-8.4.0beta1/ext/hash/hash_sha_ni.c:95:19: note: expanded from macro 'MSG4'
   95 |         W[(i + 0) % 4] = _mm_sha256msg1_epu32(W[(i + 0) % 4], W[(i + 1) % 4]);  \
      |                          ^
/builds/alpine/aports/testing/php84/src/php-8.4.0beta1/ext/hash/hash_sha_ni.c:149:2: error: '__builtin_ia32_palignr128' needs target feature ssse3
/builds/alpine/aports/testing/php84/src/php-8.4.0beta1/ext/hash/hash_sha_ni.c:105:3: note: expanded from macro 'RNDMSG'
  105 |                 MSG4(W, i + 4);                                 \
      |                 ^
/builds/alpine/aports/testing/php84/src/php-8.4.0beta1/ext/hash/hash_sha_ni.c:97:6: note: expanded from macro 'MSG4'
   97 |             _mm_alignr_epi8(W[(i + 3) % 4], W[(i + 2) % 4], 4));                \
      |             ^
/usr/lib/llvm18/lib/clang/18/include/tmmintrin.h:157:13: note: expanded from macro '_mm_alignr_epi8'
  157 |   ((__m128i)__builtin_ia32_palignr128((__v16qi)(__m128i)(a), \
      |             ^
fatal error: too many errors emitted, stopping now [-ferror-limit=]

and x86

In file included from /builds/alpine/aports/testing/php84/src/php-8.4.0beta1/ext/spl/spl_fixedarray.c:22:
In file included from /builds/alpine/aports/testing/php84/src/php-8.4.0beta1/main/php.h:31:
In file included from /builds/alpine/aports/testing/php84/src/php-8.4.0beta1/Zend/zend.h:34:
/builds/alpine/aports/testing/php84/src/php-8.4.0beta1/Zend/zend_ast.h:252:21: warning: fastcall calling convention is not supported on variadic function [-Wignored-attributes]
  252 | ZEND_API zend_ast * ZEND_FASTCALL zend_ast_create_n(unsigned kind, ...);
      |                     ^
/builds/alpine/aports/testing/php84/src/php-8.4.0beta1/Zend/zend_portability.h:306:39: note: expanded from macro 'ZEND_FASTCALL'
  306 | # define ZEND_FASTCALL __attribute__((fastcall))
      |                                       ^
In file included from /builds/alpine/aports/testing/php84/src/php-8.4.0beta1/ext/spl/spl_fixedarray.c:22:
In file included from /builds/alpine/aports/testing/php84/src/php-8.4.0beta1/main/php.h:31:
In file included from /builds/alpine/aports/testing/php84/src/php-8.4.0beta1/Zend/zend.h:34:
/builds/alpine/aports/testing/php84/src/php-8.4.0beta1/Zend/zend_ast.h:253:21: warning: fastcall calling convention is not supported on variadic function [-Wignored-attributes]
  253 | ZEND_API zend_ast * ZEND_FASTCALL zend_ast_create_ex_n(zend_ast_kind kind, unsigned attr, ...);
      |                     ^
/builds/alpine/aports/testing/php84/src/php-8.4.0beta1/Zend/zend_portability.h:306:39: note: expanded from macro 'ZEND_FASTCALL'
  306 | # define ZEND_FASTCALL __attribute__((fastcall))
      |                                       ^
2 warnings generated.

PHP Version

PHP 8.4.0beta1

Operating System

Alpinelinux

[TimWolla]

The first one is related to #15152.

[petk]

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

[cmb69]

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.

[petk]

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

Ok. I see. Better, yes. Thanks.

[cmb69]

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

[arnaud-lb]

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-src/ext/hash/hash_sha_ni.c

Lines 34 to 36 in bf1b0eb

	# if PHP_HASH_INTRIN_SHA_RESOLVER
	static __m128i be32dec_128(const uint8_t * src) __attribute__((target("ssse3")));
	void SHA256_Transform_shani(uint32_t state[PHP_STATIC_RESTRICT 8], const uint8_t block[PHP_STATIC_RESTRICT 64]) __attribute__((target("ssse3,sha")));

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

php-src/configure.ac

Lines 524 to 527 in bf1b0eb

	AS_CASE([$host_alias], [*-*-*android*|*-*-*uclibc*|*-*-*musl*|*openbsd*], [true], [
	if test "`uname -s 2>/dev/null`" != "FreeBSD" || test "`uname -U 2>/dev/null`" -ge 1200000; then
	AX_GCC_FUNC_ATTRIBUTE([ifunc])
	AX_GCC_FUNC_ATTRIBUTE([target])

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.

[cmb69]

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

[arnaud-lb]

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

[andypost]

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

[andypost]

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

[TimWolla]

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.

[cmb69]

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

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

[TimWolla]

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.

[petk]

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.

[andypost]

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

[petk]

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

[TimWolla]

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.

[NattyNarwhal]

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?

[TimWolla]

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

[andypost]

Oh, yep, looks beta3 is better option

[NattyNarwhal]

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

[andypost]

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

[petk]

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.

[NattyNarwhal]

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.