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

From: Mathias Krause <minipli@grsecurity.net>
Date: Jan 31, 2023, 07:48 UTC
Message-ID: <d1c4cbad-bbb8-d610-5e27-970d96dd7a74@grsecurity.net>
In-Reply-To: <xmqqk0131zxi.fsf@gitster.g>

On 30.01.23 22:21, Junio C Hamano wrote:

Show 33 quoted lines

> Junio C Hamano <gitster@pobox.com> writes:
> 
>> Having said all that, I do not mind queuing v2 if the "use *NO_JIT
>> to disable" is added to the message to help users who are forced to
>> redo the query.
> 
> In the meantime, here is what I plan to apply on top of v2 while
> queuing it.  The message given to die() should lack the terminating
> LF, and the overlong line can and should be split at operator
> boundary.
> 
> Thanks.
> 
>  grep.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git c/grep.c w/grep.c
> index 59afc3f07f..42f184bd09 100644
> --- c/grep.c
> +++ w/grep.c
> @@ -357,7 +357,11 @@ 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",
> +			    p->pattern, jitret,
> +			    pcre2_jit_functional() 
> +			    ? "\nPerhaps prefix (*NO_GIT) to your pattern?" 
> +			    : "");
>  		}
>  
>  		/*

Looks sensible, but maybe something like below would be even better?

diff --git a/grep.c b/grep.c
index 59afc3f07fc9..e0144ba77e7a 100644
--- a/grep.c
+++ b/grep.c
@@ -357,7 +357,13 @@ 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);
+			int do_clip = p->patternlen > 64;
+			int clip_len = do_clip ? 64 : p->patternlen;
+			die("Couldn't JIT the PCRE2 pattern '%.*s'%s, got '%d'%s",
+			    clip_len, p->pattern, do_clip ? "..." : "", jitret,
+			    pcre2_jit_functional()
+			    ? "\nPerhaps prefix (*NO_JIT) to your pattern?"
+			    : "");
 		}
 
 		/*

It'll ensure, git will be printing the hint even for very long patterns,
like the one I was testing this with ("$(perl -e 'print "(.)" x 4000')").

Thanks,
Mathias