Ticket #5230 (closed bug: fixed)

Opened 2 years ago

Last modified 2 years ago

Segfault in cgrun064

Reported by: daniel.is.fischer Owned by: tibbe
Priority: normal Milestone:
Component: Compiler Version: 7.1
Keywords: Cc: johan.tibell@…
Operating System: Unknown/Multiple Architecture: x86
Type of failure: None/Unknown Difficulty:
Test Case: Blocked By:
Blocking: Related Tickets:

Description

cgrun064 failed (all ways) with a segfault:

=====> cgrun064(normal) 1467 of 2801 [0, 14, 0]
cd ./codeGen/should_run && '/home/dafis/Haskell/Hacking/Gits/gcd/bindisttest/install dir/bin/ghc' -fforce-recomp -dcore-lint -dcmm-lint -dno-debug-output -no-user-package-conf -rtsopts -optl-lz -o cgrun064 cgrun064.hs    >cgrun064.comp.stderr 2>&1
cd ./codeGen/should_run && ./cgrun064    </dev/null >cgrun064.run.stdout 2>cgrun064.run.stderr
/bin/sh: Zeile 1: 20142 Speicherzugriffsfehler  ./cgrun064 < /dev/null > cgrun064.run.stdout 2> cgrun064.run.stderr
Wrong exit code (expected 0 , actual 139 )
Stdout:

Stderr:

*** unexpected failure for cgrun064(normal)

No idea yet what may cause it.

Attachments

Change History

Changed 2 years ago by tibbe

  • cc johan.tibell@… added
  • owner set to tibbe

Changed 2 years ago by daniel.is.fischer

freezeArray is at least one cause for the segfaults. With the simple example

