-
Notifications
You must be signed in to change notification settings - Fork 8.1k
TSRM: make CG, EG, SCNG and AG compile-time offsets #22287
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -862,7 +862,7 @@ extern "C++" { | |
| /** @deprecated */ | ||
| #define ZEND_CGG_DIAGNOSTIC_IGNORED_END ZEND_DIAGNOSTIC_IGNORED_END | ||
|
|
||
| #if defined(__cplusplus) | ||
| #if defined(__cplusplus) || defined(__STDC_VERSION__) && (__STDC_VERSION__ >= 202311L) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. technically unrelated, but I didn't want to copy an incomplete definition into tsrm.c
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. see also #21228.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd be in favour, getting php-src and extensions to compile in extremely old environments is a pain anyway (I do rhel-7 builds for frankenphp). Should I replace this then?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Not yet, probably. See: https://news-web.php.net/php.internals/130714 |
||
| # define ZEND_STATIC_ASSERT(c, m) static_assert((c), m) | ||
| #elif defined(__STDC_VERSION__) && (__STDC_VERSION__ >= 201112L) /* C11 */ | ||
| # define ZEND_STATIC_ASSERT(c, m) _Static_assert((c), m) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2842,6 +2842,12 @@ PHPAPI bool php_tsrm_startup_ex(int expected_threads) | |
| { | ||
| bool ret = tsrm_startup(expected_threads, 1, 0, NULL); | ||
| php_reserve_tsrm_memory(); | ||
| /* Must reserve exactly the prefix laid out by ZEND_*_OFFSET (zend_globals.h). */ | ||
| tsrm_reserve_fast_front( | ||
| TSRM_ALIGNED_SIZE(sizeof(zend_compiler_globals)) + | ||
| TSRM_ALIGNED_SIZE(sizeof(zend_executor_globals)) + | ||
| TSRM_ALIGNED_SIZE(sizeof(zend_php_scanner_globals)) + | ||
| TSRM_ALIGNED_SIZE(zend_mm_globals_size())); // AG size, exposed through function call | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. really unhappy about this, same issue as below: |
||
| (void)ts_resource(0); | ||
| return ret; | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so this is a two-fold problem:
In C++ this would be a simple consteval function in the header, but I have no idea how to make it work in C. So I kept the definition private, but force it to the specific size it has.