Hi Anatol, Anatol Belski wrote:
> Hi Christoph, > >> -----Original Message----- >> From: Christoph Becker [mailto:cmbecke...@gmx.de] >> Sent: Saturday, July 25, 2015 6:02 PM >> To: Anatol Belski <anatol....@belski.net>; 'Pierre Joye' >> <pierre....@gmail.com> >> Cc: 'PHP internals' <internals@lists.php.net> >> Subject: Re: [PHP-DEV] PCRE JIT stack size limit >> >> Hi Anatol, >> >> Anatol Belski wrote: >> >>>> -----Original Message----- >>>> From: Christoph Becker [mailto:cmbecke...@gmx.de] >>>> Sent: Saturday, July 25, 2015 2:25 AM >>>> To: Anatol Belski <anatol....@belski.net>; 'Pierre Joye' >>>> <pierre....@gmail.com> >>>> Cc: 'PHP internals' <internals@lists.php.net> >>>> Subject: Re: [PHP-DEV] PCRE JIT stack size limit >>>> >>>> I would not suggest to change the return value of preg_match() and friends. >>>> FALSE seems to be appropriate here. Instead I suggest to add a new >>>> error constant which would be returned from preg_last_error(). >>>> >>> Yep, sounds feasible. At least users can be informed about this kind of >>> error, >> seems a constant is feasible for 7.0. >> >> That would be nice. I'll make a respective PR. > Thanks. > >> >>> Btw I've just noticed that the snippet I've posted earlier won't currently >>> work. >>> >>> If (false === preg_match(",evil pattern,", ...)) { >>> If (OUT_OF_JIT_STACK == pcre_last_error()) { >>> ini_set("pcre.jit", 0); >>> // retry >>> } >>> } >>> >>> The retry would fail even when JIT is disabled on demand. This is >>> actually rather unexpected, maybe something in PCRE JIT state cannot >>> be recovered once JIT compilation has failed. But actually, even >>> without running into debugging, it is also a bad sign telling that the >>> behavior can get complicated ( >> >> Indeed! That's caused by our PCRE cache. The cached regex is alread studied >> with JIT info, as all that happens in pcre_get_compiled-regex_cache[1]. So >> switching pcre.jit at runtime may not have the desired effect. I'm not sure >> if that >> has to be regarded as bug, and whether we should do something about it. One >> solution might be to clear the regex cache whenever the ini setting changes. >> I'll >> have a closer look. >> > Good catch. Maybe another option could be implementing the PHP pattern above > In C. Say > > - check what pcre_exec delivered > - if it’s the JIT stack error - look for the pattern in the cache > - if it's found - remove it from the cache > - recompile and cache > - exec a non JIT pattern version > > However this would silently ignore JIT, not transparent for user. Maybe > another > option could be introducing a user space function to clear a particular > pattern from > the cache. When expunging the full cache, it'll for sure have a negative > performance > effect when the cache isn't empty. But it's actually logic as one can base it > on the idea - > either JIT is enabled for everything, or it is not (for everything). Definitely interesting ideas. However, currently I'm leaning towards a full-fledged solution in PHP 7.1 (whatever that may be), and to only add the error constant for PHP 7.0. Users can set pcre.jit=0 in php.ini and not change the setting during runtime, if necessary. FWIW, some hours ago I've implemented the clean whole cache solution: <https://github.com/cmb69/php-src/commit/c41d78046da48db2fa8f5bd82fd6ac58099288f8>. >>> Running for the INI option and some custom JIT stack functionality needs >> more brainstorming and test. Please ping when you have an initial >> implementation, I'd be glad to help testing, etc. anyway. >> >> All in all it seems that we probably should have an RFC for that. I'll >> consider to >> write one, but there's no need to hurry – too late for 7.0 anyway. :) >> > Yep, the whole needs a good consideration about how to do it best without a > big > impact on the user side. Also probably it could be worth it to check how PCRE2 > behaves in this cases, maybe it were worthwhile to upgrade to PCRE2 for 7.1. Indeed, offering support for PCRE2 should be considered for 7.1. And also support of the JIT fast path API: <http://www.pcre.org/original/doc/html/pcrejit.html#SEC11>. Regards -- Christoph M. Becker -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
