Subject: bug#24159: closed (Re: bug#24159: [PATCH] dfa: minor fix for whether dfa is fast or not)

From: [email-masked] (GNU bug Tracking System)
Date: Sat, 06 Aug 2016 05:03:02 +0000

Your bug report

#24159: [PATCH] dfa: minor fix for whether dfa is fast or not

which was filed against the grep package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
24159: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=24159
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#24159: [PATCH] dfa: minor fix for whether dfa is fast or not
From: Jim Meyering <[email-masked]>
To: Norihiro Tanaka <[email-masked]>
Cc: [email-masked]
Date: Fri, 5 Aug 2016 22:02:31 -0700
List-Id: <debbugs-submit.debbugs.gnu.org>

On Fri, Aug 5, 2016 at 7:32 PM, Norihiro Tanaka <[email-masked]> wrote:
>
> On Fri, 5 Aug 2016 13:29:43 -0700
> Jim Meyering <[email-masked]> wrote:
>
>> On Fri, Aug 5, 2016 at 4:30 AM, Norihiro Tanaka <[email-masked]> wrote:
>> > dfaoptimize() is not set fast flag even if it is success, but it is wrong.
>> > If success, dfa matcher uses algorithm for single byte, and it is so fast.
>> >
>> > I think this bug does not affect for grep, but it will affect with the
>> > patch that I just sent to gawk.
>>
>> Thank you for the patch.
>> I was going to push it with the attached slightly updated log message.
>> Note however that grep does use that -> fast member via dfasearch.c's
>> use of the dfaisfast function.
>> But then I realized I should at least verify with "make check", and
>> found that this makes grep's dfa-match test fail.
>> Thus, I will not be pushing it as-is.
>
> Thanks for review and adjustment.  I re-ran all tests including dfa-match,
> and they were passwd again in my machine.  Next, I will re-run them on
> Fedora24, as my machine is RHEL 6.8 and GCC 4.4.7 which is too old.
>
> However, I do not know why dfa-match test fails on your machine.
> dfa-match test does not use grep.  It directly calls dfa functions through
> dfa-match-aux executable in order to test codes of dfa which grep does
> not use.  dfa-match-aux does not referer to the ->fast member.

I have examined the logs, which suggest it was a false positive in a
parallelized "make check" run, due to that test's 3-second timeout. I
have tried repeatedly to reproduce that failure, so far without
success, but in coreutils development, with parallelized tests, we
fixed many hard-to-reproduce tests with small timeout limits like this
-- most of them now use 10 seconds as the limit, so I will change this
one, too (and several others) with the attached patch.

I have pushed your patch.
From df8c2b662b68bc81ca6941da45f639ff806a3037 Mon Sep 17 00:00:00 2001
From: Jim Meyering <[email-masked]>
Date: Fri, 5 Aug 2016 21:53:45 -0700
Subject: [PATCH] tests: standardize on 10-second timeouts to avoid rare false
 failure

In a parallel test run, it is not unusual to exceed a timeout of
1-3 seconds.  Increase several from 3 or fewer to 10 seconds.
* tests/skip-device: Increase timeout from 2 to 10 seconds.
* tests/grep-dev-null-out: Likewise, but s/1/10/.
* tests/pcre-invalid-utf8-input: Likewise, but s/3/10/.
* tests/dfa-match: Likewise.
* tests/pcre-invalid-utf8-infloop: Likewise.
* tests/pcre-infloop: Likewise.
* tests/max-count-overread: Likewise.
* tests/invalid-multibyte-infloop: Likewise.
Prompted by http://bugs.gnu.org/24159.
---
 tests/dfa-match                 | 2 +-
 tests/grep-dev-null-out         | 2 +-
 tests/invalid-multibyte-infloop | 4 ++--
 tests/max-count-overread        | 2 +-
 tests/pcre-infloop              | 2 +-
 tests/pcre-invalid-utf8-infloop | 2 +-
 tests/pcre-invalid-utf8-input   | 8 ++++----
 tests/skip-device               | 2 +-
 8 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/tests/dfa-match b/tests/dfa-match
index 373c83c..846ab1f 100755
--- a/tests/dfa-match
+++ b/tests/dfa-match
@@ -35,7 +35,7 @@ fi

 fail2=0
 in=$(printf "bb\nbb")
-timeout 3 dfa-match-aux a "$in" 1 > out || fail2=1
+timeout 10 dfa-match-aux a "$in" 1 > out || fail2=1
 compare /dev/null out || fail2=1
 if test $fail2 -ne 0; then
   warn_ 'dfa-match test #2 failed\n'
diff --git a/tests/grep-dev-null-out b/tests/grep-dev-null-out
index f30700d..7f0e1c5 100755
--- a/tests/grep-dev-null-out
+++ b/tests/grep-dev-null-out
@@ -6,6 +6,6 @@
 require_timeout_

 ${AWK-awk} 'BEGIN {while (1) print "x"}' </dev/null |
-  timeout 1 grep x >/dev/null || fail=1
+  timeout 10 grep x >/dev/null || fail=1

 Exit $fail
