Skip to content

Fix GH-12643: Stack limit tests failing on ppc64le #12779

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

Merged
merged 2 commits into from
Dec 10, 2023

Conversation

arnaud-lb
Copy link
Member

Some tests are setting the max_allowed_stack_size too low for ppc64le builds. In this PR I increase the max_allowed_stack_size in these tests, and also clarify the max_allowed_stack_size error message.

Fixes GH-12643

These tests were setting zend.max_allowed_stack_size to a small value. On ppc64le
this value is too small and the limit may be reached too soon.
@arnaud-lb arnaud-lb marked this pull request as ready for review November 25, 2023 11:45
@@ -2311,7 +2311,7 @@ static zend_never_inline ZEND_COLD void ZEND_FASTCALL zend_use_new_element_for_s
#ifdef ZEND_CHECK_STACK_LIMIT
static zend_never_inline ZEND_COLD void ZEND_FASTCALL zend_call_stack_size_error(void)
{
zend_throw_error(NULL, "Maximum call stack size of %zu bytes reached. Infinite recursion?",
zend_throw_error(NULL, "Maximum call stack size of %zu bytes (zend.max_allowed_stack_size-zend.reserved_stack_size) reached. Infinite recursion?",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
zend_throw_error(NULL, "Maximum call stack size of %zu bytes (zend.max_allowed_stack_size-zend.reserved_stack_size) reached. Infinite recursion?",
zend_throw_error(NULL, "Maximum call stack size of %zu bytes (zend.max_allowed_stack_size - zend.reserved_stack_size) reached. Infinite recursion?",

Make it clearer why the size is not exactly zend.max_allowed_stack_size
@arnaud-lb arnaud-lb merged commit b1516d9 into php:PHP-8.3 Dec 10, 2023
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.

3 participants