address argument to atomic operation must be a pointer to non-const _Atomic type

[ryandesign]

Description

Hello! I'm the maintainer of php in MacPorts, and I noticed this build failure of php 8.2.0alpha2 and 8.2.0alpha1 on macOS 10.13 with Xcode 9.4.1 and its Apple clang 902.0.39.2:

Zend/zend_atomic.h:85:9: error: address argument to atomic operation must be a pointer to non-const _Atomic type ('const _Atomic(bool) *' invalid)
        return __c11_atomic_load(&obj->value, __ATOMIC_SEQ_CST);
               ^                 ~~~~~~~~~~~

Here is a full failed build log from macOS 10.13.

I also verified the problem on macOS 10.13 with Xcode 10.1 and its Apple clang 1000.11.45.5.

It also fails similarly on macOS 10.12, OS X 10.11, and OS X 10.10.

The build succeeds on macOS 10.14 with Xcode 10.3 and its Apple clang 1001.0.46.4 and on newer systems. Here is a full successful build log from macOS 10.14.

Curiously the build also succeeds on much older systems like OS X 10.9, OS X 10.8, and Mac OS X 10.7 with their older versions of Xcode and clang.

I see "c11" in the error message but I don't see -std=c11 being passed to the compiler. Thinking this might be a situation where certain older compilers would support whatever you're wanting to do if only you would ask the compiler to be in c11 mode, I tried adding -std=c11 on macOS 10.13 with Xcode 10.1 and its Apple clang 1000.11.45.5, and I also tried -std=gnu11, but neither of those changed the error.

php 8.1.7 and earlier don't have this problem.

PHP Version

8.2.0alpha2

Operating System

macOS 10.13.6

[twose]

Seems related to #8327, could you have a try with the patch in #8764? (or just use this patch)

bson-atomic also used type casting to avoid such problems, to be on the safe side, I've added type-casts to all calls.

cc @morrisonlevi @cmb69

[ryandesign]

I apologize for my delay in following up on this.

The problem still occurs with PHP 8.2.0RC7.

could you have a try with the patch in #8764? (or just use this patch)

This patch did not apply cleanly to PHP 8.2.0RC7 but I updated the patch (php82-atomic.patch.txt) and it built successfully. But there seems to be disagreement in #8764 about whether that's the correct fix.

In Zend/zend_atomic.h it says:

#if __has_feature(c_atomic)
#define	HAVE_C11_ATOMICS 1
#elif ZEND_GCC_PREREQ(4, 7)
#define	HAVE_GNUC_ATOMICS 1
#elif defined(__GNUC__)
#define	HAVE_SYNC_ATOMICS 1
#elif !defined(ZEND_WIN32)
#define HAVE_NO_ATOMICS 1
#endif

If I just change the first line to:

#if 0

that also allows it to build successfully. And, as I said above, earlier clangs (that presumably don't have the c_atomic feature) also compile it fine. So it seems the problem is that old versions of clang that claim to have the c_atomic feature don't have it in the way that PHP wants. Specifically it seems that the initial version of the C11 standard did not have the const qualifier on atomic_load:

https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1807.htm

So another solution might be for PHP to detect at configure time whether atomic_load has a const qualifier, and only then define HAVE_C11_ATOMICS.

[cmb69]

The problem still occurs with PHP 8.2.0RC7.

@morrisonlevi, could you please have a look at this?

[twose]

requote this link here again:
llvm/llvm-project@b4b1f59

and discussion:
http://lists.llvm.org/pipermail/cfe-dev/2018-May/058129.html

I also hope it can be fixed soon :)

[2konrad]

I can confirm the fix mentioned by @ryandesign works when trying to install brew install php on macOS 10.13.
I replaced __has_feature(c_atomic) with 0 in Zend/zend_atomic.h, re-packed the php source code again and placed it in the brew cache directory (brew --cache).
However would be good if this could be integrated in the recipe at some point.

[ryandesign]

Specifically it seems that the initial version of the C11 standard did not have the const qualifier on atomic_load:

https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1807.htm

So another solution might be for PHP to detect at configure time whether atomic_load has a const qualifier, and only then define HAVE_C11_ATOMICS.

It was recently pointed out to me that I misinterpreted this: The C11 standard did not have the const qualifier; C17 does. Therefore, the problem is simply solved by not defining HAVE_C11_ATOMICS unless __STDC_VERSION__ >= 201710L is also true. And at that point it probably makes sense to rename HAVE_C11_ATOMICS to HAVE_C17_ATOMICS. I've proposed this in #11931.

What had confused me into thinking that there was a "revised C11 standard" is that newer compilers that support C17 also use the const qualifier in C11 mode.

[morrisonlevi]

The issue is specifically for older compilers which haven't fixed this errata yet. It's hard to know exactly which compilers these are. Can you please share your compiler name and version?

For example, even for C11, clang is supposed to allow const here. There's no reason it shouldn't, so it fixed it even for C11 mode. I'm trying to find when exactly the fixes landed.

[ryandesign]

As I've said, I believe the issue would affect compilers which support C11 mode and which predate the existence of the C17 standard which made the change to the constness.

I said in my report that I observed it on OS X 10.10–10.13, and it would have been on the MacPorts automated buildbot system, on which the compiler versions are:

OS X Yosemite v10.10.5 (14F2511)
Xcode v7.2.1 (7C1002)
Apple LLVM version 7.0.2 (clang-700.1.81)

OS X El Capitan v10.11.6 (15G22010)
Xcode v8.2.1 (8C1002)
Apple LLVM version 8.0.0 (clang-800.0.42.1)

macOS Sierra v10.12.6 (16G2136)
Xcode v9.2 (9C40b)
Apple LLVM version 9.0.0 (clang-900.0.39.2)

macOS High Sierra v10.13.6 (17G14042)
Xcode v9.4.1 (9F2000)
Apple LLVM version 9.1.0 (clang-902.0.39.2)

I also said "I also verified the problem on macOS 10.13 with Xcode 10.1 and its Apple clang 1000.11.45.5" which must have been on my own machine, not the buildbot system.

We did not see the problem on the macOS 10.14 or later build machines. The 10.14 builder has this version:

macOS Mojave v10.14.6 (18G9323)
Xcode v10.3 (10G8)
Apple LLVM version 10.0.1 (clang-1001.0.46.4)