diff --git a/tests/invalid-multibyte-infloop b/tests/invalid-multibyte-infloop
index d7c6165..5b3bdfc 100755
--- a/tests/invalid-multibyte-infloop
+++ b/tests/invalid-multibyte-infloop
@@ -14,7 +14,7 @@ encode AA > input
 fail=0

 # Before 2.15, this would infloop.
-LC_ALL=en_US.UTF-8 timeout 3 grep -aF $(encode A) input > out
+LC_ALL=en_US.UTF-8 timeout 10 grep -aF $(encode A) input > out
 status=$?
 if test $status -eq 0; then
   compare input out
@@ -26,7 +26,7 @@ fi || fail=1

 echo 'Binary file input matches' >binary-file-matches

-LC_ALL=en_US.UTF-8 timeout 3 grep -F $(encode A) input > out
+LC_ALL=en_US.UTF-8 timeout 10 grep -F $(encode A) input > out
 status=$?
 if test $status -eq 0; then
   compare binary-file-matches out
diff --git a/tests/max-count-overread b/tests/max-count-overread
index d7aafd6..23c45cb 100755
--- a/tests/max-count-overread
+++ b/tests/max-count-overread
@@ -9,7 +9,7 @@ fail=0

 echo x > exp || framework_failure_

-yes x | timeout 3 grep -m1 x > out || fail=1
+yes x | timeout 10 grep -m1 x > out || fail=1
 compare exp out || fail=1

 Exit $fail
diff --git a/tests/pcre-infloop b/tests/pcre-infloop
index 2f93e70..179bf86 100755
--- a/tests/pcre-infloop
+++ b/tests/pcre-infloop
@@ -27,7 +27,7 @@ printf 'a\201b\r' > in || framework_failure_

 fail=0

-LC_ALL=en_US.UTF-8 timeout 3 grep -P 'a.?..b' in
+LC_ALL=en_US.UTF-8 timeout 10 grep -P 'a.?..b' in
 test $? = 1 || fail_ "libpcre's match function appears to infloop"

 Exit $fail
diff --git a/tests/pcre-invalid-utf8-infloop b/tests/pcre-invalid-utf8-infloop
index e96f1f5..e0503bc 100755
--- a/tests/pcre-invalid-utf8-infloop
+++ b/tests/pcre-invalid-utf8-infloop
@@ -18,7 +18,7 @@ fail=0
 printf '\201_\0' > in || framework_failure_
 printf '_\n' > exp || framework_failure_

-LC_ALL=en_US.UTF-8 timeout 3 grep -aoP _ in > out 2> err || fail=1
+LC_ALL=en_US.UTF-8 timeout 10 grep -aoP _ in > out 2> err || fail=1

 compare exp out || fail=1
 compare /dev/null err || fail=1
diff --git a/tests/pcre-invalid-utf8-input b/tests/pcre-invalid-utf8-input
index fec23ae..17c1d4e 100755
--- a/tests/pcre-invalid-utf8-input
+++ b/tests/pcre-invalid-utf8-input
@@ -1,5 +1,5 @@
 #! /bin/sh
-# Ensure that grep -P doesn't abort for invalid multi-byte input
+# Ensure that grep -P doesn't abort or infloop for invalid multi-byte input
 #
 # Copyright (C) 2013-2016 Free Software Foundation, Inc.
 #
@@ -17,15 +17,15 @@ fail=0

 printf 'j\202j\nj\nk\202\n' > in || framework_failure_

-LC_ALL=en_US.UTF-8 timeout 3 grep -P j in
+LC_ALL=en_US.UTF-8 timeout 10 grep -P j in
 test $? -eq 0 || fail=1

-LC_ALL=en_US.UTF-8 timeout 3 grep -P 'k$' in
+LC_ALL=en_US.UTF-8 timeout 10 grep -P 'k$' in
 test $? -eq 1 || fail=1

 echo k >exp

-LC_ALL=en_US.UTF-8 timeout 3 grep -aoP 'k*' in >out || fail=1
+LC_ALL=en_US.UTF-8 timeout 10 grep -aoP 'k*' in >out || fail=1
 compare exp out || fail=1

 Exit $fail
diff --git a/tests/skip-device b/tests/skip-device
index e73bad3..d6368f1 100755
--- a/tests/skip-device
+++ b/tests/skip-device
@@ -10,7 +10,7 @@ echo foo | grep --devices=skip foo || fail=1

 require_timeout_
 mkfifo myfifo || framework_failure_
-timeout 2s grep -D skip foo myfifo
+timeout 10 grep -D skip foo myfifo
 test $? -eq 1 || fail=1

 Exit $fail
-- 
2.8.0-rc2

Subject: [PATCH] dfa: minor fix for whether dfa is fast or not
From: Norihiro Tanaka <[email-masked]>
To: <[email-masked]>
Date: Fri, 05 Aug 2016 20:30:49 +0900
List-Id: <debbugs-submit.debbugs.gnu.org>

dfaoptimize() is not set fast flag even if it is success, but it is wrong.
If success, dfa matcher uses algorithm for single byte, and it is so fast.

I think this bug does not affect for grep, but it will affect with the
patch that I just sent to gawk.