Skip to content

zend call stack fixing stack limit for macOs arm64.#13319

Closed
devnexen wants to merge 1 commit intophp:masterfrom
devnexen:apple_arm64_stacksize
Closed

zend call stack fixing stack limit for macOs arm64.#13319
devnexen wants to merge 1 commit intophp:masterfrom
devnexen:apple_arm64_stacksize

Conversation

@devnexen
Copy link
Copy Markdown
Member

@devnexen devnexen commented Feb 3, 2024

8MB sounded a prudent size for older 10.9 macOs release, however with newer mac with arm64, it triggers a stack overflow.

@devnexen devnexen requested a review from iluuu1994 as a code owner February 3, 2024 15:33
@devnexen devnexen requested review from arnaud-lb and removed request for iluuu1994 February 3, 2024 15:33
Copy link
Copy Markdown
Member

@arnaud-lb arnaud-lb left a comment

Choose a reason for hiding this comment

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

Thank you! The code change looks good to me, but the new comments are redundant with the existing comment bellow them.

We could make the change self-explanatory by merging the code paths. For example (in pseudo code) :

if (pthread_main_np() && !arm64) {
    /* existing comment explaining the workaround for older OSX versions */
    max_size = 8 * 1024 * 1024;
} else {
    max_size = pthread_get_stacksize_np(pthread_self());
}

@devnexen devnexen force-pushed the apple_arm64_stacksize branch from 04cd85a to 8165853 Compare February 5, 2024 11:38
@divinity76
Copy link
Copy Markdown
Contributor

divinity76 commented Feb 5, 2024

#ifndef __aarch64__
	/* pthread_get_stacksize_np() returns a too low value for the main
	 * thread in OSX 10.9, 10.10:
	 * https://mail.openjdk.org/pipermail/hotspot-dev/2013-October/011353.html
	 */
	if (pthread_main_np()) {
		max_size = 8 * 1024 * 1024;
	} else
#endif
	max_size = pthread_get_stacksize_np(pthread_self());

should be slightly faster on modern macos

edit: fixed a logic error

8MB sounded a prudent size for older 10.9 macOs release, however
with newer mac with arm64, it triggers a stack overflow.
@devnexen devnexen force-pushed the apple_arm64_stacksize branch from 8165853 to 7f770c1 Compare February 5, 2024 12:39
@divinity76
Copy link
Copy Markdown
Contributor

LGTM 👍

@devnexen devnexen closed this in b320aab Feb 5, 2024
arnaud-lb pushed a commit to arnaud-lb/php-src that referenced this pull request Aug 6, 2025
8MB sounded a prudent size for older 10.9 macOs release, however
with newer mac with arm64, it triggers a stack overflow.

close phpGH-13319
@arnaud-lb arnaud-lb mentioned this pull request Aug 6, 2025
arnaud-lb pushed a commit that referenced this pull request Aug 7, 2025
8MB sounded a prudent size for older 10.9 macOs release, however
with newer mac with arm64, it triggers a stack overflow.

Cherry picks b320aab (GH-13319) from PHP-8.4.
Closes GH-19390.
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