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

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 27, 2023, 17:39 UTC
Message-ID: <xmqq1qnfancf.fsf@gitster.g>
In-Reply-To: <xmqqbkmk9bsn.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> writes:

Show 10 quoted lines

> Yes, the "instead of failing hard, fall back" makes sense.  Just
> that I do not see why the runtime test is a good thing to have.  In
> short, we are not in the business of catching bugs in pcre2_jit
> implementations, so if they say they cannot compile the pattern (I
> would even say I doubt the point of checking the return code to
> ensure it is NOMEMORY), it would be fine to let the interpreter
> codepath to inspect the pattern and diagnose problems with it, or
> take the slow match without JIT.
>
> What am I missing?

Note that I've seen and recently re-read the discussion that leads to https://lore.kernel.org/git/f680b274-fa85-6624-096a-7753a2671c15@grsecurity.net/

I suspect that this auto-probe is related to solving "the user thinks JIT is in use but because of failing JIT the user's pattern is getting horrible performance" somehow. But I do not think a hard failure is a good approach to help users in such a situation.

After such a failure, the user can prefix "(*NO_JIT)" to the pattern and retry, or give up the operation altogether and not get a useful result, but wouldn't it be far more helpful to just fallback as if (*NO_JIT) was on from the beginning?

Also I notice that p->pcre2_jit_on is per "struct grep_pat", so it is not like "once we see a pathological pattern, we turn off JIT completely for other patterns", right? That is, if you have

    git grep -P -e "$A" -e "$B"

and we fail to compile "$A" (for whatever reason), we could still (attempt to) compile "$B". Perhaps $A was too complex or was incompatible with JIT combined with other options, but $B may be easy enough to still be JITtable, in which case we would match with the JITted version of $B with interpreted version of $A, instead of failing, right?

THanks.