{-# LANGUAGE MagicHash, UnboxedTuples #-}
module Try where

import GHC.Base
import GHC.ST

list :: [Int]
list = toList ar 5
  where
    ar = runST $ do
        src <- newArray 16 3
        freezeArray src 4 10

data Array a = Array { unArray :: Array# a }
data MArray s a = MArray { unMArray :: MutableArray# s a }

newArray :: Int -> a -> ST s (MArray s a)
newArray (I# n#) a = ST $ \s# -> case newArray# n# a s# of
    (# s2#, marr# #) -> (# s2#, MArray marr# #)

indexArray :: Array a -> Int -> a
indexArray arr (I# i#) = case indexArray# (unArray arr) i# of
    (# a #) -> a

writeArray :: MArray s a -> Int -> a -> ST s ()
writeArray marr (I# i#) a = ST $ \ s# ->
    case writeArray# (unMArray marr) i# a s# of
        s2# -> (# s2#, () #)

freezeArray :: MArray s a -> Int -> Int -> ST s (Array a)
freezeArray src (I# six#) (I# n#) = ST $ \ s# ->
    case unsafeFreezeArray# (unMArray src) s# of
      (# s2#, arr# #) -> (# s2#, Array arr# #)
--     case freezeArray# (unMArray src) six# n# s# of
--         (# s2#, arr# #) -> (# s2#, Array arr# #)

toList :: Array a -> Int -> [a]
toList arr n = go 0
  where
    go i | i >= n = []
         | otherwise = indexArray arr i : go (i+1)

it works. Using freezeArray# instead, it segfaults. But it also segfaults on copying when not using freezeArray# for freezing, so it's not the only culprit.

Changed 2 years ago by tibbe

Here's a simpler example:

{-# LANGUAGE MagicHash, UnboxedTuples #-}
module Main (main) where

import Control.Exception
import GHC.Exts hiding (copyArray#)
import GHC.PrimopWrappers (copyArray#)
import GHC.ST

main :: IO ()
main = evaluate $ runST $ ST $ \ s -> case newArray# 32# 0 s of
    (# s2, src #) -> case unsafeFreezeArray# src s2 of
        (# s3, src #) -> case newArray# 32# 1 s3 of
            (# s4, dst #) -> case copyArray# src 0# dst 0# 1# s4 of
                s5 -> (# s5, () #)

Changed 2 years ago by tibbe

  • os changed from Linux to Unknown/Multiple

I've stared at the assembly for GHC.PrimopWrappers.copyArray# (as reported by GDB's disassemble command) but I can't figure out what's going on. Here's the (partially annotated) assembly:

ghczmprim_GHCziPrimopWrappers_copyArrayzh_info:
mov    0x0(%ebp),%eax           ; eax = src
mov    0x4(%ebp),%ecx           ; ecx = src_off
mov    0x8(%ebp),%edx           ; edx = dst
mov    %eax,0x40(%esp)          ; save src on the stack
mov    0xc(%ebp),%eax           ; eax = dst_off
mov    %eax,0x4c(%esp)          ; save dst_off on the stack
mov    0x10(%ebp),%eax          ; eax = n
mov    %eax,0x58(%esp)          ; save n on the stack
mov    0x2430e0,%eax            ; set header
mov    %eax,(%edx)              ; set header cont.
sub    $0x4,%esp
mov    0x5c(%esp),%eax          ; eax = n
shl    $0x2,%eax                ; eax = BYTES(n)
push   %eax                     ; 3rd memcpy arg = BYTES(n)
shl    $0x2,%ecx                ; ecx = BYTES(src_off)
mov    0x48(%esp),%eax          ; eax = src
add    $0xc,%eax                ; eax = src + 12
add    %ecx,%eax                ; eax = src + 12 + BYTES(src_off)
push   %eax                     ; 2nd memcpy arg = src + 12 + BYTES(src_off)
mov    0x58(%esp),%eax          ; eax = dst_off
mov    %eax,%ecx                ; ecx = dst_off
shl    $0x2,%ecx                ; ecx = BYTES(dst_off)
lea    0xc(%edx),%eax           ; eax = dst + 12
add    %ecx,%eax                ; eax = dst + 12 + BYTES(dst_off)
push   %eax                     ; 1st memcpy arg = dst + 12 + BYTES(dst_off)
mov    %edx,0x74(%esp)          ; save edx
call   0x2426da <dyld_stub_memcpy> ; caller-saves: eax, ecx, edx
add    $0x10,%esp               ; stack clean-up
mov    0x4c(%esp),%eax          ; eax = n
mov    %eax,%ecx                ; ecx = n
shr    $0x7,%ecx                ; ecx = n >> 7
sub    $0x7,%esp                ; ??? Why subtract 7?
mov    $0x1,%edx                ; edx = 1
sub    %ecx,%edx                ; edx = 1 - (n >> 7)
mov    %ecx,0x74(%esp)          ; save edx
mov    0x5c(%esp),%ecx
add    %ecx,%eax
shr    $0x7,%eax
add    %edx,%eax
push   %eax
push   $0x1
mov    0x70(%esp),%eax
mov    0x4(%eax),%ecx           ; crash here
shl    $0x2,%ecx
mov    0x7c(%esp),%edx
add    %edx,%ecx
add    $0xc,%eax
add    %ecx,%eax
push   %eax
call   0x2426e6 <dyld_stub_memset>
add    $0x10,%esp
add    $0x14,%ebp
jmp    *0x0(%ebp)
sbb    %al,(%eax)
add    %al,(%eax)
xchg   %ax,%ax

I recommend reading it side-by-side with emitCopyArray from compiler/codeGen/CgPrimOp.hs.

The assembly makes sense to me up to the point where 7 is subtracted from the stack pointer.

Changed 2 years ago by tibbe

  • status changed from new to patch

Fixed.

commit 820c8e4d7b627aa67b6eb302f8417bcccee3fa81
Author: Johan Tibell <johan.tibell@gmail.com>
Date:   Tue Jun 7 14:44:23 2011 +0200

    Fix segfault in array copy primops on 32-bit
    
    The second argument to C's memset was passed as a W8 while memset
    expects an int.

Please merge for 7.2.

Changed 2 years ago by tibbe

This raises an interesting question: with the direct call's to the C functions in CgPrimOp being replaced with the CallishMachOps David Terei added, who is responsible for making sure that we call C's memcpy with a value of the right width? In the new world order I think emitMemset should be called with a W8 value and each backend should do the right thing e.g. emit the value as is in the LLVM backend and convert the value to word width in the backends that call C's memcpy (e.g. the x86 backend).

Changed 2 years ago by dterei

  • status changed from patch to closed
  • resolution set to fixed

Applied patch in d0faaa6fa0cecd23c5670fd199e9206275313666. Thanks!

Note: See TracTickets for help on using tickets.