Opened 6 months ago

Closed 6 months ago

Last modified 6 months ago

#8425 closed bug (fixed)

ghc-7.6.3: crossmodule inline leads to buggy code (-O2)

Reported by: slyfox Owned by:
Priority: normal Milestone:
Component: Compiler Version: 7.6.3
Keywords: Cc:
Operating System: Unknown/Multiple Architecture: x86_64 (amd64)
Type of failure: Runtime crash Difficulty: Unknown
Test Case: stranal/should_run/T8425 Blocked By:
Blocking: Related Tickets:

Description

The bug is found as a crash of gf-3.4 ('Grammatical Framework') package compiled with -O2.
With some debugging I've come to roughly following minimal example:

This code compiles incorrectly:

import BuggyOpt() -- bug inducer!

import qualified Data.Map as Map
import Data.Array.IArray

{-# NOINLINE mkLin #-}
mkLin :: Array Int (Array Int Int) -> Map.Map (Array Int Int) Int -> Int -> (Map.Map (Array Int Int) Int, Int)
mkLin mseqs seqs seqid =
    let seq_ = mseqs ! seqid
    -- lookup/insert pair. exactly as in 'BuggyOpt' module
    in case Map.lookup seq_ seqs of
        Just seqid' -> (seqs,seqid')
        Nothing    -> let seqid'' = Map.size seqs
                          seqs'   = Map.insert seq_ seqid'' seqs
                      in (seqs',seqid'')

And this works fine:

--import BuggyOpt() -- bug inducer!

import qualified Data.Map as Map
import Data.Array.IArray

{-# NOINLINE mkLin #-}
mkLin :: Array Int (Array Int Int) -> Map.Map (Array Int Int) Int -> Int -> (Map.Map (Array Int Int) Int, Int)
mkLin mseqs seqs seqid =
    let seq_ = mseqs ! seqid
    -- lookup/insert pair. exactly as in 'BuggyOpt' module
    in case Map.lookup seq_ seqs of
        Just seqid' -> (seqs,seqid')
        Nothing    -> let seqid'' = Map.size seqs
                          seqs'   = Map.insert seq_ seqid'' seqs
                      in (seqs',seqid'')

Attached minimal test tarball and my results on it are:

 [("Project name","The Glorious Glasgow Haskell Compilation System")
 ,("GCC extra via C opts"," -fwrapv")
 ,("C compiler command","x86_64-pc-linux-gnu-gcc")
 ,("C compiler flags"," -fno-stack-protector  -Wl,--hash-size=31 -Wl,--reduce-memory-overheads")
 ,("ar command","/usr/bin/ar")
 ,("ar flags","q")
 ,("ar supports at file","YES")
 ,("touch command","touch")
 ,("dllwrap command","/bin/false")
 ,("windres command","/bin/false")
 ,("perl command","/usr/bin/perl")
 ,("target os","OSLinux")
 ,("target arch","ArchX86_64")
 ,("target word size","8")
 ,("target has GNU nonexec stack","True")
 ,("target has .ident directive","True")
 ,("target has subsections via symbols","False")
 ,("LLVM llc command","/usr/bin/llc")
 ,("LLVM opt command","/usr/bin/opt")
 ,("Project version","7.6.3")
 ,("Booter version","7.6.3")
 ,("Stage","2")
 ,("Build platform","x86_64-unknown-linux")
 ,("Host platform","x86_64-unknown-linux")
 ,("Target platform","x86_64-unknown-linux")
 ,("Have interpreter","YES")
 ,("Object splitting supported","YES")
 ,("Have native code generator","YES")
 ,("Support SMP","YES")
 ,("Unregisterised","NO")
 ,("Tables next to code","YES")
 ,("RTS ways","l debug  thr thr_debug thr_l thr_p dyn debug_dyn thr_dyn thr_debug_dyn")
 ,("Leading underscore","NO")
 ,("Debug on","False")
 ,("LibDir","/usr/lib64/ghc-7.6.3")
 ,("Global Package DB","/usr/lib64/ghc-7.6.3/package.conf.d")
 ,("Gcc Linker flags","[\"-Wl,--hash-size=31\",\"-Wl,--reduce-memory-overheads\"]")
 ,("Ld Linker flags","[\"--hash-size=31\",\"--reduce-memory-overheads\"]")
 ]
RUN good
[1 of 2] Compiling Susp             ( good/Susp.hs, good/Susp.o )
[2 of 2] Compiling Main             ( good/main.hs, good/main.o )
Linking good/main ...
(fromList [(array (0,0) [(0,42)],0)],0)
RUN bad
[1 of 3] Compiling BuggyOpt         ( bad/BuggyOpt.hs, bad/BuggyOpt.o )
[2 of 3] Compiling Susp             ( bad/Susp.hs, bad/Susp.o )
[3 of 3] Compiling Main             ( bad/main.hs, bad/main.o )
Linking bad/main ...
main: Error in array index; 0 not in range [0..0)
TEST FAILED in bad

What we see here is the test with useless import mysteriously fails:

main: Error in array index; 0 not in range [0..0)

while without import it works fine:

(fromList [(array (0,0) [(0,42)],0)],0)

The functions in both BuggyOpt? and Susp modules contain suspiciously similar code:

  case Map.lookup seq' seqs of
    Just id' -> (seqs,id')
    Nothing -> let last_seq = Map.size seqs
               in (Map.insert seq' last_seq seqs, last_seq)

But in BuggyOpt? ghc somehow deduces some arguments
and applies that effect to (unrelated, but looking similar)
Susp module.

Thanks!

Attachments (3)

inliner-bug.tar.gz (1.2 KB) - added by slyfox 6 months ago.
inliner-bug.zip (29.3 KB) - added by monoidal 6 months ago.
version with inlined libraries
inliner-bug-v2.tar.gz (2.2 KB) - added by slyfox 6 months ago.
inliner-bug-v2.tar.gz - compile-time failure

Download all attachments as: .zip

Change History (11)

Changed 6 months ago by slyfox

comment:1 follow-up: Changed 6 months ago by carter

I believe this is a known issue in http://ghc.haskell.org/trac/ghc/ticket/5550 and http://ghc.haskell.org/trac/ghc/ticket/7944 ?

could you try compiling the code with HEAD and see if the error is still there?

comment:2 Changed 6 months ago by monoidal

That's a different ticket. I can reproduce the "Error in array index" in 7.6, but in HEAD it works fine. I will close the ticket soon, but leave some time in case someone more experienced knows what's going on.

comment:3 in reply to: ↑ 1 Changed 6 months ago by slyfox

Replying to carter:

I believe this is a known issue in http://ghc.haskell.org/trac/ghc/ticket/5550 and http://ghc.haskell.org/trac/ghc/ticket/7944 ?

could you try compiling the code with HEAD and see if the error is still there?

Yeah, head works:

# ./run_test.bash
...

,("Project version","7.7.20131009")
,("Booter version","7.6.3")
,("Stage","2")
,("Build platform","x86_64-unknown-linux")
,("Host platform","x86_64-unknown-linux")
,("Target platform","x86_64-unknown-linux")
,("Have interpreter","YES")
,("Object splitting supported","YES")
,("Have native code generator","YES")
,("Support SMP","YES")
,("Tables next to code","YES")
,("RTS ways","l debug thr thr_debug thr_l thr_p dyn debug_dyn thr_dyn thr_debug_dyn l_dyn thr_l_dyn")
,("Support dynamic-too","YES")
,("Support parallel --make","YES")
,("Dynamic by default","NO")
,("GHC Dynamic","YES")
,("Leading underscore","NO")
,("Debug on","False")
,("LibDir?","/var/tmp/portage/dev-lang/ghc-9999/work/ghc-9999/inplace/lib")
,("Global Package DB","/var/tmp/portage/dev-lang/ghc-9999/work/ghc-9999/inplace/lib/package.conf.d")
]

RUN good
[1 of 2] Compiling Susp ( good/Susp.hs, good/Susp.o )
[2 of 2] Compiling Main ( good/main.hs, good/main.o )
Linking good/main ...
(fromList [(array (0,0) [(0,42)],0)],0)
RUN bad
[1 of 3] Compiling BuggyOpt? ( bad/BuggyOpt.hs, bad/BuggyOpt.o )
[2 of 3] Compiling Susp ( bad/Susp.hs, bad/Susp.o )
[3 of 3] Compiling Main ( bad/main.hs, bad/main.o )
Linking bad/main ...
(fromList [(array (0,0) [(0,42)],0)],0)

But why are those bugs related to this issue?
AFAIU they are compile-time issues.

May I ask you to point to related commitspossibly fixing this?

And what workaround would you suggest for this particular code?
I'm using {-# NOINLINE #-} in random places inside miscompiled
function.

Thanks!

Changed 6 months ago by monoidal

version with inlined libraries

Changed 6 months ago by slyfox

inliner-bug-v2.tar.gz - compile-time failure

comment:4 Changed 6 months ago by slyfox

inliner-bug-v2.tar.gz - a bit refined monoidal's variant with dropped strict field.
Now it makes final binary fail in a very fun way (maybe adds some clue):

./run_test.bash
RUN good
True
RUN bad
Main: Oops!  Entered absent arg ww_s3a6{v} [lid] ghc-prim:GHC.Types.Int{(w) tc 3J}
TEST FAILED in bad

comment:5 Changed 6 months ago by slyfox

My theory is the bug is induced by {-# INLINABLE insert #-} and {-# INLINABLE lookup #-}
pragmas.

It makes Main module use BuggyOpt?.$sinsert specialisation (with it's environment taken into account).
It's seen nicely when we build Main in both modes:

good (all ok: Base.lookup):

==================== Occurrence analysis ====================
[snip instances]
Main.mkLin
  :: Arr.Array GHC.Types.Int
     -> Base.Map (Arr.Array GHC.Types.Int) GHC.Types.Int
[LclIdX,
 Arity=1,
 Unf=Unf{Src=<vanilla>, TopLvl=True, Arity=1, Value=True,
         ConLike=True, WorkFree=True, Expandable=True,
         Guidance=IF_ARGS [0] 110 0}]
Main.mkLin =
  \ (mseqs_amq :: Arr.Array GHC.Types.Int) ->
    case Base.lookup
           @ (Arr.Array GHC.Types.Int)
           @ (GHC.Prim.Any *)
           (Arr.$fEqArray @ GHC.Types.Int)
           mseqs_amq
           (Base.empty @ (Arr.Array GHC.Types.Int) @ (GHC.Prim.Any *))
    of _ { __DEFAULT ->
    Base.insert
      @ (Arr.Array GHC.Types.Int)
      @ GHC.Types.Int
      $dOrd_apD
      mseqs_amq
      (GHC.Types.I# 1)
      (Base.empty @ (Arr.Array GHC.Types.Int) @ GHC.Types.Int)
    }

bad (note BuggyOpt?.$sinsert on an early phase):

==================== Occurrence analysis ====================
Main.mkLin
  :: Arr.Array GHC.Types.Int
     -> Base.Map (Arr.Array GHC.Types.Int) GHC.Types.Int
[LclIdX,
 Arity=1,
 Unf=Unf{Src=<vanilla>, TopLvl=True, Arity=1, Value=True,
         ConLike=True, WorkFree=True, Expandable=True,
         Guidance=IF_ARGS [0] 100 0}]
Main.mkLin =
  \ (mseqs_amy :: Arr.Array GHC.Types.Int) ->
    case Base.lookup
           @ (Arr.Array GHC.Types.Int)
           @ (GHC.Prim.Any *)
           (Arr.$fEqArray @ GHC.Types.Int)
           mseqs_amy
           (Base.empty @ (Arr.Array GHC.Types.Int) @ (GHC.Prim.Any *))
    of _ { __DEFAULT ->
    BuggyOpt.$sinsert
      @ GHC.Types.Int
      mseqs_amy
      (GHC.Types.I# 1)
      (Base.empty @ (Arr.Array GHC.Types.Int) @ GHC.Types.Int)
    }

Does it look suspicious?

comment:6 Changed 6 months ago by slyfox

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

As noted by monoidal in irc the nature of error points to strictness analyzer (like in bug #7737)

Looks like changeset:0831a12ea2fc73c33652eeec1adc79fa19700578 and later
changed the logic substantially, thus there is no simple thing for me to
try to pull into 7.6 branch to test the theory.

Marking as fixed. Maybe, tests is worth pulling in the test suite
(it's a bit big though). Sorry for the noise.

Thanks!

comment:7 Changed 6 months ago by Simon Peyton Jones <simonpj@…>

comment:8 Changed 6 months ago by simonpj

  • Test Case set to stranal/should_run/T8425

I added it as a test case. Thanks.

Simon

Note: See TracTickets for help on using tickets.