fromIntegral (2^64 :: Integer) == (0 :: Natural)

added Tbug ghc-bignum needs triage labels

This is a regression from GHC 8.10:

$ /opt/ghc/8.10.4/bin/ghc -O1 -fforce-recomp powmod.hs && ./powmod
[1 of 1] Compiling Main             ( powmod.hs, powmod.o )
Linking powmod ...
0

changed milestone to %9.0.2

added Phighest incorrect runtime result labels

Seems powModNatural is an innocent victim, the real culprit is fromIntegral :: Integer -> Natural.

import Data.Bits
import GHC.Natural

main = print $ show (fromIntegral n :: Natural)
  where
    n = 1 `shiftL` 64 :: Integer
    {-# NOINLINE n #-}

$ ghc -O1 fromIntegral.hs && ./fromIntegral
[1 of 1] Compiling Main             ( fromIntegral.hs, fromIntegral.o )
Linking fromIntegral ...
"0"

This looks pretty bad.

This was fixed by !4980 (closed) and will be present in 9.0.2. I think the regression test T19345 covers the same ground, so we don't have to add another one.

closed

It is very nice indeed that it is already fixed, but what about mitigation plan and public communication?

It looks like any library dealing with fromIntegral and Natural is affected, silently returning an incorrect result. This is outright dangerous. And almost invisible in testing, because QuickCheck tests rarely generate inputs of such magnitude.

In my case the bug hit two libraries, both with cryptographic applications, and I was lucky to have thorough enough tests. What is the best way to fix them?

Simplest might be to refuse to comipiple with __GLASGOW_HASKELL == 901, users will have to use 8.x or >= 9.0.2. I am not sure that more complex work-arounds are warranted.

Yes, one way around is just skip GHC 9.0.1, probably by base /= 4.15.0.0 in cabal file. I wonder if there are other, less disruptive options, but they require a way to audit external dependencies. Potentially one can run a build logging triggered rewrite rules, and compare logs between GHC 9.0.1 and GHC HEAD. It does not sound quite appealing, but once affected libraries are identified, they can be fixed with

#if __GLASGOW_HASKELL__ == 900 && __GLASGOW_HASKELL_PATCHLEVEL1__ == 1
import Prelude hiding (fromIntegral)

fromIntegral = fromInteger . toInteger

Still not perfect, because it can slow down other, legit applications of fromIntegral in the same file.

reopened

removed needs triage label

changed title from {-powModNatural 0 (1 shiftL 64) 2 /= 0-} to fromIntegral (2^64 :: Integer) == (0 :: Natural)

Ouch this is a bad bug. Crashing would be better :(

Yes this bug is awful, sorry about that :/

It is very nice indeed that it is already fixed, but what about mitigation plan and public communication?

@bgamari When is 9.0.2 expected? Could we release it soon with a big warning about not using 9.0.1 for anything mission critical?

Orthogonally to 9.0.2, I believe it is worth to warn public about the issue, before more people get hurt. IMO it would be more appropriate if such warning comes from GHC developers, even if in form of email to Haskell Cafe or whatever.

It might very well be that my perception is biased; if others do not share my feelings, feel free to close, I do not mean to dictate others what to do.

I fully agree. I was hoping that we could release 9.0.2 soon enough to propose a simple solution. Anyway I've sent the email: https://mail.haskell.org/pipermail/haskell-cafe/2021-March/133540.html I'll post it on reddit too.

@hsyl20 thanks, this is very much appreciated. Closing now.

closed

mentioned in issue haskell/ghcup-hs#297 (closed)

fromIntegral (2^64 :: Integer) == (0 :: Natural)

Summary

Steps to reproduce

Expected behavior

Environment

Child items ...

Activity