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

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 30, 2023, 18:54 UTC
Message-ID: <xmqq357r4zvk.fsf@gitster.g>
In-Reply-To: <9b5a1113-84f1-1651-bffc-6382462057dd@grsecurity.net>

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?

Show 24 quoted lines

> 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