{"thread":{"id":"51542","subject":"[RFC PATCH] grep: allow for run time disabling of JIT in PCRE","startedAt":"2019-07-28T23:54:31Z","lastAt":"2019-08-05T20:17:01Z","messageCount":23,"participants":["Carlo Marcelo Arenas Belón","Carlo Arenas","Junio C Hamano","Ævar Arnfjörð Bjarmason","René Scharfe","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"379456","messageId":"20190728235427.41425-1-carenas@gmail.com","threadId":"51542","inReplyTo":null,"subject":"[RFC PATCH] grep: allow for run time disabling of JIT in PCRE","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2019-07-28T23:54:27Z","receivedAt":"2019-07-28T23:54:31Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"PCRE1 allowed for a compile time flag to disable JIT, but PCRE2 never\nhad one, forcing the use of JIT if -P was requested.\n\nAfter ed0479ce3d (Merge branch 'ab/no-kwset' into next, 2019-07-15)\nthe PCRE2 engine will be used more broadly and therefore adding this\nknob will give users a fallback for situations like the one observed\nin OpenBSD with a JIT enabled PCRE2, because of W^X restrictions:\n\n  $ git grep 'foo bar'\n  fatal: Couldn't JIT the PCRE2 pattern 'foo bar', got '-48'\n  $ git grep -G 'foo bar'\n  fatal: Couldn't JIT the PCRE2 pattern 'foo bar', got '-48'\n  $ git grep -E 'foo bar'\n  fatal: Couldn't JIT the PCRE2 pattern 'foo bar', got '-48'\n  $ git grep -F 'foo bar'\n  fatal: Couldn't JIT the PCRE2 pattern 'foo bar', got '-48'\n\nSigned-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>\n---\n Documentation/git-grep.txt |  4 ++++\n grep.c                     | 15 +++++++++++++--\n grep.h                     |  1 +\n 3 files changed, 18 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt\nindex c89fb569e3..ff544bdeec 100644\n--- a/Documentation/git-grep.txt\n+++ b/Documentation/git-grep.txt\n@@ -69,6 +69,10 @@ grep.fallbackToNoIndex::\n \tIf set to true, fall back to git grep --no-index if git grep\n \tis executed outside of a git repository.  Defaults to false.\n \n+pcre.jit::\n+\tIf set to false, disable JIT when using PCRE.  Defaults to\n+\ttrue.\n+\n \n OPTIONS\n -------\ndiff --git a/grep.c b/grep.c\nindex c7c06ae08d..3524d353dd 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -56,6 +56,7 @@ void init_grep_defaults(struct repository *repo)\n \topt->repo = repo;\n \topt->relative = 1;\n \topt->pathname = 1;\n+\topt->pcre_jit = 1;\n \topt->max_depth = -1;\n \topt->pattern_type_option = GREP_PATTERN_TYPE_UNSPECIFIED;\n \tcolor_set(opt->colors[GREP_COLOR_CONTEXT], \"\");\n@@ -125,6 +126,12 @@ int grep_config(const char *var, const char *value, void *cb)\n \t\treturn 0;\n \t}\n \n+\tif (!strcmp(var, \"pcre.jit\")) {\n+\t\tint is_bool;\n+\t\topt->pcre_jit = git_config_bool_or_int(var, value, &is_bool);\n+\t\treturn 0;\n+\t}\n+\n \tif (!strcmp(var, \"color.grep\"))\n \t\topt->color = git_config_colorbool(var, value);\n \tif (!strcmp(var, \"color.grep.match\")) {\n@@ -163,6 +170,7 @@ void grep_init(struct grep_opt *opt, struct repository *repo, const char *prefix\n \topt->pattern_tail = &opt->pattern_list;\n \topt->header_tail = &opt->header_list;\n \n+\topt->pcre_jit = def->pcre_jit;\n \topt->only_matching = def->only_matching;\n \topt->color = def->color;\n \topt->extended_regexp_option = def->extended_regexp_option;\n@@ -393,7 +401,8 @@ static void compile_pcre1_regexp(struct grep_pat *p, const struct grep_opt *opt)\n \t\tdie(\"%s\", error);\n \n #ifdef GIT_PCRE1_USE_JIT\n-\tpcre_config(PCRE_CONFIG_JIT, &p->pcre1_jit_on);\n+\tif (opt->pcre_jit)\n+\t\tpcre_config(PCRE_CONFIG_JIT, &p->pcre1_jit_on);\n #endif\n }\n \n@@ -489,7 +498,9 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt\n \t\tcompile_regexp_failed(p, (const char *)&errbuf);\n \t}\n \n-\tpcre2_config(PCRE2_CONFIG_JIT, &p->pcre2_jit_on);\n+\tif (opt->pcre_jit)\n+\t\tpcre2_config(PCRE2_CONFIG_JIT, &p->pcre2_jit_on);\n+\n \tif (p->pcre2_jit_on) {\n \t\tjitret = pcre2_jit_compile(p->pcre2_pattern, PCRE2_JIT_COMPLETE);\n \t\tif (jitret)\ndiff --git a/grep.h b/grep.h\nindex c0c71eb4a9..fff152e606 100644\n--- a/grep.h\n+++ b/grep.h\n@@ -151,6 +151,7 @@ struct grep_opt {\n \tint allow_textconv;\n \tint extended;\n \tint use_reflog_filter;\n+\tint pcre_jit;\n \tint pcre1;\n \tint pcre2;\n \tint relative;\n-- \n2.22.0\n\n"},{"id":"379457","messageId":"CAPUEsphhsbdeutJ8=XrBx-y0shf+64O9o55AeQBhnXYso_1PyQ@mail.gmail.com","threadId":"51542","inReplyTo":"20190728235427.41425-1-carenas@gmail.com","subject":"Re: [RFC PATCH] grep: allow for run time disabling of JIT in PCRE","fromName":"Carlo Arenas","fromEmail":"carenas@gmail.com","sentAt":"2019-07-29T00:09:16Z","receivedAt":"2019-07-29T00:09:29Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"On Sun, Jul 28, 2019 at 4:54 PM Carlo Marcelo Arenas Belón\n<carenas@gmail.com> wrote:\n> @@ -125,6 +126,12 @@ int grep_config(const char *var, const char *value, void *cb)\n>                 return 0;\n>         }\n>\n> +       if (!strcmp(var, \"pcre.jit\")) {\n> +               int is_bool;\n> +               opt->pcre_jit = git_config_bool_or_int(var, value, &is_bool);\n> +               return 0;\n> +       }\n> +\n>         if (!strcmp(var, \"color.grep\"))\n>                 opt->color = git_config_colorbool(var, value);\n>         if (!strcmp(var, \"color.grep.match\")) {\n\nusing git_config_bool_or_int, as I am hoping a future version will use\na third value (maybe -1) to\nindicate JIT will be tried first, but then the interpreter will be\nused in case JIT is not available (as\nrecommended in PCRE)\n\nnot sure also about the right name and where to document this flag, as\nthis is not only restricted to\nthe grep subcommand and the issue it is working around will be also\nrelevant for log (including pickaxe)\n\nCarlo\n"},{"id":"379469","messageId":"xmqqimrll9ow.fsf@gitster-ct.c.googlers.com","threadId":"51542","inReplyTo":"20190728235427.41425-1-carenas@gmail.com","subject":"Re: [RFC PATCH] grep: allow for run time disabling of JIT in PCRE","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-07-29T04:57:19Z","receivedAt":"2019-07-29T04:57:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Carlo Marcelo Arenas Belón  <carenas@gmail.com> writes:\n\n> PCRE1 allowed for a compile time flag to disable JIT, but PCRE2 never\n> had one, forcing the use of JIT if -P was requested.\n>\n> After ed0479ce3d (Merge branch 'ab/no-kwset' into next, 2019-07-15)\n> the PCRE2 engine will be used more broadly and therefore adding this\n> knob will give users a fallback for situations like the one observed\n> in OpenBSD with a JIT enabled PCRE2, because of W^X restrictions:\n>\n>   $ git grep 'foo bar'\n>   fatal: Couldn't JIT the PCRE2 pattern 'foo bar', got '-48'\n>   $ git grep -G 'foo bar'\n>   fatal: Couldn't JIT the PCRE2 pattern 'foo bar', got '-48'\n>   $ git grep -E 'foo bar'\n>   fatal: Couldn't JIT the PCRE2 pattern 'foo bar', got '-48'\n>   $ git grep -F 'foo bar'\n>   fatal: Couldn't JIT the PCRE2 pattern 'foo bar', got '-48'\n\n;-) Yeah, we should have known that security-paranoid distros would\nhave W^X issues with this series, too.\n\nI am not sure I like a config-only knob like this,\nthough---shouldn't we have a command line knob to turn jit off\nfirst, and then for those who gets tired of having to type it all\nthe time add the configuration to flip the default for them?\n\nOther than that, the feature itself makes quite a lot of sense.\n\n>\n> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>\n> ---\n>  Documentation/git-grep.txt |  4 ++++\n>  grep.c                     | 15 +++++++++++++--\n>  grep.h                     |  1 +\n>  3 files changed, 18 insertions(+), 2 deletions(-)\n>\n> diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt\n> index c89fb569e3..ff544bdeec 100644\n> --- a/Documentation/git-grep.txt\n> +++ b/Documentation/git-grep.txt\n> @@ -69,6 +69,10 @@ grep.fallbackToNoIndex::\n>  \tIf set to true, fall back to git grep --no-index if git grep\n>  \tis executed outside of a git repository.  Defaults to false.\n>  \n> +pcre.jit::\n> +\tIf set to false, disable JIT when using PCRE.  Defaults to\n> +\ttrue.\n> +\n>  \n>  OPTIONS\n>  -------\n> diff --git a/grep.c b/grep.c\n> index c7c06ae08d..3524d353dd 100644\n> --- a/grep.c\n> +++ b/grep.c\n> @@ -56,6 +56,7 @@ void init_grep_defaults(struct repository *repo)\n>  \topt->repo = repo;\n>  \topt->relative = 1;\n>  \topt->pathname = 1;\n> +\topt->pcre_jit = 1;\n>  \topt->max_depth = -1;\n>  \topt->pattern_type_option = GREP_PATTERN_TYPE_UNSPECIFIED;\n>  \tcolor_set(opt->colors[GREP_COLOR_CONTEXT], \"\");\n> @@ -125,6 +126,12 @@ int grep_config(const char *var, const char *value, void *cb)\n>  \t\treturn 0;\n>  \t}\n>  \n> +\tif (!strcmp(var, \"pcre.jit\")) {\n> +\t\tint is_bool;\n> +\t\topt->pcre_jit = git_config_bool_or_int(var, value, &is_bool);\n> +\t\treturn 0;\n> +\t}\n> +\n>  \tif (!strcmp(var, \"color.grep\"))\n>  \t\topt->color = git_config_colorbool(var, value);\n>  \tif (!strcmp(var, \"color.grep.match\")) {\n> @@ -163,6 +170,7 @@ void grep_init(struct grep_opt *opt, struct repository *repo, const char *prefix\n>  \topt->pattern_tail = &opt->pattern_list;\n>  \topt->header_tail = &opt->header_list;\n>  \n> +\topt->pcre_jit = def->pcre_jit;\n>  \topt->only_matching = def->only_matching;\n>  \topt->color = def->color;\n>  \topt->extended_regexp_option = def->extended_regexp_option;\n> @@ -393,7 +401,8 @@ static void compile_pcre1_regexp(struct grep_pat *p, const struct grep_opt *opt)\n>  \t\tdie(\"%s\", error);\n>  \n>  #ifdef GIT_PCRE1_USE_JIT\n> -\tpcre_config(PCRE_CONFIG_JIT, &p->pcre1_jit_on);\n> +\tif (opt->pcre_jit)\n> +\t\tpcre_config(PCRE_CONFIG_JIT, &p->pcre1_jit_on);\n>  #endif\n>  }\n>  \n> @@ -489,7 +498,9 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt\n>  \t\tcompile_regexp_failed(p, (const char *)&errbuf);\n>  \t}\n>  \n> -\tpcre2_config(PCRE2_CONFIG_JIT, &p->pcre2_jit_on);\n> +\tif (opt->pcre_jit)\n> +\t\tpcre2_config(PCRE2_CONFIG_JIT, &p->pcre2_jit_on);\n> +\n>  \tif (p->pcre2_jit_on) {\n>  \t\tjitret = pcre2_jit_compile(p->pcre2_pattern, PCRE2_JIT_COMPLETE);\n>  \t\tif (jitret)\n> diff --git a/grep.h b/grep.h\n> index c0c71eb4a9..fff152e606 100644\n> --- a/grep.h\n> +++ b/grep.h\n> @@ -151,6 +151,7 @@ struct grep_opt {\n>  \tint allow_textconv;\n>  \tint extended;\n>  \tint use_reflog_filter;\n> +\tint pcre_jit;\n>  \tint pcre1;\n>  \tint pcre2;\n>  \tint relative;\n"},{"id":"379472","messageId":"CAPUEspiwTi2WfpTK_kxTEuKZrKJF2+_KZHsiaGmjctKR4BbtTw@mail.gmail.com","threadId":"51542","inReplyTo":"xmqqimrll9ow.fsf@gitster-ct.c.googlers.com","subject":"Re: [RFC PATCH] grep: allow for run time disabling of JIT in PCRE","fromName":"Carlo Arenas","fromEmail":"carenas@gmail.com","sentAt":"2019-07-29T05:29:34Z","receivedAt":"2019-07-29T05:29:47Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"On Sun, Jul 28, 2019 at 9:57 PM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> I am not sure I like a config-only knob like this,\n> though---shouldn't we have a command line knob to turn jit off\n> first, and then for those who gets tired of having to type it all\n> the time add the configuration to flip the default for them?\n\nare you suggesting to add a --pcre-jit parameter to both grep and log?\n\nguess using pcre.jit for the configuration makes sense as far as\nit is documented on both man pages, or would this imply this should go\ninstead into core?\n\nI was expecting this to be set system (or at least global) wide most\nof the time like\n(ex: core.ignorecase, credential.helper) neither of which I can relate\nto a command line parameter.\n\n> Other than that, the feature itself makes quite a lot of sense.\n\nshould I target maint/master?, next conflicts because of ab/no-kwset and it also\nconflicts with other in fly features too (some not even in pu), which is why now\nis based on pu and depends on ab/pcre-jit-fixes\n\nCarlo\n"},{"id":"379479","messageId":"8736ip6wzk.fsf@evledraar.gmail.com","threadId":"51542","inReplyTo":"20190728235427.41425-1-carenas@gmail.com","subject":"Re: [RFC PATCH] grep: allow for run time disabling of JIT in PCRE","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-07-29T08:55:27Z","receivedAt":"2019-07-29T08:55:32Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Jul 29 2019, Carlo Marcelo Arenas Belón wrote:\n\n> PCRE1 allowed for a compile time flag to disable JIT, but PCRE2 never\n> had one, forcing the use of JIT if -P was requested.\n\nWhat's that PCRE1 compile-time flag?\n\n> After ed0479ce3d (Merge branch 'ab/no-kwset' into next, 2019-07-15)\n> the PCRE2 engine will be used more broadly and therefore adding this\n> knob will give users a fallback for situations like the one observed\n> in OpenBSD with a JIT enabled PCRE2, because of W^X restrictions:\n>\n>   $ git grep 'foo bar'\n>   fatal: Couldn't JIT the PCRE2 pattern 'foo bar', got '-48'\n>   $ git grep -G 'foo bar'\n>   fatal: Couldn't JIT the PCRE2 pattern 'foo bar', got '-48'\n>   $ git grep -E 'foo bar'\n>   fatal: Couldn't JIT the PCRE2 pattern 'foo bar', got '-48'\n>   $ git grep -F 'foo bar'\n>   fatal: Couldn't JIT the PCRE2 pattern 'foo bar', got '-48'\n\nYeah that obviously sucks more with ab/no-kwset, but that seems like a\ncase where -P would have been completely broken before, and therefore I\ncan't imagine the package ever passed \"make test\". Or is W^X also\nexposed as some run-time option on OpenBSD?\n\nI.e. aside from the merits of such a setting in general these examples\nseem like just working around something that should be fixed at make\nall/test time, or maybe I'm missing something.\n\nTo the extent that we'd want to make this sort of thing configurable, I\nwonder if a continuation of my (*NO_JIT) patch isn't better, i.e. just\nadding the ability to configure some string we'd inject at the start of\nevery pattern.\n\nThat would allow for setting any other number of options in\npcre2syntax(3) without us needing to carry config for each one,\ne.g. (*LIMIT_HEAP=d), (*LIMIT_DEPTH=d) etc. It does present a larger\nfoot-gun surface though...\n"},{"id":"379491","messageId":"CAPUEspgQNCENviPYP6X790DvSgj_RpJVo2KP_39voLQnVc65pQ@mail.gmail.com","threadId":"51542","inReplyTo":"8736ip6wzk.fsf@evledraar.gmail.com","subject":"Re: [RFC PATCH] grep: allow for run time disabling of JIT in PCRE","fromName":"Carlo Arenas","fromEmail":"carenas@gmail.com","sentAt":"2019-07-29T10:26:42Z","receivedAt":"2019-07-29T10:26:56Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"On Mon, Jul 29, 2019 at 1:55 AM Ævar Arnfjörð Bjarmason\n<avarab@gmail.com> wrote:\n>\n> On Mon, Jul 29 2019, Carlo Marcelo Arenas Belón wrote:\n>\n> > PCRE1 allowed for a compile time flag to disable JIT, but PCRE2 never\n> > had one, forcing the use of JIT if -P was requested.\n>\n> What's that PCRE1 compile-time flag?\n\nNO_LIBPCRE1_JIT at GIT compile time (regardless of JIT support in the\nPCRE1 library you are using)\n\n> > After ed0479ce3d (Merge branch 'ab/no-kwset' into next, 2019-07-15)\n> > the PCRE2 engine will be used more broadly and therefore adding this\n> > knob will give users a fallback for situations like the one observed\n> > in OpenBSD with a JIT enabled PCRE2, because of W^X restrictions:\n> >\n> >   $ git grep 'foo bar'\n> >   fatal: Couldn't JIT the PCRE2 pattern 'foo bar', got '-48'\n> >   $ git grep -G 'foo bar'\n> >   fatal: Couldn't JIT the PCRE2 pattern 'foo bar', got '-48'\n> >   $ git grep -E 'foo bar'\n> >   fatal: Couldn't JIT the PCRE2 pattern 'foo bar', got '-48'\n> >   $ git grep -F 'foo bar'\n> >   fatal: Couldn't JIT the PCRE2 pattern 'foo bar', got '-48'\n>\n> Yeah that obviously sucks more with ab/no-kwset, but that seems like a\n> case where -P would have been completely broken before, and therefore I\n> can't imagine the package ever passed \"make test\". Or is W^X also\n> exposed as some run-time option on OpenBSD?\n\nironically, you could use PCRE1 since that is not using the JIT fast\npath and therefore will fallback automatically to the interpreter\n\nthere is also a convoluted way to make your binary work by moving\nit into a mount point that has been specially exempted from that W^X\nrestriction.\n\n> I.e. aside from the merits of such a setting in general these examples\n> seem like just working around something that should be fixed at make\n> all/test time, or maybe I'm missing something.\n\n1) before you could just avoid using -P and still be able to grep\n2) there is no way to tell PCRE2 to get out of the way even if you are\n    not using -P\n\nyou are right though that this is not a new problem and was reported\nbefore with patches and the last comment saying a configuration\nshould be provided.\n\n> To the extent that we'd want to make this sort of thing configurable, I\n> wonder if a continuation of my (*NO_JIT) patch isn't better, i.e. just\n> adding the ability to configure some string we'd inject at the start of\n> every pattern.\n\nlooking at the number of lines of code, it would seem the configuration\napproach is simpler.\n\n> That would allow for setting any other number of options in\n> pcre2syntax(3) without us needing to carry config for each one,\n> e.g. (*LIMIT_HEAP=d), (*LIMIT_DEPTH=d) etc. It does present a larger\n> foot-gun surface though...\n\nthe parameters I suspect users might need are not really accessible through\nthat (ex: jit stacksize).\n\nit is important to note that currently we are not preventing any user to use\nthose flags themselves in their patterns either.\n\nCarlo\n"},{"id":"379492","messageId":"20190729105955.44390-1-carenas@gmail.com","threadId":"51542","inReplyTo":"20190728235427.41425-1-carenas@gmail.com","subject":"[RFC PATCH v2] grep: allow for run time disabling of JIT in PCRE","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2019-07-29T10:59:55Z","receivedAt":"2019-07-29T10:59:59Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"PCRE1 allowed for a compile time flag to disable JIT, but PCRE2 never\nhad one, forcing the use of JIT if -P was requested.\n\nAfter ed0479ce3d (Merge branch 'ab/no-kwset' into next, 2019-07-15)\nthe PCRE2 engine will be used more broadly and therefore adding this\nknob will allow users a escape from situations where JIT might be\nproblematic.\n\nJIT will be used by default but it can be disabled with the --no-pcre-jit\noption in `git grep` or by setting 0/false into the pcre.jit config.\n\nIf a value of -1 is used instead then the following error is prevented by\nusing the interpreter when a JIT failure consistent with known security\nrestrictions is found at regex compilation time.\n\n  $ git grep 'foo bar'\n  fatal: Couldn't JIT the PCRE2 pattern 'foo bar', got '-48'\n\nSigned-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>\n---\nV2: add command line to grep as suggested by Junio\n\n Documentation/git-grep.txt | 11 +++++++++++\n builtin/grep.c             |  4 ++++\n grep.c                     | 30 ++++++++++++++++++++++++++----\n grep.h                     |  1 +\n 4 files changed, 42 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt\nindex c89fb569e3..895c6b34ec 100644\n--- a/Documentation/git-grep.txt\n+++ b/Documentation/git-grep.txt\n@@ -13,6 +13,7 @@ SYNOPSIS\n \t   [-v | --invert-match] [-h|-H] [--full-name]\n \t   [-E | --extended-regexp] [-G | --basic-regexp]\n \t   [-P | --perl-regexp]\n+\t   [-j | --[no]-pcre-jit]\n \t   [-F | --fixed-strings] [-n | --line-number] [--column]\n \t   [-l | --files-with-matches] [-L | --files-without-match]\n \t   [(-O | --open-files-in-pager) [<pager>]]\n@@ -69,6 +70,12 @@ grep.fallbackToNoIndex::\n \tIf set to true, fall back to git grep --no-index if git grep\n \tis executed outside of a git repository.  Defaults to false.\n \n+pcre.jit::\n+\tIf set to false, disable JIT when using PCRE.  Defaults to\n+\ttrue.\n+\tif set to -1 will try first to use JIT and fallback to the\n+\tinterpreter instead of returning an error.\n+\n \n OPTIONS\n -------\n@@ -175,6 +182,10 @@ providing this option will cause it to die.\n \tUse fixed strings for patterns (don't interpret pattern\n \tas a regex).\n \n+-j::\n+--[no-]pcre-jit::\n+\tDiable JIT in PCRE with --no-pcre-jit.\n+\n -n::\n --line-number::\n \tPrefix the line number to matching lines.\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 580fd38f41..b0e94875b2 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -923,6 +923,10 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOL_F(0, \"ext-grep\", &external_grep_allowed__ignored,\n \t\t\t   N_(\"allow calling of grep(1) (ignored by this build)\"),\n \t\t\t   PARSE_OPT_NOCOMPLETE),\n+\t\tOPT_GROUP(\"PCRE\"),\n+\t\tOPT_SET_INT('j', \"pcre-jit\", &opt.pcre_jit,\n+\t\t\tN_(\"when to use JIT with PCRE\"),\n+\t\t\t1),\n \t\tOPT_END()\n \t};\n \ndiff --git a/grep.c b/grep.c\nindex c7c06ae08d..d58cad0257 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -56,6 +56,7 @@ void init_grep_defaults(struct repository *repo)\n \topt->repo = repo;\n \topt->relative = 1;\n \topt->pathname = 1;\n+\topt->pcre_jit = 1;\n \topt->max_depth = -1;\n \topt->pattern_type_option = GREP_PATTERN_TYPE_UNSPECIFIED;\n \tcolor_set(opt->colors[GREP_COLOR_CONTEXT], \"\");\n@@ -125,6 +126,12 @@ int grep_config(const char *var, const char *value, void *cb)\n \t\treturn 0;\n \t}\n \n+\tif (!strcmp(var, \"pcre.jit\")) {\n+\t\tint is_bool;\n+\t\topt->pcre_jit = git_config_bool_or_int(var, value, &is_bool);\n+\t\treturn 0;\n+\t}\n+\n \tif (!strcmp(var, \"color.grep\"))\n \t\topt->color = git_config_colorbool(var, value);\n \tif (!strcmp(var, \"color.grep.match\")) {\n@@ -163,6 +170,7 @@ void grep_init(struct grep_opt *opt, struct repository *repo, const char *prefix\n \topt->pattern_tail = &opt->pattern_list;\n \topt->header_tail = &opt->header_list;\n \n+\topt->pcre_jit = def->pcre_jit;\n \topt->only_matching = def->only_matching;\n \topt->color = def->color;\n \topt->extended_regexp_option = def->extended_regexp_option;\n@@ -393,7 +401,8 @@ static void compile_pcre1_regexp(struct grep_pat *p, const struct grep_opt *opt)\n \t\tdie(\"%s\", error);\n \n #ifdef GIT_PCRE1_USE_JIT\n-\tpcre_config(PCRE_CONFIG_JIT, &p->pcre1_jit_on);\n+\tif (opt->pcre_jit)\n+\t\tpcre_config(PCRE_CONFIG_JIT, &p->pcre1_jit_on);\n #endif\n }\n \n@@ -489,11 +498,24 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt\n \t\tcompile_regexp_failed(p, (const char *)&errbuf);\n \t}\n \n-\tpcre2_config(PCRE2_CONFIG_JIT, &p->pcre2_jit_on);\n+\tif (opt->pcre_jit)\n+\t\tpcre2_config(PCRE2_CONFIG_JIT, &p->pcre2_jit_on);\n+\n \tif (p->pcre2_jit_on) {\n \t\tjitret = pcre2_jit_compile(p->pcre2_pattern, PCRE2_JIT_COMPLETE);\n-\t\tif (jitret)\n-\t\t\tdie(\"Couldn't JIT the PCRE2 pattern '%s', got '%d'\\n\", p->pattern, jitret);\n+\t\tif (jitret) {\n+\t\t\tif ((opt->pcre_jit < 0) &&\n+\t\t\t\tjitret == PCRE2_ERROR_NOMEMORY) {\n+\t\t\t\t/*\n+\t\t\t\t * JIT compiler isn't available but we can\n+\t\t\t\t * still fallback to the interpreter\n+\t\t\t\t */\n+\t\t\t\tp->pcre2_jit_on = 0;\n+\t\t\t\treturn;\n+\t\t\t}\n+\t\t\telse\n+\t\t\t\tdie(\"Couldn't JIT the PCRE2 pattern '%s', got '%d'\\n\", p->pattern, jitret);\n+\t\t}\n \n \t\t/*\n \t\t * The pcre2_config(PCRE2_CONFIG_JIT, ...) call just\ndiff --git a/grep.h b/grep.h\nindex c0c71eb4a9..fff152e606 100644\n--- a/grep.h\n+++ b/grep.h\n@@ -151,6 +151,7 @@ struct grep_opt {\n \tint allow_textconv;\n \tint extended;\n \tint use_reflog_filter;\n+\tint pcre_jit;\n \tint pcre1;\n \tint pcre2;\n \tint relative;\n\nbase-commit: 870eea81669bfff4333b37b11fedd870cd05fd90\n-- \n2.22.0\n\n"},{"id":"379493","messageId":"CAPUEspi7jvGEx_3CMPpYp7_gip0Mv=+MZdfWYoj+NJmomXJTHw@mail.gmail.com","threadId":"51542","inReplyTo":"20190729105955.44390-1-carenas@gmail.com","subject":"Re: [RFC PATCH v2] grep: allow for run time disabling of JIT in PCRE","fromName":"Carlo Arenas","fromEmail":"carenas@gmail.com","sentAt":"2019-07-29T11:33:45Z","receivedAt":"2019-07-29T11:33:59Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"Known Issues:\n* PCRE1 is broken, but fixing it would make more sense on top of the\ntopic[1] (not in pu)\n* it depends on the current ab/pcre-jit-fixes that is missing 1\ncritical commit in pu\n* no tests yet; would need to extend it on top of the debug from Beat\nand test-tool changes from Ævar, neither of which are final\n* need to build on top of pu and will need further changes to be ready\nfor next/master/maint\n\nThe code has been tested in OpenBSD with PCRE2 (latest from svn, but\nany version should work if they are JIT enabled), it is\nexpected to also work in NetBSD (even with PAX enabled kernels) and\nmacOS 10.13.6 but haven't yet tested them.  HardenedBSD\nwill likely segfault unless pcre.jit=0 as described in the original report[2]\n\nTesting with SElinux and PAX enabled for Linux encouraged\n\nCarlo\n\n[1] https://public-inbox.org/git/20190726202642.7986-1-carenas@gmail.com/\n[2] https://public-inbox.org/git/20181209230024.43444-1-carenas@gmail.com/\n"},{"id":"379494","messageId":"87wog15834.fsf@evledraar.gmail.com","threadId":"51542","inReplyTo":"CAPUEspgQNCENviPYP6X790DvSgj_RpJVo2KP_39voLQnVc65pQ@mail.gmail.com","subject":"Re: [RFC PATCH] grep: allow for run time disabling of JIT in PCRE","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-07-29T12:38:39Z","receivedAt":"2019-07-29T12:38:45Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Jul 29 2019, Carlo Arenas wrote:\n\n> On Mon, Jul 29, 2019 at 1:55 AM Ævar Arnfjörð Bjarmason\n> <avarab@gmail.com> wrote:\n>>\n>> On Mon, Jul 29 2019, Carlo Marcelo Arenas Belón wrote:\n>>\n>> > PCRE1 allowed for a compile time flag to disable JIT, but PCRE2 never\n>> > had one, forcing the use of JIT if -P was requested.\n>>\n>> What's that PCRE1 compile-time flag?\n>\n> NO_LIBPCRE1_JIT at GIT compile time (regardless of JIT support in the\n> PCRE1 library you are using)\n\nAh of course, I was reading this as \"regexp\ncompile-time\". I.e. something like (*NO_JIT). No *such* thing exists for\nPCRE v1 JIT AFAIK as exposed by git-grep.\n\n>> > After ed0479ce3d (Merge branch 'ab/no-kwset' into next, 2019-07-15)\n>> > the PCRE2 engine will be used more broadly and therefore adding this\n>> > knob will give users a fallback for situations like the one observed\n>> > in OpenBSD with a JIT enabled PCRE2, because of W^X restrictions:\n>> >\n>> >   $ git grep 'foo bar'\n>> >   fatal: Couldn't JIT the PCRE2 pattern 'foo bar', got '-48'\n>> >   $ git grep -G 'foo bar'\n>> >   fatal: Couldn't JIT the PCRE2 pattern 'foo bar', got '-48'\n>> >   $ git grep -E 'foo bar'\n>> >   fatal: Couldn't JIT the PCRE2 pattern 'foo bar', got '-48'\n>> >   $ git grep -F 'foo bar'\n>> >   fatal: Couldn't JIT the PCRE2 pattern 'foo bar', got '-48'\n>>\n>> Yeah that obviously sucks more with ab/no-kwset, but that seems like a\n>> case where -P would have been completely broken before, and therefore I\n>> can't imagine the package ever passed \"make test\". Or is W^X also\n>> exposed as some run-time option on OpenBSD?\n>\n> ironically, you could use PCRE1 since that is not using the JIT fast\n> path and therefore will fallback automatically to the interpreter\n\n...because OpenBSD PCRE v1 was compiled with --disable-jit before, but\ntheir v2 package has --enable-jit, it just doesn't work at all? Is this\nyour custom built git + OpenBSD packages of PCRE coming with the OS?\n\nI don't use OpenBSD, but isn't this their recipe? Seems they use \"make\ntest\", and don't compile with PCRE at all if I'm reading it right:\nhttps://github.com/openbsd/ports/blob/master/devel/git/Makefile\n\n> there is also a convoluted way to make your binary work by moving\n> it into a mount point that has been specially exempted from that W^X\n> restriction.\n>\n>> I.e. aside from the merits of such a setting in general these examples\n>> seem like just working around something that should be fixed at make\n>> all/test time, or maybe I'm missing something.\n>\n> 1) before you could just avoid using -P and still be able to grep\n> 2) there is no way to tell PCRE2 to get out of the way even if you are\n>     not using -P\n\nRight, no arguments at all about ab/no-kwset making this worse (re: your\n#1). I just really prefer not to expose/document config for what\n*should* be something purely internal if the X-Y problem is a bug being\nexposed that we should just fix.\n\nParticularly because I think it's a losing battle to provide run-time\noptions for what are surely a *lot* of \"make test\" failures.\n\nIf it really is unavoidable to detect this until runtime in some common\nconfigurations I have no problem with it, I just haven't encountered\nthat so far.\n\n> you are right though that this is not a new problem and was reported\n> before with patches and the last comment saying a configuration\n> should be provided.\n\npatches = your recent\nhttps://public-inbox.org/git/20181209230024.43444-2-carenas@gmail.com/\nor something earlier?\n\nThat patch seems sane without having tested it. Seems like the\nequivalent of what we do with v1 with PCRE2_JIT_COMPLETE.\n\nI *am* curious if there's setups where fixing the code for PCRE v1 isn't\npurely an academic exercise. Is there a reason for why these platforms\ncan't just move to PCRE v2 in principle (dumpster fires in \"next\"\nnon-withstanding)?\n\n>> To the extent that we'd want to make this sort of thing configurable, I\n>> wonder if a continuation of my (*NO_JIT) patch isn't better, i.e. just\n>> adding the ability to configure some string we'd inject at the start of\n>> every pattern.\n>\n> looking at the number of lines of code, it would seem the configuration\n> approach is simpler.\n>\n>> That would allow for setting any other number of options in\n>> pcre2syntax(3) without us needing to carry config for each one,\n>> e.g. (*LIMIT_HEAP=d), (*LIMIT_DEPTH=d) etc. It does present a larger\n>> foot-gun surface though...\n>\n> the parameters I suspect users might need are not really accessible through\n> that (ex: jit stacksize).\n>\n> it is important to note that currently we are not preventing any user to use\n> those flags themselves in their patterns either.\n"},{"id":"379504","messageId":"2f9cc063-96fc-dc99-0e29-230f1e12c1d3@web.de","threadId":"51542","inReplyTo":"20190729105955.44390-1-carenas@gmail.com","subject":"Re: [RFC PATCH v2] grep: allow for run time disabling of JIT in PCRE","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2019-07-29T15:11:19Z","receivedAt":"2019-07-29T15:11:40Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 29.07.19 um 12:59 schrieb Carlo Marcelo Arenas Belón:\n> PCRE1 allowed for a compile time flag to disable JIT, but PCRE2 never\n> had one, forcing the use of JIT if -P was requested.\n>\n> After ed0479ce3d (Merge branch 'ab/no-kwset' into next, 2019-07-15)\n> the PCRE2 engine will be used more broadly and therefore adding this\n> knob will allow users a escape from situations where JIT might be\n> problematic.\n>\n> JIT will be used by default but it can be disabled with the --no-pcre-jit\n> option in `git grep` or by setting 0/false into the pcre.jit config.\n>\n> If a value of -1 is used instead then the following error is prevented by\n> using the interpreter when a JIT failure consistent with known security\n> restrictions is found at regex compilation time.\n>\n>   $ git grep 'foo bar'\n>   fatal: Couldn't JIT the PCRE2 pattern 'foo bar', got '-48'\n>\n> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>\n> ---\n> V2: add command line to grep as suggested by Junio\n>\n>  Documentation/git-grep.txt | 11 +++++++++++\n>  builtin/grep.c             |  4 ++++\n>  grep.c                     | 30 ++++++++++++++++++++++++++----\n>  grep.h                     |  1 +\n>  4 files changed, 42 insertions(+), 4 deletions(-)\n>\n> diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt\n> index c89fb569e3..895c6b34ec 100644\n> --- a/Documentation/git-grep.txt\n> +++ b/Documentation/git-grep.txt\n> @@ -13,6 +13,7 @@ SYNOPSIS\n>  \t   [-v | --invert-match] [-h|-H] [--full-name]\n>  \t   [-E | --extended-regexp] [-G | --basic-regexp]\n>  \t   [-P | --perl-regexp]\n> +\t   [-j | --[no]-pcre-jit]\n\nDo users care?  Enough so to add a short option for this?\n\n>  \t   [-F | --fixed-strings] [-n | --line-number] [--column]\n>  \t   [-l | --files-with-matches] [-L | --files-without-match]\n>  \t   [(-O | --open-files-in-pager) [<pager>]]\n> @@ -69,6 +70,12 @@ grep.fallbackToNoIndex::\n>  \tIf set to true, fall back to git grep --no-index if git grep\n>  \tis executed outside of a git repository.  Defaults to false.\n>\n> +pcre.jit::\n> +\tIf set to false, disable JIT when using PCRE.  Defaults to\n> +\ttrue.\n> +\tif set to -1 will try first to use JIT and fallback to the\n> +\tinterpreter instead of returning an error.\n\nWhy not implement only -1, without adding this config setting?\n\n> +\n>\n>  OPTIONS\n>  -------\n> @@ -175,6 +182,10 @@ providing this option will cause it to die.\n>  \tUse fixed strings for patterns (don't interpret pattern\n>  \tas a regex).\n>\n> +-j::\n> +--[no-]pcre-jit::\n> +\tDiable JIT in PCRE with --no-pcre-jit.\n\n\"Disable\".\n\nRené\n"},{"id":"379511","messageId":"xmqqh874vikk.fsf@gitster-ct.c.googlers.com","threadId":"51542","inReplyTo":"2f9cc063-96fc-dc99-0e29-230f1e12c1d3@web.de","subject":"Re: [RFC PATCH v2] grep: allow for run time disabling of JIT in PCRE","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-07-29T17:47:39Z","receivedAt":"2019-07-29T17:47:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe <l.s.r@web.de> writes:\n\n>> +pcre.jit::\n>> +\tIf set to false, disable JIT when using PCRE.  Defaults to\n>> +\ttrue.\n>> +\tif set to -1 will try first to use JIT and fallback to the\n>> +\tinterpreter instead of returning an error.\n>\n> Why not implement only -1, without adding this config setting?\n\n... nor command line option.  If we have an auto-fallback, I would\nthink that makes the most sense.  IIRC the first iteration with only\nthe configuration was really about working around the (non-working)\npcre-jit---if we can self-detect and skip a non-working case, that\nwould allow us to drop end-user facing knobs, which is ideal.\n\nThanks for a doze of sanity.\n\n"},{"id":"379567","messageId":"CAPUEspjo3Uo8KtSYQ=uh+_gPEjb+dBdSPgsEVE1j1bOMPF=0DA@mail.gmail.com","threadId":"51542","inReplyTo":"xmqqh874vikk.fsf@gitster-ct.c.googlers.com","subject":"Re: [RFC PATCH v2] grep: allow for run time disabling of JIT in PCRE","fromName":"Carlo Arenas","fromEmail":"carenas@gmail.com","sentAt":"2019-07-30T00:49:35Z","receivedAt":"2019-07-30T00:49:48Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"On Mon, Jul 29, 2019 at 10:47 AM Junio C Hamano <gitster@pobox.com> wrote:\n> René Scharfe <l.s.r@web.de> writes:\n> >> +pcre.jit::\n> >> +    If set to false, disable JIT when using PCRE.  Defaults to\n> >> +    true.\n> >> +    if set to -1 will try first to use JIT and fallback to the\n> >> +    interpreter instead of returning an error.\n> >\n> > Why not implement only -1, without adding this config setting?\n>\n> ... nor command line option.  If we have an auto-fallback, I would\n> think that makes the most sense.  IIRC the first iteration with only\n> the configuration was really about working around the (non-working)\n> pcre-jit---if we can self-detect and skip a non-working case, that\n> would allow us to drop end-user facing knobs, which is ideal.\n\nbecause that was proposed earlier[1] and wasn't accepted ;)\n\nthe main pushback though I got was that doing the fallback would degrade\nperformance and so it was suggested[2] that keeping the error should be\npossible somehow (with the implication it will add yet another macro)\n\nsince living without grep -P was a reasonable tradeoff at that time got\npunted, but the need to find a solution for this become more urgent once\nit was announced[3] PCRE2 would be used also used outside -P\n\n> Thanks for a doze of sanity.\n\nObviously I am biased, but I kind of like the knob as it allows the user\nmore flexibility to tweak the internals of grep and because we had\nmade those internals already visible (ex: not handling any library\nerrors ourselves and just aborting with a pcre error), but without any\nflexibility to fix those problems themselves (unless they open the code\nand rebuild, in most cases)\n\nthe comment from the user that reported[4] a regression with GNU grep\nbecause of JIT stack size and that I quote below is representative of how\nthat layering violation affect users, and while git users are more likely\nthan grep users to do the code tweaking needed, they could use some\nhelp.\n\n\"As using the JIT can not be turned off at runtime, nor can the stacksize\nbe controlled without patching + recompiling, this breaks previously\nworking expressions for me, so I consider this a new regression,\nintroduced with b06f7a29a58bbdd5866afc1e92dba3fdc9e2ed59 .\n\nI tested that increasing the stack-size to 1 M fixes the problem for me.\nA better fix could maybe consist of a better error message, allowing\nstack-size control at runtime and / or making JIT optional at runtime.\"\n\nmaking JIT optional at runtime is therefore the title of this patch and as\nI mentioned in some other thread it might be even useful to us for our\nown tests.\n\nCarlo\n\n[1] https://public-inbox.org/git/20181209230024.43444-3-carenas@gmail.com/\n[2] https://public-inbox.org/git/87zhtbn5xb.fsf@evledraar.gmail.com/\n[3] https://public-inbox.org/git/CAPUEspjKxQFiRgmfb2SuR_xpVu4=MN66kGEeBK1pHdBgXQbv7Q@mail.gmail.com/\n[4] https://www.mail-archive.com/bug-grep@gnu.org/msg05762.html\n"},{"id":"379576","messageId":"CAPUEspiqKim_Ypc+PPZPKvqQu9DvW7D+mHxB9cyheo8FDQs3VQ@mail.gmail.com","threadId":"51542","inReplyTo":"87wog15834.fsf@evledraar.gmail.com","subject":"Re: [RFC PATCH] grep: allow for run time disabling of JIT in PCRE","fromName":"Carlo Arenas","fromEmail":"carenas@gmail.com","sentAt":"2019-07-30T13:01:53Z","receivedAt":"2019-07-30T13:02:06Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"On Mon, Jul 29, 2019 at 5:38 AM Ævar Arnfjörð Bjarmason\n<avarab@gmail.com> wrote:\n> On Mon, Jul 29 2019, Carlo Arenas wrote:\n> > On Mon, Jul 29, 2019 at 1:55 AM Ævar Arnfjörð Bjarmason\n> > <avarab@gmail.com> wrote:\n> >>\n> >> On Mon, Jul 29 2019, Carlo Marcelo Arenas Belón wrote:\n> >>\n> >> > PCRE1 allowed for a compile time flag to disable JIT, but PCRE2 never\n> >> > had one, forcing the use of JIT if -P was requested.\n> >>\n> >> What's that PCRE1 compile-time flag?\n> >\n> > NO_LIBPCRE1_JIT at GIT compile time (regardless of JIT support in the\n> > PCRE1 library you are using)\n>\n> Ah of course, I was reading this as \"regexp\n> compile-time\". I.e. something like (*NO_JIT). No *such* thing exists for\n> PCRE v1 JIT AFAIK as exposed by git-grep.\n\ncorrect, but there are still other knobs like (*UTF), (*UCP), (?m) or\n(?i) that also\naffect some of our assumptions.\n\n> >> > After ed0479ce3d (Merge branch 'ab/no-kwset' into next, 2019-07-15)\n> >> > the PCRE2 engine will be used more broadly and therefore adding this\n> >> > knob will give users a fallback for situations like the one observed\n> >> > in OpenBSD with a JIT enabled PCRE2, because of W^X restrictions:\n> >> >\n> >> >   $ git grep 'foo bar'\n> >> >   fatal: Couldn't JIT the PCRE2 pattern 'foo bar', got '-48'\n> >> >   $ git grep -G 'foo bar'\n> >> >   fatal: Couldn't JIT the PCRE2 pattern 'foo bar', got '-48'\n> >> >   $ git grep -E 'foo bar'\n> >> >   fatal: Couldn't JIT the PCRE2 pattern 'foo bar', got '-48'\n> >> >   $ git grep -F 'foo bar'\n> >> >   fatal: Couldn't JIT the PCRE2 pattern 'foo bar', got '-48'\n> >>\n> >> Yeah that obviously sucks more with ab/no-kwset, but that seems like a\n> >> case where -P would have been completely broken before, and therefore I\n> >> can't imagine the package ever passed \"make test\". Or is W^X also\n> >> exposed as some run-time option on OpenBSD?\n> >\n> > ironically, you could use PCRE1 since that is not using the JIT fast\n> > path and therefore will fallback automatically to the interpreter\n>\n> ...because OpenBSD PCRE v1 was compiled with --disable-jit before, but\n> their v2 package has --enable-jit, it just doesn't work at all? Is this\n> your custom built git + OpenBSD packages of PCRE coming with the OS?\n\nsorry for the confusion, custom builds of both PCRE and git.\nand I was referring to the fact that after 685668faaa (grep: stop\nusing a custom JIT stack with PCRE v1, 2019-07-26)\nand with my patches to avoid UTF-8 issues, git + PCRE1 was a much pleasant\nexperience than git + PCRE2 in OpenBSD (which is also why I didn't\neven care about fixing it for pcre.jit=!1 yet)\n\nas shown by :\n$ git grep 'foo bar' Documentation | cat\nDocumentation/RelNotes/1.8.0.txt:   when the user says \"git checkout\n-b -t foo bar\" (e.g. \"-t\" is not a\nDocumentation/git-rev-list.txt: $ git rev-list foo bar ^baz\nDocumentation/gitattributes.txt:abc foo bar baz\n$ git grep -P 'foo bar' Documentation | cat\nDocumentation/RelNotes/1.8.0.txt:   when the user says \"git checkout\n-b -t foo bar\" (e.g. \"-t\" is not a\nDocumentation/git-rev-list.txt: $ git rev-list foo bar ^baz\nDocumentation/gitattributes.txt:abc foo bar baz\n$ git grep -P 'foo[ ]bar' Documentation | cat\nDocumentation/RelNotes/1.8.0.txt:   when the user says \"git checkout\n-b -t foo bar\" (e.g. \"-t\" is not a\nDocumentation/git-rev-list.txt: $ git rev-list foo bar ^baz\nDocumentation/gitattributes.txt:abc foo bar baz\n$ dmesg | grep git\ngit-grep(87484): mmap W^X violation\n\nthe last of which might be suppressed with `-c pcre.jit=0` once I get\nto fix that\n\n> I don't use OpenBSD, but isn't this their recipe? Seems they use \"make\n> test\", and don't compile with PCRE at all if I'm reading it right:\n> https://github.com/openbsd/ports/blob/master/devel/git/Makefile\n\nyes, I was using OpenBSD as a testbase where issues with JIT would\nbe easily reproducible; their packagers are smart enough not to enable\nJIT by default in PCRE or even link it with git as you pointed out.\n\nNetBSD/HardenedBSD might be a better example of a native default package that\nwould be affected in its standard configuration, if that is what you\nwere looking for.\n\n> >> I.e. aside from the merits of such a setting in general these examples\n> >> seem like just working around something that should be fixed at make\n> >> all/test time, or maybe I'm missing something.\n> >\n> > 1) before you could just avoid using -P and still be able to grep\n> > 2) there is no way to tell PCRE2 to get out of the way even if you are\n> >     not using -P\n>\n> Right, no arguments at all about ab/no-kwset making this worse (re: your\n> #1). I just really prefer not to expose/document config for what\n> *should* be something purely internal if the X-Y problem is a bug being\n> exposed that we should just fix.\n>\n> Particularly because I think it's a losing battle to provide run-time\n> options for what are surely a *lot* of \"make test\" failures.\n\nnot sure I understand.  The knob was there to give flexibility to the user\nto decide for himself how he wants to use the application and how that\nuse fits in their environment.\n\nwe can't predict either of those with 100% certainty and while it makes\nsense we will impose some constrains we should understand that the\nmore inflexible those are, the more users will be alienated.\n\nthis specific constrain is particularly silly, even PCRE recommends using\nJIT only as an optimization and fallback to the interpreter but we don't\nfollow that advice and the least we could do is give a escape hatch to\nthe users.\n\n> If it really is unavoidable to detect this until runtime in some common\n> configurations I have no problem with it, I just haven't encountered\n> that so far.\n\nguess it is sort of a chicken/egg problem and we would rather have\nOpenBSD never linking their git with PCRE.\n\nFWIW we can't know ahead of time if someone setup will include\nrunning a PAX/SELinux enabled kernel on their otherwise regular\nuserspace encountering this problem.\n\nthere is also the case of Linux distributions without official packages\n(like Gentoo and Arch) where each user decides which options they\nwant to use on their packages at their own time.\n\n> patches = your recent\n> https://public-inbox.org/git/20181209230024.43444-2-carenas@gmail.com/\n> or something earlier?\n>\n> That patch seems sane without having tested it. Seems like the\n> equivalent of what we do with v1 with PCRE2_JIT_COMPLETE.\n\nI am missing context here; that patch was obsoleted by your ab/pcre-jit-fixes\nbranch and has nothing to do with PCRE2_JIT_COMPLETE.\n\nif I recall correctly, was waiting on feedback on the series on top of your\noriginal pcre-jit-fixes branch (with only 3 patches) and that was posted in:\nhttps://public-inbox.org/git/20190726202642.7986-1-carenas@gmail.com/T/#u\n\n> I *am* curious if there's setups where fixing the code for PCRE v1 isn't\n> purely an academic exercise. Is there a reason for why these platforms\n> can't just move to PCRE v2 in principle (dumpster fires in \"next\"\n> non-withstanding)?\n\nhow can you expect me (or anyone else) to answer that question? obviously\nwe don't know, I personally for sure have no problem.\n\nbrian mentioned[1] some CentOS 6 users that don't have PCRE2 in their systems,\nI mentioned Xcode's git in macOS as likely not updating since it uses\na system library\nand that is also used by several other applications (like Safari)\n\nbut regardless of that, I see no reason for making their life more\ndifficult; PCRE1 is\nwidely used, it is still being supported, and they can use PCRE2 as an\nalternative\nmost of the time (at least with git)\n\nCarlo\n\n[1] https://public-inbox.org/git/20190615191514.GD8616@genre.crustytoothpaste.net/\n"},{"id":"379588","messageId":"22b9cf06-ebbb-a0c1-2cc9-7f09d61d3d9b@web.de","threadId":"51542","inReplyTo":"CAPUEspjo3Uo8KtSYQ=uh+_gPEjb+dBdSPgsEVE1j1bOMPF=0DA@mail.gmail.com","subject":"Re: [RFC PATCH v2] grep: allow for run time disabling of JIT in PCRE","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2019-07-30T17:55:17Z","receivedAt":"2019-07-30T17:55:42Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 30.07.19 um 02:49 schrieb Carlo Arenas:\n> On Mon, Jul 29, 2019 at 10:47 AM Junio C Hamano <gitster@pobox.com> wrote:\n>> René Scharfe <l.s.r@web.de> writes:\n>>>> +pcre.jit::\n>>>> +    If set to false, disable JIT when using PCRE.  Defaults to\n>>>> +    true.\n>>>> +    if set to -1 will try first to use JIT and fallback to the\n>>>> +    interpreter instead of returning an error.\n>>>\n>>> Why not implement only -1, without adding this config setting?\n>>\n>> ... nor command line option.  If we have an auto-fallback, I would\n>> think that makes the most sense.  IIRC the first iteration with only\n>> the configuration was really about working around the (non-working)\n>> pcre-jit---if we can self-detect and skip a non-working case, that\n>> would allow us to drop end-user facing knobs, which is ideal.\n>\n> because that was proposed earlier[1] and wasn't accepted ;)\n\nSo you add all those knobs for Ævar?\n\nUsers on OpenBSD would get an error whenever they used -P?  And they\nare expected to discover an option for turning off said error?  That\ncould be handled automatically?\n\nThis sounds like bullying to me.  I simply wouldn't use -P anymore if\nthat happened to me.  If that error was returned without -P, I'd\ncontemplate switching to the Silver Searcher or a similar tool.\n\n> the main pushback though I got was that doing the fallback would degrade\n> performance and so it was suggested[2] that keeping the error should be\n> possible somehow (with the implication it will add yet another macro)\n\nThe slowdown by the fallback should be minimal -- just the extra\nwasted time to compile the pattern to machine code.  Compilation time is\nprobably (hopefully?) dwarfed by search time.\n\nIIUC, Ævar's concern was more about being able to discover when JIT is\nnot available for some reason.  Printing a warning with --debug or\n--verbose could help with that.\n\nPersonally I don't care about JIT at all and wouldn't want to see such a\nwarning and certainly prefer not to get any error message about it,\neven more so since there is nothing I can do about it.  (Disabling\nsecurity features to get faster search sounds sounds like a no-go.)\n\n> since living without grep -P was a reasonable tradeoff at that time got\n> punted, but the need to find a solution for this become more urgent once\n> it was announced[3] PCRE2 would be used also used outside -P\n\nRight.\n\n> Obviously I am biased, but I kind of like the knob as it allows the user\n> more flexibility to tweak the internals of grep and because we had\n> made those internals already visible (ex: not handling any library\n> errors ourselves and just aborting with a pcre error), but without any\n> flexibility to fix those problems themselves (unless they open the code\n> and rebuild, in most cases)\n>\n> the comment from the user that reported[4] a regression with GNU grep\n> because of JIT stack size and that I quote below is representative of how\n> that layering violation affect users, and while git users are more likely\n> than grep users to do the code tweaking needed, they could use some\n> help.\n\nSo JIT can cause other problems, and some of them we don't (or can't)\nhandle in our code?  Turning on an unstable feature without a way to\ndisable it sounds like a bad idea indeed.  Is it really that bad?  Can\nwe grow the stack on demand, for example, as GNU grep does now in\nresponse to the bug you mentioned (http://bugs.gnu.org/19833)?  Are\nthere more such examples?\n\n> making JIT optional at runtime is therefore the title of this patch and as\n> I mentioned in some other thread it might be even useful to us for our\n> own tests.\n\nTestability is a valid concern, especially if the JIT code is\nconsidered, well, unfinished.\n\nRené\n"},{"id":"379659","messageId":"nycvar.QRO.7.76.6.1907311421140.21907@tvgsbejvaqbjf.bet","threadId":"51542","inReplyTo":"20190728235427.41425-1-carenas@gmail.com","subject":"Re: [RFC PATCH] grep: allow for run time disabling of JIT in PCRE","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-07-31T12:24:02Z","receivedAt":"2019-07-31T12:24:14Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 28 Jul 2019, Carlo Marcelo Arenas Belón wrote:\n\n> PCRE1 allowed for a compile time flag to disable JIT, but PCRE2 never\n> had one, forcing the use of JIT if -P was requested.\n>\n> After ed0479ce3d (Merge branch 'ab/no-kwset' into next, 2019-07-15)\n> the PCRE2 engine will be used more broadly and therefore adding this\n> knob will give users a fallback for situations like the one observed\n> in OpenBSD with a JIT enabled PCRE2, because of W^X restrictions:\n\nJust so that nobody else needs to feel like an idiot for not knowing\nwhat on the burning planet \"W^X\" is supposed to mean:\nhttps://en.wikipedia.org/wiki/W%5EX says it is essentially an OS-level\nmechanism to prevent executing code that was written into memory by the\nsame process, i.e. JIT.\n\nMakes me wonder whether node.js works on OpenBSD, or any decently fast\nweb browser like Firefox or Chromium...\n\nBut I digress. I just wanted to chime in with the results of my web hunt\nfor that \"W^X\" term.\n\nCiao,\nDscho\n\n>\n>   $ git grep 'foo bar'\n>   fatal: Couldn't JIT the PCRE2 pattern 'foo bar', got '-48'\n>   $ git grep -G 'foo bar'\n>   fatal: Couldn't JIT the PCRE2 pattern 'foo bar', got '-48'\n>   $ git grep -E 'foo bar'\n>   fatal: Couldn't JIT the PCRE2 pattern 'foo bar', got '-48'\n>   $ git grep -F 'foo bar'\n>   fatal: Couldn't JIT the PCRE2 pattern 'foo bar', got '-48'\n>\n> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>\n> ---\n>  Documentation/git-grep.txt |  4 ++++\n>  grep.c                     | 15 +++++++++++++--\n>  grep.h                     |  1 +\n>  3 files changed, 18 insertions(+), 2 deletions(-)\n>\n> diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt\n> index c89fb569e3..ff544bdeec 100644\n> --- a/Documentation/git-grep.txt\n> +++ b/Documentation/git-grep.txt\n> @@ -69,6 +69,10 @@ grep.fallbackToNoIndex::\n>  \tIf set to true, fall back to git grep --no-index if git grep\n>  \tis executed outside of a git repository.  Defaults to false.\n>\n> +pcre.jit::\n> +\tIf set to false, disable JIT when using PCRE.  Defaults to\n> +\ttrue.\n> +\n>\n>  OPTIONS\n>  -------\n> diff --git a/grep.c b/grep.c\n> index c7c06ae08d..3524d353dd 100644\n> --- a/grep.c\n> +++ b/grep.c\n> @@ -56,6 +56,7 @@ void init_grep_defaults(struct repository *repo)\n>  \topt->repo = repo;\n>  \topt->relative = 1;\n>  \topt->pathname = 1;\n> +\topt->pcre_jit = 1;\n>  \topt->max_depth = -1;\n>  \topt->pattern_type_option = GREP_PATTERN_TYPE_UNSPECIFIED;\n>  \tcolor_set(opt->colors[GREP_COLOR_CONTEXT], \"\");\n> @@ -125,6 +126,12 @@ int grep_config(const char *var, const char *value, void *cb)\n>  \t\treturn 0;\n>  \t}\n>\n> +\tif (!strcmp(var, \"pcre.jit\")) {\n> +\t\tint is_bool;\n> +\t\topt->pcre_jit = git_config_bool_or_int(var, value, &is_bool);\n> +\t\treturn 0;\n> +\t}\n> +\n>  \tif (!strcmp(var, \"color.grep\"))\n>  \t\topt->color = git_config_colorbool(var, value);\n>  \tif (!strcmp(var, \"color.grep.match\")) {\n> @@ -163,6 +170,7 @@ void grep_init(struct grep_opt *opt, struct repository *repo, const char *prefix\n>  \topt->pattern_tail = &opt->pattern_list;\n>  \topt->header_tail = &opt->header_list;\n>\n> +\topt->pcre_jit = def->pcre_jit;\n>  \topt->only_matching = def->only_matching;\n>  \topt->color = def->color;\n>  \topt->extended_regexp_option = def->extended_regexp_option;\n> @@ -393,7 +401,8 @@ static void compile_pcre1_regexp(struct grep_pat *p, const struct grep_opt *opt)\n>  \t\tdie(\"%s\", error);\n>\n>  #ifdef GIT_PCRE1_USE_JIT\n> -\tpcre_config(PCRE_CONFIG_JIT, &p->pcre1_jit_on);\n> +\tif (opt->pcre_jit)\n> +\t\tpcre_config(PCRE_CONFIG_JIT, &p->pcre1_jit_on);\n>  #endif\n>  }\n>\n> @@ -489,7 +498,9 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt\n>  \t\tcompile_regexp_failed(p, (const char *)&errbuf);\n>  \t}\n>\n> -\tpcre2_config(PCRE2_CONFIG_JIT, &p->pcre2_jit_on);\n> +\tif (opt->pcre_jit)\n> +\t\tpcre2_config(PCRE2_CONFIG_JIT, &p->pcre2_jit_on);\n> +\n>  \tif (p->pcre2_jit_on) {\n>  \t\tjitret = pcre2_jit_compile(p->pcre2_pattern, PCRE2_JIT_COMPLETE);\n>  \t\tif (jitret)\n> diff --git a/grep.h b/grep.h\n> index c0c71eb4a9..fff152e606 100644\n> --- a/grep.h\n> +++ b/grep.h\n> @@ -151,6 +151,7 @@ struct grep_opt {\n>  \tint allow_textconv;\n>  \tint extended;\n>  \tint use_reflog_filter;\n> +\tint pcre_jit;\n>  \tint pcre1;\n>  \tint pcre2;\n>  \tint relative;\n> --\n> 2.22.0\n>\n>\n"},{"id":"379660","messageId":"nycvar.QRO.7.76.6.1907311426290.21907@tvgsbejvaqbjf.bet","threadId":"51542","inReplyTo":"20190729105955.44390-1-carenas@gmail.com","subject":"Re: [RFC PATCH v2] grep: allow for run time disabling of JIT in PCRE","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-07-31T12:32:16Z","receivedAt":"2019-07-31T12:32:35Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 29 Jul 2019, Carlo Marcelo Arenas Belón wrote:\n\n>   $ git grep 'foo bar'\n>   fatal: Couldn't JIT the PCRE2 pattern 'foo bar', got '-48'\n\nMy immediate reaction to this error message was: That's not helpful.\nWhat is `-48` supposed to mean? Why do we even think it sensible to\nthrow such an error message at the end user? Can't we do a much better\njob translating that into something that makes actual sense without\nknowing implementation details?\n\nBut then, I realized that -48 must be a well-known constant in PCRE2,\nand my reaction transformed into something much more hopeful: why don't\nwe detect the situation where the JIT'ed code was not actually\nexecutable [*1*], and fall back to the non-JIT'ed code path ourselves,\nwithout troubling the end user (maybe warning, but maybe better not lest\nwe annoy the user with something pointless)?\n\nEven after finding out that -48 disappointingly means\nPCRE2_ERROR_NOMEMORY (as opposed to something like\nPCRE2_ERROR_CANNOT_EXECUTE_JIT_CODE), I like the idea of not bothering\nend users and doing the sensible fallback under the hood.\n\nCiao,\nDscho\n\nFootnote *1*: Why anybody would think it sensible to build a PCRE2 with\nJIT on an OS that does not allow executing code that was written by the\nsame process is beyond me. Or is there a mode in OpenBSD that *does*\nallow JIT'ed code to be executed?\n"},{"id":"379661","messageId":"nycvar.QRO.7.76.6.1907311434080.21907@tvgsbejvaqbjf.bet","threadId":"51542","inReplyTo":"CAPUEspjo3Uo8KtSYQ=uh+_gPEjb+dBdSPgsEVE1j1bOMPF=0DA@mail.gmail.com","subject":"Re: [RFC PATCH v2] grep: allow for run time disabling of JIT in PCRE","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-07-31T12:36:08Z","receivedAt":"2019-07-31T12:37:02Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Carlo,\n\nOn Mon, 29 Jul 2019, Carlo Arenas wrote:\n\n> On Mon, Jul 29, 2019 at 10:47 AM Junio C Hamano <gitster@pobox.com> wrote:\n> > René Scharfe <l.s.r@web.de> writes:\n> > >> +pcre.jit::\n> > >> +    If set to false, disable JIT when using PCRE.  Defaults to\n> > >> +    true.\n> > >> +    if set to -1 will try first to use JIT and fallback to the\n> > >> +    interpreter instead of returning an error.\n> > >\n> > > Why not implement only -1, without adding this config setting?\n> >\n> > ... nor command line option.  If we have an auto-fallback, I would\n> > think that makes the most sense.  IIRC the first iteration with only\n> > the configuration was really about working around the (non-working)\n> > pcre-jit---if we can self-detect and skip a non-working case, that\n> > would allow us to drop end-user facing knobs, which is ideal.\n>\n> because that was proposed earlier[1] and wasn't accepted ;)\n> [...]\n> [1] https://public-inbox.org/git/20181209230024.43444-3-carenas@gmail.com/\n\nFor the record, I read\nhttps://public-inbox.org/git/xmqqh8flkgs2.fsf@gitster-ct.c.googlers.com/\nas encouraging a slightly more powerful argument in favor. Junio seemed\nto hope that PCRE2's own `pcre2grep` would behave that way, and that\nwould give us plenty reason to just imitate it.\n\nI don't know whether `pcre2grep` behaves that way, even if it does not,\nI think the benefits of the auto fallback to the end user are\nconsiderable.\n\nCiao,\nDscho\n"},{"id":"379667","messageId":"87o91a5k0d.fsf@evledraar.gmail.com","threadId":"51542","inReplyTo":"nycvar.QRO.7.76.6.1907311426290.21907@tvgsbejvaqbjf.bet","subject":"Re: [RFC PATCH v2] grep: allow for run time disabling of JIT in PCRE","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-07-31T14:57:54Z","receivedAt":"2019-07-31T14:57:58Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Jul 31 2019, Johannes Schindelin wrote:\n\n> Hi,\n>\n> On Mon, 29 Jul 2019, Carlo Marcelo Arenas Belón wrote:\n>\n>>   $ git grep 'foo bar'\n>>   fatal: Couldn't JIT the PCRE2 pattern 'foo bar', got '-48'\n>\n> My immediate reaction to this error message was: That's not helpful.\n> What is `-48` supposed to mean? Why do we even think it sensible to\n> throw such an error message at the end user? Can't we do a much better\n> job translating that into something that makes actual sense without\n> knowing implementation details?\n>\n> But then, I realized that -48 must be a well-known constant in PCRE2,\n> and my reaction transformed into something much more hopeful: why don't\n> we detect the situation where the JIT'ed code was not actually\n> executable [*1*], and fall back to the non-JIT'ed code path ourselves,\n> without troubling the end user (maybe warning, but maybe better not lest\n> we annoy the user with something pointless)?\n>\n> Even after finding out that -48 disappointingly means\n> PCRE2_ERROR_NOMEMORY (as opposed to something like\n> PCRE2_ERROR_CANNOT_EXECUTE_JIT_CODE), I like the idea of not bothering\n> end users and doing the sensible fallback under the hood.\n>\n> Ciao,\n> Dscho\n>\n> Footnote *1*: Why anybody would think it sensible to build a PCRE2 with\n> JIT on an OS that does not allow executing code that was written by the\n> same process is beyond me. Or is there a mode in OpenBSD that *does*\n> allow JIT'ed code to be executed?\n\nWe do detect if JIT isn't supported and fall back. That's what the\npcre2_config(PCRE2_CONFIG_JIT, &p->pcre2_jit_on) code in grep.c\ndoes. This and is the subsequent pcre2_pattern_info() call is how PCRE\ndocuments that you should do this.\n\nWhat hasn't been supported is all of that saying \"yes, I support JIT\"\nand the feature then fail whaling. I had not encountered that before.\n\nSo far that seems like because Carlo just built a completely broken PCRE\nv2 package, so I don't know if that's worth supporting on our\nside. I.e. this isn't something I think could plausibly happen in the\nwild.\n\nThat should *not* be confused with me thinking other stuff Carlo's\nraised is a non-issue, e.g. running into the JIT stack limit etc. Some\nof that's clearly bugs in our/my grep.c code that need fixing.\n"},{"id":"379691","messageId":"xmqqlfweqis3.fsf@gitster-ct.c.googlers.com","threadId":"51542","inReplyTo":"nycvar.QRO.7.76.6.1907311434080.21907@tvgsbejvaqbjf.bet","subject":"Re: [RFC PATCH v2] grep: allow for run time disabling of JIT in PCRE","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-07-31T16:18:52Z","receivedAt":"2019-07-31T16:19:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> For the record, I read\n> https://public-inbox.org/git/xmqqh8flkgs2.fsf@gitster-ct.c.googlers.com/\n> as encouraging a slightly more powerful argument in favor. Junio seemed\n> to hope that PCRE2's own `pcre2grep` would behave that way, and that\n> would give us plenty reason to just imitate it.\n>\n> I don't know whether `pcre2grep` behaves that way, even if it does not,\n> I think the benefits of the auto fallback to the end user are\n> considerable.\n\nThanks for digging that up ;-)  I do agree with what was said there.\n\nJIT is merely an optimization, and we should be able to work without\nit and should not even bother the users with warning messages when\nwe have to choose non-JIT codepath.  Those who care about debugging\ncan use a \"--debug\" option or something to figure out if their build\non a particular pattern is or is not using JIT.\n\nI think I read somebody (Carlo?) made that argument in the thread\nearlier, and I agree with that sentiment.\n"},{"id":"379905","messageId":"CAPUEsphQVjptjZVh2mmXWdDj4rEohgtWw5UWUmN_qKc0XC9psg@mail.gmail.com","threadId":"51542","inReplyTo":"87o91a5k0d.fsf@evledraar.gmail.com","subject":"Re: [RFC PATCH v2] grep: allow for run time disabling of JIT in PCRE","fromName":"Carlo Arenas","fromEmail":"carenas@gmail.com","sentAt":"2019-08-04T00:25:45Z","receivedAt":"2019-08-04T00:25:59Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"On Wed, Jul 31, 2019 at 7:57 AM Ævar Arnfjörð Bjarmason\n<avarab@gmail.com> wrote:\n> What hasn't been supported is all of that saying \"yes, I support JIT\"\n> and the feature then fail whaling. I had not encountered that before.\n>\n> So far that seems like because Carlo just built a completely broken PCRE\n> v2 package, so I don't know if that's worth supporting on our\n> side. I.e. this isn't something I think could plausibly happen in the\n> wild.\n\nsince you are in Debian please follow the instructions here:\n\n  https://wiki.debian.org/SELinux/Setup\n\nno need to rebuild git or pcre (but to enable selinux will need to\nreboot twice), then type as root the following:\n\n  # set enforce 1\n\nCarlo\n"},{"id":"379909","messageId":"20190804031409.32764-1-carenas@gmail.com","threadId":"51542","inReplyTo":"20190729105955.44390-1-carenas@gmail.com","subject":"[RFC PATCH v3] grep: treat PCRE2 jit compilation memory error as non fatal","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2019-08-04T03:14:09Z","receivedAt":"2019-08-04T03:14:30Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"94da9193a6 (grep: add support for PCRE v2, 2017-06-01) uses the\nJIT fast path unless JIT support has not been compiled in the\nlinked library.\n\nStarting from 10.23 of PCRE2, pcre2grep ignores any errors from\npcre2_jit_cpmpile as a workaround for their bug1749[1] and we\nshould do too, so that the interpreter could be used as a fallback\nin cases where JIT was not available because of a security policy.\n\nTo be conservative, we are restricting initially the error to the\nknown error that would be returned in that case (and to be documented\nas such in a future release of PCRE) and printing a warning so that\ncorrective action could be taken.\n\n[1] https://bugs.exim.org/show_bug.cgi?id=1749\n\nSigned-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>\n---\n grep.c | 8 +++++++-\n 1 file changed, 7 insertions(+), 1 deletion(-)\n\ndiff --git a/grep.c b/grep.c\nindex f7c3a5803e..593a1cb7a0 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -525,7 +525,13 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt\n \tif (p->pcre2_jit_on == 1) {\n \t\tjitret = pcre2_jit_compile(p->pcre2_pattern, PCRE2_JIT_COMPLETE);\n \t\tif (jitret)\n-\t\t\tdie(\"Couldn't JIT the PCRE2 pattern '%s', got '%d'\\n\", p->pattern, jitret);\n+\t\t\tif (jitret == PCRE2_ERROR_NOMEMORY) {\n+\t\t\t\twarning(\"JIT couldn't be used in PCRE2\");\n+\t\t\t\tp->pcre2_jit_on = 0;\n+\t\t\t\treturn;\n+\t\t\t}\n+\t\t\telse\n+\t\t\t\tdie(\"Couldn't JIT the PCRE2 pattern '%s', got '%d'\\n\", p->pattern, jitret);\n \n \t\t/*\n \t\t * The pcre2_config(PCRE2_CONFIG_JIT, ...) call just\n-- \n2.23.0.rc1\n\n"},{"id":"379910","messageId":"CAPUEspjOtzFxDLrQzLLU7+Q6Z6V8P0=qNrH2NrOzL7Q884krrQ@mail.gmail.com","threadId":"51542","inReplyTo":"20190804031409.32764-1-carenas@gmail.com","subject":"Re: [RFC PATCH v3] grep: treat PCRE2 jit compilation memory error as non fatal","fromName":"Carlo Arenas","fromEmail":"carenas@gmail.com","sentAt":"2019-08-04T07:43:17Z","receivedAt":"2019-08-04T07:43:31Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"PROs:\n* it works (only for PCRE2) and tested in OpenBSD, NetBSD, macOS, Linux (Debian)\n* it applies everywhere (even pu) without conflicts\n* it doesn't introduce any regressions in tests (tested in Debian with\nSElinux in enforcing mode)\n* it is simple\n\nCONs:\n* HardenedBSD still segfaults (bugfix proposed[1] to sljit/pcre)\n* warning is noisy (at least once per thread) and might be even\nineffective as it goes to stderr while stdout with most the output\ngoes to a pager\n* too conservative (pcre2grep shows all errors from pcre2_jit_compile\nshould be ignored)\n* no tests\n\nKnown Issues:\n* code is ugly (it even triggers a warning if you have the right compiler)\n* code is suspiciously similar to one[2] that was rejected, but\nhopefully commit message is better\n* code is incomplete (PCRE1 has too many conflicting changes in flight\nto attempt a similar fix)\n* there are obvious blind spots in the tests that need fixing, and a\nlot more testing in other platforms/architectures\n* git still will sometimes die because the non fast path has UTF-8 issues\n\nI still think the pcre.jit flag knob might be useful to workaround\nsome of the issues detailed in CONs but probably with a different\ndefinition:\nunset -> fallback (try JIT but use interpreter if that didn't work)\nfalse -> don't even try to use JIT\ntrue -> print warning and maybe even die (if we really think that is useful)\n\nsome performance numbers below for the perl tests\n\nwith JIT enabled (in non enforcing SELinux)\n\nTest                                            this tree\n---------------------------------------------------------------\n7820.3: perl grep 'how.to'                      0.56(0.29+0.60)\n7820.7: perl grep '^how to'                     0.49(0.29+0.54)\n7820.11: perl grep '[how] to'                   0.54(0.39+0.51)\n7820.15: perl grep '(e.t[^ ]*|v.ry) rare'       0.60(0.45+0.58)\n7820.19: perl grep 'm(ú|u)lt.b(æ|y)te'          0.58(0.30+0.61)\n\nwith \"fallback to interpreter\" (in enforcing SELinux)\n\nTest                                            this tree\n---------------------------------------------------------------\n7820.3: perl grep 'how.to'                      0.64(0.59+0.56)\n7820.7: perl grep '^how to'                     1.83(2.91+0.56)\n7820.11: perl grep '[how] to'                   2.07(3.33+0.61)\n7820.15: perl grep '(e.t[^ ]*|v.ry) rare'       2.89(4.91+0.66)\n7820.19: perl grep 'm(ú|u)lt.b(æ|y)te'          0.78(0.86+0.55)\n\n[1] https://github.com/zherczeg/sljit/pull/2\n[2] https://public-inbox.org/git/20181209230024.43444-3-carenas@gmail.com/\n"},{"id":"379953","messageId":"xmqqef1zmkp5.fsf@gitster-ct.c.googlers.com","threadId":"51542","inReplyTo":"CAPUEspjOtzFxDLrQzLLU7+Q6Z6V8P0=qNrH2NrOzL7Q884krrQ@mail.gmail.com","subject":"Re: [RFC PATCH v3] grep: treat PCRE2 jit compilation memory error as non fatal","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-08-05T20:16:54Z","receivedAt":"2019-08-05T20:17:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Carlo Arenas <carenas@gmail.com> writes:\n\n> * code is suspiciously similar to one[2] that was rejected, but\n> hopefully commit message is better\n> ...\n> [2] https://public-inbox.org/git/20181209230024.43444-3-carenas@gmail.com/\n\nI do not recall ever rejecting that one.\n\nIt did not come with a good proposed log message to be accepted\nas-is, so I do not find it surprising that I did not pick it up, was\nwaiting for a new iteration and then everybody forgot about it.\n\nBut that is quite different from getting rejected (with the\nconnotation that \"don't attempt this bad idea again, unless the\nworld changes drastically\").\n\nIn any case, this round looks a lot more reasoned.  I personally do\nnot think the warning() is a good idea.  As I said in the old\ndiscussion, we by default should treat JIT as a mere optimization,\nand we should stay out of the way most of the time.\n\nAn additional \"must have JIT or we will die\" [*1*] can be added on\ntop of this change, if somebody really cares.\n\nThanks.\n\n\n[Reference]\n\n*1* https://public-inbox.org/git/87pnu9yekk.fsf@evledraar.gmail.com/\n"}]}