RE: ghc 8.4.1 and trac 13930

[Simon Peyton Jones via ghc-devs]

Wow.  Could you open a ticket?

I just tried with 8.2.2 which is what I have on this laptop, but it printed 
"all is well".  Does that mean it was fine in 8.2, went wrong in 8.4.1 and was 
fixed in 8.4.2?

Simon

| -Original Message-
| From: Evan Laforge <qdun...@gmail.com>
| Sent: 02 May 2018 19:39
| To: Simon Peyton Jones <simo...@microsoft.com>
| Cc: ghc-devs@haskell.org
| Subject: Re: ghc 8.4.1 and trac 13930
| 
| Ok, here's a short module:
| 
| import qualified Control.Exception as Exception
| 
| main :: IO ()
| main = do
| unserialize
| putStrLn "all is well"
| 
| unserialize :: IO Char
| unserialize =
| if definitelyTrue
| then do
| return 'a'
| else do
| Exception.evaluate (error "wrong place")
| 
| {-# NOINLINE definitelyTrue #-}
| definitelyTrue :: Bool
| definitelyTrue = True
| 
| 
| When compiled with -O on 8.4.1, this should print "wrong place".
| Without -O, or with 8.4.2, or if True can be inlined, or without
| evaluate, all is well.
___
ghc-devs mailing list
ghc-devs@haskell.org
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs

Re: ghc 8.4.1 and trac 13930

[Evan Laforge]

Ok, here's a short module:

import qualified Control.Exception as Exception

main :: IO ()
main = do
unserialize
putStrLn "all is well"

unserialize :: IO Char
unserialize =
if definitelyTrue
then do
return 'a'
else do
Exception.evaluate (error "wrong place")

{-# NOINLINE definitelyTrue #-}
definitelyTrue :: Bool
definitelyTrue = True


When compiled with -O on 8.4.1, this should print "wrong place".
Without -O, or with 8.4.2, or if True can be inlined, or without
evaluate, all is well.
___
ghc-devs mailing list
ghc-devs@haskell.org
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs

Re: ghc 8.4.1 and trac 13930

[Evan Laforge]

On Wed, May 2, 2018 at 1:24 AM, Simon Peyton Jones
 wrote:
> |  I recently noticed that with -O1, ghc was optimizing some code
> |
> |  if Trace.traceShowId "b" $ True
> |  then ...
> |  else ...
> |
> |  to always evaluate the 'else' branch.
>
> Are you certain this is #13930?  I don't see an obvious connection.  It seems 
> really really terrible to "optimise" True to False!
>
> I think #13930 was fixed by #5129, which in turn was about discarding a call 
> to 'evaluate'.  That is different to turning True to False.
>
> But there's probably some more complicated context to your use-case that 
> means my understanding is faulty.
>
> If you are confident that it's securely fixed, well and good. But when bugs 
> disappear I always worry that they are still there, just concealed by some 
> other change.

I'm not totally confident, which is I why I asked.  It does seem to be
related to the presence of Exception.evaluate, but it also comes and
goes depending on how many things are in the condition and branches.

It does seem to be gone in 8.4.1, but I'm also a bit nervous when I
don't know exactly why something was fixed.  I'll try to reduce this
to as small an expression as possible that still triggers True ->
False.
___
ghc-devs mailing list
ghc-devs@haskell.org
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs

RE: ghc 8.4.1 and trac 13930

[Simon Peyton Jones via ghc-devs]

|  I recently noticed that with -O1, ghc was optimizing some code
|  
|  if Trace.traceShowId "b" $ True
|  then ...
|  else ...
|  
|  to always evaluate the 'else' branch.  

Are you certain this is #13930?  I don't see an obvious connection.  It seems 
really really terrible to "optimise" True to False!

I think #13930 was fixed by #5129, which in turn was about discarding a call to 
'evaluate'.  That is different to turning True to False.

But there's probably some more complicated context to your use-case that means 
my understanding is faulty.

If you are confident that it's securely fixed, well and good. But when bugs 
disappear I always worry that they are still there, just concealed by some 
other change.

Simon


|  -Original Message-
|  From: ghc-devs  On Behalf Of Evan
|  Laforge
|  Sent: 02 May 2018 04:10
|  To: ghc-devs@haskell.org
|  Subject: ghc 8.4.1 and trac 13930
|  
|  I recently noticed that with -O1, ghc was optimizing some code
|  
|  if Trace.traceShowId "b" $ True
|  then return $ Left $ Serialize.BadMagic (Serialize.magicBytes
|  magic) file_magic
|  else first Serialize.UnserializeError <$> Exception.evaluate
|  (Serialize.decode rest)
|  
|  to always evaluate the 'else' branch.  This is fixed in 8.4.2, so I'm
|  pretty sure it's https://ghc.haskell.org/trac/ghc/ticket/13930
|  
|  I assume there's no point trying to get a minimal reproduction, since
|  it's a known issue and fixed.  Still,
|  https://downloads.haskell.org/~ghc/8.4.2/docs/html/users_guide/8.4.2-
|  notes.html
|  says "incorrectly optimised, resulting in space leaks".
|  
|  Maybe it should instead say "incorrectly optimised, resulting in
|  taking the wrong branch in 'if' expressions"?  That's a bit more
|  alarming, and is a stronger "upgrade to 8.4.2" signal.
|  ___
|  ghc-devs mailing list
|  ghc-devs@haskell.org
|  http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs
___
ghc-devs mailing list
ghc-devs@haskell.org
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs

Re: ghc 8.4.1 and trac 13930

[Evan Laforge]

On Tue, May 1, 2018 at 9:58 PM, Ben Gamari  wrote:
> Yes, I suppose the language in the release notes does rather understate
> the degree of the incorrectness.
>
> I'll push a new version of the manual with some stronger language
> tomorrow.

Good deal, thanks for the quick response.
___
ghc-devs mailing list
ghc-devs@haskell.org
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs

Re: ghc 8.4.1 and trac 13930

[Ben Gamari]

Evan Laforge  writes:

> I recently noticed that with -O1, ghc was optimizing some code
>
> if Trace.traceShowId "b" $ True
> then return $ Left $ Serialize.BadMagic (Serialize.magicBytes
> magic) file_magic
> else first Serialize.UnserializeError <$> Exception.evaluate
> (Serialize.decode rest)
>
> to always evaluate the 'else' branch.  This is fixed in 8.4.2, so I'm
> pretty sure it's https://ghc.haskell.org/trac/ghc/ticket/13930
>
> I assume there's no point trying to get a minimal reproduction, since
> it's a known issue and fixed.  Still,
> https://downloads.haskell.org/~ghc/8.4.2/docs/html/users_guide/8.4.2-notes.html
> says "incorrectly optimised, resulting in space leaks".
>
> Maybe it should instead say "incorrectly optimised, resulting in
> taking the wrong branch in 'if' expressions"?  That's a bit more
> alarming, and is a stronger "upgrade to 8.4.2" signal.

Yes, I suppose the language in the release notes does rather understate
the degree of the incorrectness.

I'll push a new version of the manual with some stronger language
tomorrow.

Cheers,

- Ben


signature.asc
Description: PGP signature
___
ghc-devs mailing list
ghc-devs@haskell.org
http://mail.haskell.org/cgi-bin/mailman/listinfo/ghc-devs