Skip to content

Fix regression on platforms without ZEND_CHECK_STACK_LIMIT set #16284

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

Conversation

NattyNarwhal
Copy link
Member

There was a guard on one instance of this, but not the other. Unsure if this is the best place for this; the functions are #ifdef gated in zend_execute.c, but not the associated header; perhaps it should be #else with a nop version of the function in that case? I added a guard at this call site to be consistent with other places.

Fixes build on ppc64 CI. I couldn't seem to reproduce it with a default build on ppc64, but CI has sanitizers and such on.

/usr/bin/powerpc64-unknown-linux-gnu-ld.bfd: ext/standard/var.o: in function `php_var_serialize_intern':
/srv/actions/.cache/act/2afa8512bdf5ceb6/hostexecutor/ext/standard/var.c:1055:(.text+0x18f64): undefined reference to `zend_call_stack_size_error'
/usr/bin/powerpc64-unknown-linux-gnu-ld.bfd: ext/standard/var.o: in function `php_var_serialize_intern':
/srv/actions/.cache/act/2afa8512bdf5ceb6/hostexecutor/ext/standard/var.c:1055:(.text+0x18f64): undefined reference to `zend_call_stack_size_error'
/usr/bin/powerpc64-unknown-linux-gnu-ld.bfd: ext/standard/var.o: in function `php_var_serialize_intern':
/srv/actions/.cache/act/2afa8512bdf5ceb6/hostexecutor/ext/standard/var.c:1055:(.text+0x18f64): undefined reference to `zend_call_stack_size_error'
/usr/bin/powerpc64-unknown-linux-gnu-ld.bfd: ext/standard/var.o: in function `php_var_serialize_intern':
/srv/actions/.cache/act/2afa8512bdf5ceb6/hostexecutor/ext/standard/var.c:1055:(.text+0x18f64): undefined reference to `zend_call_stack_size_error'
clang-17: error: linker command failed with exit code 1 (use -v to see invocation)

There was a guard on one instance of this, but not the other. Unsure if
this is the best place for this.
@NattyNarwhal NattyNarwhal requested a review from bukka as a code owner October 7, 2024 16:49
@nielsdos
Copy link
Member

nielsdos commented Oct 7, 2024

This needs to go in 8.4 and up

@NattyNarwhal
Copy link
Member Author

Oops, thanks for noticing. I'll reopen and target 8.4.

@NattyNarwhal
Copy link
Member Author

Replaced with GH-16285

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.

2 participants