From: René Scharfe <hidden> Date: 2021-12-18 19:50:28
compile_pcre2_pattern() currently uses the option PCRE2_UTF only for
patterns with non-ASCII characters. Patterns with ASCII wildcards can
match non-ASCII strings, though. Without that option PCRE2 mishandles
UTF-8 input, though -- it matches parts of multi-byte characters. Fix
that by using PCRE2_UTF even for ASCII-only patterns.
This is a remake of the reverted ae39ba431a (grep/pcre2: fix an edge
case concerning ascii patterns and UTF-8 data, 2021-10-15). The change
to the condition and the test are simplified and more targeted.
Original-patch-by: Hamza Mahfooz [off-list ref]
Signed-off-by: René Scharfe <redacted>
---
grep.c | 2 +-
t/t7812-grep-icase-non-ascii.sh | 6 ++++++
2 files changed, 7 insertions(+), 1 deletion(-)
From: René Scharfe <hidden> Date: 2021-12-18 19:53:32
Patterns that contain no wildcards and don't have to be case-folded are
literal. Give this condition a name to increase the readability of the
boolean expression for enabling the option PCRE2_UTF.
Signed-off-by: René Scharfe <redacted>
---
grep.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Patterns that contain no wildcards and don't have to be case-folded are
literal. Give this condition a name to increase the readability of the
boolean expression for enabling the option PCRE2_UTF.
Signed-off-by: René Scharfe <redacted>
---
grep.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
I think for this and 1/2 it would be really nice to pick up a version of
Hamza's CI changes:
https://lore.kernel.org/git/20211118084143.279174-2-someguy@effective-light.com/
Aside: Not needed for this change, but I wonder if we could benefit minutely
from:
#ifdef PCRE2_LITERAL
options |= PCRE2_LITERAL;
#endif
It'll save PCRE2 the small effort of finding that we've got no metacharacters.
From: SZEDER Gábor <hidden> Date: 2022-01-29 17:25:49
On Sat, Dec 18, 2021 at 08:50:02PM +0100, René Scharfe wrote:
quoted hunk
compile_pcre2_pattern() currently uses the option PCRE2_UTF only for
patterns with non-ASCII characters. Patterns with ASCII wildcards can
match non-ASCII strings, though. Without that option PCRE2 mishandles
UTF-8 input, though -- it matches parts of multi-byte characters. Fix
that by using PCRE2_UTF even for ASCII-only patterns.
This is a remake of the reverted ae39ba431a (grep/pcre2: fix an edge
case concerning ascii patterns and UTF-8 data, 2021-10-15). The change
to the condition and the test are simplified and more targeted.
Original-patch-by: Hamza Mahfooz [off-list ref]
Signed-off-by: René Scharfe <redacted>
---
grep.c | 2 +-
t/t7812-grep-icase-non-ascii.sh | 6 ++++++
2 files changed, 7 insertions(+), 1 deletion(-)
I tried to use 'git grep -P' for the first time ever, and it hung
right away, spinning all CPUs at 100%. I could narrow it down, both
the complexity of the pattern and the size of the input, see the test
below, and it bisects to this patch.
--- >8 ---
#!/bin/sh
test_description='test'
. ./test-lib.sh
test_expect_success PCRE 'test' '
# LC_ALL=C works
LC_ALL=en_US.UTF-8 &&
cat >ascii <<-\EOF &&
foo
bar
baz
EOF
cat >utf8 <<-\EOF &&
foo
bar
báz
EOF
git add ascii utf8 &&
# These all work as expected:
git grep --threads=1 -P " " ascii &&
git grep --threads=1 -P "^ " ascii &&
git grep --threads=1 -P "\s" ascii &&
git grep --threads=1 -P "^\s" ascii &&
git grep --threads=1 -P " " utf8 &&
git grep --threads=1 -P "^ " utf8 &&
git grep --threads=1 -P "\s" utf8 &&
# This hangs (but it does work with basic and extended regexp):
git grep --threads=1 -P "^\s" utf8
'
test_done
From: René Scharfe <hidden> Date: 2022-01-30 07:55:20
Am 29.01.22 um 18:25 schrieb SZEDER Gábor:
On Sat, Dec 18, 2021 at 08:50:02PM +0100, René Scharfe wrote:
quoted
compile_pcre2_pattern() currently uses the option PCRE2_UTF only for
patterns with non-ASCII characters. Patterns with ASCII wildcards can
match non-ASCII strings, though. Without that option PCRE2 mishandles
UTF-8 input, though -- it matches parts of multi-byte characters. Fix
that by using PCRE2_UTF even for ASCII-only patterns.
This is a remake of the reverted ae39ba431a (grep/pcre2: fix an edge
case concerning ascii patterns and UTF-8 data, 2021-10-15). The change
to the condition and the test are simplified and more targeted.
Original-patch-by: Hamza Mahfooz [off-list ref]
Signed-off-by: René Scharfe <redacted>
---
grep.c | 2 +-
t/t7812-grep-icase-non-ascii.sh | 6 ++++++
2 files changed, 7 insertions(+), 1 deletion(-)
I tried to use 'git grep -P' for the first time ever, and it hung
right away, spinning all CPUs at 100%. I could narrow it down, both
the complexity of the pattern and the size of the input, see the test
below, and it bisects to this patch.
--- >8 ---
#!/bin/sh
test_description='test'
. ./test-lib.sh
test_expect_success PCRE 'test' '
# LC_ALL=C works
LC_ALL=en_US.UTF-8 &&
cat >ascii <<-\EOF &&
foo
bar
baz
EOF
cat >utf8 <<-\EOF &&
foo
bar
báz
EOF
git add ascii utf8 &&
# These all work as expected:
git grep --threads=1 -P " " ascii &&
git grep --threads=1 -P "^ " ascii &&
git grep --threads=1 -P "\s" ascii &&
git grep --threads=1 -P "^\s" ascii &&
git grep --threads=1 -P " " utf8 &&
git grep --threads=1 -P "^ " utf8 &&
git grep --threads=1 -P "\s" utf8 &&
# This hangs (but it does work with basic and extended regexp):
git grep --threads=1 -P "^\s" utf8
'
test_done
I get the following result and no hang with PCRE2 10.39:
utf8: bar
utf8: báz
e0c6029 (Fix inifinite loop when a single byte newline is searched in
JIT., 2020-05-29) [1] sounds like it might have fixed it. It's part of
version 10.36.
Do you still get the error when you disable JIT, i.e. when you use the
pattern "(*NO_JIT)^\s" instead?
René
[1] https://github.com/PhilipHazel/pcre2/commit/e0c6029a62db9c2161941ecdf459205382d4d379
From: SZEDER Gábor <hidden> Date: 2022-01-30 09:04:58
On Sun, Jan 30, 2022 at 08:55:02AM +0100, René Scharfe wrote:
Am 29.01.22 um 18:25 schrieb SZEDER Gábor:
quoted
On Sat, Dec 18, 2021 at 08:50:02PM +0100, René Scharfe wrote:
quoted
compile_pcre2_pattern() currently uses the option PCRE2_UTF only for
patterns with non-ASCII characters. Patterns with ASCII wildcards can
match non-ASCII strings, though. Without that option PCRE2 mishandles
UTF-8 input, though -- it matches parts of multi-byte characters. Fix
that by using PCRE2_UTF even for ASCII-only patterns.
This is a remake of the reverted ae39ba431a (grep/pcre2: fix an edge
case concerning ascii patterns and UTF-8 data, 2021-10-15). The change
to the condition and the test are simplified and more targeted.
Original-patch-by: Hamza Mahfooz [off-list ref]
Signed-off-by: René Scharfe <redacted>
---
grep.c | 2 +-
t/t7812-grep-icase-non-ascii.sh | 6 ++++++
2 files changed, 7 insertions(+), 1 deletion(-)
I tried to use 'git grep -P' for the first time ever, and it hung
right away, spinning all CPUs at 100%. I could narrow it down, both
the complexity of the pattern and the size of the input, see the test
below, and it bisects to this patch.
--- >8 ---
#!/bin/sh
test_description='test'
. ./test-lib.sh
test_expect_success PCRE 'test' '
# LC_ALL=C works
LC_ALL=en_US.UTF-8 &&
cat >ascii <<-\EOF &&
foo
bar
baz
EOF
cat >utf8 <<-\EOF &&
foo
bar
báz
EOF
git add ascii utf8 &&
# These all work as expected:
git grep --threads=1 -P " " ascii &&
git grep --threads=1 -P "^ " ascii &&
git grep --threads=1 -P "\s" ascii &&
git grep --threads=1 -P "^\s" ascii &&
git grep --threads=1 -P " " utf8 &&
git grep --threads=1 -P "^ " utf8 &&
git grep --threads=1 -P "\s" utf8 &&
# This hangs (but it does work with basic and extended regexp):
git grep --threads=1 -P "^\s" utf8
'
test_done
I get the following result and no hang with PCRE2 10.39:
utf8: bar
utf8: báz
e0c6029 (Fix inifinite loop when a single byte newline is searched in
JIT., 2020-05-29) [1] sounds like it might have fixed it. It's part of
version 10.36.
I saw this hang on two Ubuntu 20.04 based boxes, which predate that
fix you mention only by a month or two, and apparently the almost two
years since then was not enough for this fix to trickle down into
updated 20.04 pcre packages, because:
Do you still get the error when you disable JIT, i.e. when you use the
pattern "(*NO_JIT)^\s" instead?
No, with this pattern it works as expected.
So is there a more convenient way to disable PCRE JIT in Git? FWIW,
(non-git) 'grep -P' works with the same patterns.
From: René Scharfe <hidden> Date: 2022-01-30 13:33:16
Am 30.01.22 um 10:04 schrieb SZEDER Gábor:
On Sun, Jan 30, 2022 at 08:55:02AM +0100, René Scharfe wrote:
quoted
e0c6029 (Fix inifinite loop when a single byte newline is searched in
JIT., 2020-05-29) [1] sounds like it might have fixed it. It's part of
version 10.36.
I saw this hang on two Ubuntu 20.04 based boxes, which predate that
fix you mention only by a month or two, and apparently the almost two
years since then was not enough for this fix to trickle down into
updated 20.04 pcre packages, because:
quoted
Do you still get the error when you disable JIT, i.e. when you use the
pattern "(*NO_JIT)^\s" instead?
No, with this pattern it works as expected.
So is there a more convenient way to disable PCRE JIT in Git? FWIW,
(non-git) 'grep -P' works with the same patterns.
I don't know a better way. We could do it automatically, though:
--- >8 ---
Subject: [PATCH] grep: disable JIT on PCRE2 before 10.36 to avoid endless loop
Commit e0c6029 (Fix inifinite loop when a single byte newline is
searched in JIT., 2020-05-29) of PCRE2 adds the following point to its
ChangeLog for version 10.36:
2. Fix inifinite loop when a single byte newline is searched in JIT when
invalid utf8 mode is enabled.
Avoid that bug on older versions (which are still reportedly found in
the wild) by disabling the JIT when handling UTF-8.
Reported-by: SZEDER Gábor <redacted>
Signed-off-by: René Scharfe <redacted>
---
Not sure how to test it. Killing git grep after a second or so seems a
bit clumsy. timeout(1) from GNU coreutils at least allows doing that
from the shell, but it's not a standard tool. Perhaps we need a new
test helper for that purpose?
grep.c | 8 ++++++++
1 file changed, 8 insertions(+)
On Sun, Jan 30, 2022 at 08:55:02AM +0100, René Scharfe wrote:
quoted
e0c6029 (Fix inifinite loop when a single byte newline is searched in
JIT., 2020-05-29) [1] sounds like it might have fixed it. It's part of
version 10.36.
I saw this hang on two Ubuntu 20.04 based boxes, which predate that
fix you mention only by a month or two, and apparently the almost two
years since then was not enough for this fix to trickle down into
updated 20.04 pcre packages, because:
quoted
Do you still get the error when you disable JIT, i.e. when you use the
pattern "(*NO_JIT)^\s" instead?
No, with this pattern it works as expected.
So is there a more convenient way to disable PCRE JIT in Git? FWIW,
(non-git) 'grep -P' works with the same patterns.
I don't know a better way. We could do it automatically, though:
--- >8 ---
Subject: [PATCH] grep: disable JIT on PCRE2 before 10.36 to avoid endless loop
Commit e0c6029 (Fix inifinite loop when a single byte newline is
searched in JIT., 2020-05-29) of PCRE2 adds the following point to its
ChangeLog for version 10.36:
2. Fix inifinite loop when a single byte newline is searched in JIT when
invalid utf8 mode is enabled.
Avoid that bug on older versions (which are still reportedly found in
the wild) by disabling the JIT when handling UTF-8.
Reported-by: SZEDER Gábor <redacted>
Signed-off-by: René Scharfe <redacted>
---
Not sure how to test it. Killing git grep after a second or so seems a
bit clumsy. timeout(1) from GNU coreutils at least allows doing that
from the shell, but it's not a standard tool. Perhaps we need a new
test helper for that purpose?
grep.c | 8 ++++++++
1 file changed, 8 insertions(+)
Better to quote this as PhilipHazel/pcre2@e0c6029 or something, i.e. to
indicate that it's not git.git's commit.
+ * single byte newline is searched in JIT., 2020-05-29).
+ */
+ if (options & PCRE2_MATCH_INVALID_UTF)
+ p->pcre2_jit_on = 0;
It seems rather heavy-hande, but I can't think of a better way to deal
with this, i.e. if we selectively use JIT on older versions, surely we
run into the match-bytes-but-want-chars bug you were fixing.
+#endif
if (p->pcre2_jit_on) {
jitret = pcre2_jit_compile(p->pcre2_pattern, PCRE2_JIT_COMPLETE);
if (jitret)
From: René Scharfe <hidden> Date: 2022-02-05 17:00:19
Am 31.01.22 um 22:01 schrieb Ævar Arnfjörð Bjarmason:
On Sun, Jan 30 2022, René Scharfe wrote:
quoted
Am 30.01.22 um 10:04 schrieb SZEDER Gábor:
quoted
On Sun, Jan 30, 2022 at 08:55:02AM +0100, René Scharfe wrote:
quoted
e0c6029 (Fix inifinite loop when a single byte newline is searched in
JIT., 2020-05-29) [1] sounds like it might have fixed it. It's part of
version 10.36.
I saw this hang on two Ubuntu 20.04 based boxes, which predate that
fix you mention only by a month or two, and apparently the almost two
years since then was not enough for this fix to trickle down into
updated 20.04 pcre packages, because:
quoted
Do you still get the error when you disable JIT, i.e. when you use the
pattern "(*NO_JIT)^\s" instead?
No, with this pattern it works as expected.
So is there a more convenient way to disable PCRE JIT in Git? FWIW,
(non-git) 'grep -P' works with the same patterns.
I don't know a better way. We could do it automatically, though:
--- >8 ---
Subject: [PATCH] grep: disable JIT on PCRE2 before 10.36 to avoid endless loop
Commit e0c6029 (Fix inifinite loop when a single byte newline is
searched in JIT., 2020-05-29) of PCRE2 adds the following point to its
ChangeLog for version 10.36:
2. Fix inifinite loop when a single byte newline is searched in JIT when
invalid utf8 mode is enabled.
Avoid that bug on older versions (which are still reportedly found in
the wild) by disabling the JIT when handling UTF-8.
Reported-by: SZEDER Gábor <redacted>
Signed-off-by: René Scharfe <redacted>
---
Not sure how to test it. Killing git grep after a second or so seems a
bit clumsy. timeout(1) from GNU coreutils at least allows doing that
from the shell, but it's not a standard tool. Perhaps we need a new
test helper for that purpose?
https://mywiki.wooledge.org/BashFAQ/068 offers a Perl-based Shell
function or aborting a program if it takes too long:
doalarm() { perl -e 'alarm shift; exec @ARGV' -- "$@"; }
It doesn't waste time when the program finishes faster and seems to work
fine with git grep.
I can't actually test the effectiveness of the patch because PCRE2's
JIT doesn't work on my development machine at all (Apple M1), as I just
discovered. :-/ While we know that disabling JIT helps, we didn't
actually determine, yet, if e0c6029 (Fix inifinite loop when a single
byte newline is searched in JIT., 2020-05-29) really fixes the "^\s"
bug.
So I have to abandon this patch, unfortunately. Any volunteer to pick
it up?
Better to quote this as PhilipHazel/pcre2@e0c6029 or something, i.e. to
indicate that it's not git.git's commit.
The context should suffice for a human reader to understand that it's a
PCRE2 about a bug fix, but I can see some program turning hex strings
into repo-local links getting confused. Is there a standard cross-repo
citation format? Using a Github link might be the most practical way
for the near term, I suspect.
quoted
+ * single byte newline is searched in JIT., 2020-05-29).
+ */
+ if (options & PCRE2_MATCH_INVALID_UTF)
+ p->pcre2_jit_on = 0;
It seems rather heavy-hande, but I can't think of a better way to deal
with this, i.e. if we selectively use JIT on older versions, surely we
run into the match-bytes-but-want-chars bug you were fixing.
quoted
+#endif
if (p->pcre2_jit_on) {
jitret = pcre2_jit_compile(p->pcre2_pattern, PCRE2_JIT_COMPLETE);
if (jitret)
From: SZEDER Gábor <hidden> Date: 2022-02-06 10:08:27
On Sat, Feb 05, 2022 at 06:00:02PM +0100, René Scharfe wrote:
quoted
quoted
--- >8 ---
Subject: [PATCH] grep: disable JIT on PCRE2 before 10.36 to avoid endless loop
Commit e0c6029 (Fix inifinite loop when a single byte newline is
searched in JIT., 2020-05-29) of PCRE2 adds the following point to its
ChangeLog for version 10.36:
2. Fix inifinite loop when a single byte newline is searched in JIT when
invalid utf8 mode is enabled.
Avoid that bug on older versions (which are still reportedly found in
the wild) by disabling the JIT when handling UTF-8.
Reported-by: SZEDER Gábor <redacted>
Signed-off-by: René Scharfe <redacted>
---
Not sure how to test it. Killing git grep after a second or so seems a
bit clumsy. timeout(1) from GNU coreutils at least allows doing that
from the shell, but it's not a standard tool. Perhaps we need a new
test helper for that purpose?
https://mywiki.wooledge.org/BashFAQ/068 offers a Perl-based Shell
function or aborting a program if it takes too long:
doalarm() { perl -e 'alarm shift; exec @ARGV' -- "$@"; }
It doesn't waste time when the program finishes faster and seems to work
fine with git grep.
I can't actually test the effectiveness of the patch because PCRE2's
JIT doesn't work on my development machine at all (Apple M1), as I just
discovered. :-/ While we know that disabling JIT helps, we didn't
actually determine, yet, if e0c6029 (Fix inifinite loop when a single
byte newline is searched in JIT., 2020-05-29) really fixes the "^\s"
bug.
So I have to abandon this patch, unfortunately. Any volunteer to pick
it up?
FWIW, I built Git with your patch and USE_LIBPCRE2=YesPlease and run
the test suite, and it succeeded. Though I can't judge how much is
this actually worth.
Am 31.01.22 um 22:01 schrieb Ævar Arnfjörð Bjarmason:
quoted
On Sun, Jan 30 2022, René Scharfe wrote:
quoted
Am 30.01.22 um 10:04 schrieb SZEDER Gábor:
quoted
On Sun, Jan 30, 2022 at 08:55:02AM +0100, René Scharfe wrote:
quoted
e0c6029 (Fix inifinite loop when a single byte newline is searched in
JIT., 2020-05-29) [1] sounds like it might have fixed it. It's part of
version 10.36.
I saw this hang on two Ubuntu 20.04 based boxes, which predate that
fix you mention only by a month or two, and apparently the almost two
years since then was not enough for this fix to trickle down into
updated 20.04 pcre packages, because:
quoted
Do you still get the error when you disable JIT, i.e. when you use the
pattern "(*NO_JIT)^\s" instead?
No, with this pattern it works as expected.
So is there a more convenient way to disable PCRE JIT in Git? FWIW,
(non-git) 'grep -P' works with the same patterns.
I don't know a better way. We could do it automatically, though:
--- >8 ---
Subject: [PATCH] grep: disable JIT on PCRE2 before 10.36 to avoid endless loop
Commit e0c6029 (Fix inifinite loop when a single byte newline is
searched in JIT., 2020-05-29) of PCRE2 adds the following point to its
ChangeLog for version 10.36:
2. Fix inifinite loop when a single byte newline is searched in JIT when
invalid utf8 mode is enabled.
Avoid that bug on older versions (which are still reportedly found in
the wild) by disabling the JIT when handling UTF-8.
Reported-by: SZEDER Gábor <redacted>
Signed-off-by: René Scharfe <redacted>
---
Not sure how to test it. Killing git grep after a second or so seems a
bit clumsy. timeout(1) from GNU coreutils at least allows doing that
from the shell, but it's not a standard tool. Perhaps we need a new
test helper for that purpose?
https://mywiki.wooledge.org/BashFAQ/068 offers a Perl-based Shell
function or aborting a program if it takes too long:
doalarm() { perl -e 'alarm shift; exec @ARGV' -- "$@"; }
It doesn't waste time when the program finishes faster and seems to work
fine with git grep.
I can't actually test the effectiveness of the patch because PCRE2's
JIT doesn't work on my development machine at all (Apple M1), as I just
discovered. :-/ While we know that disabling JIT helps, we didn't
actually determine, yet, if e0c6029 (Fix inifinite loop when a single
byte newline is searched in JIT., 2020-05-29) really fixes the "^\s"
bug.
So I have to abandon this patch, unfortunately. Any volunteer to pick
it up?
We can test it in CI, and have a proposed patch from Hamza Mahfooz to do
so. See
https://lore.kernel.org/git/211220.865yrjszg4.gmgdl@evledraar.gmail.com/
There's been some minor changes to the main.yml since then, but I think
you should be able to just pick that patch up, adjust it, apply whatever
changes you want to test on top, and push it to github.
From: René Scharfe <hidden> Date: 2022-02-17 21:14:45
Am 12.02.22 um 21:46 schrieb Ævar Arnfjörð Bjarmason:
On Sat, Feb 05 2022, René Scharfe wrote:
quoted
I can't actually test the effectiveness of the patch because PCRE2's
JIT doesn't work on my development machine at all (Apple M1), as I just
discovered. :-/ While we know that disabling JIT helps, we didn't
actually determine, yet, if e0c6029 (Fix inifinite loop when a single
byte newline is searched in JIT., 2020-05-29) really fixes the "^\s"
bug.
So I have to abandon this patch, unfortunately. Any volunteer to pick
it up?
We can test it in CI, and have a proposed patch from Hamza Mahfooz to do
so. See
https://lore.kernel.org/git/211220.865yrjszg4.gmgdl@evledraar.gmail.com/
There's been some minor changes to the main.yml since then, but I think
you should be able to just pick that patch up, adjust it, apply whatever
changes you want to test on top, and push it to github.
Good idea! Except the "just" is not justified, I feel. I learned that
- t7810 fails with PCRE2 built with --disable-unicode because it uses
\p{...} unconditionally, and that's not supported without Unicode
support -- no idea how to detect that and skip those tests except
by trying and maybe looking for the note that "this version of PCRE2
does not have support for \P, \p, or \X", which somehow feels iffy,
- PCRE2 10.35 doesn't build on Ubuntu x64 without adding -mshstk to
CFLAGS, and that's the version I wanted to test,
- many of the Unicode related tests require Islandic language support,
and "sudo apt-get -y install `check-language-support -l is`"
installs it,
- the condition for our workaround for bug 2642 is reversed,
- with that fixed I can't trigger the endless loop.
So perhaps that's the only fix we need here -- or perhaps I got
confused by the multitude of options.
--- >8 ---
Subject: [PATCH] grep: fix triggering PCRE2_NO_START_OPTIMIZE workaround
PCRE2 bug 2642 was fixed in version 10.36. Our 95ca1f987e (grep/pcre2:
better support invalid UTF-8 haystacks, 2021-01-24) worked around it on
older versions by setting the flag PCRE2_NO_START_OPTIMIZE. 797c359978
(grep/pcre2: use compile-time PCREv2 version test, 2021-02-18) switched
it around to set the flag on 10.36 and higher instead, while it claimed
to use "the same test done at compile-time".
Switch the condition back to apply the workaround on PCRE2 versions
_before_ 10.36.
Signed-off-by: René Scharfe <redacted>
---
grep.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)