git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] grep: skip UTF8 checks explicitally

From
Carlo Arenas <carenas@gmail.com>
Date
Jul 23, 2019, 03:50 UTC
Message-ID
<CAPUEspg1nUoPApTk5J2r_-9psxTTSC7nRAPw_X9no+2sFVSxAA@mail.gmail.com>
In-Reply-To
<87muh57t5r.fsf@evledraar.gmail.com>

On Mon, Jul 22, 2019 at 12:42 PM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

Show 7 quoted lines
>
> On Mon, Jul 22 2019, Johannes Schindelin wrote:
>
> > So I am fine with this patch.
>
> I'm not, not because I dislike the approach. I haven't made up my mind
> yet.

my bad, I should had explained better the regression I was trying to fix with this patch :

$ git version git version 2.22.0.709.g102302147b.dirty $ git grep "Nguyễn Thái" .mailmap:Nguyễn Thái Ngọc Duy <pclouds@gmail.com> fatal: pcre2_match failed with error code -22: UTF-8 error: isolated byte with 0x80 bit set $ git grep -I "Nguyễn Thái" .mailmap:Nguyễn Thái Ngọc Duy <pclouds@gmail.com> po/TEAMS:Members: Nguyễn Thái Ngọc Duy <pclouds AT gmail.com> po/vi.po:# Nguyễn Thái Ngọc Duy <pclouds@gmail.com>, 2012. t/t1302-repo-version.sh:# Copyright (c) 2007 Nguyễn Thái Ngọc Duy t/t2104-update-index-skip-worktree.sh:# Copyright (c) 2008 Nguyễn Thái Ngọc Duy fatal: pcre2_match failed with error code -8: UTF-8 error: byte 2 top bits not 0x80

Show 7 quoted lines
> I stopped paying attention to this error-with-not-JIT discussion when I
> heard that some other series going into next for Windows fixed that
> issue[1]
>
> But now we have it again in some form? My ab/no-kwset has a lot of tests
> for encodings & locales combined with grep, don't some of those trigger
> this? If so we should make any such failure a test & part of this patch.

I don't have a windows environment to test, and I admit I wasn't following clearly the whole conversation but at least in my case, I never got any test to fail, and I haven't seen any test with broken UTF-8.

I apologize for not sending a test and will correct that, but I am concerned that according to PCRE documentation the behaviour is undefined and therefore the test might be flacky.

> Right now we don't have the info of whether we're really using the JIT
> or not, but that would be easy to add to grep's --debug mode for use in
> a test prereq.

I would think that extending testtool would be a better option, I have indeed a patch checking for UTF-8 support using git directly which I hadn't sent just because I thought it looked too hacky, but agree it will be nice to have all those dependencies clear and the corresponding tests otherwise

It is interesting to note that because of the conservative way we enable UTF-8 then it will depend on several factors if the problematic code gets triggered

> As noted in [2] I'd be inclined to go the other way, if we indeed have
> some cases where PCRE skips its own checks does not dying actually give
> us anything useful? I'd think not, so just ignoring the issue seems like
> the wrong thing to do.

we could reenable the checks by moving out of the JIT fast path in pcre so that pcre2_match is used for all matches (with JIT compiled as an optimization) and that might have the added benefit of solving the PCRE errors when JIT is broken[1]

$ git version git version 2.22.0 $ git grep "Nguyễn Thái" .mailmap:Nguyễn Thái Ngọc Duy <pclouds@gmail.com> po/TEAMS:Members: Nguyễn Thái Ngọc Duy <pclouds AT gmail.com> po/vi.po:# Nguyễn Thái Ngọc Duy <pclouds@gmail.com>, 2012. t/t1302-repo-version.sh:# Copyright (c) 2007 Nguyễn Thái Ngọc Duy t/t2104-update-index-skip-worktree.sh:# Copyright (c) 2008 Nguyễn Thái Ngọc Duy t/t7011-skip-worktree-reading.sh:# Copyright (c) 2008 Nguyễn Thái Ngọc Duy t/t7012-skip-worktree-writing.sh:# Copyright (c) 2008 Nguyễn Thái Ngọc Duy

important to note that at least on my system (macOS 10.14.6) the output of all engines for grep is the same for a UTF-8 pattern match, even if using invalid UTF-8 in the corpus, and I would expect that to be a better indicative of correctness

FWIW GNU grep (with -P) also ignores UTF-8 errors using the same flag and even PCRE seems to be going in the other direction with the recent addition of PCRE2_MATCH_INVALID_UTF[1]

$ od -b int.p 0000000 102 145 154 303 263 156 012 303 $ pcre2grep -U 'Beló' int.p Belón

Carlo

[1] https://public-inbox.org/git/20181209230024.43444-1-carenas@gmail.com/ [2] https://lists.exim.org/lurker/message/20190528.141422.2af1e386.gl.html

Previous: Ævar Arnfjörð BjarmasonNext: Johannes Schindelin
Message 13 of 26 in “grep: skip UTF8 checks explicitally”
  1. grep: skip UTF8 checks explicitallyCarlo Marcelo Arenas Belón, Jul 21, 2019
  2. Eric SunshineJul 21, 2019
  3. Johannes SchindelinJul 22, 2019
  4. grep: skip UTF8 checks explicitlyCarlo Marcelo Arenas Belón, Jul 22, 2019
  5. Junio C HamanoJul 24, 2019
  6. Johannes SchindelinJul 24, 2019
  7. Junio C HamanoJul 24, 2019
  8. grep: skip UTF8 checks explicitlyCarlo Marcelo Arenas Belón, Aug 28, 2019
  9. Carlo ArenasAug 28, 2019
  10. Carlo ArenasSep 9, 2019
  11. Junio C HamanoSep 9, 2019
  12. Ævar Arnfjörð BjarmasonJul 22, 2019
  13. Carlo ArenasJul 23, 2019
  14. Johannes SchindelinJul 23, 2019
  15. Carlo ArenasJul 24, 2019
  16. Johannes SchindelinJul 24, 2019
  17. Ævar Arnfjörð BjarmasonJul 24, 2019
  18. Junio C HamanoJul 24, 2019
  19. Johannes SchindelinJul 25, 2019
  20. Junio C HamanoJul 25, 2019
  21. Johannes SchindelinJul 25, 2019
  22. Ævar Arnfjörð BjarmasonJul 26, 2019
  23. Carlo ArenasJul 26, 2019
  24. Ævar Arnfjörð BjarmasonJul 26, 2019
  25. Junio C HamanoJul 26, 2019
  26. Ævar Arnfjörð BjarmasonJul 26, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.