Re: [PATCH v2 2/8] grep: stop "using" a custom JIT stack with PCRE v2

[Carlo Arenas <carenas@gmail.com>]

On Fri, Jul 26, 2019 at 8:08 AM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
>
> As noted in [3] there are known regexes that will fail with the lower
> stack limit, the way GNU grep fixed it is interesting, although I
> believe the implementation is overly verbose, they could make PCRE v2
> handle that gradual re-allocation, that's what min/max memory is
> for.

The part I liked about the grep implementation was how they went above
and beyond to make sure there was no abstraction leak and at the end
the end user doesn't even see a PCRE error message.

Presume thought that the end user we have is different, and might make
sense to expose them to the underlying mechanism, but in that case we
should also provide them with knobs to tweak (like the one I proposed
to disable jit, and that in this case might be to set a stacksize)

> So we might end up bringing this back, I'm more inclined to just kick
> such cases upstairs to PCRE maintainers as a bug, perhaps they'll add
> some overall "just allocate more then" flag to make this easier. In
> any case there's no functional change here, we didn't have a custom
> stack, so let's apply this first, we can always revert it later.

agree, LGTM other than by the comment below

> diff --git a/grep.c b/grep.c
> index 95af88cb74..4b1e917ac5 100644
> --- a/grep.c
> +++ b/grep.c
> @@ -534,14 +534,6 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt
>                         p->pcre2_jit_on = 0;
>                         return;

this return and brackets no longer needed

Carlo