Skip to content

Increase the PCRE JIT stack size #2910

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

Closed
wants to merge 1 commit into from
Closed

Conversation

khromov
Copy link

@khromov khromov commented Nov 9, 2017

Related issues:
https://bugs.php.net/bug.php?id=70110
https://bugs.exim.org/show_bug.cgi?id=1663

From the PCRE docs:
A maximum stack size of 512K to 1M should be more than enough for any pattern.

https://www.pcre.org/original/doc/html/pcre_jit_stack_alloc.html

This also affects applications all over the web that fail by the strange behaviour that preg_ calls return errors for valid expressions. Some highlights:
https://www.drupal.org/node/2792877

Heroku have defaulted pcre.jit = 0, presumably due to this issue.
https://devcenter.heroku.com/changelog-items/853

Related issues:
https://bugs.php.net/bug.php?id=70110
https://bugs.exim.org/show_bug.cgi?id=1663

From the PCRE docs:
**A maximum stack size of 512K to 1M should be more than enough for any pattern. **

https://www.pcre.org/original/doc/html/pcre_jit_stack_alloc.html

This also affects applications all over the web that fail by the strange behaviour that `preg_` calls return errors for valid expressions. Some highlights:
https://www.drupal.org/node/2792877
@cmb69
Copy link
Member

cmb69 commented Nov 9, 2017

See also a related discussion which happened on internals quite a while ago.

@khromov
Copy link
Author

khromov commented Nov 9, 2017

@cmb69 Thanks for the background. That discussion seems to have "died out" after one test regexp passed when increasing the stack limit to 64K, but this is far below the recommended maximum of 512K-1M. Coming from a developer standpoint, it's very scary to not be able to trust the result of preg_ functions, so I implore the core devs to consider this patch which bumps us to the absolute minimum required to fit "any pattern" according to PCRE docs.

@donatj
Copy link

donatj commented Nov 9, 2017

To me the worst part of the preg_ situation is they fail without warning or exception - you have to actually check preg_last_error to know if it worked.

That's… horrid.

@weltling
Copy link
Contributor

weltling commented Nov 16, 2017

The pattern preg_match('/^(A{1,2}B)+$$/',str_repeat('AB',8192)); from #70110 crashes 5.6, 7.0 and so forth when JIT is disabled. Same with preg_match('~(a)*~', str_repeat('a', 5431)); from there. The crash is not a JIT issue but a recursion one. I don't see how this bug matches JIT stack limit question.

@khromov regarding the Drupal issue you've linked, were it possible you to post some short reproduce code as it doesn't seem clear from the ticket and that patch isn't merged in Drupal.

Thanks.

@ettoredn
Copy link

ettoredn commented Dec 7, 2018

This issue seems to persist when running WordPress 5.0 and PHP 5.3.0. Are there are solutions besides disabling PCRE JIT via php.ini ?

@cmb69
Copy link
Member

cmb69 commented Dec 7, 2018

@ettoredn You can't disable PCRE JIT in PHP 5.3.0, since it is only available as of PHP 7.0.0.

@Majkl578
Copy link
Contributor

Majkl578 commented Dec 7, 2018

@ettoredn Yes, using something less ancient - PHP 7.3 currently has stack size limit of 192 MiB.

@ettoredn
Copy link

ettoredn commented Dec 7, 2018

Sorry, that was a silly typo. I am experiencing the issue in PHP 7.3.0!

Warning: preg_match_all(): JIT compilation failed: no more memory in wp-includes/formatting.php on line 229

macOS 10.13.6, PHP 7.3.0, XDebug 2.7.0beta1

@nikic
Copy link
Member

nikic commented Dec 7, 2018

@ettoredn That error does not look stack size related, it's an OOM during pattern compilation, not execution. Is it possible to reproduce this with an isolated code sample?

@nikic
Copy link
Member

nikic commented Dec 7, 2018

@ettoredn Also, please report this issue on bugs.php.net as well, I think this might be some PCRE2 related regression.

@ettoredn
Copy link

ettoredn commented Dec 7, 2018

Thanks. I reported the issue https://bugs.php.net/bug.php?id=77260

centminmod added a commit to centminmod/centminmod that referenced this pull request Oct 27, 2019
Added 2 variables below which can be set in persistent config file. When enabled PHP_PCREJIT_STACKSIZE_ADJUST='y' and PHP_PCREJIT_STACKSIZE='512' are set in persistent config file /etc/centminmod/custom_config.inc prior to centmin.sh menu option 5 recompiles/updates, allows you to raise PHP's default PCRE JIT stack size php/php-src#2910 from current defaults which for PHP 7.2 is 64 x 1024 max and PHP 7.3 is 192 x 1024 max and PHP 7.4 is 256 x 1024 max. PHP_PCREJIT_STACKSIZE defines the value to raise to i.e. 512 x 1024 for PCRE_JIT_STACK_MAX_SIZE which is hard coded and set by PHP. Disabled by default PHP_PCREJIT_STACKSIZE_ADJUST='n'.

PHP_PCREJIT_STACKSIZE_ADJUST='y'
PHP_PCREJIT_STACKSIZE='512'
@mvorisek
Copy link
Contributor

I belive it can be closed, the stack size limit is now 192KB

#define PCRE_JIT_STACK_MAX_SIZE (192 * 1024)

@nikic
Copy link
Member

nikic commented Mar 16, 2021

Right, I haven't heard about any stack size issues recently anymore, so I think the current 192KB size is sufficient. Let's reconsider if there are actual issues.

@nikic nikic closed this Mar 16, 2021
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.

9 participants