∞-loop, space==bound in primMapByteStringBounded (#204)

* ∞-loop, space==bound in primMapByteStringBounded In primMapByteStringBounded, when the potentially needed space for encoding one more byte (the 'bound' of the provided BoundedPrim) is exactly equal to the free space in the output buffer, we must fill the buffer, rather than signal BufferFull. Otherwise, when we ask for "bounded" more bytes, no new space is allocated, and an infinite loop follows. * Fix findSubstrings CI * Cabal file fixes - Add missing other-modules - Update QuickCheck bounds - Use same QuickCheck bounds in tests .cabal file. * Add 8.10 to test matrix Also dropping "head" which (being 8.7) is no longer worth testing. Noting: - GHC 7.0 fails to build QuickCheck 1.13, because it pulls in "text" 1.2.4.0 which in turn fails to build. - GHC 7.2 fails the "hPutBuilder test", because random Unicode Strings don't roundtrip read/write in GHC 7.2. Fixing this requires limiting the range of test inputs when GHC is too old. Co-authored-by: Viktor Dukhovni <[email protected]>
haskell · May 6, 2020 · 26ef95d · 26ef95d
1 parent 38ad056
commit 26ef95d
Show file tree

Hide file tree

Showing 5 changed files with 56 additions and 27 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -1,8 +1,10 @@
 env:
+ ### GHC 7.0.4 fails to build "text" 1.2.4.0 for QuickCheck
  - GHCVER=7.0.4 CABALVER=1.16
+ ### GHC 7.2.2 fails "hPutBuilder" tests due to encoding roundtrip issues
+ - GHCVER=7.2.2 CABALVER=1.16
  # we have to use CABALVER=1.16 for GHC<7.6 as well, as there's
  # no package for earlier cabal versions in the PPA
- - GHCVER=7.2.2 CABALVER=1.16
  - GHCVER=7.4.2 CABALVER=1.16
  - GHCVER=7.6.3 CABALVER=1.16
  - GHCVER=7.8.4 CABALVER=1.18
@@ -12,7 +14,9 @@ env:
  - GHCVER=8.4.4 CABALVER=2.2
  - GHCVER=8.6.3 CABALVER=2.4
  - GHCVER=8.8.1 CABALVER=2.4
- - GHCVER=head  CABALVER=2.4
+ - GHCVER=8.10.1 CABALVER=2.4
+ ### The head in "ppa", currently 8.7, is no longer worth testing.
+ # - GHCVER=head  CABALVER=2.4
 
 matrix:
   allow_failures:

diff --git a/Data/ByteString/Builder/Prim.hs b/Data/ByteString/Builder/Prim.hs
@@ -652,7 +652,7 @@ primMapByteStringBounded w =
               touchForeignPtr ifp -- input buffer consumed
               k br
 
-          | op0 `plusPtr` bound < ope =
+          | op0 `plusPtr` bound <= ope =
               goPartial (ip0 `plusPtr` min outRemaining inpRemaining)
 
           | otherwise  = return $ bufferFull bound op0 (goBS ip0)

diff --git a/bytestring.cabal b/bytestring.cabal
@@ -54,7 +54,8 @@ Author:              Don Stewart,
 Maintainer:          Duncan Coutts <[email protected]>
 Homepage:            https://github.com/haskell/bytestring
 Bug-reports:         https://github.com/haskell/bytestring/issues
-Tested-With:         GHC==7.10.1, GHC==7.8.4, GHC==7.6.3, GHC==7.4.2, GHC==6.12.3
+Tested-With:         GHC==8.10.1, GHC==8.8.1, GHC==8.6.3, GHC==8.4.4, GHC==8.2.2,
+                     GHC==8.0.2, GHC==7.10.3, GHC==7.8.4, GHC==7.6.3, GHC==7.4.2
 Build-Type:          Simple
 Cabal-Version:       >= 1.10
 extra-source-files:  README.md Changelog.md
@@ -150,10 +151,19 @@ test-suite prop-compiled
   other-modules:    Rules
                     QuickCheckUtils
                     TestFramework
+                    Data.ByteString
+                    Data.ByteString.Char8
+                    Data.ByteString.Internal
+                    Data.ByteString.Lazy
+                    Data.ByteString.Lazy.Char8
+                    Data.ByteString.Lazy.Internal
+                    Data.ByteString.Short
+                    Data.ByteString.Short.Internal
+                    Data.ByteString.Unsafe
   hs-source-dirs:   . tests
   build-depends:    base, ghc-prim, deepseq, random, directory,
                     test-framework, test-framework-quickcheck2,
-                    QuickCheck >= 2.10 && < 2.14
+                    QuickCheck >= 2.10 && < 2.15
   c-sources:        cbits/fpstring.c
   include-dirs:     include
   ghc-options:      -fwarn-unused-binds
@@ -191,14 +201,31 @@ test-suite test-builder
   type:             exitcode-stdio-1.0
   hs-source-dirs:   . tests tests/builder
   main-is:          TestSuite.hs
-  other-modules:    Data.ByteString.Builder.Tests
-                    Data.ByteString.Builder.Prim.Tests
+  other-modules:    Data.ByteString
+                    Data.ByteString.Internal
+                    Data.ByteString.Lazy
+                    Data.ByteString.Lazy.Internal
+                    Data.ByteString.Short
+                    Data.ByteString.Short.Internal
+                    Data.ByteString.Unsafe
+                    Data.ByteString.Builder
+                    Data.ByteString.Builder.ASCII
+                    Data.ByteString.Builder.Extra
+                    Data.ByteString.Builder.Internal
+                    Data.ByteString.Builder.Prim
+                    Data.ByteString.Builder.Prim.ASCII
+                    Data.ByteString.Builder.Prim.Binary
+                    Data.ByteString.Builder.Prim.Internal
+                    Data.ByteString.Builder.Prim.Internal.Base16
+                    Data.ByteString.Builder.Prim.Internal.Floating
+                    Data.ByteString.Builder.Prim.Internal.UncheckedShifts
                     Data.ByteString.Builder.Prim.TestUtils
+                    Data.ByteString.Builder.Prim.Tests
+                    Data.ByteString.Builder.Tests
                     TestFramework
-
   build-depends:    base, ghc-prim,
                     deepseq,
-                    QuickCheck                 >= 2.10 && < 2.14,
+                    QuickCheck                 >= 2.10 && < 2.15,
                     byteorder                  == 1.0.*,
                     dlist                      >= 0.5 && < 0.9,
                     directory,

diff --git a/tests/Properties.hs b/tests/Properties.hs
@@ -1240,31 +1240,29 @@ prop_initsBB xs = inits xs == map P.unpack (P.inits (P.pack xs))
 
 prop_tailsBB xs = tails xs == map P.unpack (P.tails (P.pack xs))
 
+prop_findSubstringsBB :: String -> Int -> Int -> Bool
 prop_findSubstringsBB s x l
-    = C.findSubstrings (C.pack p) (C.pack s) == naive_findSubstrings p s
+    -- coerce to 8-bit alphabet to avoid false-positives
+    = let s8 = C.unpack $ C.pack s
+          -- we look for some random substring of the test string
+          p = take (model l) $ drop (model x) s8
+      in C.findSubstrings (C.pack p) (C.pack s8) == naive_findSubstrings p s8
   where
-    _ = l :: Int
-    _ = x :: Int
-
-    -- we look for some random substring of the test string
-    p = take (model l) $ drop (model x) s
-
     -- naive reference implementation
     naive_findSubstrings :: String -> String -> [Int]
-    naive_findSubstrings p s = [x | x <- [0..length s], p `isPrefixOf` drop x s]
+    naive_findSubstrings p q = [x | x <- [0..length q], p `isPrefixOf` drop x q]
 
+prop_findSubstringBB :: String -> Int -> Int -> Bool
 prop_findSubstringBB s x l
-    = C.findSubstring (C.pack p) (C.pack s) == naive_findSubstring p s
+    -- coerce to 8-bit alphabet to avoid false-positives
+    = let s8 = C.unpack $ C.pack s
+          -- we look for some random substring of the test string
+          p = take (model l) $ drop (model x) s8
+      in C.findSubstring (C.pack p) (C.pack s8) == naive_findSubstring p s8
   where
-    _ = l :: Int
-    _ = x :: Int
-
-    -- we look for some random substring of the test string
-    p = take (model l) $ drop (model x) s
-
     -- naive reference implementation
     naive_findSubstring :: String -> String -> Maybe Int
-    naive_findSubstring p s = listToMaybe [x | x <- [0..length s], p `isPrefixOf` drop x s]
+    naive_findSubstring p q = listToMaybe [x | x <- [0..length q], p `isPrefixOf` drop x q]
 
 -- correspondance between break and breakSubstring
 prop_breakSubstringBB c l

diff --git a/tests/bytestring-tests.cabal b/tests/bytestring-tests.cabal
@@ -27,7 +27,7 @@ executable prop-compiled
   hs-source-dirs:   . ..
   build-depends:    base, ghc-prim, deepseq, random, directory,
                     test-framework, test-framework-quickcheck2,
-                    QuickCheck >= 2.3 && < 2.8
+                    QuickCheck >= 2.10 && < 2.15
   c-sources:        ../cbits/fpstring.c
   include-dirs:     ../include
   cpp-options:      -DHAVE_TEST_FRAMEWORK=1
@@ -70,7 +70,7 @@ executable test-builder
 
   build-depends:    base, ghc-prim,
                     deepseq,
-                    QuickCheck                 >= 2.4 && < 3,
+                    QuickCheck                 >= 2.10 && < 2.15,
                     byteorder                  == 1.0.*,
                     dlist                      >= 0.5 && < 0.9,
                     directory,