{"thread":{"id":"62332","subject":"git grep does not find all occurrences on macOS","startedAt":"2024-10-14T13:34:14Z","lastAt":"2024-10-22T05:58:41Z","messageCount":6,"participants":["David Gstir","Taylor Blau","René Scharfe"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"505018","messageId":"771C9C4E-45F1-4F71-B3A9-4E8E4A9CAC1F@sigma-star.at","threadId":"62332","inReplyTo":null,"subject":"git grep does not find all occurrences on macOS","fromName":"David Gstir","fromEmail":"david@sigma-star.at","sentAt":"2024-10-14T13:34:02Z","receivedAt":"2024-10-14T13:34:14Z","isPatch":false,"sender":{"key":"david@sigma-star.at","avatar":null},"body":"Hi!\n\nI encountered rather subtle issue on in git 2.47.0 on macOS 14.7 (installed from Homebrew):\n\ngit grep will not find all occurrences of string patterns containing a “.” under some\nconditions. In my case I have an ISO-8859 encoded text file which contains umlauts.\nIf the string I’m grepping for occurs after a non-ASCII character in this file, git grep\nwill not find it.\n\nI’ve put up a reproducer here https://github.com/iokill/repro-git-grep-issue, but the gist\nof it is \"git grep quz.baz\" on the ISO-8859-encoded file below will not return anything,\nwhen it should return the line \"quz.baz=3\":\n\n-->8-------------\nfoo=bar\numlauts=äöü\nquz.baz=3 \n--8<-------------\n\nFrom what I’ve found so far, this occurs on macOS, but not on Linux and it has to be connected\nto the regex matching code, because the issue does not occur with --fixed-strings or --perl-regexp.\n\nThanks!\n- David"},{"id":"505099","messageId":"Zw22sTxyWCbczZy8@nand.local","threadId":"62332","inReplyTo":"771C9C4E-45F1-4F71-B3A9-4E8E4A9CAC1F@sigma-star.at","subject":"Re: git grep does not find all occurrences on macOS","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2024-10-15T00:26:25Z","receivedAt":"2024-10-15T00:26:32Z","isPatch":false,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Mon, Oct 14, 2024 at 03:34:02PM +0200, David Gstir wrote:\n> Hi!\n>\n> I encountered rather subtle issue on in git 2.47.0 on macOS 14.7 (installed from Homebrew):\n>\n> git grep will not find all occurrences of string patterns containing a “.” under some\n> conditions. In my case I have an ISO-8859 encoded text file which contains umlauts.\n> If the string I’m grepping for occurs after a non-ASCII character in this file, git grep\n> will not find it.\n>\n> I’ve put up a reproducer here https://github.com/iokill/repro-git-grep-issue, but the gist\n> of it is \"git grep quz.baz\" on the ISO-8859-encoded file below will not return anything,\n> when it should return the line \"quz.baz=3\":\n\nInteresting. I have a macOS system, but don't build Git on it regularly\nenough to test.\n\nI CC'd some other active contributors who have recently touched grep.c,\nperhaps they may have a better insight into what is going on here.\n\nThanks,\nTaylor\n"},{"id":"505207","messageId":"0eaf6c1c-7205-4897-ba19-e5ba03194012@web.de","threadId":"62332","inReplyTo":"Zw22sTxyWCbczZy8@nand.local","subject":"Re: git grep does not find all occurrences on macOS","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2024-10-15T20:15:27Z","receivedAt":"2024-10-15T20:20:56Z","isPatch":false,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 15.10.24 um 02:26 schrieb Taylor Blau:\n> On Mon, Oct 14, 2024 at 03:34:02PM +0200, David Gstir wrote:\n>> Hi!\n>>\n>> I encountered rather subtle issue on in git 2.47.0 on macOS 14.7 (installed from Homebrew):\n>>\n>> git grep will not find all occurrences of string patterns containing a “.” under some\n>> conditions. In my case I have an ISO-8859 encoded text file which contains umlauts.\n>> If the string I’m grepping for occurs after a non-ASCII character in this file, git grep\n>> will not find it.\n>>\n>> I’ve put up a reproducer here https://github.com/iokill/repro-git-grep-issue, but the gist\n>> of it is \"git grep quz.baz\" on the ISO-8859-encoded file below will not return anything,\n>> when it should return the line \"quz.baz=3\":\n\nCan reproduce on macOS 15.0.1. Bisects to 1819ad327b (grep: fix\nmultibyte regex handling under macOS, 2022-08-26), which enabled the use\nof the system's regex engine. grep(1) does find that line.\n\nregexec(3) returns REG_ILLSEQ (illegal byte sequence) for that file,\nwhich makes sense. Interpreting that result as a non-match of the\nwhole file is not the best way to handle it, though. Reporting the\nerror would be one option. Turning off lookahead and matching each line\nseparately might be better.\n\nWould setting the attribute working-tree-encoding help here? Not fully:\nThe file would be converted to UTF-8 before commit, but git grep without\na tree argument would still read the raw file, without conversion.\nShouldn't it respect the attribute and call convert_to_git()?\n\nUsing -P to use Perl regular expressions would work in the example.\n\nRené\n\n"},{"id":"505531","messageId":"7a51a68e-5f9d-4444-a568-9ca180bc4c6b@web.de","threadId":"62332","inReplyTo":"0eaf6c1c-7205-4897-ba19-e5ba03194012@web.de","subject":"[PATCH] grep: disable lookahead on error","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2024-10-20T11:02:32Z","receivedAt":"2024-10-20T11:02:54Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"regexec(3) can fail. E.g. on macOS it fails if it is used with an UTF-8\nlocale to match a valid regex against a buffer containing invalid UTF-8\ncharacters.\n\ngit grep has two ways to search for matches in a file: Either it splits\nits contents into lines and matches them separately, or it matches the\nwhole content and figures out line boundaries later. The latter is done\nby look_ahead() and it's quicker in the common case where most files\ndon't contain a match.\n\nFall back to line-by-line matching if look_ahead() encounters an\nregexec(3) error by propagating errors out of patmatch() and bailing out\nof look_ahead() if there is one. This way we at least can find matches\nin lines that contain only valid characters. That matches the behavior\nof grep(1) on macOS.\n\npcre2match() dies if pcre2_jit_match() or pcre2_match() fail, but since\nwe use the flag PCRE2_MATCH_INVALID_UTF it handles invalid UTF-8\ncharacters gracefully. So implement the fall-back only for regexec(3)\nand leave the PCRE2 matching unchanged.\n\nReported-by: David Gstir \nSigned-off-by: René Scharfe \n---\n grep.c | 30 ++++++++++++++++++++----------\n t/t7810-grep.sh | 9 +++++++++\n 2 files changed, 29 insertions(+), 10 deletions(-)\n\ndiff --git a/grep.c b/grep.c\nindex 701e58de04..506f0423c8 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -906,15 +906,17 @@ static int patmatch(struct grep_pat *p,\n \t\t const char *line, const char *eol,\n \t\t regmatch_t *match, int eflags)\n {\n-\tint hit;\n-\n \tif (p->pcre2_pattern)\n-\t\thit = !pcre2match(p, line, eol, match, eflags);\n-\telse\n-\t\thit = !regexec_buf(&p->regexp, line, eol - line, 1, match,\n-\t\t\t\t eflags);\n+\t\treturn !pcre2match(p, line, eol, match, eflags);\n\n-\treturn hit;\n+\tswitch (regexec_buf(&p->regexp, line, eol - line, 1, match, eflags)) {\n+\tcase 0:\n+\t\treturn 1;\n+\tcase REG_NOMATCH:\n+\t\treturn 0;\n+\tdefault:\n+\t\treturn -1;\n+\t}\n }\n\n static void strip_timestamp(const char *bol, const char **eol_p)\n@@ -952,6 +954,8 @@ static int headerless_match_one_pattern(struct grep_pat *p,\n\n again:\n \thit = patmatch(p, bol, eol, pmatch, eflags);\n+\tif (hit < 0)\n+\t\thit = 0;\n\n \tif (hit && p->word_regexp) {\n \t\tif ((pmatch[0].rm_so < 0) ||\n@@ -1461,6 +1465,8 @@ static int look_ahead(struct grep_opt *opt,\n \t\tregmatch_t m;\n\n \t\thit = patmatch(p, bol, bol + *left_p, &m, 0);\n+\t\tif (hit < 0)\n+\t\t\treturn -1;\n \t\tif (!hit || m.rm_so < 0 || m.rm_eo < 0)\n \t\t\tcontinue;\n \t\tif (earliest < 0 || m.rm_so < earliest)\n@@ -1655,9 +1661,13 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle\n \t\tif (try_lookahead\n \t\t && !(last_hit\n \t\t\t && (show_function ||\n-\t\t\t lno <= last_hit + opt->post_context))\n-\t\t && look_ahead(opt, &left, &lno, &bol))\n-\t\t\tbreak;\n+\t\t\t lno <= last_hit + opt->post_context))) {\n+\t\t\thit = look_ahead(opt, &left, &lno, &bol);\n+\t\t\tif (hit < 0)\n+\t\t\t\ttry_lookahead = 0;\n+\t\t\telse if (hit)\n+\t\t\t\tbreak;\n+\t\t}\n \t\teol = end_of_line(bol, &left);\n\n \t\tif ((ctx == GREP_CONTEXT_HEAD) && (eol == bol))\ndiff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\nindex af2cf2f78a..64ac4f04ee 100755\n--- a/t/t7810-grep.sh\n+++ b/t/t7810-grep.sh\n@@ -87,6 +87,7 @@ test_expect_success setup '\n \t# Still a no-op.\n \tfunction dummy() {}\n \tEOF\n+\tprintf \"\\200\\nASCII\\n\" >invalid-utf8 &&\n \tif test_have_prereq FUNNYNAMES\n \tthen\n \t\techo unusual >\"\\\"unusual\\\" pathname\" &&\n@@ -534,6 +535,14 @@ do\n \t\ttest_cmp expected actual\n \t'\n\n+\ttest_expect_success \"grep $L searches past invalid lines on UTF-8 locale\" '\n+\t\tLC_ALL=en_US.UTF-8 git grep A. invalid-utf8 >actual &&\n+\t\tcat >expected <<-EOF &&\n+\t\tinvalid-utf8:ASCII\n+\t\tEOF\n+\t\ttest_cmp expected actual\n+\t'\n+\n \ttest_expect_success FUNNYNAMES \"grep $L should quote unusual pathnames\" '\n \t\tcat >expected <<-EOF &&\n \t\t${HC}\"\\\"unusual\\\" pathname\":unusual\n--\n2.47.0\n"},{"id":"505785","messageId":"ZxbOQXNn7Y5v2MPB@nand.local","threadId":"62332","inReplyTo":"7a51a68e-5f9d-4444-a568-9ca180bc4c6b@web.de","subject":"Re: [PATCH] grep: disable lookahead on error","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2024-10-21T21:57:21Z","receivedAt":"2024-10-21T21:57:23Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Sun, Oct 20, 2024 at 01:02:32PM +0200, René Scharfe wrote:\n> ---\n> grep.c | 30 ++++++++++++++++++++----------\n> t/t7810-grep.sh | 9 +++++++++\n> 2 files changed, 29 insertions(+), 10 deletions(-)\n\nThanks, René. The patch looks good to me, but it would be nice to hear\nfrom David who could confirm that it fixes the issue on his end as well.\n\nThanks,\nTaylor\n"},{"id":"505808","messageId":"494E7060-B824-4663-B49D-A645A8A67711@sigma-star.at","threadId":"62332","inReplyTo":"7a51a68e-5f9d-4444-a568-9ca180bc4c6b@web.de","subject":"Re: [PATCH] grep: disable lookahead on error","fromName":"David Gstir","fromEmail":"david@sigma-star.at","sentAt":"2024-10-22T05:58:28Z","receivedAt":"2024-10-22T05:58:41Z","isPatch":true,"sender":{"key":"david@sigma-star.at","avatar":null},"body":"Hi René,\n\n> On 20.10.2024, at 13:02, René Scharfe wrote:\n> \n> regexec(3) can fail. E.g. on macOS it fails if it is used with an UTF-8\n> locale to match a valid regex against a buffer containing invalid UTF-8\n> characters.\n> \n> git grep has two ways to search for matches in a file: Either it splits\n> its contents into lines and matches them separately, or it matches the\n> whole content and figures out line boundaries later. The latter is done\n> by look_ahead() and it's quicker in the common case where most files\n> don't contain a match.\n> \n> Fall back to line-by-line matching if look_ahead() encounters an\n> regexec(3) error by propagating errors out of patmatch() and bailing out\n> of look_ahead() if there is one. This way we at least can find matches\n> in lines that contain only valid characters. That matches the behavior\n> of grep(1) on macOS.\n> \n> pcre2match() dies if pcre2_jit_match() or pcre2_match() fail, but since\n> we use the flag PCRE2_MATCH_INVALID_UTF it handles invalid UTF-8\n> characters gracefully. So implement the fall-back only for regexec(3)\n> and leave the PCRE2 matching unchanged.\n> \n> Reported-by: David Gstir \n> Signed-off-by: René Scharfe \n\nthanks for fixing this! I’ve tested it on my end and your patch works. Feel free to add my Tested-By.\n\nThanks,\nDavid"}]}