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