#7270 closed bug (fixed)

Incorrect optimization with Data.ByteString.append

Reported by: ocheron Owned by: duncan
Priority: highest Milestone: 7.6.2
Component: libraries (other) Version: 7.6.1
Keywords: Cc: hackage.haskell.org@…, conrad@…
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Incorrect result at runtime Difficulty: Unknown
Test Case: Blocked By:
Blocking: Related Tickets:

Description

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"

Change History (12)

comment:1 Changed 19 months ago by 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.

comment:2 Changed 19 months ago by igloo

  • Difficulty set to Unknown
  • Owner set to igloo

comment:3 Changed 19 months ago by igloo

  • Milestone set to 7.6.2
  • Owner changed from igloo to duncan
  • Priority changed from normal to highest

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

comment:4 follow-up: Changed 19 months ago by duncan

  • Resolution set to fixed
  • Status changed from new to closed

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.

comment:5 Changed 19 months ago by igloo

  • Owner duncan deleted
  • Resolution fixed deleted
  • Status changed from closed to new

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

comment:6 Changed 19 months ago by igloo

  • Owner set to igloo

comment:7 in reply to: ↑ 4 Changed 19 months ago by 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.

comment:8 Changed 18 months ago by igloo

  • Owner changed from igloo to duncan

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?

comment:9 Changed 18 months ago by liyang

  • Cc hackage.haskell.org@… added

comment:10 Changed 18 months ago by conrad

  • Cc conrad@… added

comment:11 Changed 17 months ago by duncan

  • Status changed from new to merge

Oops, dunno how that happened. Fixed.

comment:12 Changed 17 months ago by igloo

  • Resolution set to fixed
  • Status changed from merge to closed

Merged

Note: See TracTickets for help on using tickets.