Skip to content
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

Increase zend.reserved_stack_size minimum value in ASAN/MSAN builds #11073

Closed
wants to merge 1 commit into from

Conversation

arnaud-lb
Copy link
Member

zend.reserved_stack_size is the size of the stack space that is reserved for native code called by the VM. This reserved space is similar to the shadow space described in https://pangin.pro/posts/stack-overflow-handling.

The minimum and default value of zend.reserved_stack_size is ZEND_ALLOCA_MAX_SIZE + some empirically chosen value. If this is too low, the program may crash as if no stack limit was in place.

ASAN/MSAN builds use considerably more stack space due to instrumentation, so this default value is too low in these builds. https://clang.llvm.org/docs/AddressSanitizer.html#limitations mentions an increase of up to 3x, but the stack usage of individual functions can increase much more than that. For example, here is a stack trace annotated with stack usage of each function as reported by -fstack-usage:

                                         non-asan asan

total                                    5176     52912

zend_mm_small_size_to_bin                40       88
zend_mm_alloc_heap                       96       424
_zend_mm_alloc                           64       120
zend_mm_realloc_heap                     192      1224
_erealloc                                96       184
_safe_erealloc                           64       152
zend_stack_push                          48       296
enter_nesting                            48       152
lex_scan                                 1056     30424
zendlex                                  80       152
zendparse                                2448     17688
zend_compile                             224      280
compile_file                             304      184
compile_filename                         144      216
zend_include_or_eval                     176      408
ZEND_INCLUDE_OR_EVAL_SPEC_CONST_HANDLER  96       920
execute_ex         <-- stack limit check is made here

In this PR I increase the minimum and default value of zend.reserved_stack_size by 10x in ASAN/MSAN builds.

@arnaud-lb arnaud-lb force-pushed the reserved-stack-asan branch from e3317bf to f64086a Compare July 7, 2023 12:25
@arnaud-lb arnaud-lb marked this pull request as ready for review July 7, 2023 16:42
Copy link
Member

@iluuu1994 iluuu1994 left a comment

Choose a reason for hiding this comment

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

Unfortunately, this doesn't seem to resolve the issue for me locally. I still get stack-overflow errors for most tests. stack_limit_005.phpt gives me "Uncaught Error: Call to a member function f() on null", and stack_limit_013.phpt no output at all. I'm on gcc version 13.1.1.

@arnaud-lb
Copy link
Member Author

I will close this PR then, as the stack usage is too unpredictable under instrumentation. Since my initial goal was to avoid stack overflows while fuzzing, it might be preferable to just set zend.reserved_stack_size to an arbitrarily large value in the fuzzer's hardcoded ini settings.

@arnaud-lb arnaud-lb closed this Jul 14, 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.

2 participants