Skip to content

Zend/zend_atomic.h: fix build for upcoming gcc-14#12821

Closed
trofi wants to merge 1 commit into
php:PHP-8.2from
trofi:zend.h-gcc-14
Closed

Zend/zend_atomic.h: fix build for upcoming gcc-14#12821
trofi wants to merge 1 commit into
php:PHP-8.2from
trofi:zend.h-gcc-14

Conversation

@trofi

@trofi trofi commented Nov 28, 2023

Copy link
Copy Markdown

In https://gcc.gnu.org/PR60512 gcc-14 implemented c_atomic feature and exposed build failure on php as:

/build/php-8.2.13/Zend/zend_atomic.h: In function 'zend_atomic_bool_store_ex':
/build/php-8.2.13/Zend/zend_atomic.h:96:9: warning: implicit declaration of function '__c11_atomic_store'; did you mean '__atomic_store'? [-Wimplicit-function-declaration]
   96 |         __c11_atomic_store(&obj->value, desired, __ATOMIC_SEQ_CST);
      |         ^~~~~~~~~~~~~~~~~~
      |         __atomic_store
...
ld: /build/php-8.2.13/Zend/zend_atomic.h:96:(.text+0x1301): undefined reference to `__c11_atomic_store'
ld: /build/php-8.2.13/Zend/zend_atomic.h:96:(.text+0x1e49): undefined reference to `__c11_atomic_store'

As I understand the build failure happens because php uses clang's internal functions prefixed with __c11_* that implement atomics instead of standard-defined atomic_* ones. gcc implements ony standard-defined ones.

The change fixes build for me.

@bwoebi

bwoebi commented Nov 28, 2023

Copy link
Copy Markdown
Member

\cc @morrisonlevi

Comment thread Zend/zend_atomic.h Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe if you use the <stdatomic.h> builtins, you need to use memory_order_seq_cst from <stdatomi.h> as well, not the implementation-specific __ATOMIC_SEQ_CST constant.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great catch! Completely forgot about the arguments. Changed to memory_order_seq_cst in all 3 places.

In gcc.gnu.org/PR60512 `gcc-14` implemented `c_atomic` feature and
exposed build failure on `php` as:

    /build/php-8.2.13/Zend/zend_atomic.h: In function 'zend_atomic_bool_store_ex':
    /build/php-8.2.13/Zend/zend_atomic.h:96:9: warning: implicit declaration of function '__c11_atomic_store'; did you mean '__atomic_store'? [-Wimplicit-function-declaration]
       96 |         __c11_atomic_store(&obj->value, desired, __ATOMIC_SEQ_CST);
          |         ^~~~~~~~~~~~~~~~~~
          |         __atomic_store
    ...
    ld: /build/php-8.2.13/Zend/zend_atomic.h:96:(.text+0x1301): undefined reference to `__c11_atomic_store'
    ld: /build/php-8.2.13/Zend/zend_atomic.h:96:(.text+0x1e49): undefined reference to `__c11_atomic_store'

As I understand the build failure happens because `php` uses `clang`'s
internal functions prefixed with `__c11_*` that implement atomics instead
of standard-defined `atomic_*` ones. `gcc` implements ony standard-defined
ones.

The change fixes build for me.
@trofi trofi changed the title Zend/zend_atomic.h: fix build for upcomit gcc-14 Zend/zend_atomic.h: fix build for upcoming gcc-14 Dec 4, 2023
@crrodriguez

Copy link
Copy Markdown
Contributor

Note that #ifndef ZTS ... memory_order AFAIK needs to be memory_order_relaxed instead.

@trofi

trofi commented Feb 18, 2024

Copy link
Copy Markdown
Author

Was worked around by 7252660

@trofi trofi closed this Feb 18, 2024
@trofi trofi deleted the zend.h-gcc-14 branch February 18, 2024 21:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants