{"thread":{"id":"51506","subject":"[PATCH] grep: use custom JIT stack with pcre2","startedAt":"2019-07-21T19:40:57Z","lastAt":"2021-01-24T17:30:14Z","messageCount":70,"participants":["Carlo Marcelo Arenas Belón","Ævar Arnfjörð Bjarmason","Junio C Hamano","Carlo Arenas","Andreas Schwab","Johannes Schindelin","Jeff King","Todd Zullinger","Johannes Sixt","Ramsay Jones"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"379141","messageId":"20190721194052.15440-1-carenas@gmail.com","threadId":"51506","inReplyTo":null,"subject":"[PATCH] grep: use custom JIT stack with pcre2","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2019-07-21T19:40:52Z","receivedAt":"2019-07-21T19:40:57Z","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) allocate\na stack and assign it to a match context, but never pass it to\npcre2_jit_match, using instead the default.\n\nSigned-off-by: Carlo Marcelo Arenas Belón \n---\nThis might have positive performance consequences (per the comments)\nbut haven't tested them; if there is no difference might be better\ninstead to remove the stack and match_context and save the related\nmemory, since it seems the default was working fine anyway.\n\n grep.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/grep.c b/grep.c\nindex 146093f590..ff76907ceb 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -564,7 +564,7 @@ static int pcre2match(struct grep_pat *p, const char *line, const char *eol,\n \tif (p->pcre2_jit_on)\n \t\tret = pcre2_jit_match(p->pcre2_pattern, (unsigned char *)line,\n \t\t\t\t eol - line, 0, flags, p->pcre2_match_data,\n-\t\t\t\t NULL);\n+\t\t\t\t p->pcre2_match_context);\n \telse\n \t\tret = pcre2_match(p->pcre2_pattern, (unsigned char *)line,\n \t\t\t\t eol - line, 0, flags, p->pcre2_match_data,\n-- \n2.22.0\n\n"},{"id":"379203","messageId":"20190724151415.3698-1-avarab@gmail.com","threadId":"51506","inReplyTo":"20190721194052.15440-1-carenas@gmail.com","subject":"[PATCH 0/3] grep: PCRE JIT fixes","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-07-24T15:14:12Z","receivedAt":"2019-07-24T15:14:50Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"There's a couple of patches fixing mistakes in the JIT code I added\nfor PCRE in <20190722181923.21572-1-dev+git@drbeat.li> and\n<20190721194052.15440-1-carenas@gmail.com>\n\nThis small series proposes to replace both of those. In both cases I\nthink we're better off just removing the relevant code. The commit\nmessages for the patches themselves make the case for that.\n\nÆvar Arnfjörð Bjarmason (3):\n grep: remove overly paranoid BUG(...) code\n grep: stop \"using\" a custom JIT stack with PCRE v2\n grep: stop using a custom JIT stack with PCRE v1\n\n grep.c | 46 ++++++----------------------------------------\n grep.h | 9 ---------\n 2 files changed, 6 insertions(+), 49 deletions(-)\n\n-- \n2.22.0.455.g172b71a6c5\n\n"},{"id":"379204","messageId":"20190724151415.3698-3-avarab@gmail.com","threadId":"51506","inReplyTo":"20190721194052.15440-1-carenas@gmail.com","subject":"[PATCH 2/3] grep: stop \"using\" a custom JIT stack with PCRE v2","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-07-24T15:14:14Z","receivedAt":"2019-07-24T15:14:54Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"As reported in [1] the code I added in 94da9193a6 (\"grep: add support\nfor PCRE v2\", 2017-06-01) to use a custom JIT stack has never\nworked. It was incorrectly copy/pasted from code I added in\nfbaceaac47 (\"grep: add support for the PCRE v1 JIT API\", 2017-05-25),\nwhich did work.\n\nThus our intention of starting with 1 byte of stack at a maximum of 1\nMB didn't happen, we'd always use the 32 KB stack provided by PCRE\nv2's jit_machine_stack_exec()[2]. The reason I allocated a custom\nstack at all was this advice in pcrejit(3) (same in pcre2jit(3)):\n\n \"By default, it uses 32KiB on the machine stack. However, some\n large or complicated patterns need more than this\"\n\nSince we've haven't had any reports of users running into\nPCRE2_ERROR_JIT_STACKLIMIT in the wild I think we can safely assume\nthat we can just use the library defaults instead and drop this\ncode. This won't change with the wider use of PCRE v2 in\ned0479ce3d (\"Merge branch 'ab/no-kwset' into next\", 2019-07-15), a\nfixed string search is not a \"large or complicated pattern\".\n\nFor good measure I ran the performance test noted in 94da9193a6,\nalthough the command is simpler now due to my 0f50c8e32c (\"Makefile:\nremove the NO_R_TO_GCC_LINKER flag\", 2019-05-17):\n\n GIT_PERF_REPEAT_COUNT=30 GIT_PERF_LARGE_REPO=~/g/linux GIT_PERF_MAKE_OPTS='-j8 USE_LIBPCRE2=Y CFLAGS=-O3 LIBPCREDIR=/home/avar/g/pcre2/inst' ./run HEAD~ HEAD p7820-grep-engines.sh\n\nJust the /perl/ results are:\n\n Test HEAD~ HEAD\n ---------------------------------------------------------------------------------------\n 7820.3: perl grep 'how.to' 0.17(0.27+0.65) 0.17(0.24+0.68) +0.0%\n 7820.7: perl grep '^how to' 0.16(0.23+0.66) 0.16(0.23+0.67) +0.0%\n 7820.11: perl grep '[how] to' 0.18(0.35+0.62) 0.18(0.33+0.65) +0.0%\n 7820.15: perl grep '(e.t[^ ]*|v.ry) rare' 0.17(0.45+0.54) 0.17(0.49+0.50) +0.0%\n 7820.19: perl grep 'm(ú|u)lt.b(æ|y)te' 0.16(0.33+0.58) 0.16(0.29+0.62) +0.0%\n\nSo, as expected there's no change, and running with valgrind reveals\nthat we have fewer allocations now.\n\n1. https://public-inbox.org/git/20190721194052.15440-1-carenas@gmail.com/\n2. I didn't really intend to start with 1 byte, looking at the PCRE v2\n code again what happened is that I cargo-culted some of PCRE v2's\n own test code which was meant to test re-allocations. It's more\n sane to start with say 32 KB with a max of 1 MB, as pcre2grep.c\n does.\n\nReported-by: Carlo Marcelo Arenas Belón \nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n grep.c | 10 ----------\n grep.h | 4 ----\n 2 files changed, 14 deletions(-)\n\ndiff --git a/grep.c b/grep.c\nindex be4282fef3..20ce95270a 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -546,14 +546,6 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt\n \t\t\tp->pcre2_jit_on = 0;\n \t\t\treturn;\n \t\t}\n-\n-\t\tp->pcre2_jit_stack = pcre2_jit_stack_create(1, 1024 * 1024, NULL);\n-\t\tif (!p->pcre2_jit_stack)\n-\t\t\tdie(\"Couldn't allocate PCRE2 JIT stack\");\n-\t\tp->pcre2_match_context = pcre2_match_context_create(NULL);\n-\t\tif (!p->pcre2_match_context)\n-\t\t\tdie(\"Couldn't allocate PCRE2 match context\");\n-\t\tpcre2_jit_stack_assign(p->pcre2_match_context, NULL, p->pcre2_jit_stack);\n \t}\n }\n \n@@ -597,8 +589,6 @@ static void free_pcre2_pattern(struct grep_pat *p)\n \tpcre2_compile_context_free(p->pcre2_compile_context);\n \tpcre2_code_free(p->pcre2_pattern);\n \tpcre2_match_data_free(p->pcre2_match_data);\n-\tpcre2_jit_stack_free(p->pcre2_jit_stack);\n-\tpcre2_match_context_free(p->pcre2_match_context);\n }\n #else /* !USE_LIBPCRE2 */\n static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt)\ndiff --git a/grep.h b/grep.h\nindex 1875880f37..a65f4a1ae1 100644\n--- a/grep.h\n+++ b/grep.h\n@@ -29,8 +29,6 @@ typedef int pcre_jit_stack;\n typedef int pcre2_code;\n typedef int pcre2_match_data;\n typedef int pcre2_compile_context;\n-typedef int pcre2_match_context;\n-typedef int pcre2_jit_stack;\n #endif\n #include \"kwset.h\"\n #include \"thread-utils.h\"\n@@ -94,8 +92,6 @@ struct grep_pat {\n \tpcre2_code *pcre2_pattern;\n \tpcre2_match_data *pcre2_match_data;\n \tpcre2_compile_context *pcre2_compile_context;\n-\tpcre2_match_context *pcre2_match_context;\n-\tpcre2_jit_stack *pcre2_jit_stack;\n \tuint32_t pcre2_jit_on;\n \tkwset_t kws;\n \tunsigned fixed:1;\n-- \n2.22.0.455.g172b71a6c5\n\n"},{"id":"379205","messageId":"20190724151415.3698-2-avarab@gmail.com","threadId":"51506","inReplyTo":"20190721194052.15440-1-carenas@gmail.com","subject":"[PATCH 1/3] grep: remove overly paranoid BUG(...) code","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-07-24T15:14:13Z","receivedAt":"2019-07-24T15:14:55Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Remove code that would trigger if pcre_config() or pcre2_config() was\nso broken that \"do we have JIT?\" wouldn't return a boolean.\n\nI added this code back in fbaceaac47 (\"grep: add support for the PCRE\nv1 JIT API\", 2017-05-25) and then as noted in [1] incorrectly\ncopy/pasted some of it in 94da9193a6 (\"grep: add support for PCRE v2\",\n2017-06-01).\n\nLet's just remove it instead of fixing that bug. Being this paranoid\nabout what PCRE returns is crossing the line into unreasonable\nparanoia.\n\n1. https://public-inbox.org/git/20190722181923.21572-1-dev+git@drbeat.li/\n\nReported-by: Beat Bolli \nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n grep.c | 10 ++--------\n 1 file changed, 2 insertions(+), 8 deletions(-)\n\ndiff --git a/grep.c b/grep.c\nindex f7c3a5803e..be4282fef3 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -406,14 +406,11 @@ static void compile_pcre1_regexp(struct grep_pat *p, const struct grep_opt *opt)\n \n #ifdef GIT_PCRE1_USE_JIT\n \tpcre_config(PCRE_CONFIG_JIT, &p->pcre1_jit_on);\n-\tif (p->pcre1_jit_on == 1) {\n+\tif (p->pcre1_jit_on) {\n \t\tp->pcre1_jit_stack = pcre_jit_stack_alloc(1, 1024 * 1024);\n \t\tif (!p->pcre1_jit_stack)\n \t\t\tdie(\"Couldn't allocate PCRE JIT stack\");\n \t\tpcre_assign_jit_stack(p->pcre1_extra_info, NULL, p->pcre1_jit_stack);\n-\t} else if (p->pcre1_jit_on != 0) {\n-\t\tBUG(\"The pcre1_jit_on variable should be 0 or 1, not %d\",\n-\t\t p->pcre1_jit_on);\n \t}\n #endif\n }\n@@ -522,7 +519,7 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt\n \t}\n \n \tpcre2_config(PCRE2_CONFIG_JIT, &p->pcre2_jit_on);\n-\tif (p->pcre2_jit_on == 1) {\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@@ -557,9 +554,6 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt\n \t\tif (!p->pcre2_match_context)\n \t\t\tdie(\"Couldn't allocate PCRE2 match context\");\n \t\tpcre2_jit_stack_assign(p->pcre2_match_context, NULL, p->pcre2_jit_stack);\n-\t} else if (p->pcre2_jit_on != 0) {\n-\t\tBUG(\"The pcre2_jit_on variable should be 0 or 1, not %d\",\n-\t\t p->pcre1_jit_on);\n \t}\n }\n \n-- \n2.22.0.455.g172b71a6c5\n\n"},{"id":"379206","messageId":"20190724151415.3698-4-avarab@gmail.com","threadId":"51506","inReplyTo":"20190721194052.15440-1-carenas@gmail.com","subject":"[PATCH 3/3] grep: stop using a custom JIT stack with PCRE v1","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-07-24T15:14:15Z","receivedAt":"2019-07-24T15:14:57Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Simplify the PCRE v1 code for the same reasons as for the PCRE v2 code\nin the last commit. Unlike with v2 we actually used the custom stack\nin v1, but let's use PCRE's built-in 32 KB one instead, since\nexperience with v2 shows that's enough. Most distros are already using\nv2 as a default, and the underlying sljit code is the same.\n\nUnfortunately we can't just pass a NULL to pcre_jit_exec() as with\npcre2_jit_match(). Unlike the v2 function it doesn't support\nthat. Instead we need to use the fatter pcre_exec() if we'd like the\nsame behavior.\n\nThis will make things slightly slower than on the fast-path function,\nbut it's OK since we care less about v1 performance these days since\nwe have and recommend v2. Running a similar performance test as what I\nran in fbaceaac47 (\"grep: add support for the PCRE v1 JIT API\",\n2017-05-25) via:\n\n GIT_PERF_REPEAT_COUNT=30 GIT_PERF_LARGE_REPO=~/g/linux GIT_PERF_MAKE_OPTS='-j8 USE_LIBPCRE1=Y CFLAGS=-O3 LIBPCREDIR=/home/avar/g/pcre/inst' ./run HEAD~ HEAD p7820-grep-engines.sh\n\nGives us this, just the /perl/ results:\n\n Test HEAD~ HEAD\n ---------------------------------------------------------------------------------------\n 7820.3: perl grep 'how.to' 0.19(0.67+0.52) 0.19(0.65+0.52) +0.0%\n 7820.7: perl grep '^how to' 0.19(0.78+0.44) 0.19(0.72+0.49) +0.0%\n 7820.11: perl grep '[how] to' 0.39(2.13+0.43) 0.40(2.10+0.46) +2.6%\n 7820.15: perl grep '(e.t[^ ]*|v.ry) rare' 0.44(2.55+0.37) 0.45(2.47+0.41) +2.3%\n 7820.19: perl grep 'm(ú|u)lt.b(æ|y)te' 0.23(1.06+0.42) 0.22(1.03+0.43) -4.3%\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n grep.c | 28 +++++-----------------------\n grep.h | 5 -----\n 2 files changed, 5 insertions(+), 28 deletions(-)\n\ndiff --git a/grep.c b/grep.c\nindex 20ce95270a..6b52fed53a 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -406,12 +406,6 @@ static void compile_pcre1_regexp(struct grep_pat *p, const struct grep_opt *opt)\n \n #ifdef GIT_PCRE1_USE_JIT\n \tpcre_config(PCRE_CONFIG_JIT, &p->pcre1_jit_on);\n-\tif (p->pcre1_jit_on) {\n-\t\tp->pcre1_jit_stack = pcre_jit_stack_alloc(1, 1024 * 1024);\n-\t\tif (!p->pcre1_jit_stack)\n-\t\t\tdie(\"Couldn't allocate PCRE JIT stack\");\n-\t\tpcre_assign_jit_stack(p->pcre1_extra_info, NULL, p->pcre1_jit_stack);\n-\t}\n #endif\n }\n \n@@ -423,18 +417,9 @@ static int pcre1match(struct grep_pat *p, const char *line, const char *eol,\n \tif (eflags & REG_NOTBOL)\n \t\tflags |= PCRE_NOTBOL;\n \n-#ifdef GIT_PCRE1_USE_JIT\n-\tif (p->pcre1_jit_on) {\n-\t\tret = pcre_jit_exec(p->pcre1_regexp, p->pcre1_extra_info, line,\n-\t\t\t\t eol - line, 0, flags, ovector,\n-\t\t\t\t ARRAY_SIZE(ovector), p->pcre1_jit_stack);\n-\t} else\n-#endif\n-\t{\n-\t\tret = pcre_exec(p->pcre1_regexp, p->pcre1_extra_info, line,\n-\t\t\t\teol - line, 0, flags, ovector,\n-\t\t\t\tARRAY_SIZE(ovector));\n-\t}\n+\tret = pcre_exec(p->pcre1_regexp, p->pcre1_extra_info, line,\n+\t\t\teol - line, 0, flags, ovector,\n+\t\t\tARRAY_SIZE(ovector));\n \n \tif (ret < 0 && ret != PCRE_ERROR_NOMATCH)\n \t\tdie(\"pcre_exec failed with error code %d\", ret);\n@@ -451,14 +436,11 @@ static void free_pcre1_regexp(struct grep_pat *p)\n {\n \tpcre_free(p->pcre1_regexp);\n #ifdef GIT_PCRE1_USE_JIT\n-\tif (p->pcre1_jit_on) {\n+\tif (p->pcre1_jit_on)\n \t\tpcre_free_study(p->pcre1_extra_info);\n-\t\tpcre_jit_stack_free(p->pcre1_jit_stack);\n-\t} else\n+\telse\n #endif\n-\t{\n \t\tpcre_free(p->pcre1_extra_info);\n-\t}\n \tpcre_free((void *)p->pcre1_tables);\n }\n #else /* !USE_LIBPCRE1 */\ndiff --git a/grep.h b/grep.h\nindex a65f4a1ae1..a405fc870c 100644\n--- a/grep.h\n+++ b/grep.h\n@@ -14,13 +14,9 @@\n #ifndef GIT_PCRE_STUDY_JIT_COMPILE\n #define GIT_PCRE_STUDY_JIT_COMPILE 0\n #endif\n-#if PCRE_MAJOR <= 8 && PCRE_MINOR < 20\n-typedef int pcre_jit_stack;\n-#endif\n #else\n typedef int pcre;\n typedef int pcre_extra;\n-typedef int pcre_jit_stack;\n #endif\n #ifdef USE_LIBPCRE2\n #define PCRE2_CODE_UNIT_WIDTH 8\n@@ -86,7 +82,6 @@ struct grep_pat {\n \tregex_t regexp;\n \tpcre *pcre1_regexp;\n \tpcre_extra *pcre1_extra_info;\n-\tpcre_jit_stack *pcre1_jit_stack;\n \tconst unsigned char *pcre1_tables;\n \tint pcre1_jit_on;\n \tpcre2_code *pcre2_pattern;\n-- \n2.22.0.455.g172b71a6c5\n\n"},{"id":"379210","messageId":"xmqq1ryfs8ws.fsf@gitster-ct.c.googlers.com","threadId":"51506","inReplyTo":"20190724151415.3698-1-avarab@gmail.com","subject":"Re: [PATCH 0/3] grep: PCRE JIT fixes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-07-24T16:18:43Z","receivedAt":"2019-07-24T16:18:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> There's a couple of patches fixing mistakes in the JIT code I added\n> for PCRE in <20190722181923.21572-1-dev+git@drbeat.li> and\n> <20190721194052.15440-1-carenas@gmail.com>\n>\n> This small series proposes to replace both of those. In both cases I\n> think we're better off just removing the relevant code. The commit\n> messages for the patches themselves make the case for that.\n\nI am not sure about the BUG() that practically never triggered so\nfar (AFAICT, the check that guards the BUG() would trigger only if\nwe later introduced a bug, calling the code to compile when we are\nnot asked to do so)---wouldn't it be better to leave it in while\nthere still are people who are touching the vicinity?\n\nThe other two I am perfectly OK with. It is easy to resurrect the\nsupport for v1 (which may not even be needed for long) and resurrect\nthe support for v2 with Carlo's fix, if it later turns out that some\nusers may need to use a more complex pattern.\n\nThanks.\n\n> Ævar Arnfjörð Bjarmason (3):\n> grep: remove overly paranoid BUG(...) code\n> grep: stop \"using\" a custom JIT stack with PCRE v2\n> grep: stop using a custom JIT stack with PCRE v1\n>\n> grep.c | 46 ++++++----------------------------------------\n> grep.h | 9 ---------\n> 2 files changed, 6 insertions(+), 49 deletions(-)\n"},{"id":"379211","messageId":"xmqqwog7qu2r.fsf@gitster-ct.c.googlers.com","threadId":"51506","inReplyTo":"20190724151415.3698-3-avarab@gmail.com","subject":"Re: [PATCH 2/3] grep: stop \"using\" a custom JIT stack with PCRE v2","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-07-24T16:24:28Z","receivedAt":"2019-07-24T16:24:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Since we've haven't had any reports of users running into\n> PCRE2_ERROR_JIT_STACKLIMIT in the wild I think we can safely assume\n> that we can just use the library defaults instead and drop this\n> code.\n\nDoes everybody use pcre2 with JIT with Git these days, or only those\nwho want to live near the bleeding edge?\n\n> This won't change with the wider use of PCRE v2 in\n> ed0479ce3d (\"Merge branch 'ab/no-kwset' into next\", 2019-07-15), a\n> fixed string search is not a \"large or complicated pattern\".\n\nIn any case, if we were not \"using\" the custom stack anyway for v2,\nthis change does not hurt anybody, possibly other than those who\nwill learn about pcre2 support by reading this message and experiments\nwith larger patterns. And it should be simple to wire it back if it\nbecomes necessary later.\n\nThanks for cleaning up.\n"},{"id":"379214","messageId":"87k1c76vyw.fsf@evledraar.gmail.com","threadId":"51506","inReplyTo":"xmqq1ryfs8ws.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 0/3] grep: PCRE JIT fixes","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-07-24T20:03:51Z","receivedAt":"2019-07-24T20:07:34Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Jul 24 2019, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> There's a couple of patches fixing mistakes in the JIT code I added\n>> for PCRE in <20190722181923.21572-1-dev+git@drbeat.li> and\n>> <20190721194052.15440-1-carenas@gmail.com>\n>>\n>> This small series proposes to replace both of those. In both cases I\n>> think we're better off just removing the relevant code. The commit\n>> messages for the patches themselves make the case for that.\n>\n> I am not sure about the BUG() that practically never triggered so\n> far (AFAICT, the check that guards the BUG() would trigger only if\n> we later introduced a bug, calling the code to compile when we are\n> not asked to do so)---wouldn't it be better to leave it in while\n> there still are people who are touching the vicinity?\n\nThe BUG() in 1/3 is just checking if pcre2?_config() returns a boolean\nwhen promised, so it amounts to black-box testing of that library.\n\nI think code in that style is overly paranoid and verbose, it's\nreasonable to just trust the library in that case.\n\nI think the reason it ended up in the codebase in the first place was\nconverting some first-draft implementation I wrote where I was being\nmore paranoid about using the PCRE API as a black box.\n\n> The other two I am perfectly OK with. It is easy to resurrect the\n> support for v1 (which may not even be needed for long) and resurrect\n> the support for v2 with Carlo's fix, if it later turns out that some\n> users may need to use a more complex pattern.\n>\n> Thanks.\n>\n>> Ævar Arnfjörð Bjarmason (3):\n>> grep: remove overly paranoid BUG(...) code\n>> grep: stop \"using\" a custom JIT stack with PCRE v2\n>> grep: stop using a custom JIT stack with PCRE v1\n>>\n>> grep.c | 46 ++++++----------------------------------------\n>> grep.h | 9 ---------\n>> 2 files changed, 6 insertions(+), 49 deletions(-)\n"},{"id":"379215","messageId":"87imrr6vv2.fsf@evledraar.gmail.com","threadId":"51506","inReplyTo":"xmqqwog7qu2r.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 2/3] grep: stop \"using\" a custom JIT stack with PCRE v2","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-07-24T20:06:09Z","receivedAt":"2019-07-24T20:20:49Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Jul 24 2019, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> Since we've haven't had any reports of users running into\n>> PCRE2_ERROR_JIT_STACKLIMIT in the wild I think we can safely assume\n>> that we can just use the library defaults instead and drop this\n>> code.\n>\n> Does everybody use pcre2 with JIT with Git these days, or only those\n> who want to live near the bleeding edge?\n\nMy informal survey of various package recipies suggests that all the big\n*nix distros are using it by default now, so we have a lot of users in\nthe wild, including in the just-released Debian stable.\n\nSo I'm confidend that if there were issues with e.g. it dying on\npatterns in practical use we'd have heard about them.\n\n>> This won't change with the wider use of PCRE v2 in\n>> ed0479ce3d (\"Merge branch 'ab/no-kwset' into next\", 2019-07-15), a\n>> fixed string search is not a \"large or complicated pattern\".\n>\n> In any case, if we were not \"using\" the custom stack anyway for v2,\n> this change does not hurt anybody, possibly other than those who\n> will learn about pcre2 support by reading this message and experiments\n> with larger patterns. And it should be simple to wire it back if it\n> becomes necessary later.\n\n*nod*\n"},{"id":"379240","messageId":"CAPUEspjj+fG8QDmf=bZXktfpLgkgiu34HTjKLhm-cmEE04FE-A@mail.gmail.com","threadId":"51506","inReplyTo":"87imrr6vv2.fsf@evledraar.gmail.com","subject":"Re: [PATCH 2/3] grep: stop \"using\" a custom JIT stack with PCRE v2","fromName":"Carlo Arenas","fromEmail":"carenas@gmail.com","sentAt":"2019-07-25T05:11:04Z","receivedAt":"2019-07-25T05:11:18Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"On Wed, Jul 24, 2019 at 1:20 PM Ævar Arnfjörð Bjarmason\n wrote:\n> On Wed, Jul 24 2019, Junio C Hamano wrote:\n> >\n> > Does everybody use pcre2 with JIT with Git these days, or only those\n> > who want to live near the bleeding edge?\n>\n> My informal survey of various package recipies suggests that all the big\n> *nix distros are using it by default now, so we have a lot of users in\n> the wild, including in the just-released Debian stable.\n\nFWIW neither OpenBSD or NetBSD enable JIT, and the git that comes\nwith Xcode (AKA Apple Git) doesn't either, while still using PCRE1\n\n> > In any case, if we were not \"using\" the custom stack anyway for v2,\n> > this change does not hurt anybody, possibly other than those who\n> > will learn about pcre2 support by reading this message and experiments\n> > with larger patterns. And it should be simple to wire it back if it\n> > becomes necessary later.\n>\n> *nod*\n\nthe following pattern fails unless 1MB stack is available:\n\n '^([/](?!/)|[^/])*~/.*'\n\nthe workaround implemented in GNU grep (that uses PCRE1) and the related\ndiscussion[1] are a very interesting read\n\nCarlo\n\n[1] https://www.mail-archive.com/bug-grep@gnu.org/msg05763.html\n"},{"id":"379335","messageId":"CAPUEspiCFup4wvNwOA+egiAjkUEPgU+YnU8x2DfKhdbqTdOV3w@mail.gmail.com","threadId":"51506","inReplyTo":"20190724151415.3698-4-avarab@gmail.com","subject":"Re: [PATCH 3/3] grep: stop using a custom JIT stack with PCRE v1","fromName":"Carlo Arenas","fromEmail":"carenas@gmail.com","sentAt":"2019-07-26T13:15:02Z","receivedAt":"2019-07-26T13:15:16Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"since this moves PCRE1 out of the JIT fast path, introduces the\nregression where git grep will abort if there is binary data or non\nUTF-8 text in the repository/log and should be IMHO hold out until a\nfix for that can be merged.\n\nthis also needs additional changes to better support NO_LIBPCRE1_JIT,\npatch to follow\n\nCarlo\n"},{"id":"379337","messageId":"87h8787vmt.fsf@evledraar.gmail.com","threadId":"51506","inReplyTo":"CAPUEspiCFup4wvNwOA+egiAjkUEPgU+YnU8x2DfKhdbqTdOV3w@mail.gmail.com","subject":"Re: [PATCH 3/3] grep: stop using a custom JIT stack with PCRE v1","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-07-26T13:50:18Z","receivedAt":"2019-07-26T13:50:33Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Jul 26 2019, Carlo Arenas wrote:\n\n> since this moves PCRE1 out of the JIT fast path,\n\nI think you're mostly replying to the wrong thread. None of the patches\nI've sent disable PCRE v1 JIT, as the performance numbers show. The JIT\nstack is resized, and for v2 some dead code removed.\n\n> introduces the regression where git grep will abort if there is binary\n> data or non UTF-8 text in the repository/log and should be IMHO hold\n> out until a fix for that can be merged.\n\nYou're talking about the kwset series, not this cleanup series.\n\n> this also needs additional changes to better support NO_LIBPCRE1_JIT,\n> patch to follow\n\nLooking forward to it, thanks!\n"},{"id":"379341","messageId":"CAPUEsphZJ_Uv9o1-yDpjNLA_q-f7gWXz9g1gCY2pYAYN8ri40g@mail.gmail.com","threadId":"51506","inReplyTo":"87h8787vmt.fsf@evledraar.gmail.com","subject":"Re: [PATCH 3/3] grep: stop using a custom JIT stack with PCRE v1","fromName":"Carlo Arenas","fromEmail":"carenas@gmail.com","sentAt":"2019-07-26T14:12:34Z","receivedAt":"2019-07-26T14:12:47Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"On Fri, Jul 26, 2019 at 6:50 AM Ævar Arnfjörð Bjarmason\n wrote:\n>\n> On Fri, Jul 26 2019, Carlo Arenas wrote:\n>\n> > since this moves PCRE1 out of the JIT fast path,\n>\n> I think you're mostly replying to the wrong thread. None of the patches\n> I've sent disable PCRE v1 JIT, as the performance numbers show. The JIT\n> stack is resized, and for v2 some dead code removed.\n\nI didn't mean JIT was disabled, but that we are calling now the regular\nPCRE1 function which does UTF-8 validation (unlike the one used before)\n\n> > introduces the regression where git grep will abort if there is binary\n> > data or non UTF-8 text in the repository/log and should be IMHO hold\n> > out until a fix for that can be merged.\n>\n> You're talking about the kwset series, not this cleanup series.\n\na combination of both (as seen in pu) and that will also happen in next if\nthis series get merged there.\n\nbefore this cleanup series, a git compiled against PCRE1 and not using\nNO_LIBPCRE1_JIT will use the jit fast path function and therefore would\nhave no problems with binary or non UTF-8 content in the repository, but\nwill regress after.\n\nCarlo\n"},{"id":"379343","messageId":"87ftms7t6s.fsf@evledraar.gmail.com","threadId":"51506","inReplyTo":"CAPUEsphZJ_Uv9o1-yDpjNLA_q-f7gWXz9g1gCY2pYAYN8ri40g@mail.gmail.com","subject":"Re: [PATCH 3/3] grep: stop using a custom JIT stack with PCRE v1","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-07-26T14:43:07Z","receivedAt":"2019-07-26T14:43:11Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Jul 26 2019, Carlo Arenas wrote:\n\n> On Fri, Jul 26, 2019 at 6:50 AM Ævar Arnfjörð Bjarmason\n> wrote:\n>>\n>> On Fri, Jul 26 2019, Carlo Arenas wrote:\n>>\n>> > since this moves PCRE1 out of the JIT fast path,\n>>\n>> I think you're mostly replying to the wrong thread. None of the patches\n>> I've sent disable PCRE v1 JIT, as the performance numbers show. The JIT\n>> stack is resized, and for v2 some dead code removed.\n>\n> I didn't mean JIT was disabled, but that we are calling now the regular\n> PCRE1 function which does UTF-8 validation (unlike the one used before)\n>\n>> > introduces the regression where git grep will abort if there is binary\n>> > data or non UTF-8 text in the repository/log and should be IMHO hold\n>> > out until a fix for that can be merged.\n>>\n>> You're talking about the kwset series, not this cleanup series.\n>\n> a combination of both (as seen in pu) and that will also happen in next if\n> this series get merged there.\n>\n> before this cleanup series, a git compiled against PCRE1 and not using\n> NO_LIBPCRE1_JIT will use the jit fast path function and therefore would\n> have no problems with binary or non UTF-8 content in the repository, but\n> will regress after.\n\nI see. Yes you're right, I misread pcrejit(3) about how the \"fast path\nAPI\" worked (or more accurately, misremembered). Yes, this is now a new\ncaveat.\n\nI have some patches on top of next I'm about to send that hopefully make\nthis whole thing less of a mess.\n"},{"id":"379344","messageId":"20190726150818.6373-2-avarab@gmail.com","threadId":"51506","inReplyTo":"20190724151415.3698-1-avarab@gmail.com","subject":"[PATCH v2 1/8] grep: remove overly paranoid BUG(...) code","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-07-26T15:08:11Z","receivedAt":"2019-07-26T15:09:01Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Remove code that would trigger if pcre_config() or pcre2_config() was\nso broken that \"do we have JIT?\" wouldn't return a boolean.\n\nI added this code back in fbaceaac47 (\"grep: add support for the PCRE\nv1 JIT API\", 2017-05-25) and then as noted in f002532784 (\"grep: print\nthe pcre2_jit_on value\", 2019-07-22) incorrectly copy/pasted some of\nit in 94da9193a6 (\"grep: add support for PCRE v2\", 2017-06-01).\n\nLet's just remove this code. Being this paranoid about the\npcre2?_config() function itself being broken is crossing the line into\nunreasonable paranoia.\n\nReported-by: Beat Bolli \nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n grep.c | 10 ++--------\n 1 file changed, 2 insertions(+), 8 deletions(-)\n\ndiff --git a/grep.c b/grep.c\nindex 0937c5bfff..95af88cb74 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -394,14 +394,11 @@ static void compile_pcre1_regexp(struct grep_pat *p, const struct grep_opt *opt)\n \n #ifdef GIT_PCRE1_USE_JIT\n \tpcre_config(PCRE_CONFIG_JIT, &p->pcre1_jit_on);\n-\tif (p->pcre1_jit_on == 1) {\n+\tif (p->pcre1_jit_on) {\n \t\tp->pcre1_jit_stack = pcre_jit_stack_alloc(1, 1024 * 1024);\n \t\tif (!p->pcre1_jit_stack)\n \t\t\tdie(\"Couldn't allocate PCRE JIT stack\");\n \t\tpcre_assign_jit_stack(p->pcre1_extra_info, NULL, p->pcre1_jit_stack);\n-\t} else if (p->pcre1_jit_on != 0) {\n-\t\tBUG(\"The pcre1_jit_on variable should be 0 or 1, not %d\",\n-\t\t p->pcre1_jit_on);\n \t}\n #endif\n }\n@@ -510,7 +507,7 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt\n \t}\n \n \tpcre2_config(PCRE2_CONFIG_JIT, &p->pcre2_jit_on);\n-\tif (p->pcre2_jit_on == 1) {\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@@ -545,9 +542,6 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt\n \t\tif (!p->pcre2_match_context)\n \t\t\tdie(\"Couldn't allocate PCRE2 match context\");\n \t\tpcre2_jit_stack_assign(p->pcre2_match_context, NULL, p->pcre2_jit_stack);\n-\t} else if (p->pcre2_jit_on != 0) {\n-\t\tBUG(\"The pcre2_jit_on variable should be 0 or 1, not %d\",\n-\t\t p->pcre2_jit_on);\n \t}\n }\n \n-- \n2.22.0.455.g172b71a6c5\n\n"},{"id":"379345","messageId":"20190726150818.6373-3-avarab@gmail.com","threadId":"51506","inReplyTo":"20190724151415.3698-1-avarab@gmail.com","subject":"[PATCH v2 2/8] grep: stop \"using\" a custom JIT stack with PCRE v2","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-07-26T15:08:12Z","receivedAt":"2019-07-26T15:09:04Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"As reported in [1] the code I added in 94da9193a6 (\"grep: add support\nfor PCRE v2\", 2017-06-01) to use a custom JIT stack has never\nworked. It was incorrectly copy/pasted from code I added in\nfbaceaac47 (\"grep: add support for the PCRE v1 JIT API\", 2017-05-25),\nwhich did work.\n\nThus our intention of starting with 1 byte of stack at a maximum of 1\nMB didn't happen, we'd always use the 32 KB stack provided by PCRE\nv2's jit_machine_stack_exec()[2]. The reason I allocated a custom\nstack at all was this advice in pcrejit(3) (same in pcre2jit(3)):\n\n \"By default, it uses 32KiB on the machine stack. However, some\n large or complicated patterns need more than this\"\n\nSince we've haven't had any reports of users running into\nPCRE2_ERROR_JIT_STACKLIMIT in the wild I think we can safely assume\nthat we can just use the library defaults instead and drop this\ncode. This won't change with the wider use of PCRE v2 in\ned0479ce3d (\"Merge branch 'ab/no-kwset' into next\", 2019-07-15), a\nfixed string search is not a \"large or complicated pattern\".\n\nFor good measure I ran the performance test noted in 94da9193a6,\nalthough the command is simpler now due to my 0f50c8e32c (\"Makefile:\nremove the NO_R_TO_GCC_LINKER flag\", 2019-05-17):\n\n GIT_PERF_REPEAT_COUNT=30 GIT_PERF_LARGE_REPO=~/g/linux GIT_PERF_MAKE_OPTS='-j8 USE_LIBPCRE2=Y CFLAGS=-O3 LIBPCREDIR=/home/avar/g/pcre2/inst' ./run HEAD~ HEAD p7820-grep-engines.sh\n\nJust the /perl/ results are:\n\n Test HEAD~ HEAD\n ---------------------------------------------------------------------------------------\n 7820.3: perl grep 'how.to' 0.17(0.27+0.65) 0.17(0.24+0.68) +0.0%\n 7820.7: perl grep '^how to' 0.16(0.23+0.66) 0.16(0.23+0.67) +0.0%\n 7820.11: perl grep '[how] to' 0.18(0.35+0.62) 0.18(0.33+0.65) +0.0%\n 7820.15: perl grep '(e.t[^ ]*|v.ry) rare' 0.17(0.45+0.54) 0.17(0.49+0.50) +0.0%\n 7820.19: perl grep 'm(ú|u)lt.b(æ|y)te' 0.16(0.33+0.58) 0.16(0.29+0.62) +0.0%\n\nSo, as expected there's no change, and running with valgrind reveals\nthat we have fewer allocations now.\n\nAs noted in [3] there are known regexes that will fail with the lower\nstack limit, the way GNU grep fixed it is interesting, although I\nbelieve the implementation is overly verbose, they could make PCRE v2\nhandle that gradual re-allocation, that's what min/max memory is\nfor.\n\nSo we might end up bringing this back, I'm more inclined to just kick\nsuch cases upstairs to PCRE maintainers as a bug, perhaps they'll add\nsome overall \"just allocate more then\" flag to make this easier. In\nany case there's no functional change here, we didn't have a custom\nstack, so let's apply this first, we can always revert it later.\n\n1. https://public-inbox.org/git/20190721194052.15440-1-carenas@gmail.com/\n2. I didn't really intend to start with 1 byte, looking at the PCRE v2\n code again what happened is that I cargo-culted some of PCRE v2's\n own test code which was meant to test re-allocations. It's more\n sane to start with say 32 KB with a max of 1 MB, as pcre2grep.c\n does.\n3. https://public-inbox.org/git/CAPUEspjj+fG8QDmf=bZXktfpLgkgiu34HTjKLhm-cmEE04FE-A@mail.gmail.com/\n\nReported-by: Carlo Marcelo Arenas Belón \nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n grep.c | 10 ----------\n grep.h | 4 ----\n 2 files changed, 14 deletions(-)\n\ndiff --git a/grep.c b/grep.c\nindex 95af88cb74..4b1e917ac5 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -534,14 +534,6 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt\n \t\t\tp->pcre2_jit_on = 0;\n \t\t\treturn;\n \t\t}\n-\n-\t\tp->pcre2_jit_stack = pcre2_jit_stack_create(1, 1024 * 1024, NULL);\n-\t\tif (!p->pcre2_jit_stack)\n-\t\t\tdie(\"Couldn't allocate PCRE2 JIT stack\");\n-\t\tp->pcre2_match_context = pcre2_match_context_create(NULL);\n-\t\tif (!p->pcre2_match_context)\n-\t\t\tdie(\"Couldn't allocate PCRE2 match context\");\n-\t\tpcre2_jit_stack_assign(p->pcre2_match_context, NULL, p->pcre2_jit_stack);\n \t}\n }\n \n@@ -585,8 +577,6 @@ static void free_pcre2_pattern(struct grep_pat *p)\n \tpcre2_compile_context_free(p->pcre2_compile_context);\n \tpcre2_code_free(p->pcre2_pattern);\n \tpcre2_match_data_free(p->pcre2_match_data);\n-\tpcre2_jit_stack_free(p->pcre2_jit_stack);\n-\tpcre2_match_context_free(p->pcre2_match_context);\n }\n #else /* !USE_LIBPCRE2 */\n static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt)\ndiff --git a/grep.h b/grep.h\nindex d35a137fcb..4d8e300175 100644\n--- a/grep.h\n+++ b/grep.h\n@@ -29,8 +29,6 @@ typedef int pcre_jit_stack;\n typedef int pcre2_code;\n typedef int pcre2_match_data;\n typedef int pcre2_compile_context;\n-typedef int pcre2_match_context;\n-typedef int pcre2_jit_stack;\n #endif\n #include \"thread-utils.h\"\n #include \"userdiff.h\"\n@@ -93,8 +91,6 @@ struct grep_pat {\n \tpcre2_code *pcre2_pattern;\n \tpcre2_match_data *pcre2_match_data;\n \tpcre2_compile_context *pcre2_compile_context;\n-\tpcre2_match_context *pcre2_match_context;\n-\tpcre2_jit_stack *pcre2_jit_stack;\n \tuint32_t pcre2_jit_on;\n \tunsigned fixed:1;\n \tunsigned ignore_case:1;\n-- \n2.22.0.455.g172b71a6c5\n\n"},{"id":"379346","messageId":"20190726150818.6373-4-avarab@gmail.com","threadId":"51506","inReplyTo":"20190724151415.3698-1-avarab@gmail.com","subject":"[PATCH v2 3/8] grep: stop using a custom JIT stack with PCRE v1","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-07-26T15:08:13Z","receivedAt":"2019-07-26T15:09:04Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Simplify the PCRE v1 code for the same reasons as for the PCRE v2 code\nin the last commit. Unlike with v2 we actually used the custom stack\nin v1, but let's use PCRE's built-in 32 KB one instead, since\nexperience with v2 shows that's enough. Most distros are already using\nv2 as a default, and the underlying sljit code is the same.\n\nUnfortunately we can't just pass a NULL to pcre_jit_exec() as with\npcre2_jit_match(). Unlike the v2 function it doesn't support\nthat. Instead we need to use the fatter pcre_exec() if we'd like the\nsame behavior.\n\nThis will make things slightly slower than on the fast-path function,\nbut it's OK since we care less about v1 performance these days since\nwe have and recommend v2. Running a similar performance test as what I\nran in fbaceaac47 (\"grep: add support for the PCRE v1 JIT API\",\n2017-05-25) via:\n\n GIT_PERF_REPEAT_COUNT=30 GIT_PERF_LARGE_REPO=~/g/linux GIT_PERF_MAKE_OPTS='-j8 USE_LIBPCRE1=Y CFLAGS=-O3 LIBPCREDIR=/home/avar/g/pcre/inst' ./run HEAD~ HEAD p7820-grep-engines.sh\n\nGives us this, just the /perl/ results:\n\n Test HEAD~ HEAD\n ---------------------------------------------------------------------------------------\n 7820.3: perl grep 'how.to' 0.19(0.67+0.52) 0.19(0.65+0.52) +0.0%\n 7820.7: perl grep '^how to' 0.19(0.78+0.44) 0.19(0.72+0.49) +0.0%\n 7820.11: perl grep '[how] to' 0.39(2.13+0.43) 0.40(2.10+0.46) +2.6%\n 7820.15: perl grep '(e.t[^ ]*|v.ry) rare' 0.44(2.55+0.37) 0.45(2.47+0.41) +2.3%\n 7820.19: perl grep 'm(ú|u)lt.b(æ|y)te' 0.23(1.06+0.42) 0.22(1.03+0.43) -4.3%\n\nIt will also implicitly re-enable UTF-8 validation for PCRE v1. As\nnoted in [1] we now have cases as a result where PCRE v1 is more eager\nto error out. Subsequent patches will fix that for v2, and I think\nit's fair to tell v1 users \"just upgrade\" and not worry about that\nedge case for v1.\n\n1. https://public-inbox.org/git/CAPUEsphZJ_Uv9o1-yDpjNLA_q-f7gWXz9g1gCY2pYAYN8ri40g@mail.gmail.com/\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n grep.c | 28 +++++-----------------------\n grep.h | 5 -----\n 2 files changed, 5 insertions(+), 28 deletions(-)\n\ndiff --git a/grep.c b/grep.c\nindex 4b1e917ac5..9c2b259771 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -394,12 +394,6 @@ static void compile_pcre1_regexp(struct grep_pat *p, const struct grep_opt *opt)\n \n #ifdef GIT_PCRE1_USE_JIT\n \tpcre_config(PCRE_CONFIG_JIT, &p->pcre1_jit_on);\n-\tif (p->pcre1_jit_on) {\n-\t\tp->pcre1_jit_stack = pcre_jit_stack_alloc(1, 1024 * 1024);\n-\t\tif (!p->pcre1_jit_stack)\n-\t\t\tdie(\"Couldn't allocate PCRE JIT stack\");\n-\t\tpcre_assign_jit_stack(p->pcre1_extra_info, NULL, p->pcre1_jit_stack);\n-\t}\n #endif\n }\n \n@@ -411,18 +405,9 @@ static int pcre1match(struct grep_pat *p, const char *line, const char *eol,\n \tif (eflags & REG_NOTBOL)\n \t\tflags |= PCRE_NOTBOL;\n \n-#ifdef GIT_PCRE1_USE_JIT\n-\tif (p->pcre1_jit_on) {\n-\t\tret = pcre_jit_exec(p->pcre1_regexp, p->pcre1_extra_info, line,\n-\t\t\t\t eol - line, 0, flags, ovector,\n-\t\t\t\t ARRAY_SIZE(ovector), p->pcre1_jit_stack);\n-\t} else\n-#endif\n-\t{\n-\t\tret = pcre_exec(p->pcre1_regexp, p->pcre1_extra_info, line,\n-\t\t\t\teol - line, 0, flags, ovector,\n-\t\t\t\tARRAY_SIZE(ovector));\n-\t}\n+\tret = pcre_exec(p->pcre1_regexp, p->pcre1_extra_info, line,\n+\t\t\teol - line, 0, flags, ovector,\n+\t\t\tARRAY_SIZE(ovector));\n \n \tif (ret < 0 && ret != PCRE_ERROR_NOMATCH)\n \t\tdie(\"pcre_exec failed with error code %d\", ret);\n@@ -439,14 +424,11 @@ static void free_pcre1_regexp(struct grep_pat *p)\n {\n \tpcre_free(p->pcre1_regexp);\n #ifdef GIT_PCRE1_USE_JIT\n-\tif (p->pcre1_jit_on) {\n+\tif (p->pcre1_jit_on)\n \t\tpcre_free_study(p->pcre1_extra_info);\n-\t\tpcre_jit_stack_free(p->pcre1_jit_stack);\n-\t} else\n+\telse\n #endif\n-\t{\n \t\tpcre_free(p->pcre1_extra_info);\n-\t}\n \tpcre_free((void *)p->pcre1_tables);\n }\n #else /* !USE_LIBPCRE1 */\ndiff --git a/grep.h b/grep.h\nindex 4d8e300175..ce2d72571f 100644\n--- a/grep.h\n+++ b/grep.h\n@@ -14,13 +14,9 @@\n #ifndef GIT_PCRE_STUDY_JIT_COMPILE\n #define GIT_PCRE_STUDY_JIT_COMPILE 0\n #endif\n-#if PCRE_MAJOR <= 8 && PCRE_MINOR < 20\n-typedef int pcre_jit_stack;\n-#endif\n #else\n typedef int pcre;\n typedef int pcre_extra;\n-typedef int pcre_jit_stack;\n #endif\n #ifdef USE_LIBPCRE2\n #define PCRE2_CODE_UNIT_WIDTH 8\n@@ -85,7 +81,6 @@ struct grep_pat {\n \tregex_t regexp;\n \tpcre *pcre1_regexp;\n \tpcre_extra *pcre1_extra_info;\n-\tpcre_jit_stack *pcre1_jit_stack;\n \tconst unsigned char *pcre1_tables;\n \tint pcre1_jit_on;\n \tpcre2_code *pcre2_pattern;\n-- \n2.22.0.455.g172b71a6c5\n\n"},{"id":"379347","messageId":"20190726150818.6373-5-avarab@gmail.com","threadId":"51506","inReplyTo":"20190724151415.3698-1-avarab@gmail.com","subject":"[PATCH v2 4/8] grep: consistently use \"p->fixed\" in compile_regexp()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-07-26T15:08:14Z","receivedAt":"2019-07-26T15:09:07Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"At the start of this function we do:\n\n p->fixed = opt->fixed;\n\nIt's less confusing to use that variable consistently that switch back\n& forth between the two.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n grep.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/grep.c b/grep.c\nindex 9c2b259771..b94e998680 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -616,7 +616,7 @@ static void compile_regexp(struct grep_pat *p, struct grep_opt *opt)\n \t\tdie(_(\"given pattern contains NULL byte (via -f ). This is only supported with -P under PCRE v2\"));\n \n \tpat_is_fixed = is_fixed(p->pattern, p->patternlen);\n-\tif (opt->fixed || pat_is_fixed) {\n+\tif (p->fixed || pat_is_fixed) {\n #ifdef USE_LIBPCRE2\n \t\topt->pcre2 = 1;\n \t\tif (pat_is_fixed) {\n-- \n2.22.0.455.g172b71a6c5\n\n"},{"id":"379348","messageId":"20190726150818.6373-6-avarab@gmail.com","threadId":"51506","inReplyTo":"20190724151415.3698-1-avarab@gmail.com","subject":"[PATCH v2 5/8] grep: create a \"is_fixed\" member in \"grep_pat\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-07-26T15:08:15Z","receivedAt":"2019-07-26T15:09:09Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This change paves the way for later using this value the regex compile\nfunctions themselves.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n grep.c | 7 +++----\n grep.h | 1 +\n 2 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/grep.c b/grep.c\nindex b94e998680..6d60e2e557 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -606,7 +606,6 @@ static void compile_regexp(struct grep_pat *p, struct grep_opt *opt)\n {\n \tint err;\n \tint regflags = REG_NEWLINE;\n-\tint pat_is_fixed;\n \n \tp->word_regexp = opt->word_regexp;\n \tp->ignore_case = opt->ignore_case;\n@@ -615,11 +614,11 @@ static void compile_regexp(struct grep_pat *p, struct grep_opt *opt)\n \tif (memchr(p->pattern, 0, p->patternlen) && !opt->pcre2)\n \t\tdie(_(\"given pattern contains NULL byte (via -f ). This is only supported with -P under PCRE v2\"));\n \n-\tpat_is_fixed = is_fixed(p->pattern, p->patternlen);\n-\tif (p->fixed || pat_is_fixed) {\n+\tp->is_fixed = is_fixed(p->pattern, p->patternlen);\n+\tif (p->fixed || p->is_fixed) {\n #ifdef USE_LIBPCRE2\n \t\topt->pcre2 = 1;\n-\t\tif (pat_is_fixed) {\n+\t\tif (p->is_fixed) {\n \t\t\tcompile_pcre2_pattern(p, opt);\n \t\t} else {\n \t\t\t/*\ndiff --git a/grep.h b/grep.h\nindex ce2d72571f..c0c71eb4a9 100644\n--- a/grep.h\n+++ b/grep.h\n@@ -88,6 +88,7 @@ struct grep_pat {\n \tpcre2_compile_context *pcre2_compile_context;\n \tuint32_t pcre2_jit_on;\n \tunsigned fixed:1;\n+\tunsigned is_fixed:1;\n \tunsigned ignore_case:1;\n \tunsigned word_regexp:1;\n };\n-- \n2.22.0.455.g172b71a6c5\n\n"},{"id":"379349","messageId":"20190726150818.6373-8-avarab@gmail.com","threadId":"51506","inReplyTo":"20190724151415.3698-1-avarab@gmail.com","subject":"[PATCH v2 7/8] grep: do not enter PCRE2_UTF mode on fixed matching","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-07-26T15:08:17Z","receivedAt":"2019-07-26T15:09:09Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"As discussed in the last commit partially fix a bug introduced in\nb65abcafc7 (\"grep: use PCRE v2 for optimized fixed-string search\",\n2019-07-01). Because PCRE v2, unlike kwset, validates its UTF-8 input\nwe'd die on e.g.:\n\n fatal: pcre2_match failed with error code -22: UTF-8 error:\n isolated byte with 0x80 bit set\n\nWhen grepping a non-ASCII fixed string. This is a more general problem\nthat's hard to fix, but we can at least fix the most common case of\ngrepping for a fixed string without \"-i\". I can't think of a reason\nfor why we'd turn on PCRE2_UTF when matching byte-for-byte like that.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n grep.c | 3 ++-\n t/t7812-grep-icase-non-ascii.sh | 4 ++--\n 2 files changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/grep.c b/grep.c\nindex 5bc0f4f32a..c7c06ae08d 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -472,7 +472,8 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt\n \t\t}\n \t\toptions |= PCRE2_CASELESS;\n \t}\n-\tif (!opt->ignore_locale && is_utf8_locale() && has_non_ascii(p->pattern))\n+\tif (!opt->ignore_locale && is_utf8_locale() && has_non_ascii(p->pattern) &&\n+\t !(!opt->ignore_case && (p->fixed || p->is_fixed)))\n \t\toptions |= PCRE2_UTF;\n \n \tp->pcre2_pattern = pcre2_compile((PCRE2_SPTR)p->pattern,\ndiff --git a/t/t7812-grep-icase-non-ascii.sh b/t/t7812-grep-icase-non-ascii.sh\nindex 96c3572056..531eb59d57 100755\n--- a/t/t7812-grep-icase-non-ascii.sh\n+++ b/t/t7812-grep-icase-non-ascii.sh\n@@ -68,9 +68,9 @@ test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: grep ASCII from invalid UT\n '\n \n test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: grep non-ASCII from invalid UTF-8 data' '\n-\ttest_might_fail git grep -h \"æ\" invalid-0x80 >actual &&\n+\tgit grep -h \"æ\" invalid-0x80 >actual &&\n \ttest_cmp expected actual &&\n-\ttest_must_fail git grep -h \"(*NO_JIT)æ\" invalid-0x80 &&\n+\tgit grep -h \"(*NO_JIT)æ\" invalid-0x80 &&\n \ttest_cmp expected actual\n '\n \n-- \n2.22.0.455.g172b71a6c5\n\n"},{"id":"379351","messageId":"20190726150818.6373-7-avarab@gmail.com","threadId":"51506","inReplyTo":"20190724151415.3698-1-avarab@gmail.com","subject":"[PATCH v2 6/8] grep: stess test PCRE v2 on invalid UTF-8 data","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-07-26T15:08:16Z","receivedAt":"2019-07-26T15:09:11Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Since my b65abcafc7 (\"grep: use PCRE v2 for optimized fixed-string\nsearch\", 2019-07-01) we've been dying on invalid UTF-8 data when\ngrepping for fixed strings if the following are all true:\n\n * The subject string is non-ASCII (e.g. \"ævar\")\n * We're under a is_utf8_locale(), e.g. \"en_US.UTF-8\", not \"C\"\n * We compiled with PCRE v2\n * That PCRE v2 did not have JIT support\n\nThe last of those is why this wasn't caught earlier, per pcre2jit(3):\n\n \"unless PCRE2_NO_UTF_CHECK is set, a UTF subject string is tested\n for validity. In the interests of speed, these checks do not\n happen on the JIT fast path, and if invalid data is passed, the\n result is undefined.\"\n\nI.e. the subject being matched against our pattern was invalid, but we\nwere lucky and getting away with it on the JIT path, but the non-JIT\none is stricter.\n\nThis patch does nothing to fix that, instead we sneak in support for\nfixed patterns starting with \"(*NO_JIT)\", this disables the PCRE v2\njit with implicit fixed-string matching for testing, see\npcre2syntax(3) the syntax.\n\nThis is technically a change in behavior, but it's so obscure that I\nfigured it was OK. We'd previously consider this an invalid regular\nexpression as regcomp() would die on it, now we feed it to the PCRE v2\nfixed-string path. I thought this was better than introducing yet\nanother GIT_TEST_* environment variable.\n\nWe're also relying on a behavior of PCRE v2 that technically could\nchange, but I think the test coverage is worth dipping our toe into\nsome somewhat undefined behavior.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n grep.c | 10 ++++++++++\n t/t7812-grep-icase-non-ascii.sh | 28 ++++++++++++++++++++++++++++\n 2 files changed, 38 insertions(+)\n\ndiff --git a/grep.c b/grep.c\nindex 6d60e2e557..5bc0f4f32a 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -615,6 +615,16 @@ static void compile_regexp(struct grep_pat *p, struct grep_opt *opt)\n \t\tdie(_(\"given pattern contains NULL byte (via -f ). This is only supported with -P under PCRE v2\"));\n \n \tp->is_fixed = is_fixed(p->pattern, p->patternlen);\n+#ifdef USE_LIBPCRE2\n+ if (!p->fixed && !p->is_fixed) {\n+\t const char *no_jit = \"(*NO_JIT)\";\n+\t const int no_jit_len = strlen(no_jit);\n+\t if (starts_with(p->pattern, no_jit) &&\n+\t\t is_fixed(p->pattern + no_jit_len,\n+\t\t\t p->patternlen - no_jit_len))\n+\t\t p->is_fixed = 1;\n+ }\n+#endif\n \tif (p->fixed || p->is_fixed) {\n #ifdef USE_LIBPCRE2\n \t\topt->pcre2 = 1;\ndiff --git a/t/t7812-grep-icase-non-ascii.sh b/t/t7812-grep-icase-non-ascii.sh\nindex 0c685d3598..96c3572056 100755\n--- a/t/t7812-grep-icase-non-ascii.sh\n+++ b/t/t7812-grep-icase-non-ascii.sh\n@@ -53,4 +53,32 @@ test_expect_success REGEX_LOCALE 'pickaxe -i on non-ascii' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: setup invalid UTF-8 data' '\n+\tprintf \"\\\\200\\\\n\" >invalid-0x80 &&\n+\techo \"ævar\" >expected &&\n+\tcat expected >>invalid-0x80 &&\n+\tgit add invalid-0x80\n+'\n+\n+test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: grep ASCII from invalid UTF-8 data' '\n+\tgit grep -h \"var\" invalid-0x80 >actual &&\n+\ttest_cmp expected actual &&\n+\tgit grep -h \"(*NO_JIT)var\" invalid-0x80 >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: grep non-ASCII from invalid UTF-8 data' '\n+\ttest_might_fail git grep -h \"æ\" invalid-0x80 >actual &&\n+\ttest_cmp expected actual &&\n+\ttest_must_fail git grep -h \"(*NO_JIT)æ\" invalid-0x80 &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: grep non-ASCII from invalid UTF-8 data with -i' '\n+\ttest_might_fail git grep -hi \"Æ\" invalid-0x80 >actual &&\n+\ttest_cmp expected actual &&\n+\ttest_must_fail git grep -hi \"(*NO_JIT)Æ\" invalid-0x80 &&\n+\ttest_cmp expected actual\n+'\n+\n test_done\n-- \n2.22.0.455.g172b71a6c5\n\n"},{"id":"379350","messageId":"20190726150818.6373-1-avarab@gmail.com","threadId":"51506","inReplyTo":"20190724151415.3698-1-avarab@gmail.com","subject":"[PATCH v2 0/8] grep: PCRE JIT fixes + ab/no-kwset fix","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-07-26T15:08:10Z","receivedAt":"2019-07-26T15:09:12Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"1-3 here are a re-roll on \"next\". I figured that was easier for\neveryone with the state of the in-flight patches, it certainly was for\nme. Sorry Junio if this creates a mess for you.\n\n4-8 are a \"fix\" for the UTF-8 matching error noted in Carlo's \"grep:\nskip UTF8 checks explicitally\" in\nhttps://public-inbox.org/git/20190721183115.14985-1-carenas@gmail.com/\n\nAs noted the bug isn't fully fixed until 8/8, and that patch relies on\nunreleased PCRE v2 code. I'm hoping that with 7/8 we're in a good\nenough state to limp forward as noted in the rationale of those\ncommits.\n\nÆvar Arnfjörð Bjarmason (8):\n grep: remove overly paranoid BUG(...) code\n grep: stop \"using\" a custom JIT stack with PCRE v2\n grep: stop using a custom JIT stack with PCRE v1\n grep: consistently use \"p->fixed\" in compile_regexp()\n grep: create a \"is_fixed\" member in \"grep_pat\"\n grep: stess test PCRE v2 on invalid UTF-8 data\n grep: do not enter PCRE2_UTF mode on fixed matching\n grep: optimistically use PCRE2_MATCH_INVALID_UTF\n\n Makefile | 1 +\n grep.c | 68 +++++++++++----------------------\n grep.h | 13 ++-----\n t/helper/test-pcre2-config.c | 12 ++++++\n t/helper/test-tool.c | 1 +\n t/helper/test-tool.h | 1 +\n t/t7812-grep-icase-non-ascii.sh | 39 +++++++++++++++++++\n 7 files changed, 80 insertions(+), 55 deletions(-)\n create mode 100644 t/helper/test-pcre2-config.c\n\n-- \n2.22.0.455.g172b71a6c5\n\n"},{"id":"379352","messageId":"20190726150818.6373-9-avarab@gmail.com","threadId":"51506","inReplyTo":"20190724151415.3698-1-avarab@gmail.com","subject":"[PATCH v2 8/8] grep: optimistically use PCRE2_MATCH_INVALID_UTF","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-07-26T15:08:18Z","receivedAt":"2019-07-26T15:09:15Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"As discussed in the \"grep: stess test PCRE v2 on invalid UTF-8 data\"\ncommit leading up to this one there's a regression in\nb65abcafc7 (\"grep: use PCRE v2 for optimized fixed-string search\",\n2019-07-01) when matching UTF-8 data.\n\nThis ultimately isn't straightforward to just \"fix\", because the kwset\nbackend was so dumb about icase matching that we'd skip it entirely on\nnon-ASCII. See the code removed in 48de2a768c (\"grep: remove the kwset\noptimization\", 2019-07-01).\n\nJust going back to the C library for those isn't ideal, since it's\nlikely to be even dumber about these mixed-encoding cases.\n\nSo let's support this \"properly\" using the PCRE2_MATCH_INVALID_UTF\nflag. This is new code that's not in any released PCRE v2 version, so\nwe might need a fix that emulates it somehow. I figure that the case\nthat with the non-icase bug out of the way this is obscure enough to\ntell people \"upgrade your PCRE v2 too!'. It'll likely be released by\nthe time we release the git version this commit is part of.\n\nWe can't just use PCRE2_NO_UTF_CHECK instead for the reasons discussed\nin [1].\n\n1. https://public-inbox.org/git/87lfwn70nb.fsf@evledraar.gmail.com/\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 1 +\n grep.c | 2 +-\n grep.h | 3 +++\n t/helper/test-pcre2-config.c | 12 ++++++++++++\n t/helper/test-tool.c | 1 +\n t/helper/test-tool.h | 1 +\n t/t7812-grep-icase-non-ascii.sh | 13 ++++++++++++-\n 7 files changed, 31 insertions(+), 2 deletions(-)\n create mode 100644 t/helper/test-pcre2-config.c\n\ndiff --git a/Makefile b/Makefile\nindex bd246f2989..dd38d5e527 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -726,6 +726,7 @@ TEST_BUILTINS_OBJS += test-oidmap.o\n TEST_BUILTINS_OBJS += test-online-cpus.o\n TEST_BUILTINS_OBJS += test-parse-options.o\n TEST_BUILTINS_OBJS += test-path-utils.o\n+TEST_BUILTINS_OBJS += test-pcre2-config.o\n TEST_BUILTINS_OBJS += test-pkt-line.o\n TEST_BUILTINS_OBJS += test-prio-queue.o\n TEST_BUILTINS_OBJS += test-reach.o\ndiff --git a/grep.c b/grep.c\nindex c7c06ae08d..8b8b9efe12 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -474,7 +474,7 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt\n \t}\n \tif (!opt->ignore_locale && is_utf8_locale() && has_non_ascii(p->pattern) &&\n \t !(!opt->ignore_case && (p->fixed || p->is_fixed)))\n-\t\toptions |= PCRE2_UTF;\n+\t\toptions |= (PCRE2_UTF | PCRE2_MATCH_INVALID_UTF);\n \n \tp->pcre2_pattern = pcre2_compile((PCRE2_SPTR)p->pattern,\n \t\t\t\t\t p->patternlen, options, &error, &erroffset,\ndiff --git a/grep.h b/grep.h\nindex c0c71eb4a9..506f05b97b 100644\n--- a/grep.h\n+++ b/grep.h\n@@ -21,6 +21,9 @@ typedef int pcre_extra;\n #ifdef USE_LIBPCRE2\n #define PCRE2_CODE_UNIT_WIDTH 8\n #include \n+#ifndef PCRE2_MATCH_INVALID_UTF\n+#define PCRE2_MATCH_INVALID_UTF 0\n+#endif\n #else\n typedef int pcre2_code;\n typedef int pcre2_match_data;\ndiff --git a/t/helper/test-pcre2-config.c b/t/helper/test-pcre2-config.c\nnew file mode 100644\nindex 0000000000..5258fdddba\n--- /dev/null\n+++ b/t/helper/test-pcre2-config.c\n@@ -0,0 +1,12 @@\n+#include \"test-tool.h\"\n+#include \"cache.h\"\n+#include \"grep.h\"\n+\n+int cmd__pcre2_config(int argc, const char **argv)\n+{\n+\tif (argc == 2 && !strcmp(argv[1], \"has-PCRE2_MATCH_INVALID_UTF\")) {\n+\t\tint value = PCRE2_MATCH_INVALID_UTF;\n+\t\treturn !value;\n+\t}\n+\treturn 1;\n+}\ndiff --git a/t/helper/test-tool.c b/t/helper/test-tool.c\nindex ce7e89028c..e022ce0e48 100644\n--- a/t/helper/test-tool.c\n+++ b/t/helper/test-tool.c\n@@ -40,6 +40,7 @@ static struct test_cmd cmds[] = {\n \t{ \"online-cpus\", cmd__online_cpus },\n \t{ \"parse-options\", cmd__parse_options },\n \t{ \"path-utils\", cmd__path_utils },\n+\t{ \"pcre2-config\", cmd__pcre2_config },\n \t{ \"pkt-line\", cmd__pkt_line },\n \t{ \"prio-queue\", cmd__prio_queue },\n \t{ \"reach\", cmd__reach },\ndiff --git a/t/helper/test-tool.h b/t/helper/test-tool.h\nindex f805bb39ae..acd8af2a9d 100644\n--- a/t/helper/test-tool.h\n+++ b/t/helper/test-tool.h\n@@ -30,6 +30,7 @@ int cmd__oidmap(int argc, const char **argv);\n int cmd__online_cpus(int argc, const char **argv);\n int cmd__parse_options(int argc, const char **argv);\n int cmd__path_utils(int argc, const char **argv);\n+int cmd__pcre2_config(int argc, const char **argv);\n int cmd__pkt_line(int argc, const char **argv);\n int cmd__prio_queue(int argc, const char **argv);\n int cmd__reach(int argc, const char **argv);\ndiff --git a/t/t7812-grep-icase-non-ascii.sh b/t/t7812-grep-icase-non-ascii.sh\nindex 531eb59d57..848d46e4f9 100755\n--- a/t/t7812-grep-icase-non-ascii.sh\n+++ b/t/t7812-grep-icase-non-ascii.sh\n@@ -74,11 +74,22 @@ test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: grep non-ASCII from invali\n \ttest_cmp expected actual\n '\n \n-test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: grep non-ASCII from invalid UTF-8 data with -i' '\n+test_lazy_prereq PCRE2_MATCH_INVALID_UTF '\n+\ttest-tool pcre2-config has-PCRE2_MATCH_INVALID_UTF\n+'\n+\n+test_expect_success GETTEXT_LOCALE,LIBPCRE2,!PCRE2_MATCH_INVALID_UTF 'PCRE v2: grep non-ASCII from invalid UTF-8 data with -i' '\n \ttest_might_fail git grep -hi \"Æ\" invalid-0x80 >actual &&\n \ttest_cmp expected actual &&\n \ttest_must_fail git grep -hi \"(*NO_JIT)Æ\" invalid-0x80 &&\n \ttest_cmp expected actual\n '\n \n+test_expect_success GETTEXT_LOCALE,LIBPCRE2,PCRE2_MATCH_INVALID_UTF 'PCRE v2: grep non-ASCII from invalid UTF-8 data with -i' '\n+\tgit grep -hi \"Æ\" invalid-0x80 >actual &&\n+\ttest_cmp expected actual &&\n+\tgit grep -hi \"(*NO_JIT)Æ\" invalid-0x80 &&\n+\ttest_cmp expected actual\n+'\n+\n test_done\n-- \n2.22.0.455.g172b71a6c5\n\n"},{"id":"379393","messageId":"20190726202642.7986-1-carenas@gmail.com","threadId":"51506","inReplyTo":"87ftms7t6s.fsf@evledraar.gmail.com","subject":"[RFC PATCH 0/2] PCRE1 cleanup","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2019-07-26T20:26:40Z","receivedAt":"2019-07-26T20:26:50Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"Sent as an RFC since it was meant to be applied against ab/pcre-jit-fixes\nbut that is likely to change with the reroll of that branch.\n\n [PATCH 1/2] grep: make sure NO_LIBPCRE1_JIT disable JIT in PCRE1\n [PATCH 2/2] grep: refactor and simplify PCRE1 support\n\nThe end result could be squashed together before merging but sent as\nindependentt changes to make review easier, with the first change doing\nthe minimum required to make PCRE1 great again.\n\n Makefile | 9 ++-------\n grep.c | 15 +++++++++------\n grep.h | 11 -----------\n 3 files changed, 11 insertions(+), 24 deletions(-)\n\nbase-commit: 0f8c4ddfdddb72dc62d76864f5d3d31f136c7129\n-- \n2.22.0\n"},{"id":"379394","messageId":"20190726202642.7986-2-carenas@gmail.com","threadId":"51506","inReplyTo":"20190726202642.7986-1-carenas@gmail.com","subject":"[RFC PATCH 1/2] grep: make sure NO_LIBPCRE1_JIT disable JIT in PCRE1","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2019-07-26T20:26:41Z","receivedAt":"2019-07-26T20:26:51Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"e87de7cab4 (\"grep: un-break building with PCRE < 8.32\", 2017-05-25)\nadded a restriction for JIT support that is no longer needed after\npcre_jit_exec() calls were removed.\n\nReorganize the definitions in grep.h so that JIT support could be\ndetected early and NO_LIBPCRE1_JIT could be used reliably to enforce\nJIT doesn't get used.\n\nSigned-off-by: Carlo Marcelo Arenas Belón \n---\n Makefile | 9 ++-------\n grep.h | 4 +---\n 2 files changed, 3 insertions(+), 10 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 11ccea4071..7e0e6cc129 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -34,13 +34,8 @@ all::\n # library. Support for version 1 will likely be removed in some future\n # release of Git, as upstream has all but abandoned it.\n #\n-# When using USE_LIBPCRE1, define NO_LIBPCRE1_JIT if the PCRE v1\n-# library is compiled without --enable-jit. We will auto-detect\n-# whether the version of the PCRE v1 library in use has JIT support at\n-# all, but we unfortunately can't auto-detect whether JIT support\n-# hasn't been compiled in in an otherwise JIT-supporting version. If\n-# you have link-time errors about a missing `pcre_jit_exec` define\n-# this, or recompile PCRE v1 with --enable-jit.\n+# When using USE_LIBPCRE1, define NO_LIBPCRE1_JIT if you want to\n+# disable JIT even if supported by your library.\n #\n # Define LIBPCREDIR=/foo/bar if your PCRE header and library files are\n # in /foo/bar/include and /foo/bar/lib directories. Which version of\ndiff --git a/grep.h b/grep.h\nindex a405fc870c..2a74e28d94 100644\n--- a/grep.h\n+++ b/grep.h\n@@ -3,14 +3,12 @@\n #include \"color.h\"\n #ifdef USE_LIBPCRE1\n #include \n-#ifdef PCRE_CONFIG_JIT\n-#if PCRE_MAJOR >= 8 && PCRE_MINOR >= 32\n #ifndef NO_LIBPCRE1_JIT\n+#ifdef PCRE_CONFIG_JIT\n #define GIT_PCRE1_USE_JIT\n #define GIT_PCRE_STUDY_JIT_COMPILE PCRE_STUDY_JIT_COMPILE\n #endif\n #endif\n-#endif\n #ifndef GIT_PCRE_STUDY_JIT_COMPILE\n #define GIT_PCRE_STUDY_JIT_COMPILE 0\n #endif\n-- \n2.22.0\n"},{"id":"379395","messageId":"20190726202642.7986-3-carenas@gmail.com","threadId":"51506","inReplyTo":"20190726202642.7986-1-carenas@gmail.com","subject":"[RFC PATCH 2/2] grep: refactor and simplify PCRE1 support","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2019-07-26T20:26:42Z","receivedAt":"2019-07-26T20:26:55Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"The code used both a macro and a variable to keep track if JIT\nsupport was desired and relied on the fact that a non JIT\nenabled library will ignore a request for JIT compilation\n(as defined by the second parameter of the call to pcre_study)\n\nCleanup the multiple levels of macros used and call pcre_study\nwith the right parameter after JIT support has been confirmed\nand unless it was requested to be disabled with NO_LIBPCRE1_JIT\n\nSigned-off-by: Carlo Marcelo Arenas Belón \n---\n grep.c | 15 +++++++++------\n grep.h | 9 ---------\n 2 files changed, 9 insertions(+), 15 deletions(-)\n\ndiff --git a/grep.c b/grep.c\nindex 6b52fed53a..599765c5c1 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -386,6 +386,7 @@ static void compile_pcre1_regexp(struct grep_pat *p, const struct grep_opt *opt)\n \tconst char *error;\n \tint erroffset;\n \tint options = PCRE_MULTILINE;\n+\tint study_options = 0;\n \n \tif (opt->ignore_case) {\n \t\tif (has_non_ascii(p->pattern))\n@@ -400,13 +401,15 @@ static void compile_pcre1_regexp(struct grep_pat *p, const struct grep_opt *opt)\n \tif (!p->pcre1_regexp)\n \t\tcompile_regexp_failed(p, error);\n \n-\tp->pcre1_extra_info = pcre_study(p->pcre1_regexp, GIT_PCRE_STUDY_JIT_COMPILE, &error);\n-\tif (!p->pcre1_extra_info && error)\n-\t\tdie(\"%s\", error);\n-\n-#ifdef GIT_PCRE1_USE_JIT\n+#if defined(PCRE_CONFIG_JIT) && !defined(NO_LIBPCRE1_JIT)\n \tpcre_config(PCRE_CONFIG_JIT, &p->pcre1_jit_on);\n+\tif (p->pcre1_jit_on)\n+\t\tstudy_options = PCRE_STUDY_JIT_COMPILE;\n #endif\n+\n+\tp->pcre1_extra_info = pcre_study(p->pcre1_regexp, study_options, &error);\n+\tif (!p->pcre1_extra_info && error)\n+\t\tdie(\"%s\", error);\n }\n \n static int pcre1match(struct grep_pat *p, const char *line, const char *eol,\n@@ -435,7 +438,7 @@ static int pcre1match(struct grep_pat *p, const char *line, const char *eol,\n static void free_pcre1_regexp(struct grep_pat *p)\n {\n \tpcre_free(p->pcre1_regexp);\n-#ifdef GIT_PCRE1_USE_JIT\n+#ifdef PCRE_CONFIG_JIT\n \tif (p->pcre1_jit_on)\n \t\tpcre_free_study(p->pcre1_extra_info);\n \telse\ndiff --git a/grep.h b/grep.h\nindex 2a74e28d94..30f2503121 100644\n--- a/grep.h\n+++ b/grep.h\n@@ -3,15 +3,6 @@\n #include \"color.h\"\n #ifdef USE_LIBPCRE1\n #include \n-#ifndef NO_LIBPCRE1_JIT\n-#ifdef PCRE_CONFIG_JIT\n-#define GIT_PCRE1_USE_JIT\n-#define GIT_PCRE_STUDY_JIT_COMPILE PCRE_STUDY_JIT_COMPILE\n-#endif\n-#endif\n-#ifndef GIT_PCRE_STUDY_JIT_COMPILE\n-#define GIT_PCRE_STUDY_JIT_COMPILE 0\n-#endif\n #else\n typedef int pcre;\n typedef int pcre_extra;\n-- \n2.22.0\n"},{"id":"379396","messageId":"xmqqpnlwmthc.fsf@gitster-ct.c.googlers.com","threadId":"51506","inReplyTo":"20190726150818.6373-1-avarab@gmail.com","subject":"Re: [PATCH v2 0/8] grep: PCRE JIT fixes + ab/no-kwset fix","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-07-26T20:27:43Z","receivedAt":"2019-07-26T20:27:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> 1-3 here are a re-roll on \"next\". I figured that was easier for\n> everyone with the state of the in-flight patches, it certainly was for\n> me. Sorry Junio if this creates a mess for you.\n\nAs long as I can just apply all of them on top of no-kwset and keep\nit a single topic, it wouldn't be too much of a hassle.\n\n> 4-8 are a \"fix\" for the UTF-8 matching error noted in Carlo's \"grep:\n> skip UTF8 checks explicitally\" in\n> https://public-inbox.org/git/20190721183115.14985-1-carenas@gmail.com/\n>\n> As noted the bug isn't fully fixed until 8/8, and that patch relies on\n> unreleased PCRE v2 code. I'm hoping that with 7/8 we're in a good\n> enough state to limp forward as noted in the rationale of those\n> commits.\n\nYikes. Perhaps we should kick the no-kwset thing out of 'next' and\nstart from scratch? It does not sound that the world is ready yet.\n\nBut that is just a knee-jerk reaction before reading the actual\npatches. Let's see how they look ;-)\n\nThanks.\n\n> Ævar Arnfjörð Bjarmason (8):\n> grep: remove overly paranoid BUG(...) code\n> grep: stop \"using\" a custom JIT stack with PCRE v2\n> grep: stop using a custom JIT stack with PCRE v1\n> grep: consistently use \"p->fixed\" in compile_regexp()\n> grep: create a \"is_fixed\" member in \"grep_pat\"\n> grep: stess test PCRE v2 on invalid UTF-8 data\n> grep: do not enter PCRE2_UTF mode on fixed matching\n> grep: optimistically use PCRE2_MATCH_INVALID_UTF\n>\n> Makefile | 1 +\n> grep.c | 68 +++++++++++----------------------\n> grep.h | 13 ++-----\n> t/helper/test-pcre2-config.c | 12 ++++++\n> t/helper/test-tool.c | 1 +\n> t/helper/test-tool.h | 1 +\n> t/t7812-grep-icase-non-ascii.sh | 39 +++++++++++++++++++\n> 7 files changed, 80 insertions(+), 55 deletions(-)\n> create mode 100644 t/helper/test-pcre2-config.c\n"},{"id":"379398","messageId":"xmqqlfwkmt62.fsf@gitster-ct.c.googlers.com","threadId":"51506","inReplyTo":"20190726150818.6373-7-avarab@gmail.com","subject":"Re: [PATCH v2 6/8] grep: stess test PCRE v2 on invalid UTF-8 data","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-07-26T20:34:29Z","receivedAt":"2019-07-26T20:34:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> diff --git a/grep.c b/grep.c\n> index 6d60e2e557..5bc0f4f32a 100644\n> --- a/grep.c\n> +++ b/grep.c\n> @@ -615,6 +615,16 @@ static void compile_regexp(struct grep_pat *p, struct grep_opt *opt)\n> \t\tdie(_(\"given pattern contains NULL byte (via -f ). This is only supported with -P under PCRE v2\"));\n> \n> \tp->is_fixed = is_fixed(p->pattern, p->patternlen);\n> +#ifdef USE_LIBPCRE2\n> + if (!p->fixed && !p->is_fixed) {\n> +\t const char *no_jit = \"(*NO_JIT)\";\n> +\t const int no_jit_len = strlen(no_jit);\n> +\t if (starts_with(p->pattern, no_jit) &&\n> +\t\t is_fixed(p->pattern + no_jit_len,\n> +\t\t\t p->patternlen - no_jit_len))\n> +\t\t p->is_fixed = 1;\n\nIt is unfortunate that is_fixed() takes a counted string.\nOtherwise, using skip_prefix() to avoid \"+no_jit_len\" would have\nmade it much easier to read. i.e.\n\n\t/* an illustration that does not quite work */\n\tchar *pattern_body;\n\tif (skip_prefix(p->pattern, \"(*NO_JIT)\", &pattern_body) &&\n is_fixed(pattern_body))\n\t\tp->is_fixed = 1;\n\t\n> +test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: setup invalid UTF-8 data' '\n> +\tprintf \"\\\\200\\\\n\" >invalid-0x80 &&\n> +\techo \"ævar\" >expected &&\n> +\tcat expected >>invalid-0x80 &&\n> +\tgit add invalid-0x80\n> +'\n> +\n> +test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: grep ASCII from invalid UTF-8 data' '\n> +\tgit grep -h \"var\" invalid-0x80 >actual &&\n> +\ttest_cmp expected actual &&\n> +\tgit grep -h \"(*NO_JIT)var\" invalid-0x80 >actual &&\n> +\ttest_cmp expected actual\n> +'\n> +\n> +test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: grep non-ASCII from invalid UTF-8 data' '\n> +\ttest_might_fail git grep -h \"æ\" invalid-0x80 >actual &&\n> +\ttest_cmp expected actual &&\n> +\ttest_must_fail git grep -h \"(*NO_JIT)æ\" invalid-0x80 &&\n> +\ttest_cmp expected actual\n> +'\n> +\n> +test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: grep non-ASCII from invalid UTF-8 data with -i' '\n> +\ttest_might_fail git grep -hi \"Æ\" invalid-0x80 >actual &&\n> +\ttest_cmp expected actual &&\n> +\ttest_must_fail git grep -hi \"(*NO_JIT)Æ\" invalid-0x80 &&\n> +\ttest_cmp expected actual\n> +'\n> +\n> test_done\n"},{"id":"379399","messageId":"xmqqh878mt2c.fsf@gitster-ct.c.googlers.com","threadId":"51506","inReplyTo":"20190726150818.6373-8-avarab@gmail.com","subject":"Re: [PATCH v2 7/8] grep: do not enter PCRE2_UTF mode on fixed matching","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-07-26T20:36:43Z","receivedAt":"2019-07-26T20:36:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> When grepping a non-ASCII fixed string. This is a more general problem\n> that's hard to fix, but we can at least fix the most common case of\n> grepping for a fixed string without \"-i\". I can't think of a reason\n> for why we'd turn on PCRE2_UTF when matching byte-for-byte like that.\n\nYes, exactly. That's quite a sane and minimum fix/workaround, I\nwould think.\n\n> test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: grep non-ASCII from invalid UTF-8 data' '\n> -\ttest_might_fail git grep -h \"æ\" invalid-0x80 >actual &&\n> +\tgit grep -h \"æ\" invalid-0x80 >actual &&\n> \ttest_cmp expected actual &&\n> -\ttest_must_fail git grep -h \"(*NO_JIT)æ\" invalid-0x80 &&\n> +\tgit grep -h \"(*NO_JIT)æ\" invalid-0x80 &&\n> \ttest_cmp expected actual\n> '\n"},{"id":"379401","messageId":"xmqqd0hwmrng.fsf@gitster-ct.c.googlers.com","threadId":"51506","inReplyTo":"20190726150818.6373-9-avarab@gmail.com","subject":"Re: [PATCH v2 8/8] grep: optimistically use PCRE2_MATCH_INVALID_UTF","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-07-26T21:07:15Z","receivedAt":"2019-07-26T21:07:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> diff --git a/Makefile b/Makefile\n> index bd246f2989..dd38d5e527 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -726,6 +726,7 @@ TEST_BUILTINS_OBJS += test-oidmap.o\n> TEST_BUILTINS_OBJS += test-online-cpus.o\n> TEST_BUILTINS_OBJS += test-parse-options.o\n> TEST_BUILTINS_OBJS += test-path-utils.o\n> +TEST_BUILTINS_OBJS += test-pcre2-config.o\n\nThis won't even build with any released pcre version; shouldn't we\nmake it at least conditionally compiled code? Specifically...\n\n> TEST_BUILTINS_OBJS += test-pkt-line.o\n> TEST_BUILTINS_OBJS += test-prio-queue.o\n> TEST_BUILTINS_OBJS += test-reach.o\n> diff --git a/grep.c b/grep.c\n> index c7c06ae08d..8b8b9efe12 100644\n> --- a/grep.c\n> +++ b/grep.c\n> @@ -474,7 +474,7 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt\n> \t}\n> \tif (!opt->ignore_locale && is_utf8_locale() && has_non_ascii(p->pattern) &&\n> \t !(!opt->ignore_case && (p->fixed || p->is_fixed)))\n> -\t\toptions |= PCRE2_UTF;\n> +\t\toptions |= (PCRE2_UTF | PCRE2_MATCH_INVALID_UTF);\n> \n> \tp->pcre2_pattern = pcre2_compile((PCRE2_SPTR)p->pattern,\n> \t\t\t\t\t p->patternlen, options, &error, &erroffset,\n> diff --git a/grep.h b/grep.h\n> index c0c71eb4a9..506f05b97b 100644\n> --- a/grep.h\n> +++ b/grep.h\n> @@ -21,6 +21,9 @@ typedef int pcre_extra;\n> #ifdef USE_LIBPCRE2\n> #define PCRE2_CODE_UNIT_WIDTH 8\n> #include \n> +#ifndef PCRE2_MATCH_INVALID_UTF\n> +#define PCRE2_MATCH_INVALID_UTF 0\n> +#endif\n\n... unlike this piece of code ...\n\n> #else\n> typedef int pcre2_code;\n> typedef int pcre2_match_data;\n> diff --git a/t/helper/test-pcre2-config.c b/t/helper/test-pcre2-config.c\n> new file mode 100644\n> index 0000000000..5258fdddba\n> --- /dev/null\n> +++ b/t/helper/test-pcre2-config.c\n> @@ -0,0 +1,12 @@\n> +#include \"test-tool.h\"\n> +#include \"cache.h\"\n> +#include \"grep.h\"\n> +\n> +int cmd__pcre2_config(int argc, const char **argv)\n> +{\n> +\tif (argc == 2 && !strcmp(argv[1], \"has-PCRE2_MATCH_INVALID_UTF\")) {\n> +\t\tint value = PCRE2_MATCH_INVALID_UTF;\n\n... this part does not have any fallback definition.\n"},{"id":"379406","messageId":"87a7d0799h.fsf@evledraar.gmail.com","threadId":"51506","inReplyTo":"xmqqd0hwmrng.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 8/8] grep: optimistically use PCRE2_MATCH_INVALID_UTF","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-07-26T21:53:30Z","receivedAt":"2019-07-26T21:53:35Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Jul 26 2019, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> diff --git a/Makefile b/Makefile\n>> index bd246f2989..dd38d5e527 100644\n>> --- a/Makefile\n>> +++ b/Makefile\n>> @@ -726,6 +726,7 @@ TEST_BUILTINS_OBJS += test-oidmap.o\n>> TEST_BUILTINS_OBJS += test-online-cpus.o\n>> TEST_BUILTINS_OBJS += test-parse-options.o\n>> TEST_BUILTINS_OBJS += test-path-utils.o\n>> +TEST_BUILTINS_OBJS += test-pcre2-config.o\n>\n> This won't even build with any released pcre version; shouldn't we\n> make it at least conditionally compiled code? Specifically...\n>\n>> TEST_BUILTINS_OBJS += test-pkt-line.o\n>> TEST_BUILTINS_OBJS += test-prio-queue.o\n>> TEST_BUILTINS_OBJS += test-reach.o\n>> diff --git a/grep.c b/grep.c\n>> index c7c06ae08d..8b8b9efe12 100644\n>> --- a/grep.c\n>> +++ b/grep.c\n>> @@ -474,7 +474,7 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt\n>> \t}\n>> \tif (!opt->ignore_locale && is_utf8_locale() && has_non_ascii(p->pattern) &&\n>> \t !(!opt->ignore_case && (p->fixed || p->is_fixed)))\n>> -\t\toptions |= PCRE2_UTF;\n>> +\t\toptions |= (PCRE2_UTF | PCRE2_MATCH_INVALID_UTF);\n>>\n>> \tp->pcre2_pattern = pcre2_compile((PCRE2_SPTR)p->pattern,\n>> \t\t\t\t\t p->patternlen, options, &error, &erroffset,\n>> diff --git a/grep.h b/grep.h\n>> index c0c71eb4a9..506f05b97b 100644\n>> --- a/grep.h\n>> +++ b/grep.h\n>> @@ -21,6 +21,9 @@ typedef int pcre_extra;\n>> #ifdef USE_LIBPCRE2\n>> #define PCRE2_CODE_UNIT_WIDTH 8\n>> #include \n>> +#ifndef PCRE2_MATCH_INVALID_UTF\n>> +#define PCRE2_MATCH_INVALID_UTF 0\n>> +#endif\n>\n> ... unlike this piece of code ...\n>\n>> #else\n>> typedef int pcre2_code;\n>> typedef int pcre2_match_data;\n>> diff --git a/t/helper/test-pcre2-config.c b/t/helper/test-pcre2-config.c\n>> new file mode 100644\n>> index 0000000000..5258fdddba\n>> --- /dev/null\n>> +++ b/t/helper/test-pcre2-config.c\n>> @@ -0,0 +1,12 @@\n>> +#include \"test-tool.h\"\n>> +#include \"cache.h\"\n>> +#include \"grep.h\"\n>> +\n>> +int cmd__pcre2_config(int argc, const char **argv)\n>> +{\n>> +\tif (argc == 2 && !strcmp(argv[1], \"has-PCRE2_MATCH_INVALID_UTF\")) {\n>> +\t\tint value = PCRE2_MATCH_INVALID_UTF;\n>\n> ... this part does not have any fallback definition.\n\nIt works because we include grep.h, which'll define\nPCRE2_MATCH_INVALID_UTF=0 if pcre2.h doesn't give it to us. I've tested\nthis on PCRE versions with/without PCRE2_MATCH_INVALID_UTF and it works\n& runs/skips the appropriate tests.\n"},{"id":"379407","messageId":"878ssk795x.fsf@evledraar.gmail.com","threadId":"51506","inReplyTo":"xmqqlfwkmt62.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 6/8] grep: stess test PCRE v2 on invalid UTF-8 data","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-07-26T21:55:38Z","receivedAt":"2019-07-26T21:55:42Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Jul 26 2019, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> diff --git a/grep.c b/grep.c\n>> index 6d60e2e557..5bc0f4f32a 100644\n>> --- a/grep.c\n>> +++ b/grep.c\n>> @@ -615,6 +615,16 @@ static void compile_regexp(struct grep_pat *p, struct grep_opt *opt)\n>> \t\tdie(_(\"given pattern contains NULL byte (via -f ). This is only supported with -P under PCRE v2\"));\n>>\n>> \tp->is_fixed = is_fixed(p->pattern, p->patternlen);\n>> +#ifdef USE_LIBPCRE2\n>> + if (!p->fixed && !p->is_fixed) {\n>> +\t const char *no_jit = \"(*NO_JIT)\";\n>> +\t const int no_jit_len = strlen(no_jit);\n>> +\t if (starts_with(p->pattern, no_jit) &&\n>> +\t\t is_fixed(p->pattern + no_jit_len,\n>> +\t\t\t p->patternlen - no_jit_len))\n>> +\t\t p->is_fixed = 1;\n>\n> It is unfortunate that is_fixed() takes a counted string.\n> Otherwise, using skip_prefix() to avoid \"+no_jit_len\" would have\n> made it much easier to read. i.e.\n>\n> \t/* an illustration that does not quite work */\n> \tchar *pattern_body;\n> \tif (skip_prefix(p->pattern, \"(*NO_JIT)\", &pattern_body) &&\n> is_fixed(pattern_body))\n> \t\tp->is_fixed = 1;\n\nIndeed, but then we couldn't use this for patterns that have NUL in\nthem, which we otherwise support (and support here). So I think it's\nworth keeping it so it takes ptr+len.\n\n>> +test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: setup invalid UTF-8 data' '\n>> +\tprintf \"\\\\200\\\\n\" >invalid-0x80 &&\n>> +\techo \"ævar\" >expected &&\n>> +\tcat expected >>invalid-0x80 &&\n>> +\tgit add invalid-0x80\n>> +'\n>> +\n>> +test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: grep ASCII from invalid UTF-8 data' '\n>> +\tgit grep -h \"var\" invalid-0x80 >actual &&\n>> +\ttest_cmp expected actual &&\n>> +\tgit grep -h \"(*NO_JIT)var\" invalid-0x80 >actual &&\n>> +\ttest_cmp expected actual\n>> +'\n>> +\n>> +test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: grep non-ASCII from invalid UTF-8 data' '\n>> +\ttest_might_fail git grep -h \"æ\" invalid-0x80 >actual &&\n>> +\ttest_cmp expected actual &&\n>> +\ttest_must_fail git grep -h \"(*NO_JIT)æ\" invalid-0x80 &&\n>> +\ttest_cmp expected actual\n>> +'\n>> +\n>> +test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: grep non-ASCII from invalid UTF-8 data with -i' '\n>> +\ttest_might_fail git grep -hi \"Æ\" invalid-0x80 >actual &&\n>> +\ttest_cmp expected actual &&\n>> +\ttest_must_fail git grep -hi \"(*NO_JIT)Æ\" invalid-0x80 &&\n>> +\ttest_cmp expected actual\n>> +'\n>> +\n>> test_done\n"},{"id":"379408","messageId":"877e84792k.fsf@evledraar.gmail.com","threadId":"51506","inReplyTo":"87a7d0799h.fsf@evledraar.gmail.com","subject":"Re: [PATCH v2 8/8] grep: optimistically use PCRE2_MATCH_INVALID_UTF","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-07-26T21:57:39Z","receivedAt":"2019-07-26T21:57:43Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Jul 26 2019, Ævar Arnfjörð Bjarmason wrote:\n\n> On Fri, Jul 26 2019, Junio C Hamano wrote:\n>\n>> Ævar Arnfjörð Bjarmason writes:\n>>\n>>> diff --git a/Makefile b/Makefile\n>>> index bd246f2989..dd38d5e527 100644\n>>> --- a/Makefile\n>>> +++ b/Makefile\n>>> @@ -726,6 +726,7 @@ TEST_BUILTINS_OBJS += test-oidmap.o\n>>> TEST_BUILTINS_OBJS += test-online-cpus.o\n>>> TEST_BUILTINS_OBJS += test-parse-options.o\n>>> TEST_BUILTINS_OBJS += test-path-utils.o\n>>> +TEST_BUILTINS_OBJS += test-pcre2-config.o\n>>\n>> This won't even build with any released pcre version; shouldn't we\n>> make it at least conditionally compiled code? Specifically...\n>>\n>>> TEST_BUILTINS_OBJS += test-pkt-line.o\n>>> TEST_BUILTINS_OBJS += test-prio-queue.o\n>>> TEST_BUILTINS_OBJS += test-reach.o\n>>> diff --git a/grep.c b/grep.c\n>>> index c7c06ae08d..8b8b9efe12 100644\n>>> --- a/grep.c\n>>> +++ b/grep.c\n>>> @@ -474,7 +474,7 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt\n>>> \t}\n>>> \tif (!opt->ignore_locale && is_utf8_locale() && has_non_ascii(p->pattern) &&\n>>> \t !(!opt->ignore_case && (p->fixed || p->is_fixed)))\n>>> -\t\toptions |= PCRE2_UTF;\n>>> +\t\toptions |= (PCRE2_UTF | PCRE2_MATCH_INVALID_UTF);\n>>>\n>>> \tp->pcre2_pattern = pcre2_compile((PCRE2_SPTR)p->pattern,\n>>> \t\t\t\t\t p->patternlen, options, &error, &erroffset,\n>>> diff --git a/grep.h b/grep.h\n>>> index c0c71eb4a9..506f05b97b 100644\n>>> --- a/grep.h\n>>> +++ b/grep.h\n>>> @@ -21,6 +21,9 @@ typedef int pcre_extra;\n>>> #ifdef USE_LIBPCRE2\n>>> #define PCRE2_CODE_UNIT_WIDTH 8\n>>> #include \n>>> +#ifndef PCRE2_MATCH_INVALID_UTF\n>>> +#define PCRE2_MATCH_INVALID_UTF 0\n>>> +#endif\n>>\n>> ... unlike this piece of code ...\n>>\n>>> #else\n>>> typedef int pcre2_code;\n>>> typedef int pcre2_match_data;\n>>> diff --git a/t/helper/test-pcre2-config.c b/t/helper/test-pcre2-config.c\n>>> new file mode 100644\n>>> index 0000000000..5258fdddba\n>>> --- /dev/null\n>>> +++ b/t/helper/test-pcre2-config.c\n>>> @@ -0,0 +1,12 @@\n>>> +#include \"test-tool.h\"\n>>> +#include \"cache.h\"\n>>> +#include \"grep.h\"\n>>> +\n>>> +int cmd__pcre2_config(int argc, const char **argv)\n>>> +{\n>>> +\tif (argc == 2 && !strcmp(argv[1], \"has-PCRE2_MATCH_INVALID_UTF\")) {\n>>> +\t\tint value = PCRE2_MATCH_INVALID_UTF;\n>>\n>> ... this part does not have any fallback definition.\n>\n> It works because we include grep.h, which'll define\n> PCRE2_MATCH_INVALID_UTF=0 if pcre2.h doesn't give it to us. I've tested\n> this on PCRE versions with/without PCRE2_MATCH_INVALID_UTF and it works\n> & runs/skips the appropriate tests.\n\nAh, I spoke too soon, of course that's all guarded by \"are we using PCRE\nv2 in general?\". I'll fix it...\n"},{"id":"379458","messageId":"CAPUEspgUiicBPKPZPyDFryj3OmtyOWVgrytqpzMq-PZNv1f1Mg@mail.gmail.com","threadId":"51506","inReplyTo":"20190726150818.6373-3-avarab@gmail.com","subject":"Re: [PATCH v2 2/8] grep: stop \"using\" a custom JIT stack with PCRE v2","fromName":"Carlo Arenas","fromEmail":"carenas@gmail.com","sentAt":"2019-07-29T00:33:47Z","receivedAt":"2019-07-29T00:34:00Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"On Fri, Jul 26, 2019 at 8:08 AM Ævar Arnfjörð Bjarmason\n wrote:\n>\n> As noted in [3] there are known regexes that will fail with the lower\n> stack limit, the way GNU grep fixed it is interesting, although I\n> believe the implementation is overly verbose, they could make PCRE v2\n> handle that gradual re-allocation, that's what min/max memory is\n> for.\n\nThe part I liked about the grep implementation was how they went above\nand beyond to make sure there was no abstraction leak and at the end\nthe end user doesn't even see a PCRE error message.\n\nPresume thought that the end user we have is different, and might make\nsense to expose them to the underlying mechanism, but in that case we\nshould also provide them with knobs to tweak (like the one I proposed\nto disable jit, and that in this case might be to set a stacksize)\n\n> So we might end up bringing this back, I'm more inclined to just kick\n> such cases upstairs to PCRE maintainers as a bug, perhaps they'll add\n> some overall \"just allocate more then\" flag to make this easier. In\n> any case there's no functional change here, we didn't have a custom\n> stack, so let's apply this first, we can always revert it later.\n\nagree, LGTM other than by the comment below\n\n> diff --git a/grep.c b/grep.c\n> index 95af88cb74..4b1e917ac5 100644\n> --- a/grep.c\n> +++ b/grep.c\n> @@ -534,14 +534,6 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt\n> p->pcre2_jit_on = 0;\n> return;\n\nthis return and brackets no longer needed\n\nCarlo\n"},{"id":"379459","messageId":"CAPUEspgStVxL=0SoAg82vxRMRGLSEKdHrT-xq6nCW1sNq7nLsw@mail.gmail.com","threadId":"51506","inReplyTo":"20190726150818.6373-4-avarab@gmail.com","subject":"Re: [PATCH v2 3/8] grep: stop using a custom JIT stack with PCRE v1","fromName":"Carlo Arenas","fromEmail":"carenas@gmail.com","sentAt":"2019-07-29T01:26:17Z","receivedAt":"2019-07-29T01:26:30Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"On Fri, Jul 26, 2019 at 8:09 AM Ævar Arnfjörð Bjarmason\n wrote:\n>\n> It will also implicitly re-enable UTF-8 validation for PCRE v1. As\n> noted in [1] we now have cases as a result where PCRE v1 is more eager\n> to error out. Subsequent patches will fix that for v2, and I think\n> it's fair to tell v1 users \"just upgrade\" and not worry about that\n> edge case for v1.\n>\n> 1. https://public-inbox.org/git/CAPUEsphZJ_Uv9o1-yDpjNLA_q-f7gWXz9g1gCY2pYAYN8ri40g@mail.gmail.com/\n\nHaven't seen any responses from packagers but there was a report[1] of\nCentOS 6 users\nthat would be affected, and would certainly make things more difficult\nto whoever is behind\nthe git binary that comes with Xcode in macOS and that is linked\nagainst a system library\n(PCRE1) that is IMHO unlikely to be upgraded.\n\nThe minimum I would expect if you want to move forward with this,\nwould be to make\ntheir git not randomly die because of non UTF-8 haystack by applying\n[2] and make clear we\nwill also introduce stack size problems like the one in [3] (as it was\ndone in the previous\ncommit)\n\nFeedback on the patchset[4] that applies on top of this to make sure\nJIT can be disabled at\ncompile time and the logic is less messy also appreciated.\n\nLet me know also how you want to keep it on sync, as IMHO makes more\nsense inside\nyour branch instead of as an independent topic.\n\nCarlo\n\nCC +Brian\n\n[1] https://public-inbox.org/git/20190615191514.GD8616@genre.crustytoothpaste.net/\n[2] https://public-inbox.org/git/20190722144350.46458-1-carenas@gmail.com/\n[3] https://public-inbox.org/git/CAPUEspjj+fG8QDmf=bZXktfpLgkgiu34HTjKLhm-cmEE04FE-A@mail.gmail.com/\n[4] https://public-inbox.org/git/20190726202642.7986-1-carenas@gmail.com/\n"},{"id":"379460","messageId":"CAPUEspgay3RnLH3pdEWyktgn8XeuiKZ8PYPNB_38gyxffmh5Jw@mail.gmail.com","threadId":"51506","inReplyTo":"20190726150818.6373-5-avarab@gmail.com","subject":"Re: [PATCH v2 4/8] grep: consistently use \"p->fixed\" in compile_regexp()","fromName":"Carlo Arenas","fromEmail":"carenas@gmail.com","sentAt":"2019-07-29T01:48:10Z","receivedAt":"2019-07-29T01:48:23Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"On Fri, Jul 26, 2019 at 8:09 AM Ævar Arnfjörð Bjarmason\n wrote:\n>\n> It's less confusing to use that variable consistently that switch back\n> & forth between the two.\n>\n> Signed-off-by: Ævar Arnfjörð Bjarmason \n> ---\n> grep.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/grep.c b/grep.c\n> index 9c2b259771..b94e998680 100644\n> --- a/grep.c\n> +++ b/grep.c\n> @@ -616,7 +616,7 @@ static void compile_regexp(struct grep_pat *p, struct grep_opt *opt)\n> die(_(\"given pattern contains NULL byte (via -f ). This is only supported with -P under PCRE v2\"));\n>\n> pat_is_fixed = is_fixed(p->pattern, p->patternlen);\n> - if (opt->fixed || pat_is_fixed) {\n> + if (p->fixed || pat_is_fixed) {\n\nat the end of this series we have:\n\n if (p->fixed || p->is_fixed)\n\nwhich doesn't make sense; at least with opt->fixed it was clear that\nwhat was meant is that grep was passed -P\n\nmaybe is_fixed shouldn't exist and fixed when applied to the pattern\nmeans we had determined it was a fixed\npattern and overridden the user selection of engine.\n\nthat at least will give us a logical way to fix the pattern reported\nin [1] and that currently requires the user to know\ngit's grep internals and know he can skip the \"is_fixed\" optimization\nby doing something like :\n\n $ git grep 'foo[ ]bar'\n\nCarlo\n\n[1] https://public-inbox.org/git/20190728235427.41425-1-carenas@gmail.com/\n"},{"id":"379466","messageId":"CAPUEspiCL+ZbcOwna7XLvW3HfDg+i3bg2GcS_1Sv=VHU3aNRoA@mail.gmail.com","threadId":"51506","inReplyTo":"20190726150818.6373-7-avarab@gmail.com","subject":"Re: [PATCH v2 6/8] grep: stess test PCRE v2 on invalid UTF-8 data","fromName":"Carlo Arenas","fromEmail":"carenas@gmail.com","sentAt":"2019-07-29T03:06:49Z","receivedAt":"2019-07-29T03:07:02Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"On Fri, Jul 26, 2019 at 8:09 AM Ævar Arnfjörð Bjarmason\n wrote:\n>\n> This patch does nothing to fix that, instead we sneak in support for\n> fixed patterns starting with \"(*NO_JIT)\", this disables the PCRE v2\n> jit with implicit fixed-string matching for testing, see\n> pcre2syntax(3) the syntax.\n\nAlternativelly; using `git -c pcre.jit=false grep ...` on top of [1],\nmight be cleaner\n\nCarlo\n\n[1] https://public-inbox.org/git/20190728235427.41425-1-carenas@gmail.com/\n"},{"id":"379480","messageId":"871ry96wiq.fsf@evledraar.gmail.com","threadId":"51506","inReplyTo":"CAPUEspgay3RnLH3pdEWyktgn8XeuiKZ8PYPNB_38gyxffmh5Jw@mail.gmail.com","subject":"Re: [PATCH v2 4/8] grep: consistently use \"p->fixed\" in compile_regexp()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-07-29T09:05:33Z","receivedAt":"2019-07-29T09:05:37Z","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 Fri, Jul 26, 2019 at 8:09 AM Ævar Arnfjörð Bjarmason\n> wrote:\n>>\n>> It's less confusing to use that variable consistently that switch back\n>> & forth between the two.\n>>\n>> Signed-off-by: Ævar Arnfjörð Bjarmason \n>> ---\n>> grep.c | 2 +-\n>> 1 file changed, 1 insertion(+), 1 deletion(-)\n>>\n>> diff --git a/grep.c b/grep.c\n>> index 9c2b259771..b94e998680 100644\n>> --- a/grep.c\n>> +++ b/grep.c\n>> @@ -616,7 +616,7 @@ static void compile_regexp(struct grep_pat *p, struct grep_opt *opt)\n>> die(_(\"given pattern contains NULL byte (via -f ). This is only supported with -P under PCRE v2\"));\n>>\n>> pat_is_fixed = is_fixed(p->pattern, p->patternlen);\n>> - if (opt->fixed || pat_is_fixed) {\n>> + if (p->fixed || pat_is_fixed) {\n>\n> at the end of this series we have:\n>\n> if (p->fixed || p->is_fixed)\n>\n> which doesn't make sense; at least with opt->fixed it was clear that\n> what was meant is that grep was passed -P\n\nI assume you mean \"was passed -F...\".\n\n> maybe is_fixed shouldn't exist and fixed when applied to the pattern\n> means we had determined it was a fixed\n> pattern and overridden the user selection of engine.\n\nThey're two flags because p->fixed is \"--fixed-strings\", and p->is_fixed\nis \"there's no metachars here\". So the former case needs escaping, as\nthe code just below might do (the two aren't mutually exclusive).\n\nI don't get how you think we can always fold them into one flag, but\nmaybe I'm missing something...\n\n> that at least will give us a logical way to fix the pattern reported\n> in [1] and that currently requires the user to know\n> git's grep internals and know he can skip the \"is_fixed\" optimization\n> by doing something like :\n>\n> $ git grep 'foo[ ]bar'\n>\n> [1] https://public-inbox.org/git/20190728235427.41425-1-carenas@gmail.com/\n\nAs I noted in a reply there this seems like a way to fix a bug in \"next\"\nwith a config knob. Yes we should fix the bug, but we've had the kwset\ncode in git for years without needing this distinction, so after we work\nout the bugs I don't see why we'd need this.\n\nThe reason we ignore the user's choice here is because you might\ne.g. set grep.patternType=extended in your config, and you'd still want\ngrepping for a fixed \"foo\" to be fast.\n"},{"id":"379482","messageId":"87zhkx5hlj.fsf@evledraar.gmail.com","threadId":"51506","inReplyTo":"871ry96wiq.fsf@evledraar.gmail.com","subject":"Re: [PATCH v2 4/8] grep: consistently use \"p->fixed\" in compile_regexp()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-07-29T09:13:12Z","receivedAt":"2019-07-29T09:13:17Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Jul 29 2019, Ævar Arnfjörð Bjarmason wrote:\n\n> On Mon, Jul 29 2019, Carlo Arenas wrote:\n>\n>> On Fri, Jul 26, 2019 at 8:09 AM Ævar Arnfjörð Bjarmason\n>> wrote:\n>>>\n>>> It's less confusing to use that variable consistently that switch back\n>>> & forth between the two.\n>>>\n>>> Signed-off-by: Ævar Arnfjörð Bjarmason \n>>> ---\n>>> grep.c | 2 +-\n>>> 1 file changed, 1 insertion(+), 1 deletion(-)\n>>>\n>>> diff --git a/grep.c b/grep.c\n>>> index 9c2b259771..b94e998680 100644\n>>> --- a/grep.c\n>>> +++ b/grep.c\n>>> @@ -616,7 +616,7 @@ static void compile_regexp(struct grep_pat *p, struct grep_opt *opt)\n>>> die(_(\"given pattern contains NULL byte (via -f ). This is only supported with -P under PCRE v2\"));\n>>>\n>>> pat_is_fixed = is_fixed(p->pattern, p->patternlen);\n>>> - if (opt->fixed || pat_is_fixed) {\n>>> + if (p->fixed || pat_is_fixed) {\n>>\n>> at the end of this series we have:\n>>\n>> if (p->fixed || p->is_fixed)\n>>\n>> which doesn't make sense; at least with opt->fixed it was clear that\n>> what was meant is that grep was passed -P\n>\n> I assume you mean \"was passed -F...\".\n>\n>> maybe is_fixed shouldn't exist and fixed when applied to the pattern\n>> means we had determined it was a fixed\n>> pattern and overridden the user selection of engine.\n>\n> They're two flags because p->fixed is \"--fixed-strings\", and p->is_fixed\n> is \"there's no metachars here\". So the former case needs escaping, as\n> the code just below might do (the two aren't mutually exclusive).\n>\n> I don't get how you think we can always fold them into one flag, but\n> maybe I'm missing something...\n>\n>> that at least will give us a logical way to fix the pattern reported\n>> in [1] and that currently requires the user to know\n>> git's grep internals and know he can skip the \"is_fixed\" optimization\n>> by doing something like :\n>>\n>> $ git grep 'foo[ ]bar'\n>>\n>> [1] https://public-inbox.org/git/20190728235427.41425-1-carenas@gmail.com/\n>\n> As I noted in a reply there this seems like a way to fix a bug in \"next\"\n> with a config knob. Yes we should fix the bug, but we've had the kwset\n> code in git for years without needing this distinction, so after we work\n> out the bugs I don't see why we'd need this.\n>\n> The reason we ignore the user's choice here is because you might\n> e.g. set grep.patternType=extended in your config, and you'd still want\n> grepping for a fixed \"foo\" to be fast.\n\n...and more generally, for any future sanity of implementation and\nmaintenance I think we should only make the promise that we support\ncertain syntax & semantics, not that the -F, -G, -E, -P options are\nguaranteed to dispatch to a given codepath.\n\nInternally we should be free to switch between those, so e.g. if a\npattern is fixed and you configure \"basic\" regexp, but we know your C\nlibrary is faster for those matches with REG_EXTENDED we should just\npass that regardless of -G or -E.\n\nOf course that means we *must* expose the same semantics (to some\nreasonable extent), which means I have a lot of bugs in \"next\" to\naddress.\n\nI'm just saying that the presence of those bugs means we should be\ninclined to fix them / back out certain changes, not work around them\nwith user-servicable knobs.\n"},{"id":"379483","messageId":"87y30h5h8v.fsf@evledraar.gmail.com","threadId":"51506","inReplyTo":"xmqqpnlwmthc.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 0/8] grep: PCRE JIT fixes + ab/no-kwset fix","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-07-29T09:20:48Z","receivedAt":"2019-07-29T09:20:52Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Jul 26 2019, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>> 1-3 here are a re-roll on \"next\". I figured that was easier for\n>> everyone with the state of the in-flight patches, it certainly was for\n>> me. Sorry Junio if this creates a mess for you.\n>\n> As long as I can just apply all of them on top of no-kwset and keep\n> it a single topic, it wouldn't be too much of a hassle.\n>\n>> 4-8 are a \"fix\" for the UTF-8 matching error noted in Carlo's \"grep:\n>> skip UTF8 checks explicitally\" in\n>> https://public-inbox.org/git/20190721183115.14985-1-carenas@gmail.com/\n>>\n>> As noted the bug isn't fully fixed until 8/8, and that patch relies on\n>> unreleased PCRE v2 code. I'm hoping that with 7/8 we're in a good\n>> enough state to limp forward as noted in the rationale of those\n>> commits.\n>\n> Yikes. Perhaps we should kick the no-kwset thing out of 'next' and\n> start from scratch? It does not sound that the world is ready yet.\n\nI have some fix-for-the-fix and was going to submit a v3 of this series,\nbut I think the more responsible thing to do at this point, especially\nwith various patches from Carlo that need to be integrated in one way or\nanother, is to back it out until the outstanding issues are addressed.\n\nIf it's not too much trouble, would you mind reverting just the two\npatches at the tip of ab/no-kwset in \"next\"? I.e.\n\n b65abcafc7 (\"grep: use PCRE v2 for optimized fixed-string search\", 2019-07-01)\n 48de2a768c (\"grep: remove the kwset optimization\", 2019-07-01)\n\nI believe the rest are all settled & haven't had any issues raised with\nthem, and those tests & preparatory fixes would be very useful to have\nin \"master\" for any re-roll without needing to be distracted by those\nchanges.\n\n> But that is just a knee-jerk reaction before reading the actual\n> patches. Let's see how they look ;-)\n>\n> Thanks.\n>\n>> Ævar Arnfjörð Bjarmason (8):\n>> grep: remove overly paranoid BUG(...) code\n>> grep: stop \"using\" a custom JIT stack with PCRE v2\n>> grep: stop using a custom JIT stack with PCRE v1\n>> grep: consistently use \"p->fixed\" in compile_regexp()\n>> grep: create a \"is_fixed\" member in \"grep_pat\"\n>> grep: stess test PCRE v2 on invalid UTF-8 data\n>> grep: do not enter PCRE2_UTF mode on fixed matching\n>> grep: optimistically use PCRE2_MATCH_INVALID_UTF\n>>\n>> Makefile | 1 +\n>> grep.c | 68 +++++++++++----------------------\n>> grep.h | 13 ++-----\n>> t/helper/test-pcre2-config.c | 12 ++++++\n>> t/helper/test-tool.c | 1 +\n>> t/helper/test-tool.h | 1 +\n>> t/t7812-grep-icase-non-ascii.sh | 39 +++++++++++++++++++\n>> 7 files changed, 80 insertions(+), 55 deletions(-)\n>> create mode 100644 t/helper/test-pcre2-config.c\n"},{"id":"379509","messageId":"xmqqpnlsvmye.fsf@gitster-ct.c.googlers.com","threadId":"51506","inReplyTo":"87y30h5h8v.fsf@evledraar.gmail.com","subject":"Re: [PATCH v2 0/8] grep: PCRE JIT fixes + ab/no-kwset fix","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-07-29T16:12:57Z","receivedAt":"2019-07-29T16:13:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> I have some fix-for-the-fix and was going to submit a v3 of this series,\n> but I think the more responsible thing to do at this point, especially\n> with various patches from Carlo that need to be integrated in one way or\n> another, is to back it out until the outstanding issues are addressed.\n>\n> If it's not too much trouble, would you mind reverting just the two\n> patches at the tip of ab/no-kwset in \"next\"? I.e.\n>\n> b65abcafc7 (\"grep: use PCRE v2 for optimized fixed-string search\", 2019-07-01)\n> 48de2a768c (\"grep: remove the kwset optimization\", 2019-07-01)\n\nAs we'd be in pre-release freeze soonish, let me revert the merge to\n'next' and drop these two tip commits from the topic branch (keeping\nthe earlier parts), and ask you two to work together to get the\npcre-jit stuff into a good shape.\n\nThanks.\n"},{"id":"379510","messageId":"xmqqlfwgvmh3.fsf@gitster-ct.c.googlers.com","threadId":"51506","inReplyTo":"87zhkx5hlj.fsf@evledraar.gmail.com","subject":"Re: [PATCH v2 4/8] grep: consistently use \"p->fixed\" in compile_regexp()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-07-29T16:23:20Z","receivedAt":"2019-07-29T16:23:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n>> The reason we ignore the user's choice here is because you might\n>> e.g. set grep.patternType=extended in your config, and you'd still want\n>> grepping for a fixed \"foo\" to be fast.\n>\n> ...and more generally, for any future sanity of implementation and\n> maintenance I think we should only make the promise that we support\n> certain syntax & semantics, not that the -F, -G, -E, -P options are\n> guaranteed to dispatch to a given codepath.\n>\n> Internally we should be free to switch between those, so e.g. if a\n> pattern is fixed and you configure \"basic\" regexp, but we know your C\n> library is faster for those matches with REG_EXTENDED we should just\n> pass that regardless of -G or -E.\n\nThat certainly is a very sensible goal.\n\n> I'm just saying that the presence of those bugs means we should be\n> inclined to fix them / back out certain changes, not work around them\n> with user-servicable knobs.\n\nAmen.\n"},{"id":"387109","messageId":"87blsyl32c.fsf_-_@igel.home","threadId":"51506","inReplyTo":"20190726150818.6373-7-avarab@gmail.com","subject":"[PATCH] t7812: add missing redirects","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2019-11-26T21:50:51Z","receivedAt":"2019-11-26T21:50:58Z","isPatch":true,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"Two tests in t7812 were missing redirects, failing to actually test the\nproduced output.\n\nFixes: 8a5999838e (\"grep: stess test PCRE v2 on invalid UTF-8 data\")\nSigned-off-by: Andreas Schwab \n---\n t/t7812-grep-icase-non-ascii.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t7812-grep-icase-non-ascii.sh b/t/t7812-grep-icase-non-ascii.sh\nindex 531eb59d57..c4528432e5 100755\n--- a/t/t7812-grep-icase-non-ascii.sh\n+++ b/t/t7812-grep-icase-non-ascii.sh\n@@ -70,14 +70,14 @@ test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: grep ASCII from invalid UT\n test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: grep non-ASCII from invalid UTF-8 data' '\n \tgit grep -h \"æ\" invalid-0x80 >actual &&\n \ttest_cmp expected actual &&\n-\tgit grep -h \"(*NO_JIT)æ\" invalid-0x80 &&\n+\tgit grep -h \"(*NO_JIT)æ\" invalid-0x80 >actual &&\n \ttest_cmp expected actual\n '\n \n test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: grep non-ASCII from invalid UTF-8 data with -i' '\n \ttest_might_fail git grep -hi \"Æ\" invalid-0x80 >actual &&\n \ttest_cmp expected actual &&\n-\ttest_must_fail git grep -hi \"(*NO_JIT)Æ\" invalid-0x80 &&\n+\ttest_must_fail git grep -hi \"(*NO_JIT)Æ\" invalid-0x80 >actual &&\n \ttest_cmp expected actual\n '\n \n-- \n2.24.0\n\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1\n\"And now for something completely different.\"\n"},{"id":"387115","messageId":"nycvar.QRO.7.76.6.1911262325390.31080@tvgsbejvaqbjf.bet","threadId":"51506","inReplyTo":"87blsyl32c.fsf_-_@igel.home","subject":"Re: [PATCH] t7812: add missing redirects","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-11-26T22:27:03Z","receivedAt":"2019-11-26T22:27:42Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\n\nOn Tue, 26 Nov 2019, Andreas Schwab wrote:\n\n> Two tests in t7812 were missing redirects, failing to actually test the\n> produced output.\n>\n> Fixes: 8a5999838e (\"grep: stess test PCRE v2 on invalid UTF-8 data\")\n\nApart from this line which is cuddled with a real footer (but is no\nfooter, and the commit reference is not in the recommended format either\nbecause it lacks the date), this patch looks fine to me.\n\nCiao,\nJohannes\n\n> Signed-off-by: Andreas Schwab \n> ---\n> t/t7812-grep-icase-non-ascii.sh | 4 ++--\n> 1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/t/t7812-grep-icase-non-ascii.sh b/t/t7812-grep-icase-non-ascii.sh\n> index 531eb59d57..c4528432e5 100755\n> --- a/t/t7812-grep-icase-non-ascii.sh\n> +++ b/t/t7812-grep-icase-non-ascii.sh\n> @@ -70,14 +70,14 @@ test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: grep ASCII from invalid UT\n> test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: grep non-ASCII from invalid UTF-8 data' '\n> \tgit grep -h \"æ\" invalid-0x80 >actual &&\n> \ttest_cmp expected actual &&\n> -\tgit grep -h \"(*NO_JIT)æ\" invalid-0x80 &&\n> +\tgit grep -h \"(*NO_JIT)æ\" invalid-0x80 >actual &&\n> \ttest_cmp expected actual\n> '\n>\n> test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: grep non-ASCII from invalid UTF-8 data with -i' '\n> \ttest_might_fail git grep -hi \"Æ\" invalid-0x80 >actual &&\n> \ttest_cmp expected actual &&\n> -\ttest_must_fail git grep -hi \"(*NO_JIT)Æ\" invalid-0x80 &&\n> +\ttest_must_fail git grep -hi \"(*NO_JIT)Æ\" invalid-0x80 >actual &&\n> \ttest_cmp expected actual\n> '\n>\n> --\n> 2.24.0\n>\n>\n> --\n> Andreas Schwab, schwab@linux-m68k.org\n> GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1\n> \"And now for something completely different.\"\n>\n"},{"id":"387118","messageId":"874kyqkzbe.fsf@igel.home","threadId":"51506","inReplyTo":"nycvar.QRO.7.76.6.1911262325390.31080@tvgsbejvaqbjf.bet","subject":"Re: [PATCH] t7812: add missing redirects","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2019-11-26T23:11:49Z","receivedAt":"2019-11-26T23:11:54Z","isPatch":true,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"On Nov 26 2019, Johannes Schindelin wrote:\n\n> footer, and the commit reference is not in the recommended format either\n> because it lacks the date),\n\nWhere is that documented?\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1\n\"And now for something completely different.\"\n"},{"id":"387144","messageId":"20191127115809.GF22221@sigill.intra.peff.net","threadId":"51506","inReplyTo":"874kyqkzbe.fsf@igel.home","subject":"Re: [PATCH] t7812: add missing redirects","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-11-27T11:58:09Z","receivedAt":"2019-11-27T11:58:12Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 27, 2019 at 12:11:49AM +0100, Andreas Schwab wrote:\n\n> On Nov 26 2019, Johannes Schindelin wrote:\n> \n> > footer, and the commit reference is not in the recommended format either\n> > because it lacks the date),\n> \n> Where is that documented?\n\nIt's mentioned as the preferred way to reference commits in\nSubmittingPatches (search for %ad).\n\nBut I don't see why it is \"not a footer\". The \"Fixes:\" key conforms to\nthe trailer syntax, and the value of a trailer is free-form. Running:\n\n git log --format='%(trailers:key=Fixes)'\n\nshows that Git is happy with it. And indeed, a few other people have\nused it before you. None of them with a date. ;)\n\n-Peff\n"},{"id":"387325","messageId":"20191130004653.8794-1-tmz@pobox.com","threadId":"51506","inReplyTo":"87blsyl32c.fsf_-_@igel.home","subject":"[PATCH] t7812: expect failure for grep -i with invalid UTF-8 data","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2019-11-30T00:46:53Z","receivedAt":"2019-11-30T00:47:08Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"When the 'grep with invalid UTF-8 data' tests were added/adjusted in\n8a5999838e (grep: stess test PCRE v2 on invalid UTF-8 data, 2019-07-26)\nand 870eea8166 (grep: do not enter PCRE2_UTF mode on fixed matching,\n2019-07-26) they lacked a redirect which caused them to falsely succeed\non most architectures. They failed on big-endian arches where the test\nnever reached the portion which was missing the redirect.\n\nA recent patch add the missing redirect and exposed the fact that the\n'PCRE v2: grep non-ASCII from invalid UTF-8 data with -i' test fails on\nall architectures.\n\nBased on the final paragraph in in 870eea8166:\n\n When grepping a non-ASCII fixed string. This is a more general problem\n that's hard to fix, but we can at least fix the most common case of\n grepping for a fixed string without \"-i\". I can't think of a reason\n for why we'd turn on PCRE2_UTF when matching byte-for-byte like that.\n\nit seems that we don't expect that the case-insensitive grep will\nsucceed. Adjust the test to reflect that expectation.\n\nSigned-off-by: Todd Zullinger \n---\n\nHi,\n\nAndreas Schwab wrote:\n> Two tests in t7812 were missing redirects, failing to actually test the\n> produced output.\n> \n> Fixes: 8a5999838e (\"grep: stess test PCRE v2 on invalid UTF-8 data\")\n> Signed-off-by: Andreas Schwab \n\nNice catch on these missing redirects. While testing the\n2.24.0 rc's this test failed only on big-endian arches (like\ns390x in Fedora). It was briefly mentioned at the time in\n<20191020002648.GZ10893@pobox.com>.\n\nAfter applying the fix to add the missing redirects, the\ntest now fails on all architectures. It seems like that is\nthe intended state and we simply need to adjust the final\ntest with `grep -i` accordingly.\n\nOf course, it's possible that I've misunderstood Ævar's\nintentions in 870eea8166 (grep: do not enter PCRE2_UTF mode\non fixed matching, 2019-07-26) or otherwise have poorly\nexplained the reasoning in my commit message.\n\nWe don't appear to test PCRE (neither v1 nor v2) in our CI\nbuilds. I added libpcre2-dev and set USE_LIBPCRE to test\nthis in travis. (Whether we want to enable that by default\nis worth discussing.)\n\nDoing so confirms that the tests (incorrectly) pass without\nthe missing redirects and fail when the redirects are added.\nInverting the expectation of test_cmp in the final `grep -i`\ntest results in a successful test run.\n\nwith PCRE2:\nhttps://travis-ci.org/tmzullinger/git/jobs/618733182\n\nwith PCRE2 and 'add missing redirects':\nhttps://travis-ci.org/tmzullinger/git/jobs/618723277\n\nwith PCRE2, 'add missing redirects' and this patch:\nhttps://travis-ci.org/tmzullinger/git/jobs/618796963\n\nThis still doesn't fix the failure on s390x. There\n\n test_might_fail git grep -hi \"Æ\" invalid-0x80 >actual\n\nfails and leaves nothing in 'actual' which causes the test\nto fail at the following\n\n test_cmp expected actual\n\nline. If we suspect the test might fail, it seems like we\nneed to account for that and not expect that 'expect' will\nmatch 'actual' as we currently do.\n\n t/t7812-grep-icase-non-ascii.sh | 7 +++++--\n 1 file changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t7812-grep-icase-non-ascii.sh b/t/t7812-grep-icase-non-ascii.sh\nindex c4528432e5..03dba6685a 100755\n--- a/t/t7812-grep-icase-non-ascii.sh\n+++ b/t/t7812-grep-icase-non-ascii.sh\n@@ -76,9 +76,12 @@ test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: grep non-ASCII from invali\n \n test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: grep non-ASCII from invalid UTF-8 data with -i' '\n \ttest_might_fail git grep -hi \"Æ\" invalid-0x80 >actual &&\n-\ttest_cmp expected actual &&\n+\tif test -s actual\n+\tthen\n+\t test_cmp expected actual\n+\tfi &&\n \ttest_must_fail git grep -hi \"(*NO_JIT)Æ\" invalid-0x80 >actual &&\n-\ttest_cmp expected actual\n+\t! test_cmp expected actual\n '\n \n test_done\n-- \n2.24.0\n\n"},{"id":"387327","messageId":"87a78ddc9o.fsf@hase.home","threadId":"51506","inReplyTo":"20191130004653.8794-1-tmz@pobox.com","subject":"Re: [PATCH] t7812: expect failure for grep -i with invalid UTF-8 data","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2019-11-30T08:00:35Z","receivedAt":"2019-11-30T08:00:46Z","isPatch":true,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"On Nov 29 2019, Todd Zullinger wrote:\n\n> When the 'grep with invalid UTF-8 data' tests were added/adjusted in\n> 8a5999838e (grep: stess test PCRE v2 on invalid UTF-8 data, 2019-07-26)\n> and 870eea8166 (grep: do not enter PCRE2_UTF mode on fixed matching,\n> 2019-07-26) they lacked a redirect which caused them to falsely succeed\n> on most architectures. They failed on big-endian arches where the test\n> never reached the portion which was missing the redirect.\n\nIt's not about big vs little endian, it's only about JIT vs non-JIT.\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5\n\"And now for something completely different.\"\n"},{"id":"387378","messageId":"xmqqo8wsypit.fsf@gitster-ct.c.googlers.com","threadId":"51506","inReplyTo":"87a78ddc9o.fsf@hase.home","subject":"Re: [PATCH] t7812: expect failure for grep -i with invalid UTF-8 data","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-12-01T16:33:14Z","receivedAt":"2019-12-01T16:33:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andreas Schwab writes:\n\n> On Nov 29 2019, Todd Zullinger wrote:\n>\n>> When the 'grep with invalid UTF-8 data' tests were added/adjusted in\n>> 8a5999838e (grep: stess test PCRE v2 on invalid UTF-8 data, 2019-07-26)\n>> and 870eea8166 (grep: do not enter PCRE2_UTF mode on fixed matching,\n>> 2019-07-26) they lacked a redirect which caused them to falsely succeed\n>> on most architectures. They failed on big-endian arches where the test\n>> never reached the portion which was missing the redirect.\n>\n> It's not about big vs little endian, it's only about JIT vs non-JIT.\n\nSo, which one of JIT / non-JIT sides did the test fail unexpectedly?\n\nShould I do s/on big-endian arches/with PCRE with JIT disabled/\nwhile queuing the patch?\n\nThanks.\n"},{"id":"387380","messageId":"87o8ws55xi.fsf@igel.home","threadId":"51506","inReplyTo":"xmqqo8wsypit.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] t7812: expect failure for grep -i with invalid UTF-8 data","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2019-12-01T17:09:13Z","receivedAt":"2019-12-01T17:09:20Z","isPatch":true,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"On Dez 01 2019, Junio C Hamano wrote:\n\n> So, which one of JIT / non-JIT sides did the test fail unexpectedly?\n\nThe non-JIT was the one that wasn't actually tested.\n\n> Should I do s/on big-endian arches/with PCRE with JIT disabled/\n> while queuing the patch?\n\nRight.\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1\n\"And now for something completely different.\"\n"},{"id":"387381","messageId":"20191201183203.GC17681@pobox.com","threadId":"51506","inReplyTo":"xmqqo8wsypit.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] t7812: expect failure for grep -i with invalid UTF-8 data","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2019-12-01T18:32:03Z","receivedAt":"2019-12-01T18:32:15Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"Hi,\n\nJunio C Hamano wrote:\n> Andreas Schwab writes:\n> \n>> On Nov 29 2019, Todd Zullinger wrote:\n>>\n>>> When the 'grep with invalid UTF-8 data' tests were added/adjusted in\n>>> 8a5999838e (grep: stess test PCRE v2 on invalid UTF-8 data, 2019-07-26)\n>>> and 870eea8166 (grep: do not enter PCRE2_UTF mode on fixed matching,\n>>> 2019-07-26) they lacked a redirect which caused them to falsely succeed\n>>> on most architectures. They failed on big-endian arches where the test\n>>> never reached the portion which was missing the redirect.\n>>\n>> It's not about big vs little endian, it's only about JIT vs non-JIT.\n> \n> So, which one of JIT / non-JIT sides did the test fail unexpectedly?\n\nOn s390x, the initial:\n\n test_might_fail git grep -hi \"Æ\" invalid-0x80 >actual\n\nfails to produce any output in actual, but since we use\ntest_might_fail, the test happily continues to:\n\n test_cmp expected actual\n\nwhich fails.\n\nThe test output from and s390x build:\n\n expecting success of 7812.11 'PCRE v2: grep non-ASCII from invalid UTF-8 data with -i':\n test_might_fail git grep -hi \"Æ\" invalid-0x80 >actual &&\n test_cmp expected actual &&\n test_must_fail git grep -hi \"(*NO_JIT)Æ\" invalid-0x80 &&\n test_cmp expected actual\n ++ test_might_fail git grep -hi Æ invalid-0x80\n ++ test_must_fail ok=success git grep -hi Æ invalid-0x80\n ++ case \"$1\" in\n ++ _test_ok=success\n ++ shift\n ++ git grep -hi Æ invalid-0x80\n fatal: pcre2_match failed with error code -22: UTF-8 error: isolated byte with 0x80 bit set\n ++ exit_code=128\n ++ test 128 -eq 0\n ++ test_match_signal 13 128\n ++ test 128 = 141\n ++ test 128 = 269\n ++ return 1\n ++ test 128 -gt 129\n ++ test 128 -eq 127\n ++ test 128 -eq 126\n ++ return 0\n ++ test_cmp expected actual\n ++ diff -u expected actual\n --- expected 2019-10-19 21:56:08.634252012 +0000\n +++ actual 2019-10-19 21:56:08.714252012 +0000\n @@ -1 +0,0 @@\n -ævar\n error: last command exited with $?=1\n not ok 11 - PCRE v2: grep non-ASCII from invalid UTF-8 data with -i\n #\n # test_might_fail git grep -hi \"Æ\" invalid-0x80 >actual &&\n # test_cmp expected actual &&\n # test_must_fail git grep -hi \"(*NO_JIT)Æ\" invalid-0x80 &&\n # test_cmp expected actual\n #\n # failed 1 among 11 test(s)\n\nAfter Andreas' missing redirect fix, that still fails on\ns390x (not surprisingly). But now systems with JIT enabled\nfail at:\n\n test_must_fail git grep -hi \"(*NO_JIT)Æ\" invalid-0x80 >actual &&\n test_cmp expected actual\n\nThough we say that the command must fail, so we shouldn't be\nsurprised that 'expect' and 'actual' don't match. It would\nbe more surprising if they did. :)\n\n> Should I do s/on big-endian arches/with PCRE with JIT disabled/\n> while queuing the patch?\n\nHere's how I changed the commit message locally. I was\ngoing to wait a day or so for any other feedback on the\nactual test changes, being a holiday weekend in the US (and\nmore generally a weekend).\n\n1: d9aeaf0c98 ! 1: d0c083db78 t7812: expect failure for grep -i with invalid UTF-8 data\n @@ Commit message\n 8a5999838e (grep: stess test PCRE v2 on invalid UTF-8 data, 2019-07-26)\n and 870eea8166 (grep: do not enter PCRE2_UTF mode on fixed matching,\n 2019-07-26) they lacked a redirect which caused them to falsely succeed\n - on most architectures. They failed on big-endian arches where the test\n - never reached the portion which was missing the redirect.\n + on most systems. The 'grep -i' test failed on systems where JIT was\n + disabled as it never reached the portion which was missing the redirect.\n \n - A recent patch add the missing redirect and exposed the fact that the\n - 'PCRE v2: grep non-ASCII from invalid UTF-8 data with -i' test fails on\n - all architectures.\n + A recent patch added the missing redirect and exposed the fact that the\n + 'PCRE v2: grep non-ASCII from invalid UTF-8 data with -i' test fails\n + regardless of whether JIT is enabled.\n \n Based on the final paragraph in in 870eea8166:\n\nThanks for pointing out the proper reasoning to use in the\ncommit message Andreas. I hadn't looked at the Fedora pcre2\npackage to see that it explicitly disables JIT on s390x.\n\nI'm not sure if s390x is supported upstream or not -- it\ndoesn't appear to have a specific entry in the sljit config\nheader¹, so it seems likely it's not well-tested at the\nleast. (Not that any of that is our concern here.)\n\n¹ https://github.com/zherczeg/sljit/blob/master/sljit_src/sljitConfigInternal.h\n\nThanks for the follow-up Junio.\n\n-- \nTodd\n"},{"id":"387385","messageId":"xmqqfti3z24k.fsf@gitster-ct.c.googlers.com","threadId":"51506","inReplyTo":"20191201183203.GC17681@pobox.com","subject":"Re: [PATCH] t7812: expect failure for grep -i with invalid UTF-8 data","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-12-02T06:13:15Z","receivedAt":"2019-12-02T06:13:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Todd Zullinger writes:\n\n> After Andreas' missing redirect fix, that still fails on\n> s390x (not surprisingly). But now systems with JIT enabled\n> fail at:\n> ...\n> Here's how I changed the commit message locally. I was\n> going to wait a day or so for any other feedback on the\n> actual test changes, being a holiday weekend in the US (and\n> more generally a weekend).\n>\n> 1: d9aeaf0c98 ! 1: d0c083db78 t7812: expect failure for grep -i with invalid UTF-8 data\n> @@ Commit message\n> 8a5999838e (grep: stess test PCRE v2 on invalid UTF-8 data, 2019-07-26)\n> and 870eea8166 (grep: do not enter PCRE2_UTF mode on fixed matching,\n> 2019-07-26) they lacked a redirect which caused them to falsely succeed\n> - on most architectures. They failed on big-endian arches where the test\n> - never reached the portion which was missing the redirect.\n> + on most systems. The 'grep -i' test failed on systems where JIT was\n> + disabled as it never reached the portion which was missing the redirect.\n> \n> - A recent patch add the missing redirect and exposed the fact that the\n> - 'PCRE v2: grep non-ASCII from invalid UTF-8 data with -i' test fails on\n> - all architectures.\n> + A recent patch added the missing redirect and exposed the fact that the\n> + 'PCRE v2: grep non-ASCII from invalid UTF-8 data with -i' test fails\n> + regardless of whether JIT is enabled.\n> \n> Based on the final paragraph in in 870eea8166:\n>\n> Thanks for pointing out the proper reasoning to use in the\n> commit message Andreas. I hadn't looked at the Fedora pcre2\n> package to see that it explicitly disables JIT on s390x.\n\nOK. I locally edited the log message to match the above. I guess\nthis forms an integral part of a topic that Andreas started with the\n\"missing redirects\" fix, so let me queue your patch directly on the\nsame topic branch, without creating a separate one.\n\nThanks, both.\n"},{"id":"415112","messageId":"20210124021229.25987-1-avarab@gmail.com","threadId":"51506","inReplyTo":"20190726150818.6373-9-avarab@gmail.com","subject":"[PATCH v3 0/4] grep: better support invalid UTF-8 haystacks","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-01-24T02:12:25Z","receivedAt":"2021-01-24T02:13:49Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This is v3 of a patch I originally sent in mid-2019:\nhttps://lore.kernel.org/git/20190726150818.6373-9-avarab@gmail.com/\n\nBack then we were near a release and the PCREv2 feature I'm using in\n4/4 wasn't in any released version. Now it's in widely used releases,\nso we can use it and fix some long-standing TODOs in invalid UTF-8\ngrep matching edge cases.\n\nÆvar Arnfjörð Bjarmason (4):\n grep/pcre2 tests: don't rely on invalid UTF-8 data test\n grep/pcre2: simplify boolean spaghetti\n grep/pcre2: further simplify boolean spaghetti\n grep/pcre2: better support invalid UTF-8 haystacks\n\n Makefile | 1 +\n grep.c | 11 +++++--\n grep.h | 4 +++\n t/helper/test-pcre2-config.c | 12 ++++++++\n t/helper/test-tool.c | 1 +\n t/helper/test-tool.h | 1 +\n t/t7812-grep-icase-non-ascii.sh | 53 ++++++++++++++++++++++++++++-----\n 7 files changed, 74 insertions(+), 9 deletions(-)\n create mode 100644 t/helper/test-pcre2-config.c\n\n-- \n2.29.2.222.g5d2a92d10f8\n\n"},{"id":"415113","messageId":"20210124021229.25987-2-avarab@gmail.com","threadId":"51506","inReplyTo":"20190726150818.6373-9-avarab@gmail.com","subject":"[PATCH v3 1/4] grep/pcre2 tests: don't rely on invalid UTF-8 data test","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-01-24T02:12:26Z","receivedAt":"2021-01-24T02:13:51Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"As noted in [1] when I originally added this test in [2] the test was\ncompletely broken as it lacked a redirect[3]. I now think this whole\nthing is overly fragile. Let's only test if we have a segfault here.\n\nBefore this the first test's \"test_cmp\" was pretty meaningless. We\nwere only testing if PCREv2 was so broken that it would spew out\nsomething completely unrelated on stdout, which isn't very plausible.\n\nIn the second test we're relying on PCREv2 forever holding to the\ncurrent behavior of the PCRE_UTF8 flag, as opposed to learning some\noptimistic graceful fallback to PCRE2_MATCH_INVALID_UTF in the\nfuture. If that happens having this test broken under bisecting would\nsuck.\n\nA follow-up commit will actually test this case in a meaningful way\nunder the PCRE2_MATCH_INVALID_UTF flag. Let's run this one\nunconditionally, and just make sure we don't segfault.\n\n1. e714b898c6 (t7812: expect failure for grep -i with invalid UTF-8\n data, 2019-11-29)\n2. 8a5999838e (grep: stess test PCRE v2 on invalid UTF-8 data,\n 2019-07-26)\n3. c74b3cbb83 (t7812: add missing redirects, 2019-11-26)\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t7812-grep-icase-non-ascii.sh | 7 +------\n 1 file changed, 1 insertion(+), 6 deletions(-)\n\ndiff --git a/t/t7812-grep-icase-non-ascii.sh b/t/t7812-grep-icase-non-ascii.sh\nindex 03dba6685a..38457c2e4f 100755\n--- a/t/t7812-grep-icase-non-ascii.sh\n+++ b/t/t7812-grep-icase-non-ascii.sh\n@@ -76,12 +76,7 @@ test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: grep non-ASCII from invali\n \n test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: grep non-ASCII from invalid UTF-8 data with -i' '\n \ttest_might_fail git grep -hi \"Æ\" invalid-0x80 >actual &&\n-\tif test -s actual\n-\tthen\n-\t test_cmp expected actual\n-\tfi &&\n-\ttest_must_fail git grep -hi \"(*NO_JIT)Æ\" invalid-0x80 >actual &&\n-\t! test_cmp expected actual\n+\ttest_might_fail git grep -hi \"(*NO_JIT)Æ\" invalid-0x80 >actual\n '\n \n test_done\n-- \n2.29.2.222.g5d2a92d10f8\n\n"},{"id":"415114","messageId":"20210124021229.25987-5-avarab@gmail.com","threadId":"51506","inReplyTo":"20190726150818.6373-9-avarab@gmail.com","subject":"[PATCH v3 4/4] grep/pcre2: better support invalid UTF-8 haystacks","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-01-24T02:12:29Z","receivedAt":"2021-01-24T02:13:59Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Improve the support for invalid UTF-8 haystacks given a non-ASCII\nneedle when using the PCREv2 backend.\n\nThis is a more complete fix for a bug I started to fix in\n870eea8166 (grep: do not enter PCRE2_UTF mode on fixed matching,\n2019-07-26), now that PCREv2 has the PCRE2_MATCH_INVALID_UTF mode we\ncan make use of it.\n\nThis fixes the sort of case described in 8a5999838e (grep: stess test\nPCRE v2 on invalid UTF-8 data, 2019-07-26), i.e.:\n\n - The subject string is non-ASCII (e.g. \"ævar\")\n - We're under a is_utf8_locale(), e.g. \"en_US.UTF-8\", not \"C\"\n - We are using --ignore-case, or we're a non-fixed pattern\n\nIf those conditions were satisfied and we matched found non-valid\nUTF-8 data PCREv2 might bark on it, in practice this only happened\nunder the JIT backend (turned on by default on most platforms).\n\nUltimately this fixes a \"regression\" in b65abcafc7 (\"grep: use PCRE v2\nfor optimized fixed-string search\", 2019-07-01), I'm putting that in\nscare-quotes because before then we wouldn't properly support these\ncomplex case-folding, locale etc. cases either, it just broke in\ndifferent ways.\n\nThere was a bug related to this the PCRE2_NO_START_OPTIMIZE flag fixed\nin PCREv2 10.36. It can be worked around by setting the\nPCRE2_NO_START_OPTIMIZE flag. Let's do that in those cases, and add\ntests for the bug.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 1 +\n grep.c | 8 +++++-\n grep.h | 4 +++\n t/helper/test-pcre2-config.c | 12 +++++++++\n t/helper/test-tool.c | 1 +\n t/helper/test-tool.h | 1 +\n t/t7812-grep-icase-non-ascii.sh | 46 ++++++++++++++++++++++++++++++++-\n 7 files changed, 71 insertions(+), 2 deletions(-)\n create mode 100644 t/helper/test-pcre2-config.c\n\ndiff --git a/Makefile b/Makefile\nindex 4edfda3e00..42a7ed96e2 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -722,6 +722,7 @@ TEST_BUILTINS_OBJS += test-online-cpus.o\n TEST_BUILTINS_OBJS += test-parse-options.o\n TEST_BUILTINS_OBJS += test-parse-pathspec-file.o\n TEST_BUILTINS_OBJS += test-path-utils.o\n+TEST_BUILTINS_OBJS += test-pcre2-config.o\n TEST_BUILTINS_OBJS += test-pkt-line.o\n TEST_BUILTINS_OBJS += test-prio-queue.o\n TEST_BUILTINS_OBJS += test-proc-receive.o\ndiff --git a/grep.c b/grep.c\nindex 242b4a3506..305c579aff 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -493,7 +493,13 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt\n \t}\n \tif (!opt->ignore_locale && is_utf8_locale() && has_non_ascii(p->pattern) &&\n \t (opt->ignore_case || !fixed))\n-\t\toptions |= PCRE2_UTF;\n+\t\toptions |= (PCRE2_UTF | PCRE2_MATCH_INVALID_UTF);\n+\n+\tif (PCRE2_MATCH_INVALID_UTF &&\n+\t options & (PCRE2_UTF | PCRE2_CASELESS) &&\n+\t !(PCRE2_MAJOR >= 10 && PCRE2_MAJOR >= 36))\n+\t\t/* Work around https://bugs.exim.org/show_bug.cgi?id=2642 fixed in 10.36 */\n+\t\toptions |= PCRE2_NO_START_OPTIMIZE;\n \n \tp->pcre2_pattern = pcre2_compile((PCRE2_SPTR)p->pattern,\n \t\t\t\t\t p->patternlen, options, &error, &erroffset,\ndiff --git a/grep.h b/grep.h\nindex b5c4e223a8..ade21c8812 100644\n--- a/grep.h\n+++ b/grep.h\n@@ -18,6 +18,10 @@ typedef int pcre2_code;\n typedef int pcre2_match_data;\n typedef int pcre2_compile_context;\n #endif\n+#ifndef PCRE2_MATCH_INVALID_UTF\n+/* PCRE2_MATCH_* dummy also with !USE_LIBPCRE2, for test-pcre2-config.c */\n+#define PCRE2_MATCH_INVALID_UTF 0\n+#endif\n #include \"thread-utils.h\"\n #include \"userdiff.h\"\n \ndiff --git a/t/helper/test-pcre2-config.c b/t/helper/test-pcre2-config.c\nnew file mode 100644\nindex 0000000000..5258fdddba\n--- /dev/null\n+++ b/t/helper/test-pcre2-config.c\n@@ -0,0 +1,12 @@\n+#include \"test-tool.h\"\n+#include \"cache.h\"\n+#include \"grep.h\"\n+\n+int cmd__pcre2_config(int argc, const char **argv)\n+{\n+\tif (argc == 2 && !strcmp(argv[1], \"has-PCRE2_MATCH_INVALID_UTF\")) {\n+\t\tint value = PCRE2_MATCH_INVALID_UTF;\n+\t\treturn !value;\n+\t}\n+\treturn 1;\n+}\ndiff --git a/t/helper/test-tool.c b/t/helper/test-tool.c\nindex 9d6d14d929..f97cd9f48a 100644\n--- a/t/helper/test-tool.c\n+++ b/t/helper/test-tool.c\n@@ -46,6 +46,7 @@ static struct test_cmd cmds[] = {\n \t{ \"parse-options\", cmd__parse_options },\n \t{ \"parse-pathspec-file\", cmd__parse_pathspec_file },\n \t{ \"path-utils\", cmd__path_utils },\n+\t{ \"pcre2-config\", cmd__pcre2_config },\n \t{ \"pkt-line\", cmd__pkt_line },\n \t{ \"prio-queue\", cmd__prio_queue },\n \t{ \"proc-receive\", cmd__proc_receive},\ndiff --git a/t/helper/test-tool.h b/t/helper/test-tool.h\nindex a6470ff62c..28072c0ad5 100644\n--- a/t/helper/test-tool.h\n+++ b/t/helper/test-tool.h\n@@ -35,6 +35,7 @@ int cmd__online_cpus(int argc, const char **argv);\n int cmd__parse_options(int argc, const char **argv);\n int cmd__parse_pathspec_file(int argc, const char** argv);\n int cmd__path_utils(int argc, const char **argv);\n+int cmd__pcre2_config(int argc, const char **argv);\n int cmd__pkt_line(int argc, const char **argv);\n int cmd__prio_queue(int argc, const char **argv);\n int cmd__proc_receive(int argc, const char **argv);\ndiff --git a/t/t7812-grep-icase-non-ascii.sh b/t/t7812-grep-icase-non-ascii.sh\nindex 38457c2e4f..e5d1e4ea68 100755\n--- a/t/t7812-grep-icase-non-ascii.sh\n+++ b/t/t7812-grep-icase-non-ascii.sh\n@@ -57,7 +57,12 @@ test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: setup invalid UTF-8 data'\n \tprintf \"\\\\200\\\\n\" >invalid-0x80 &&\n \techo \"ævar\" >expected &&\n \tcat expected >>invalid-0x80 &&\n-\tgit add invalid-0x80\n+\tgit add invalid-0x80 &&\n+\n+\t# Test for PCRE2_MATCH_INVALID_UTF bug\n+\t# https://bugs.exim.org/show_bug.cgi?id=2642\n+\tprintf \"\\\\345Aæ\\\\n\" >invalid-0xe5 &&\n+\tgit add invalid-0xe5\n '\n \n test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: grep ASCII from invalid UTF-8 data' '\n@@ -67,6 +72,13 @@ test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: grep ASCII from invalid UT\n \ttest_cmp expected actual\n '\n \n+test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: grep ASCII from invalid UTF-8 data (PCRE2 bug #2642)' '\n+\tgit grep -h \"Aæ\" invalid-0xe5 >actual &&\n+\ttest_cmp invalid-0xe5 actual &&\n+\tgit grep -h \"(*NO_JIT)Aæ\" invalid-0xe5 >actual &&\n+\ttest_cmp invalid-0xe5 actual\n+'\n+\n test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: grep non-ASCII from invalid UTF-8 data' '\n \tgit grep -h \"æ\" invalid-0x80 >actual &&\n \ttest_cmp expected actual &&\n@@ -74,9 +86,41 @@ test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: grep non-ASCII from invali\n \ttest_cmp expected actual\n '\n \n+test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: grep non-ASCII from invalid UTF-8 data (PCRE2 bug #2642)' '\n+\tgit grep -h \"Aæ\" invalid-0xe5 >actual &&\n+\ttest_cmp invalid-0xe5 actual &&\n+\tgit grep -h \"(*NO_JIT)Aæ\" invalid-0xe5 >actual &&\n+\ttest_cmp invalid-0xe5 actual\n+'\n+\n+test_lazy_prereq PCRE2_MATCH_INVALID_UTF '\n+\ttest-tool pcre2-config has-PCRE2_MATCH_INVALID_UTF\n+'\n+\n test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: grep non-ASCII from invalid UTF-8 data with -i' '\n \ttest_might_fail git grep -hi \"Æ\" invalid-0x80 >actual &&\n \ttest_might_fail git grep -hi \"(*NO_JIT)Æ\" invalid-0x80 >actual\n '\n \n+test_expect_success GETTEXT_LOCALE,LIBPCRE2,PCRE2_MATCH_INVALID_UTF 'PCRE v2: grep non-ASCII from invalid UTF-8 data with -i' '\n+\tgit grep -hi \"Æ\" invalid-0x80 >actual &&\n+\ttest_cmp expected actual &&\n+\tgit grep -hi \"(*NO_JIT)Æ\" invalid-0x80 >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success GETTEXT_LOCALE,LIBPCRE2,PCRE2_MATCH_INVALID_UTF 'PCRE v2: grep non-ASCII from invalid UTF-8 data with -i (PCRE2 bug #2642)' '\n+\tgit grep -hi \"Æ\" invalid-0xe5 >actual &&\n+\ttest_cmp invalid-0xe5 actual &&\n+\tgit grep -hi \"(*NO_JIT)Æ\" invalid-0xe5 >actual &&\n+\ttest_cmp invalid-0xe5 actual &&\n+\n+\t# Only the case of grepping the ASCII part in a way that\n+\t# relies on -i fails\n+\tgit grep -hi \"aÆ\" invalid-0xe5 >actual &&\n+\ttest_cmp invalid-0xe5 actual &&\n+\tgit grep -hi \"(*NO_JIT)aÆ\" invalid-0xe5 >actual &&\n+\ttest_cmp invalid-0xe5 actual\n+'\n+\n test_done\n-- \n2.29.2.222.g5d2a92d10f8\n\n"},{"id":"415115","messageId":"20210124021229.25987-4-avarab@gmail.com","threadId":"51506","inReplyTo":"20190726150818.6373-9-avarab@gmail.com","subject":"[PATCH v3 3/4] grep/pcre2: further simplify boolean spaghetti","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-01-24T02:12:28Z","receivedAt":"2021-01-24T02:14:01Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Follow-up the last commit by splitting the fixed check for the\nPCRE2_UTF flag into a variable.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n grep.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/grep.c b/grep.c\nindex 0bb772f727..242b4a3506 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -473,6 +473,7 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt\n \tint jitret;\n \tint patinforet;\n \tsize_t jitsizearg;\n+\tconst int fixed = p->fixed || p->is_fixed;\n \n \tassert(opt->pcre2);\n \n@@ -491,7 +492,7 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt\n \t\toptions |= PCRE2_CASELESS;\n \t}\n \tif (!opt->ignore_locale && is_utf8_locale() && has_non_ascii(p->pattern) &&\n-\t (opt->ignore_case || !(p->fixed || p->is_fixed)))\n+\t (opt->ignore_case || !fixed))\n \t\toptions |= PCRE2_UTF;\n \n \tp->pcre2_pattern = pcre2_compile((PCRE2_SPTR)p->pattern,\n-- \n2.29.2.222.g5d2a92d10f8\n\n"},{"id":"415116","messageId":"20210124021229.25987-3-avarab@gmail.com","threadId":"51506","inReplyTo":"20190726150818.6373-9-avarab@gmail.com","subject":"[PATCH v3 2/4] grep/pcre2: simplify boolean spaghetti","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-01-24T02:12:27Z","receivedAt":"2021-01-24T02:14:02Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Simplify an expression I added in 870eea8166 (grep: do not enter\nPCRE2_UTF mode on fixed matching, 2019-07-26) by using a simple\napplication of De Morgan's laws[1]. I.e.:\n\n NOT(A && B) is Equivalent to (NOT(A) OR NOT(B))\n\n1. https://en.wikipedia.org/wiki/De_Morgan%27s_laws\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n grep.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/grep.c b/grep.c\nindex efeb6dc58d..0bb772f727 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -491,7 +491,7 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt\n \t\toptions |= PCRE2_CASELESS;\n \t}\n \tif (!opt->ignore_locale && is_utf8_locale() && has_non_ascii(p->pattern) &&\n-\t !(!opt->ignore_case && (p->fixed || p->is_fixed)))\n+\t (opt->ignore_case || !(p->fixed || p->is_fixed)))\n \t\toptions |= PCRE2_UTF;\n \n \tp->pcre2_pattern = pcre2_compile((PCRE2_SPTR)p->pattern,\n-- \n2.29.2.222.g5d2a92d10f8\n\n"},{"id":"415127","messageId":"xmqqim7m292j.fsf@gitster.c.googlers.com","threadId":"51506","inReplyTo":"20210124021229.25987-3-avarab@gmail.com","subject":"Re: [PATCH v3 2/4] grep/pcre2: simplify boolean spaghetti","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-01-24T05:33:08Z","receivedAt":"2021-01-24T05:36:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> NOT(A && B) is Equivalent to (NOT(A) OR NOT(B))\n\nAt this level, however, the left one looks much simpler than the\nright one ;-)\n\n\n> \tif (!opt->ignore_locale && is_utf8_locale() && has_non_ascii(p->pattern) &&\n> -\t !(!opt->ignore_case && (p->fixed || p->is_fixed)))\n> +\t (opt->ignore_case || !(p->fixed || p->is_fixed)))\n> \t\toptions |= PCRE2_UTF;\n\nIn the context of this expression, well, I guess the rewritten one\nis probably simpler but can we explain the whole condition in fewer\nthan three lines? With or without the rewrite, it still looks too\ncomplicated to me.\n\n\n"},{"id":"415145","messageId":"ac4f3b95-49f7-7a90-b7f7-9efbb57a7392@kdbg.org","threadId":"51506","inReplyTo":"xmqqim7m292j.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v3 2/4] grep/pcre2: simplify boolean spaghetti","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2021-01-24T10:45:39Z","receivedAt":"2021-01-24T10:47:13Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 24.01.21 um 06:33 schrieb Junio C Hamano:\n> Ævar Arnfjörð Bjarmason writes:\n> \n>> NOT(A && B) is Equivalent to (NOT(A) OR NOT(B))\n> \n> At this level, however, the left one looks much simpler than the\n> right one ;-)\n> \n> \n>> \tif (!opt->ignore_locale && is_utf8_locale() && has_non_ascii(p->pattern) &&\n>> -\t !(!opt->ignore_case && (p->fixed || p->is_fixed)))\n>> +\t (opt->ignore_case || !(p->fixed || p->is_fixed)))\n>> \t\toptions |= PCRE2_UTF;\n> \n> In the context of this expression, well, I guess the rewritten one\n> is probably simpler but can we explain the whole condition in fewer\n> than three lines? With or without the rewrite, it still looks too\n> complicated to me.\n\nMake the condition\n\n \tif (!opt->ignore_locale &&\n\t is_utf8_locale() &&\n\t has_non_ascii(p->pattern) &&\n\t (opt->ignore_case ||\n\t\t(!p->fixed &&\n\t\t !p->is_fixed)))\n\t{\n \t\toptions |= PCRE2_UTF;\n\t}\n\nWith the knowledge of the equivalence\n\n (A => B) <=> (NOT(A) OR B)\n\n(A => B means \"if A then B\"), the condition makes a lot of sense when\nread aloud:\n\n if\n NOT ignore locale\n AND\n is UTF8\n AND\n has non-ASCII\n AND\n if\n NOT ignore case\n then if also\n NOT fixed\n AND\n NOT is fixed\n then\n ...\n\n\nThe codition amounts to extending a series of conjunctions with more\nconjuctions IF a condition is satisfied. That's quite sensible.\n\nYou have to swap the polarity of the first condition of || in your head,\nthough, to achieve that meaning. That works with every OR condition, BTW.\n\n-- Hannes\n"},{"id":"415150","messageId":"20210124114855.13036-2-avarab@gmail.com","threadId":"51506","inReplyTo":"20210124021229.25987-1-avarab@gmail.com","subject":"[PATCH v4 1/2] grep/pcre2 tests: don't rely on invalid UTF-8 data test","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-01-24T11:48:54Z","receivedAt":"2021-01-24T11:52:19Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"As noted in [1] when I originally added this test in [2] the test was\ncompletely broken as it lacked a redirect[3]. I now think this whole\nthing is overly fragile. Let's only test if we have a segfault here.\n\nBefore this the first test's \"test_cmp\" was pretty meaningless. We\nwere only testing if PCREv2 was so broken that it would spew out\nsomething completely unrelated on stdout, which isn't very plausible.\n\nIn the second test we're relying on PCREv2 forever holding to the\ncurrent behavior of the PCRE_UTF8 flag, as opposed to learning some\noptimistic graceful fallback to PCRE2_MATCH_INVALID_UTF in the\nfuture. If that happens having this test broken under bisecting would\nsuck.\n\nA follow-up commit will actually test this case in a meaningful way\nunder the PCRE2_MATCH_INVALID_UTF flag. Let's run this one\nunconditionally, and just make sure we don't segfault.\n\n1. e714b898c6 (t7812: expect failure for grep -i with invalid UTF-8\n data, 2019-11-29)\n2. 8a5999838e (grep: stess test PCRE v2 on invalid UTF-8 data,\n 2019-07-26)\n3. c74b3cbb83 (t7812: add missing redirects, 2019-11-26)\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t7812-grep-icase-non-ascii.sh | 7 +------\n 1 file changed, 1 insertion(+), 6 deletions(-)\n\ndiff --git a/t/t7812-grep-icase-non-ascii.sh b/t/t7812-grep-icase-non-ascii.sh\nindex 03dba6685a..38457c2e4f 100755\n--- a/t/t7812-grep-icase-non-ascii.sh\n+++ b/t/t7812-grep-icase-non-ascii.sh\n@@ -76,12 +76,7 @@ test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: grep non-ASCII from invali\n \n test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: grep non-ASCII from invalid UTF-8 data with -i' '\n \ttest_might_fail git grep -hi \"Æ\" invalid-0x80 >actual &&\n-\tif test -s actual\n-\tthen\n-\t test_cmp expected actual\n-\tfi &&\n-\ttest_must_fail git grep -hi \"(*NO_JIT)Æ\" invalid-0x80 >actual &&\n-\t! test_cmp expected actual\n+\ttest_might_fail git grep -hi \"(*NO_JIT)Æ\" invalid-0x80 >actual\n '\n \n test_done\n-- \n2.29.2.222.g5d2a92d10f8\n\n"},{"id":"415151","messageId":"20210124114855.13036-3-avarab@gmail.com","threadId":"51506","inReplyTo":"20210124021229.25987-1-avarab@gmail.com","subject":"[PATCH v4 2/2] grep/pcre2: better support invalid UTF-8 haystacks","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-01-24T11:48:55Z","receivedAt":"2021-01-24T11:52:21Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Improve the support for invalid UTF-8 haystacks given a non-ASCII\nneedle when using the PCREv2 backend.\n\nThis is a more complete fix for a bug I started to fix in\n870eea8166 (grep: do not enter PCRE2_UTF mode on fixed matching,\n2019-07-26), now that PCREv2 has the PCRE2_MATCH_INVALID_UTF mode we\ncan make use of it.\n\nThis fixes the sort of case described in 8a5999838e (grep: stess test\nPCRE v2 on invalid UTF-8 data, 2019-07-26), i.e.:\n\n - The subject string is non-ASCII (e.g. \"ævar\")\n - We're under a is_utf8_locale(), e.g. \"en_US.UTF-8\", not \"C\"\n - We are using --ignore-case, or we're a non-fixed pattern\n\nIf those conditions were satisfied and we matched found non-valid\nUTF-8 data PCREv2 might bark on it, in practice this only happened\nunder the JIT backend (turned on by default on most platforms).\n\nUltimately this fixes a \"regression\" in b65abcafc7 (\"grep: use PCRE v2\nfor optimized fixed-string search\", 2019-07-01), I'm putting that in\nscare-quotes because before then we wouldn't properly support these\ncomplex case-folding, locale etc. cases either, it just broke in\ndifferent ways.\n\nThere was a bug related to this the PCRE2_NO_START_OPTIMIZE flag fixed\nin PCREv2 10.36. It can be worked around by setting the\nPCRE2_NO_START_OPTIMIZE flag. Let's do that in those cases, and add\ntests for the bug.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 1 +\n grep.c | 8 +++++-\n grep.h | 4 +++\n t/helper/test-pcre2-config.c | 12 +++++++++\n t/helper/test-tool.c | 1 +\n t/helper/test-tool.h | 1 +\n t/t7812-grep-icase-non-ascii.sh | 46 ++++++++++++++++++++++++++++++++-\n 7 files changed, 71 insertions(+), 2 deletions(-)\n create mode 100644 t/helper/test-pcre2-config.c\n\ndiff --git a/Makefile b/Makefile\nindex 4edfda3e00..42a7ed96e2 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -722,6 +722,7 @@ TEST_BUILTINS_OBJS += test-online-cpus.o\n TEST_BUILTINS_OBJS += test-parse-options.o\n TEST_BUILTINS_OBJS += test-parse-pathspec-file.o\n TEST_BUILTINS_OBJS += test-path-utils.o\n+TEST_BUILTINS_OBJS += test-pcre2-config.o\n TEST_BUILTINS_OBJS += test-pkt-line.o\n TEST_BUILTINS_OBJS += test-prio-queue.o\n TEST_BUILTINS_OBJS += test-proc-receive.o\ndiff --git a/grep.c b/grep.c\nindex efeb6dc58d..e329f19877 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -492,7 +492,13 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt\n \t}\n \tif (!opt->ignore_locale && is_utf8_locale() && has_non_ascii(p->pattern) &&\n \t !(!opt->ignore_case && (p->fixed || p->is_fixed)))\n-\t\toptions |= PCRE2_UTF;\n+\t\toptions |= (PCRE2_UTF | PCRE2_MATCH_INVALID_UTF);\n+\n+\tif (PCRE2_MATCH_INVALID_UTF &&\n+\t options & (PCRE2_UTF | PCRE2_CASELESS) &&\n+\t !(PCRE2_MAJOR >= 10 && PCRE2_MAJOR >= 36))\n+\t\t/* Work around https://bugs.exim.org/show_bug.cgi?id=2642 fixed in 10.36 */\n+\t\toptions |= PCRE2_NO_START_OPTIMIZE;\n \n \tp->pcre2_pattern = pcre2_compile((PCRE2_SPTR)p->pattern,\n \t\t\t\t\t p->patternlen, options, &error, &erroffset,\ndiff --git a/grep.h b/grep.h\nindex b5c4e223a8..ade21c8812 100644\n--- a/grep.h\n+++ b/grep.h\n@@ -18,6 +18,10 @@ typedef int pcre2_code;\n typedef int pcre2_match_data;\n typedef int pcre2_compile_context;\n #endif\n+#ifndef PCRE2_MATCH_INVALID_UTF\n+/* PCRE2_MATCH_* dummy also with !USE_LIBPCRE2, for test-pcre2-config.c */\n+#define PCRE2_MATCH_INVALID_UTF 0\n+#endif\n #include \"thread-utils.h\"\n #include \"userdiff.h\"\n \ndiff --git a/t/helper/test-pcre2-config.c b/t/helper/test-pcre2-config.c\nnew file mode 100644\nindex 0000000000..5258fdddba\n--- /dev/null\n+++ b/t/helper/test-pcre2-config.c\n@@ -0,0 +1,12 @@\n+#include \"test-tool.h\"\n+#include \"cache.h\"\n+#include \"grep.h\"\n+\n+int cmd__pcre2_config(int argc, const char **argv)\n+{\n+\tif (argc == 2 && !strcmp(argv[1], \"has-PCRE2_MATCH_INVALID_UTF\")) {\n+\t\tint value = PCRE2_MATCH_INVALID_UTF;\n+\t\treturn !value;\n+\t}\n+\treturn 1;\n+}\ndiff --git a/t/helper/test-tool.c b/t/helper/test-tool.c\nindex 9d6d14d929..f97cd9f48a 100644\n--- a/t/helper/test-tool.c\n+++ b/t/helper/test-tool.c\n@@ -46,6 +46,7 @@ static struct test_cmd cmds[] = {\n \t{ \"parse-options\", cmd__parse_options },\n \t{ \"parse-pathspec-file\", cmd__parse_pathspec_file },\n \t{ \"path-utils\", cmd__path_utils },\n+\t{ \"pcre2-config\", cmd__pcre2_config },\n \t{ \"pkt-line\", cmd__pkt_line },\n \t{ \"prio-queue\", cmd__prio_queue },\n \t{ \"proc-receive\", cmd__proc_receive},\ndiff --git a/t/helper/test-tool.h b/t/helper/test-tool.h\nindex a6470ff62c..28072c0ad5 100644\n--- a/t/helper/test-tool.h\n+++ b/t/helper/test-tool.h\n@@ -35,6 +35,7 @@ int cmd__online_cpus(int argc, const char **argv);\n int cmd__parse_options(int argc, const char **argv);\n int cmd__parse_pathspec_file(int argc, const char** argv);\n int cmd__path_utils(int argc, const char **argv);\n+int cmd__pcre2_config(int argc, const char **argv);\n int cmd__pkt_line(int argc, const char **argv);\n int cmd__prio_queue(int argc, const char **argv);\n int cmd__proc_receive(int argc, const char **argv);\ndiff --git a/t/t7812-grep-icase-non-ascii.sh b/t/t7812-grep-icase-non-ascii.sh\nindex 38457c2e4f..e5d1e4ea68 100755\n--- a/t/t7812-grep-icase-non-ascii.sh\n+++ b/t/t7812-grep-icase-non-ascii.sh\n@@ -57,7 +57,12 @@ test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: setup invalid UTF-8 data'\n \tprintf \"\\\\200\\\\n\" >invalid-0x80 &&\n \techo \"ævar\" >expected &&\n \tcat expected >>invalid-0x80 &&\n-\tgit add invalid-0x80\n+\tgit add invalid-0x80 &&\n+\n+\t# Test for PCRE2_MATCH_INVALID_UTF bug\n+\t# https://bugs.exim.org/show_bug.cgi?id=2642\n+\tprintf \"\\\\345Aæ\\\\n\" >invalid-0xe5 &&\n+\tgit add invalid-0xe5\n '\n \n test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: grep ASCII from invalid UTF-8 data' '\n@@ -67,6 +72,13 @@ test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: grep ASCII from invalid UT\n \ttest_cmp expected actual\n '\n \n+test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: grep ASCII from invalid UTF-8 data (PCRE2 bug #2642)' '\n+\tgit grep -h \"Aæ\" invalid-0xe5 >actual &&\n+\ttest_cmp invalid-0xe5 actual &&\n+\tgit grep -h \"(*NO_JIT)Aæ\" invalid-0xe5 >actual &&\n+\ttest_cmp invalid-0xe5 actual\n+'\n+\n test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: grep non-ASCII from invalid UTF-8 data' '\n \tgit grep -h \"æ\" invalid-0x80 >actual &&\n \ttest_cmp expected actual &&\n@@ -74,9 +86,41 @@ test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: grep non-ASCII from invali\n \ttest_cmp expected actual\n '\n \n+test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: grep non-ASCII from invalid UTF-8 data (PCRE2 bug #2642)' '\n+\tgit grep -h \"Aæ\" invalid-0xe5 >actual &&\n+\ttest_cmp invalid-0xe5 actual &&\n+\tgit grep -h \"(*NO_JIT)Aæ\" invalid-0xe5 >actual &&\n+\ttest_cmp invalid-0xe5 actual\n+'\n+\n+test_lazy_prereq PCRE2_MATCH_INVALID_UTF '\n+\ttest-tool pcre2-config has-PCRE2_MATCH_INVALID_UTF\n+'\n+\n test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: grep non-ASCII from invalid UTF-8 data with -i' '\n \ttest_might_fail git grep -hi \"Æ\" invalid-0x80 >actual &&\n \ttest_might_fail git grep -hi \"(*NO_JIT)Æ\" invalid-0x80 >actual\n '\n \n+test_expect_success GETTEXT_LOCALE,LIBPCRE2,PCRE2_MATCH_INVALID_UTF 'PCRE v2: grep non-ASCII from invalid UTF-8 data with -i' '\n+\tgit grep -hi \"Æ\" invalid-0x80 >actual &&\n+\ttest_cmp expected actual &&\n+\tgit grep -hi \"(*NO_JIT)Æ\" invalid-0x80 >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success GETTEXT_LOCALE,LIBPCRE2,PCRE2_MATCH_INVALID_UTF 'PCRE v2: grep non-ASCII from invalid UTF-8 data with -i (PCRE2 bug #2642)' '\n+\tgit grep -hi \"Æ\" invalid-0xe5 >actual &&\n+\ttest_cmp invalid-0xe5 actual &&\n+\tgit grep -hi \"(*NO_JIT)Æ\" invalid-0xe5 >actual &&\n+\ttest_cmp invalid-0xe5 actual &&\n+\n+\t# Only the case of grepping the ASCII part in a way that\n+\t# relies on -i fails\n+\tgit grep -hi \"aÆ\" invalid-0xe5 >actual &&\n+\ttest_cmp invalid-0xe5 actual &&\n+\tgit grep -hi \"(*NO_JIT)aÆ\" invalid-0xe5 >actual &&\n+\ttest_cmp invalid-0xe5 actual\n+'\n+\n test_done\n-- \n2.29.2.222.g5d2a92d10f8\n\n"},{"id":"415152","messageId":"20210124114855.13036-1-avarab@gmail.com","threadId":"51506","inReplyTo":"20210124021229.25987-1-avarab@gmail.com","subject":"[PATCH v4 0/2] grep: better support invalid UTF-8 haystacks","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-01-24T11:48:53Z","receivedAt":"2021-01-24T11:52:21Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Changes since v2: Dropped mid-series unrelated refactoring. Will leave\nit to a future patch series to simplify/move/refactor this code more\ngenerally.\n\nÆvar Arnfjörð Bjarmason (2):\n grep/pcre2 tests: don't rely on invalid UTF-8 data test\n grep/pcre2: better support invalid UTF-8 haystacks\n\n Makefile | 1 +\n grep.c | 8 ++++-\n grep.h | 4 +++\n t/helper/test-pcre2-config.c | 12 ++++++++\n t/helper/test-tool.c | 1 +\n t/helper/test-tool.h | 1 +\n t/t7812-grep-icase-non-ascii.sh | 53 ++++++++++++++++++++++++++++-----\n 7 files changed, 72 insertions(+), 8 deletions(-)\n create mode 100644 t/helper/test-pcre2-config.c\n\nRange-diff:\n-: ---------- > 1: 699bb6b324 grep/pcre2 tests: don't rely on invalid UTF-8 data test\n-: ---------- > 2: e4807d6879 grep/pcre2: better support invalid UTF-8 haystacks\n-- \n2.29.2.222.g5d2a92d10f8\n\n"},{"id":"415158","messageId":"6fe69ede-d24b-1742-f699-c9af05560c0c@ramsayjones.plus.com","threadId":"51506","inReplyTo":"20210124114855.13036-3-avarab@gmail.com","subject":"Re: [PATCH v4 2/2] grep/pcre2: better support invalid UTF-8 haystacks","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2021-01-24T13:53:40Z","receivedAt":"2021-01-24T14:03:35Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 24/01/2021 11:48, Ævar Arnfjörð Bjarmason wrote:\n> Improve the support for invalid UTF-8 haystacks given a non-ASCII\n> needle when using the PCREv2 backend.\n> \n> This is a more complete fix for a bug I started to fix in\n> 870eea8166 (grep: do not enter PCRE2_UTF mode on fixed matching,\n> 2019-07-26), now that PCREv2 has the PCRE2_MATCH_INVALID_UTF mode we\n> can make use of it.\n> \n> This fixes the sort of case described in 8a5999838e (grep: stess test\n> PCRE v2 on invalid UTF-8 data, 2019-07-26), i.e.:\n> \n> - The subject string is non-ASCII (e.g. \"ævar\")\n> - We're under a is_utf8_locale(), e.g. \"en_US.UTF-8\", not \"C\"\n> - We are using --ignore-case, or we're a non-fixed pattern\n> \n> If those conditions were satisfied and we matched found non-valid\n> UTF-8 data PCREv2 might bark on it, in practice this only happened\n> under the JIT backend (turned on by default on most platforms).\n> \n> Ultimately this fixes a \"regression\" in b65abcafc7 (\"grep: use PCRE v2\n> for optimized fixed-string search\", 2019-07-01), I'm putting that in\n> scare-quotes because before then we wouldn't properly support these\n> complex case-folding, locale etc. cases either, it just broke in\n> different ways.\n> \n> There was a bug related to this the PCRE2_NO_START_OPTIMIZE flag fixed\n> in PCREv2 10.36. It can be worked around by setting the\n> PCRE2_NO_START_OPTIMIZE flag. Let's do that in those cases, and add\n> tests for the bug.\n> \n> Signed-off-by: Ævar Arnfjörð Bjarmason \n> ---\n> Makefile | 1 +\n> grep.c | 8 +++++-\n> grep.h | 4 +++\n> t/helper/test-pcre2-config.c | 12 +++++++++\n> t/helper/test-tool.c | 1 +\n> t/helper/test-tool.h | 1 +\n> t/t7812-grep-icase-non-ascii.sh | 46 ++++++++++++++++++++++++++++++++-\n> 7 files changed, 71 insertions(+), 2 deletions(-)\n> create mode 100644 t/helper/test-pcre2-config.c\n> \n> diff --git a/Makefile b/Makefile\n> index 4edfda3e00..42a7ed96e2 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -722,6 +722,7 @@ TEST_BUILTINS_OBJS += test-online-cpus.o\n> TEST_BUILTINS_OBJS += test-parse-options.o\n> TEST_BUILTINS_OBJS += test-parse-pathspec-file.o\n> TEST_BUILTINS_OBJS += test-path-utils.o\n> +TEST_BUILTINS_OBJS += test-pcre2-config.o\n> TEST_BUILTINS_OBJS += test-pkt-line.o\n> TEST_BUILTINS_OBJS += test-prio-queue.o\n> TEST_BUILTINS_OBJS += test-proc-receive.o\n> diff --git a/grep.c b/grep.c\n> index efeb6dc58d..e329f19877 100644\n> --- a/grep.c\n> +++ b/grep.c\n> @@ -492,7 +492,13 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt\n> \t}\n> \tif (!opt->ignore_locale && is_utf8_locale() && has_non_ascii(p->pattern) &&\n> \t !(!opt->ignore_case && (p->fixed || p->is_fixed)))\n> -\t\toptions |= PCRE2_UTF;\n> +\t\toptions |= (PCRE2_UTF | PCRE2_MATCH_INVALID_UTF);\n> +\n> +\tif (PCRE2_MATCH_INVALID_UTF &&\n> +\t options & (PCRE2_UTF | PCRE2_CASELESS) &&\n> +\t !(PCRE2_MAJOR >= 10 && PCRE2_MAJOR >= 36))\n ^^^^^^^^^^^^^^^^^^\nI assume that this should be s/_MAJOR/_MINOR/. ;-)\n\n> +\t\t/* Work around https://bugs.exim.org/show_bug.cgi?id=2642 fixed in 10.36 */\n> +\t\toptions |= PCRE2_NO_START_OPTIMIZE;\n> \n> \tp->pcre2_pattern = pcre2_compile((PCRE2_SPTR)p->pattern,\n> \t\t\t\t\t p->patternlen, options, &error, &erroffset,\n\nATB,\nRamsay Jones\n\n\n"},{"id":"415159","messageId":"3f32c0da-ddb9-d821-5d37-5c002b48f9f9@ramsayjones.plus.com","threadId":"51506","inReplyTo":"6fe69ede-d24b-1742-f699-c9af05560c0c@ramsayjones.plus.com","subject":"Re: [PATCH v4 2/2] grep/pcre2: better support invalid UTF-8 haystacks","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2021-01-24T14:24:26Z","receivedAt":"2021-01-24T14:25:33Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 24/01/2021 13:53, Ramsay Jones wrote:\n[snip]\n\n>> diff --git a/grep.c b/grep.c\n>> index efeb6dc58d..e329f19877 100644\n>> --- a/grep.c\n>> +++ b/grep.c\n>> @@ -492,7 +492,13 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt\n>> \t}\n>> \tif (!opt->ignore_locale && is_utf8_locale() && has_non_ascii(p->pattern) &&\n>> \t !(!opt->ignore_case && (p->fixed || p->is_fixed)))\n>> -\t\toptions |= PCRE2_UTF;\n>> +\t\toptions |= (PCRE2_UTF | PCRE2_MATCH_INVALID_UTF);\n>> +\n>> +\tif (PCRE2_MATCH_INVALID_UTF &&\n>> +\t options & (PCRE2_UTF | PCRE2_CASELESS) &&\n>> +\t !(PCRE2_MAJOR >= 10 && PCRE2_MAJOR >= 36))\n> ^^^^^^^^^^^^^^^^^^\n> I assume that this should be s/_MAJOR/_MINOR/. ;-)\n> \n\nAlthough, perhaps you want:\n\n !(((PCRE2_MAJOR * 100) + PCRE2_MINOR) >= 1036)\n\n... or something similar.\n\nATB,\nRamsay Jones\n\n\n\n"},{"id":"415160","messageId":"87a6sy75ka.fsf@evledraar.gmail.com","threadId":"51506","inReplyTo":"3f32c0da-ddb9-d821-5d37-5c002b48f9f9@ramsayjones.plus.com","subject":"Re: [PATCH v4 2/2] grep/pcre2: better support invalid UTF-8 haystacks","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-01-24T14:49:57Z","receivedAt":"2021-01-24T14:50:59Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Sun, Jan 24 2021, Ramsay Jones wrote:\n\n> On 24/01/2021 13:53, Ramsay Jones wrote:\n> [snip]\n>\n>>> diff --git a/grep.c b/grep.c\n>>> index efeb6dc58d..e329f19877 100644\n>>> --- a/grep.c\n>>> +++ b/grep.c\n>>> @@ -492,7 +492,13 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt\n>>> \t}\n>>> \tif (!opt->ignore_locale && is_utf8_locale() && has_non_ascii(p->pattern) &&\n>>> \t !(!opt->ignore_case && (p->fixed || p->is_fixed)))\n>>> -\t\toptions |= PCRE2_UTF;\n>>> +\t\toptions |= (PCRE2_UTF | PCRE2_MATCH_INVALID_UTF);\n>>> +\n>>> +\tif (PCRE2_MATCH_INVALID_UTF &&\n>>> +\t options & (PCRE2_UTF | PCRE2_CASELESS) &&\n>>> +\t !(PCRE2_MAJOR >= 10 && PCRE2_MAJOR >= 36))\n>> ^^^^^^^^^^^^^^^^^^\n>> I assume that this should be s/_MAJOR/_MINOR/. ;-)\n>> \n\nOops on the s/MAJOR/MINOR/g. Well spotted, I think I'll wait a bit more\nfor other comments for a re-roll.\n\nPerhaps Junio can be kind and do the s/_MAJOR/_MINOR/ fixup in the\nmeantime to save be from spamming the list too much...\n\nFWIW I have tested this on a verion without PCRE2_MATCH_INVALID_UTF, but\nI think I did that by manually editing the \"PCRE2_UTF\" line above, and\nthen wrote this bug.\n\n> Although, perhaps you want:\n>\n> !(((PCRE2_MAJOR * 100) + PCRE2_MINOR) >= 1036)\n>\n> ... or something similar.\n\nProbably better to use pcre2_config(PCRE2_CONFIG_VERSION) at that point\nand versioncmp() the string.\n\nFWIW we used this pattern (I added) before with PCRE, see e87de7cab4\n(grep: un-break building with PCRE < 8.32, 2017-05-25).\n\nAnd if you want to be clever ((PCRE2_MAJOR) | ((PCRE2_MINOR) << 16)) is\nprobably better, then you can get it out with bit operations :)\n\n\n"},{"id":"415162","messageId":"a75cfde9-0ac3-9af4-777c-1824063c6b0b@ramsayjones.plus.com","threadId":"51506","inReplyTo":"87a6sy75ka.fsf@evledraar.gmail.com","subject":"Re: [PATCH v4 2/2] grep/pcre2: better support invalid UTF-8 haystacks","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2021-01-24T16:10:15Z","receivedAt":"2021-01-24T16:20:35Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 24/01/2021 14:49, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Sun, Jan 24 2021, Ramsay Jones wrote:\n> \n>> On 24/01/2021 13:53, Ramsay Jones wrote:\n>> [snip]\n>>\n>>>> diff --git a/grep.c b/grep.c\n>>>> index efeb6dc58d..e329f19877 100644\n>>>> --- a/grep.c\n>>>> +++ b/grep.c\n>>>> @@ -492,7 +492,13 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt\n>>>> \t}\n>>>> \tif (!opt->ignore_locale && is_utf8_locale() && has_non_ascii(p->pattern) &&\n>>>> \t !(!opt->ignore_case && (p->fixed || p->is_fixed)))\n>>>> -\t\toptions |= PCRE2_UTF;\n>>>> +\t\toptions |= (PCRE2_UTF | PCRE2_MATCH_INVALID_UTF);\n>>>> +\n>>>> +\tif (PCRE2_MATCH_INVALID_UTF &&\n>>>> +\t options & (PCRE2_UTF | PCRE2_CASELESS) &&\n>>>> +\t !(PCRE2_MAJOR >= 10 && PCRE2_MAJOR >= 36))\n>>> ^^^^^^^^^^^^^^^^^^\n>>> I assume that this should be s/_MAJOR/_MINOR/. ;-)\n>>>\n> \n> Oops on the s/MAJOR/MINOR/g. Well spotted, I think I'll wait a bit more\n> for other comments for a re-roll.\n> \n> Perhaps Junio can be kind and do the s/_MAJOR/_MINOR/ fixup in the\n> meantime to save be from spamming the list too much...\n\nUmm, sorry for not making myself clear, _just_ changing MAJOR to\nMINOR is insufficient.\n\n> \n> FWIW I have tested this on a verion without PCRE2_MATCH_INVALID_UTF, but\n> I think I did that by manually editing the \"PCRE2_UTF\" line above, and\n> then wrote this bug.\n\nYep, I seem to have 10.34 on Linux Mint 20.1 (based on Ubuntu 20.04).\n\n> \n>> Although, perhaps you want:\n>>\n>> !(((PCRE2_MAJOR * 100) + PCRE2_MINOR) >= 1036)\n>>\n>> ... or something similar.\n> \n> Probably better to use pcre2_config(PCRE2_CONFIG_VERSION) at that point\n> and versioncmp() the string.\n\nOK, but it needs to be 'something similar' (try putting, say, MAJOR 11\nand MINOR 0->35 in your expression).\n\nATB,\nRamsay Jones\n"},{"id":"415174","messageId":"20210124172813.9547-1-avarab@gmail.com","threadId":"51506","inReplyTo":"20210124114855.13036-1-avarab@gmail.com","subject":"[PATCH v5 0/2] grep: better support invalid UTF-8 haystacks","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-01-24T17:28:11Z","receivedAt":"2021-01-24T17:29:38Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fixes a version comparison typo/thinko, pointed out by Ramsay Jones.\n\nÆvar Arnfjörð Bjarmason (2):\n grep/pcre2 tests: don't rely on invalid UTF-8 data test\n grep/pcre2: better support invalid UTF-8 haystacks\n\n Makefile | 1 +\n grep.c | 18 ++++++++++-\n grep.h | 4 +++\n t/helper/test-pcre2-config.c | 12 ++++++++\n t/helper/test-tool.c | 1 +\n t/helper/test-tool.h | 1 +\n t/t7812-grep-icase-non-ascii.sh | 53 ++++++++++++++++++++++++++++-----\n 7 files changed, 82 insertions(+), 8 deletions(-)\n create mode 100644 t/helper/test-pcre2-config.c\n\nRange-diff:\n1: 699bb6b324 = 1: 699bb6b324 grep/pcre2 tests: don't rely on invalid UTF-8 data test\n2: e4807d6879 ! 2: 04c87c04d7 grep/pcre2: better support invalid UTF-8 haystacks\n @@ grep.c: static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_\n -\t\toptions |= PCRE2_UTF;\n +\t\toptions |= (PCRE2_UTF | PCRE2_MATCH_INVALID_UTF);\n +\n -+\tif (PCRE2_MATCH_INVALID_UTF &&\n -+\t options & (PCRE2_UTF | PCRE2_CASELESS) &&\n -+\t !(PCRE2_MAJOR >= 10 && PCRE2_MAJOR >= 36))\n -+\t\t/* Work around https://bugs.exim.org/show_bug.cgi?id=2642 fixed in 10.36 */\n -+\t\toptions |= PCRE2_NO_START_OPTIMIZE;\n ++\t/* Work around https://bugs.exim.org/show_bug.cgi?id=2642 fixed in 10.36 */\n ++\tif (PCRE2_MATCH_INVALID_UTF && options & (PCRE2_UTF | PCRE2_CASELESS)) {\n ++\t\tstruct strbuf buf;\n ++\t\tint len;\n ++\t\tint err;\n ++\n ++\t\tif ((len = pcre2_config(PCRE2_CONFIG_VERSION, NULL)) < 0)\n ++\t\t\tBUG(\"pcre2_config(..., NULL) failed: %d\", len);\n ++\t\tstrbuf_init(&buf, len + 1);\n ++\t\tif ((err = pcre2_config(PCRE2_CONFIG_VERSION, buf.buf)) < 0)\n ++\t\t\tBUG(\"pcre2_config(..., buf.buf) failed: %d\", err);\n ++\t\tif (versioncmp(buf.buf, \"10.36\") < 0)\n ++\t\t\toptions |= PCRE2_NO_START_OPTIMIZE;\n ++\t\tstrbuf_release(&buf);\n ++\t}\n \n \tp->pcre2_pattern = pcre2_compile((PCRE2_SPTR)p->pattern,\n \t\t\t\t\t p->patternlen, options, &error, &erroffset,\n-- \n2.29.2.222.g5d2a92d10f8\n\n"},{"id":"415175","messageId":"20210124172813.9547-2-avarab@gmail.com","threadId":"51506","inReplyTo":"20210124114855.13036-1-avarab@gmail.com","subject":"[PATCH v5 1/2] grep/pcre2 tests: don't rely on invalid UTF-8 data test","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-01-24T17:28:12Z","receivedAt":"2021-01-24T17:29:38Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"As noted in [1] when I originally added this test in [2] the test was\ncompletely broken as it lacked a redirect[3]. I now think this whole\nthing is overly fragile. Let's only test if we have a segfault here.\n\nBefore this the first test's \"test_cmp\" was pretty meaningless. We\nwere only testing if PCREv2 was so broken that it would spew out\nsomething completely unrelated on stdout, which isn't very plausible.\n\nIn the second test we're relying on PCREv2 forever holding to the\ncurrent behavior of the PCRE_UTF8 flag, as opposed to learning some\noptimistic graceful fallback to PCRE2_MATCH_INVALID_UTF in the\nfuture. If that happens having this test broken under bisecting would\nsuck.\n\nA follow-up commit will actually test this case in a meaningful way\nunder the PCRE2_MATCH_INVALID_UTF flag. Let's run this one\nunconditionally, and just make sure we don't segfault.\n\n1. e714b898c6 (t7812: expect failure for grep -i with invalid UTF-8\n data, 2019-11-29)\n2. 8a5999838e (grep: stess test PCRE v2 on invalid UTF-8 data,\n 2019-07-26)\n3. c74b3cbb83 (t7812: add missing redirects, 2019-11-26)\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t7812-grep-icase-non-ascii.sh | 7 +------\n 1 file changed, 1 insertion(+), 6 deletions(-)\n\ndiff --git a/t/t7812-grep-icase-non-ascii.sh b/t/t7812-grep-icase-non-ascii.sh\nindex 03dba6685a..38457c2e4f 100755\n--- a/t/t7812-grep-icase-non-ascii.sh\n+++ b/t/t7812-grep-icase-non-ascii.sh\n@@ -76,12 +76,7 @@ test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: grep non-ASCII from invali\n \n test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: grep non-ASCII from invalid UTF-8 data with -i' '\n \ttest_might_fail git grep -hi \"Æ\" invalid-0x80 >actual &&\n-\tif test -s actual\n-\tthen\n-\t test_cmp expected actual\n-\tfi &&\n-\ttest_must_fail git grep -hi \"(*NO_JIT)Æ\" invalid-0x80 >actual &&\n-\t! test_cmp expected actual\n+\ttest_might_fail git grep -hi \"(*NO_JIT)Æ\" invalid-0x80 >actual\n '\n \n test_done\n-- \n2.29.2.222.g5d2a92d10f8\n\n"},{"id":"415176","messageId":"20210124172813.9547-3-avarab@gmail.com","threadId":"51506","inReplyTo":"20210124114855.13036-1-avarab@gmail.com","subject":"[PATCH v5 2/2] grep/pcre2: better support invalid UTF-8 haystacks","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-01-24T17:28:13Z","receivedAt":"2021-01-24T17:29:38Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Improve the support for invalid UTF-8 haystacks given a non-ASCII\nneedle when using the PCREv2 backend.\n\nThis is a more complete fix for a bug I started to fix in\n870eea8166 (grep: do not enter PCRE2_UTF mode on fixed matching,\n2019-07-26), now that PCREv2 has the PCRE2_MATCH_INVALID_UTF mode we\ncan make use of it.\n\nThis fixes the sort of case described in 8a5999838e (grep: stess test\nPCRE v2 on invalid UTF-8 data, 2019-07-26), i.e.:\n\n - The subject string is non-ASCII (e.g. \"ævar\")\n - We're under a is_utf8_locale(), e.g. \"en_US.UTF-8\", not \"C\"\n - We are using --ignore-case, or we're a non-fixed pattern\n\nIf those conditions were satisfied and we matched found non-valid\nUTF-8 data PCREv2 might bark on it, in practice this only happened\nunder the JIT backend (turned on by default on most platforms).\n\nUltimately this fixes a \"regression\" in b65abcafc7 (\"grep: use PCRE v2\nfor optimized fixed-string search\", 2019-07-01), I'm putting that in\nscare-quotes because before then we wouldn't properly support these\ncomplex case-folding, locale etc. cases either, it just broke in\ndifferent ways.\n\nThere was a bug related to this the PCRE2_NO_START_OPTIMIZE flag fixed\nin PCREv2 10.36. It can be worked around by setting the\nPCRE2_NO_START_OPTIMIZE flag. Let's do that in those cases, and add\ntests for the bug.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n Makefile | 1 +\n grep.c | 18 ++++++++++++-\n grep.h | 4 +++\n t/helper/test-pcre2-config.c | 12 +++++++++\n t/helper/test-tool.c | 1 +\n t/helper/test-tool.h | 1 +\n t/t7812-grep-icase-non-ascii.sh | 46 ++++++++++++++++++++++++++++++++-\n 7 files changed, 81 insertions(+), 2 deletions(-)\n create mode 100644 t/helper/test-pcre2-config.c\n\ndiff --git a/Makefile b/Makefile\nindex 4edfda3e00..42a7ed96e2 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -722,6 +722,7 @@ TEST_BUILTINS_OBJS += test-online-cpus.o\n TEST_BUILTINS_OBJS += test-parse-options.o\n TEST_BUILTINS_OBJS += test-parse-pathspec-file.o\n TEST_BUILTINS_OBJS += test-path-utils.o\n+TEST_BUILTINS_OBJS += test-pcre2-config.o\n TEST_BUILTINS_OBJS += test-pkt-line.o\n TEST_BUILTINS_OBJS += test-prio-queue.o\n TEST_BUILTINS_OBJS += test-proc-receive.o\ndiff --git a/grep.c b/grep.c\nindex efeb6dc58d..ad0af66a26 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -492,7 +492,23 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt\n \t}\n \tif (!opt->ignore_locale && is_utf8_locale() && has_non_ascii(p->pattern) &&\n \t !(!opt->ignore_case && (p->fixed || p->is_fixed)))\n-\t\toptions |= PCRE2_UTF;\n+\t\toptions |= (PCRE2_UTF | PCRE2_MATCH_INVALID_UTF);\n+\n+\t/* Work around https://bugs.exim.org/show_bug.cgi?id=2642 fixed in 10.36 */\n+\tif (PCRE2_MATCH_INVALID_UTF && options & (PCRE2_UTF | PCRE2_CASELESS)) {\n+\t\tstruct strbuf buf;\n+\t\tint len;\n+\t\tint err;\n+\n+\t\tif ((len = pcre2_config(PCRE2_CONFIG_VERSION, NULL)) < 0)\n+\t\t\tBUG(\"pcre2_config(..., NULL) failed: %d\", len);\n+\t\tstrbuf_init(&buf, len + 1);\n+\t\tif ((err = pcre2_config(PCRE2_CONFIG_VERSION, buf.buf)) < 0)\n+\t\t\tBUG(\"pcre2_config(..., buf.buf) failed: %d\", err);\n+\t\tif (versioncmp(buf.buf, \"10.36\") < 0)\n+\t\t\toptions |= PCRE2_NO_START_OPTIMIZE;\n+\t\tstrbuf_release(&buf);\n+\t}\n \n \tp->pcre2_pattern = pcre2_compile((PCRE2_SPTR)p->pattern,\n \t\t\t\t\t p->patternlen, options, &error, &erroffset,\ndiff --git a/grep.h b/grep.h\nindex b5c4e223a8..ade21c8812 100644\n--- a/grep.h\n+++ b/grep.h\n@@ -18,6 +18,10 @@ typedef int pcre2_code;\n typedef int pcre2_match_data;\n typedef int pcre2_compile_context;\n #endif\n+#ifndef PCRE2_MATCH_INVALID_UTF\n+/* PCRE2_MATCH_* dummy also with !USE_LIBPCRE2, for test-pcre2-config.c */\n+#define PCRE2_MATCH_INVALID_UTF 0\n+#endif\n #include \"thread-utils.h\"\n #include \"userdiff.h\"\n \ndiff --git a/t/helper/test-pcre2-config.c b/t/helper/test-pcre2-config.c\nnew file mode 100644\nindex 0000000000..5258fdddba\n--- /dev/null\n+++ b/t/helper/test-pcre2-config.c\n@@ -0,0 +1,12 @@\n+#include \"test-tool.h\"\n+#include \"cache.h\"\n+#include \"grep.h\"\n+\n+int cmd__pcre2_config(int argc, const char **argv)\n+{\n+\tif (argc == 2 && !strcmp(argv[1], \"has-PCRE2_MATCH_INVALID_UTF\")) {\n+\t\tint value = PCRE2_MATCH_INVALID_UTF;\n+\t\treturn !value;\n+\t}\n+\treturn 1;\n+}\ndiff --git a/t/helper/test-tool.c b/t/helper/test-tool.c\nindex 9d6d14d929..f97cd9f48a 100644\n--- a/t/helper/test-tool.c\n+++ b/t/helper/test-tool.c\n@@ -46,6 +46,7 @@ static struct test_cmd cmds[] = {\n \t{ \"parse-options\", cmd__parse_options },\n \t{ \"parse-pathspec-file\", cmd__parse_pathspec_file },\n \t{ \"path-utils\", cmd__path_utils },\n+\t{ \"pcre2-config\", cmd__pcre2_config },\n \t{ \"pkt-line\", cmd__pkt_line },\n \t{ \"prio-queue\", cmd__prio_queue },\n \t{ \"proc-receive\", cmd__proc_receive},\ndiff --git a/t/helper/test-tool.h b/t/helper/test-tool.h\nindex a6470ff62c..28072c0ad5 100644\n--- a/t/helper/test-tool.h\n+++ b/t/helper/test-tool.h\n@@ -35,6 +35,7 @@ int cmd__online_cpus(int argc, const char **argv);\n int cmd__parse_options(int argc, const char **argv);\n int cmd__parse_pathspec_file(int argc, const char** argv);\n int cmd__path_utils(int argc, const char **argv);\n+int cmd__pcre2_config(int argc, const char **argv);\n int cmd__pkt_line(int argc, const char **argv);\n int cmd__prio_queue(int argc, const char **argv);\n int cmd__proc_receive(int argc, const char **argv);\ndiff --git a/t/t7812-grep-icase-non-ascii.sh b/t/t7812-grep-icase-non-ascii.sh\nindex 38457c2e4f..e5d1e4ea68 100755\n--- a/t/t7812-grep-icase-non-ascii.sh\n+++ b/t/t7812-grep-icase-non-ascii.sh\n@@ -57,7 +57,12 @@ test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: setup invalid UTF-8 data'\n \tprintf \"\\\\200\\\\n\" >invalid-0x80 &&\n \techo \"ævar\" >expected &&\n \tcat expected >>invalid-0x80 &&\n-\tgit add invalid-0x80\n+\tgit add invalid-0x80 &&\n+\n+\t# Test for PCRE2_MATCH_INVALID_UTF bug\n+\t# https://bugs.exim.org/show_bug.cgi?id=2642\n+\tprintf \"\\\\345Aæ\\\\n\" >invalid-0xe5 &&\n+\tgit add invalid-0xe5\n '\n \n test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: grep ASCII from invalid UTF-8 data' '\n@@ -67,6 +72,13 @@ test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: grep ASCII from invalid UT\n \ttest_cmp expected actual\n '\n \n+test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: grep ASCII from invalid UTF-8 data (PCRE2 bug #2642)' '\n+\tgit grep -h \"Aæ\" invalid-0xe5 >actual &&\n+\ttest_cmp invalid-0xe5 actual &&\n+\tgit grep -h \"(*NO_JIT)Aæ\" invalid-0xe5 >actual &&\n+\ttest_cmp invalid-0xe5 actual\n+'\n+\n test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: grep non-ASCII from invalid UTF-8 data' '\n \tgit grep -h \"æ\" invalid-0x80 >actual &&\n \ttest_cmp expected actual &&\n@@ -74,9 +86,41 @@ test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: grep non-ASCII from invali\n \ttest_cmp expected actual\n '\n \n+test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: grep non-ASCII from invalid UTF-8 data (PCRE2 bug #2642)' '\n+\tgit grep -h \"Aæ\" invalid-0xe5 >actual &&\n+\ttest_cmp invalid-0xe5 actual &&\n+\tgit grep -h \"(*NO_JIT)Aæ\" invalid-0xe5 >actual &&\n+\ttest_cmp invalid-0xe5 actual\n+'\n+\n+test_lazy_prereq PCRE2_MATCH_INVALID_UTF '\n+\ttest-tool pcre2-config has-PCRE2_MATCH_INVALID_UTF\n+'\n+\n test_expect_success GETTEXT_LOCALE,LIBPCRE2 'PCRE v2: grep non-ASCII from invalid UTF-8 data with -i' '\n \ttest_might_fail git grep -hi \"Æ\" invalid-0x80 >actual &&\n \ttest_might_fail git grep -hi \"(*NO_JIT)Æ\" invalid-0x80 >actual\n '\n \n+test_expect_success GETTEXT_LOCALE,LIBPCRE2,PCRE2_MATCH_INVALID_UTF 'PCRE v2: grep non-ASCII from invalid UTF-8 data with -i' '\n+\tgit grep -hi \"Æ\" invalid-0x80 >actual &&\n+\ttest_cmp expected actual &&\n+\tgit grep -hi \"(*NO_JIT)Æ\" invalid-0x80 >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success GETTEXT_LOCALE,LIBPCRE2,PCRE2_MATCH_INVALID_UTF 'PCRE v2: grep non-ASCII from invalid UTF-8 data with -i (PCRE2 bug #2642)' '\n+\tgit grep -hi \"Æ\" invalid-0xe5 >actual &&\n+\ttest_cmp invalid-0xe5 actual &&\n+\tgit grep -hi \"(*NO_JIT)Æ\" invalid-0xe5 >actual &&\n+\ttest_cmp invalid-0xe5 actual &&\n+\n+\t# Only the case of grepping the ASCII part in a way that\n+\t# relies on -i fails\n+\tgit grep -hi \"aÆ\" invalid-0xe5 >actual &&\n+\ttest_cmp invalid-0xe5 actual &&\n+\tgit grep -hi \"(*NO_JIT)aÆ\" invalid-0xe5 >actual &&\n+\ttest_cmp invalid-0xe5 actual\n+'\n+\n test_done\n-- \n2.29.2.222.g5d2a92d10f8\n\n"},{"id":"415177","messageId":"877do26y6x.fsf@evledraar.gmail.com","threadId":"51506","inReplyTo":"a75cfde9-0ac3-9af4-777c-1824063c6b0b@ramsayjones.plus.com","subject":"Re: [PATCH v4 2/2] grep/pcre2: better support invalid UTF-8 haystacks","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-01-24T17:29:10Z","receivedAt":"2021-01-24T17:30:14Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Sun, Jan 24 2021, Ramsay Jones wrote:\n\n> On 24/01/2021 14:49, Ævar Arnfjörð Bjarmason wrote:\n>> \n>> On Sun, Jan 24 2021, Ramsay Jones wrote:\n>> \n>>> On 24/01/2021 13:53, Ramsay Jones wrote:\n>>> [snip]\n>>>\n>>>>> diff --git a/grep.c b/grep.c\n>>>>> index efeb6dc58d..e329f19877 100644\n>>>>> --- a/grep.c\n>>>>> +++ b/grep.c\n>>>>> @@ -492,7 +492,13 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt\n>>>>> \t}\n>>>>> \tif (!opt->ignore_locale && is_utf8_locale() && has_non_ascii(p->pattern) &&\n>>>>> \t !(!opt->ignore_case && (p->fixed || p->is_fixed)))\n>>>>> -\t\toptions |= PCRE2_UTF;\n>>>>> +\t\toptions |= (PCRE2_UTF | PCRE2_MATCH_INVALID_UTF);\n>>>>> +\n>>>>> +\tif (PCRE2_MATCH_INVALID_UTF &&\n>>>>> +\t options & (PCRE2_UTF | PCRE2_CASELESS) &&\n>>>>> +\t !(PCRE2_MAJOR >= 10 && PCRE2_MAJOR >= 36))\n>>>> ^^^^^^^^^^^^^^^^^^\n>>>> I assume that this should be s/_MAJOR/_MINOR/. ;-)\n>>>>\n>> \n>> Oops on the s/MAJOR/MINOR/g. Well spotted, I think I'll wait a bit more\n>> for other comments for a re-roll.\n>> \n>> Perhaps Junio can be kind and do the s/_MAJOR/_MINOR/ fixup in the\n>> meantime to save be from spamming the list too much...\n>\n> Umm, sorry for not making myself clear, _just_ changing MAJOR to\n> MINOR is insufficient.\n>\n>> \n>> FWIW I have tested this on a verion without PCRE2_MATCH_INVALID_UTF, but\n>> I think I did that by manually editing the \"PCRE2_UTF\" line above, and\n>> then wrote this bug.\n>\n> Yep, I seem to have 10.34 on Linux Mint 20.1 (based on Ubuntu 20.04).\n>\n>> \n>>> Although, perhaps you want:\n>>>\n>>> !(((PCRE2_MAJOR * 100) + PCRE2_MINOR) >= 1036)\n>>>\n>>> ... or something similar.\n>> \n>> Probably better to use pcre2_config(PCRE2_CONFIG_VERSION) at that point\n>> and versioncmp() the string.\n>\n> OK, but it needs to be 'something similar' (try putting, say, MAJOR 11\n> and MINOR 0->35 in your expression).\n\nAh yes, of course. I re-rolled a v5 with a fix for that. Thanks.\n"}]}