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

From: Carlo Arenas <carenas@gmail.com>
Date: Jul 29, 2019, 00:33 UTC
Message-ID: <CAPUEspgUiicBPKPZPyDFryj3OmtyOWVgrytqpzMq-PZNv1f1Mg@mail.gmail.com>
In-Reply-To: <20190726150818.6373-3-avarab@gmail.com>

On Fri, Jul 26, 2019 at 8:08 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

Show 6 quoted lines

>
> 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)

Show 5 quoted lines

> 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

Show 7 quoted lines

> 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