Re: [PATCH v2] grep: fall back to interpreter if JIT memory allocation fails

[Junio C Hamano <gitster@pobox.com>]

Mathias Krause <minipli@grsecurity.net> writes:

> My patch doesn't make it worse than what 'git grep' would currently be
> doing. On the contrary, actually. It allows me to use PaX's MPROTECT and
> have a functional 'git grep' as well.

I know.  But then without the "why did it fail?" logic (i.e. the v1
patch), it does not make it worse than the current code, either, and
of course allows you to use JIT-enabled pcre2 even where JIT is
impossible due to MPROTECT and whatever reasson.

> Maybe the missing piece here is simply something like below to make
> users more aware of the possibility to disable the JIT for the more
> complex cases?:

If we were to keep that "die", it is absolutely required, I would
think.  Users who got their Git with JIT-enabled pcre2 may be
viewing JIT merely as "a clever optimization the implementation is
allowed to use when able", without knowing and more importantly
without wanting to know how to disable it from within their
patterns.

But can't we drop that die() if we took the v1 route?

> diff --git a/grep.c b/grep.c
> index 59afc3f07fc9..1422f168b087 100644
> --- a/grep.c
> +++ b/grep.c
> @@ -357,7 +357,8 @@ static void compile_pcre2_pattern(struct grep_pat
> *p, const struct grep_opt *opt
>                         p->pcre2_jit_on = 0;
>                         return;
>                 } else if (jitret) {
> -                       die("Couldn't JIT the PCRE2 pattern '%s', got
> '%d'\n", p->pattern, jitret);
> +                       die("Couldn't JIT the PCRE2 pattern '%s', got
> '%d'%s\n", p->pattern, jitret,
> +                           pcre2_jit_functional() ? "\nYou might retry
> by prefixing the pattern with '(*NO_JIT)'" : "");
>                 }
>
>                 /*
>
> (Sorry about the wrapped lines, my mailer is just broken. I'll make it a
> proper patch, if such functionality is indeed wanted.)
>
> Thanks,
> Mathias