Incorrect optimization with Data.ByteString.append

[ocheron]

The following program does not give the same result with -O2 and without:

import qualified Data.ByteString as B

main = let a = B.singleton 65 in print (func a a)

func y z = B.append r s
  where
    r = B.map (succ) x
    s = B.map (succ . succ) x
    x = B.append y z

Result observed with GHC 7.6.1 (bytestring-0.10.0.0):

$ ghc --make test -fforce-recomp && ./test
[1 of 1] Compiling Main             ( test.hs, test.o )
Linking test ...
"BBCC"
$ ghc --make test -fforce-recomp -O2 && ./test
[1 of 1] Compiling Main             ( test.hs, test.o )
Linking test ...
"CCCC"

[tab]

I can also reproduce without append but using the shorter func:

func x = s `seq` r
  where
    r = B.map succ x
    s = B.map succ r

it fails at O and O2 on ghc 7.6.

[igloo]

Thanks for the report. This is due to bytestring's misuse of its inlinePerformIO; Duncan knows what to do.

Here's a small standalone program demonstrating the problem:

{-# LANGUAGE BangPatterns, MagicHash, UnboxedTuples #-}

module Main (main) where

import GHC.Base
import GHC.ForeignPtr
import Foreign

main :: IO ()
main = do let !r = make 68
              !s = make 70
          dump r
          dump s

dump :: ForeignPtr Word8 -> IO ()
dump fp = do print fp
             withForeignPtr fp $ \p -> do x <- peek p
                                          print x

make :: Word8 -> ForeignPtr Word8
make w = inlinePerformIO $ do fp <- mallocPlainForeignPtrBytes 1
                              withForeignPtr fp $ \p -> poke p w
                              return fp

inlinePerformIO :: IO a -> a
inlinePerformIO (IO m) = case m realWorld# of (# _, r #) -> r

$ ghc --make q -O
$ ./q
0x00007f95b0706010
68
0x00007f95b0706010
68

[duncan]

Committed to the upstream bytestring repo. So it'll be fixed in the next bytestring point release.

Tue Sep 25 16:21:14 BST 2012  Duncan Coutts <duncan@community.haskell.org>
  * Fix a few incorrect uses of inlinePerformIO
  The incorrect use of inlinePerformIO resulted in multiple calls to
  mallocByteString being shared, and hence two different strings sharing
  the same memory. See http://hackage.haskell.org/trac/ghc/ticket/7270
  
  Consider:
    foo x = s `seq` r
      where
        r = B.map succ x
        s = B.map succ r
  
  The B.map function used a pattern like:
    map f (PS fp s len) = inlinePerformIO $ ...
       fp' <- mallocByteString len
  
  Now, in the foo function above, we have both r and s where the compiler
  can see that the 'len' is the same in both cases, and with
  inlinePerformIO exposing everything, the compiler is free to share the
  two calls to mallocByteString len.
  
  The answer is not to use inlinePerformIO if we are allocating, but to use
  unsafeDupablePerformIO instead. Another reminder that inlinePerformIO is
  really really unsafe, and that we should be using the ST monad instead.

[igloo]

We should make sure this goes into the next GHC release.

[ocheron]

Thank you, my initial problem (with the package tls-0.9.10) is solved with this patch applied on top of bytestring-0.10.0.0. Though I had to fix imports in Data/ByteString.hs: unsafeDupablePerformIO seems to be missing.

[igloo]

Right, the patch doesn't build for me either, in either a GHC tree or as a standalone package:

[ 7 of 21] Compiling Data.ByteString  ( Data/ByteString.hs, dist/build/Data/ByteString.o )

Data/ByteString.hs:488:23: Not in scope: `unsafeDupablePerformIO'

Data/ByteString.hs:679:33: Not in scope: `unsafeDupablePerformIO'

Data/ByteString.hs:699:33: Not in scope: `unsafeDupablePerformIO'

Data/ByteString.hs:728:27: Not in scope: `unsafeDupablePerformIO'

Data/ByteString.hs:759:27: Not in scope: `unsafeDupablePerformIO'

Data/ByteString.hs:1512:38: Not in scope: `unsafeDupablePerformIO'

Duncan, could you take a look please?

[duncan]

Oops, dunno how that happened. Fixed.

[igloo]

Merged