Hi Christoph,
> -----Original Message-----
> From: Christoph Becker [mailto:cmbecke...@gmx.de]
> Sent: Saturday, July 25, 2015 12:09 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
>
> Hi Anatol,
>
> Anatol Belski wrote:
>
> > This looks like an extremely fragile topic because it depends on how
> > much stack is available to an executable. A custom JIT stack can
> > behave more stable but cannot be resized. And the main issue is that
> > the JIT stack size, machine stack size and ext/pcre cache size are
> > completely unrelated terms. For example, a binary can have not enough
> > stack, but the custom JIT stack using mmap/VirtualAlloc could even
> > succeed, but then pcre_exec will be executed and overflow the machine
> > stack. We can never know which one is exhausted first - the one for
> > the JIT compilation or the other one for the execution, or vice versa.
> >
> > Generally, moving the JIT compilation away from the machine stack and
> > increasing the PCRE cache size should be more stable against this .
> > However it's an edge case. IMHO we should not do it just to fix some
> > crazy usage. Users who need it might just turn off JIT. Normal usage
> > seems not to be affected, say loading some sane functional script,
> > which FE is done by any benchmark with WP, Symfony, etc. But moving
> > JIT compilation away from the machine stack wil lpossibly affect it.
>
> Beforehand, I'm not suggesting to change anything regarding our PCRE cache
> (PCRE_G(pcre_cache)); this seems to be fine as it is, and is indeed not
> related to
> this topic.
>
> Now please consider the following simple expression:
>
> preg_match('/^(foo)+$/', str_repeat('foo', $n))
>
> This will fail (i.e. yield FALSE) independently of pcre.jit for large enough
> $n.
> However, a user can change pcre.recursion_limit what will affect the $n limit
> (the expression will fail for smaller or larger $n), if pcre.jit=0. If
> pcre.jit=1 the
> user can't influence this boundary in any way, currently.
>
> And maybe even worse, with pcre.jit=0 the boundary is 50,000, but with
> pcre.jit=1 it is only 1,366. Of course, one can argue that this is a
> contrived
> example, and that such usage is crazy, but why do we have a default
> pcre.recursion_limit of 100,000 then? A recursion_limit of
> 2,734 would be sufficient to have a boundary of $n == 1,366.
>
The 100000 is an empirical value by my guess. It regards to the default stack
sizes on different platforms and to an average pattern. There is no prediction
that PCRE will not exhaust the stack available to the binary.
> All in all, as this example already suggests, classic execution of matching
> is done
> by recursive calls (using normal stack frames), while JIT execution of
> matching is
> iterative, using a special JIT stack.[1] I don't think it is justified to
> give users a
> setting to adjust for the former, but not for the latter (except to disable
> JIT,
> albeit JIT might bring quite some boost especially for such cases).
>
JIT without custom stack will use "32k of machine stack", by the doc. We
currently don't really give users a choice to choose between iterative and
recursive PCRE execution, it's a compile time decision. And this is again
because an iterative execution will be safer, but will affect an average case
with unnecessary thrift. In this JIT case, one could give this choice, you're
right, but we should evaluate it carefully. Fe what happens to the PHP pattern
cache if JIT stack is exhausted?
What I was more precisely talking about is like
If (false === preg_match(",evil pattern,", ...)) {
Ini_set("pcre.jit", 0);
// retry
}
So received error - no JIT. And no additional logic/overhead in ext/pcre.
Maybe custom JIT were eligible in this case, but according to the PCRE doc it
can easily bring issues as the global JIT memory can't be resized/migrated just
by one's finger click. Say one would be forced to either use the custom JIT
stack or default JIT stack from the start on. This can end up with the over
complication using a custom JIT stack for a particular pattern.
> As we're pretty late in the game for PHP 7.0, it might be best to postpone a
> new
> ini setting or other changes to PHP 7.1, but at the very least I would
> introduce a
> new error constant, say PHP_PCRE_JIT_STACKLIMIT_ERROR[2], so users get a
> more meaningful result when calling preg_last_error() than
> PHP_PCRE_INTERNAL_ERROR. And it seems to be appropriate to add a note to
> UPGRADING that pcre.jit=1 may cause some preg_*() to fail which would work
> with pcre.jit=0.
>
Yes, instead of returning false one could return an explicit error, or indicate
in any other ways. Thanks for bringing it up and maybe we'll have more info
after your further investigation. But documentation is appropriate in any cases.
Regards
Anatol
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
