summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorÖmer Sinan Ağacan <omeragacan@gmail.com>2019-05-24 11:23:01 +0300
committerBen Gamari <ben@well-typed.com>2019-06-03 23:42:15 -0400
commitc26461fcab27fbad8abafedc085a0edc3d28eb59 (patch)
tree9e4fc29324a1822851d32feee6aa914099b846b7
parent605869c7b776ce6071a31ff447998b081e0354ed (diff)
downloadhaskell-c26461fcab27fbad8abafedc085a0edc3d28eb59.tar.gz
Fix rewriting invalid shifts to errors
Fixes #16449. 5341edf3 removed a code in rewrite rules for bit shifts, which broke the "silly shift guard", causing generating invalid bit shifts or heap overflow in compile time while trying to evaluate those invalid bit shifts. The "guard" is explained in Note [Guarding against silly shifts] in PrelRules.hs. More specifically, this was the breaking change: --- a/compiler/prelude/PrelRules.hs +++ b/compiler/prelude/PrelRules.hs @@ -474,12 +474,11 @@ shiftRule shift_op ; case e1 of _ | shift_len == 0 -> return e1 - | shift_len < 0 || wordSizeInBits dflags < shift_len - -> return (mkRuntimeErrorApp rUNTIME_ERROR_ID wordPrimTy - ("Bad shift length" ++ show shift_len)) This patch reverts this change. Two new tests added: - T16449_1: The original reproducer in #16449. This was previously casing a heap overflow in compile time when CmmOpt tries to evaluate the large (invalid) bit shift in compile time, using `Integer` as the result type. Now it builds as expected. We now generate an error for the shift as expected. - T16449_2: Tests code generator for large (invalid) bit shifts.
-rw-r--r--compiler/prelude/PrelRules.hs5
-rw-r--r--testsuite/tests/codeGen/should_compile/T16449_1.hs8
-rw-r--r--testsuite/tests/codeGen/should_compile/all.T2
-rw-r--r--testsuite/tests/codeGen/should_run/T16449_2.hs9
-rw-r--r--testsuite/tests/codeGen/should_run/T16449_2.stderr1
-rw-r--r--testsuite/tests/codeGen/should_run/all.T1
6 files changed, 25 insertions, 1 deletions
diff --git a/compiler/prelude/PrelRules.hs b/compiler/prelude/PrelRules.hs
index 7111c7b07a..b0ad91db99 100644
--- a/compiler/prelude/PrelRules.hs
+++ b/compiler/prelude/PrelRules.hs
@@ -467,13 +467,16 @@ shiftRule :: (DynFlags -> Integer -> Int -> Integer) -> RuleM CoreExpr
-- Used for shift primops
-- ISllOp, ISraOp, ISrlOp :: Word# -> Int# -> Word#
-- SllOp, SrlOp :: Word# -> Int# -> Word#
--- See Note [Guarding against silly shifts]
shiftRule shift_op
= do { dflags <- getDynFlags
; [e1, Lit (LitNumber LitNumInt shift_len _)] <- getArgs
; case e1 of
_ | shift_len == 0
-> return e1
+ -- See Note [Guarding against silly shifts]
+ | shift_len < 0 || shift_len > wordSizeInBits dflags
+ -> return $ mkRuntimeErrorApp rUNTIME_ERROR_ID wordPrimTy
+ ("Bad shift length " ++ show shift_len)
-- Do the shift at type Integer, but shift length is Int
Lit (LitNumber nt x t)
diff --git a/testsuite/tests/codeGen/should_compile/T16449_1.hs b/testsuite/tests/codeGen/should_compile/T16449_1.hs
new file mode 100644
index 0000000000..b1ec71efef
--- /dev/null
+++ b/testsuite/tests/codeGen/should_compile/T16449_1.hs
@@ -0,0 +1,8 @@
+module T16449_1 where
+
+import Data.Bits (setBit)
+
+f :: Int
+f = foldl setter 0 $ zip [0..] [()]
+ where
+ setter v (ix, _) = setBit v ix
diff --git a/testsuite/tests/codeGen/should_compile/all.T b/testsuite/tests/codeGen/should_compile/all.T
index 7217d93e30..7a9229533d 100644
--- a/testsuite/tests/codeGen/should_compile/all.T
+++ b/testsuite/tests/codeGen/should_compile/all.T
@@ -52,3 +52,5 @@ test('T15723',
[ unless(have_profiling(), skip),
unless(have_dynamic(), skip),
], run_command, ['$MAKE -s --no-print-directory T15723'])
+
+test('T16449_1', normal, compile, [''])
diff --git a/testsuite/tests/codeGen/should_run/T16449_2.hs b/testsuite/tests/codeGen/should_run/T16449_2.hs
new file mode 100644
index 0000000000..3424d43e25
--- /dev/null
+++ b/testsuite/tests/codeGen/should_run/T16449_2.hs
@@ -0,0 +1,9 @@
+{-# LANGUAGE MagicHash #-}
+
+module Main where
+
+import GHC.Prim
+import GHC.Int
+
+-- Shift should be larger than the word size (e.g. 64 on 64-bit) for this test.
+main = print (I# (uncheckedIShiftL# 1# 1000#))
diff --git a/testsuite/tests/codeGen/should_run/T16449_2.stderr b/testsuite/tests/codeGen/should_run/T16449_2.stderr
new file mode 100644
index 0000000000..869a5b0f91
--- /dev/null
+++ b/testsuite/tests/codeGen/should_run/T16449_2.stderr
@@ -0,0 +1 @@
+T16449_2: Bad shift length 1000
diff --git a/testsuite/tests/codeGen/should_run/all.T b/testsuite/tests/codeGen/should_run/all.T
index 7f976b8107..ade88df6ab 100644
--- a/testsuite/tests/codeGen/should_run/all.T
+++ b/testsuite/tests/codeGen/should_run/all.T
@@ -192,3 +192,4 @@ test('T15892',
# happen, so -G1 -A32k:
extra_run_opts('+RTS -G1 -A32k -RTS') ],
compile_and_run, ['-O'])
+test('T16449_2', exit_code(1), compile_and_run, [''])