modifyIORef causes segmentation fault in FFI callback

[thalerjonathan]

Steps to Reproduce

import Data.IORef

%include C "idris_clbks_test.h"
%link C "idris_clbks_test.o"

TestCallback : Type
TestCallback = Int -> ()

testCallbackPure : IORef Integer -> Int -> ()
testCallbackPure ref x = unsafePerformIO $ do 
  putStrLn "before modifyIORef"
  modifyIORef ref (+1) -- segmentation fault occurs here
  putStrLn "after modifyIORef"

testCallbackPurePtr : IORef Integer -> IO Ptr
testCallbackPurePtr ref
  = foreign FFI_C "%wrapper" (CFnPtr TestCallback -> IO Ptr) (MkCFnPtr $ testCallbackPure ref)

setTestCallbackWithPtr : IORef Integer -> IO ()
setTestCallbackWithPtr ref = do
  ptr <- testCallbackPurePtr ref
  foreign FFI_C "registerCallback" (Ptr -> IO ()) ptr

runCallback : IO ()
runCallback = foreign FFI_C "runCallback" (IO ())

export
run : IO ()
run = do
  ref <- newIORef 42
  setTestCallbackWithPtr ref
  runCallback
  ret <- readIORef ref
  putStrLn $ "IORef = " ++ show ret

Note that is is not possible to reproduce this problem with just this code, one needs the c files as well and compile the whole thing into an executable. For this purpose I created a repository which is available here https://github.com/thalerjonathan/idris-callbacks-tests. The relevant file for this problem is https://github.com/thalerjonathan/idris-callbacks-tests/blob/master/TestModifyIORef.idr To compile this example run 'make' in the cloned folder. Make sure that in the Main.idr the TestModifyIORef.run function is called.

Expected Behavior

Program should not crash.

Observed Behavior

Program crashes with segmentation fault when modifyIORef is called.

[thalerjonathan]

Is also caused by readIORef and writeIORef alone.

[thalerjonathan]

The crash occurs in idris_readRef (). I uploaded the crashing binary https://github.com/thalerjonathan/idris-callbacks-tests/blob/master/idrisclbktest and the coredump https://github.com/thalerjonathan/idris-callbacks-tests/blob/master/core.6975 for further inspection.

[ivanperez-keera]

I don't understand. That function is simply casting a VAL into a pointer to a Ref, and then accessing the VAL inside (which is the second field in that struct). This means that either the VAL is null as a pointer (which I'm hoping one can check in idris which some unsafe casting), or it's not pointing to a struct of that type. But I don't see how. The code of this part seems straightforward:

VAL idris_newRefLock(VAL x, int outerlock) {
    Ref * cl = allocate(sizeof(*cl), outerlock);
    SETTY(cl, CT_REF);
    cl->ref = x;
    return (VAL)cl;
}

VAL idris_newRef(VAL x) {
    return idris_newRefLock(x, 0);
}

void idris_writeRef(VAL ref, VAL x) {
    Ref * r = (Ref*)ref;
    r->ref = x;
    SETTY(ref, CT_REF);
}

VAL idris_readRef(VAL ref) {
    Ref * r = (Ref*)ref;
    return r->ref;
}

[ivanperez-keera]

I guess the right way would be to compile idris's RTS with debugging symbols and track it down :|

[edwinb]

I've had a quick look, and at the moment I can only guess that something funny is going on with the callback mechanism - I'm not especially familiar with how that works though. Does anyone know enough about it to be able to take a closer look?

[melted]

I implemented it, I can take a look.

8 maj 2018 kl. 14:13 skrev Edwin Brady notifications@github.com:

I've had a quick look, and at the moment I can only guess that something funny is going on with the callback mechanism - I'm not especially familiar with how that works though. Does anyone know enough about it to be able to take a closer look?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

[melted]

I have started debugging this.

[melted]

The problem here is that a closure is constructed and passed as a callback, and the implementation can't handle that. It's obviously a thing we would want, but there needs to be a mechanism to stow away data between the creation and the use of the callback.

[melted]

It's not trivial as we would need to handle use of the same callback with different data. So the current implementation that just uses the static function pointer would have to be remodeled. Doing it at runtime would be hard. There are no closures in C and there is no way for us to pass the information of what data to retrieve on the side to the called function. I suppose one could create thunks at runtime, but that would be a major project.

If all the use sites are statically known, all the different wrappers can be generated and the dispatcher can stow away the data for the wrapper functions to use.

[ivanperez-keera]

So, if I understand correctly, it's not easy to fix, but we might get away with this so long as all arguments are passed to the callback. Could the callback receive a pointer to an IORef that it should get its arguments from, always pre-applied when the callback is installed, and use it's own logic to read from the IORef?

[ivanperez-keera]

I mean something along the following lines:

TestCallback : Type
TestCallback = ()

testCallbackPure : IORef Integer -> IORef Int -> ()
testCallbackPure ref xRef = unsafePerformIO $ do 
  putStrLn "before modifyIORef"
  x <- readIORef xRef
  modifyIORef ref (+1) -- segmentation fault occurs here
  putStrLn "after modifyIORef"

testCallbackPurePtr : IORef Integer -> IORef Int -> IO Ptr
testCallbackPurePtr ref ref2
  = foreign FFI_C "%wrapper" (CFnPtr TestCallback -> IO Ptr) (MkCFnPtr $ testCallbackPure ref ref2)

Will strictness get in the way?

[melted]

This is a solution that works, where the reference is passed explicitly:

module TestModifyIORef

import Data.IORef

%include C "idris_clbks_test.h"
%link C "idris_clbks_test.o"
%flag C "-g"

TestCallback : Type
TestCallback = Raw (IORef Integer) -> Int -> ()

testCallbackPure : Raw (IORef Integer) -> Int -> ()
testCallbackPure (MkRaw ref) x = unsafePerformIO $ do 
  putStrLn "before modifyIORef"
  modifyIORef ref (+1) -- segmentation fault occurs here
  --writeIORef ref 42
  --ret <- readIORef ref
  --putStrLn $ "readIORef: " ++ show ret
  putStrLn "after modifyIORef"

testCallbackPurePtr : IO Ptr
testCallbackPurePtr
  = foreign FFI_C "%wrapper" (CFnPtr TestCallback -> IO Ptr) (MkCFnPtr $ testCallbackPure)

setTestCallbackWithPtr : IORef Integer -> IO ()
setTestCallbackWithPtr ref = do
  ptr <- testCallbackPurePtr 
  foreign FFI_C "registerCallback" (Raw (IORef Integer) -> Ptr -> IO ()) (MkRaw ref) ptr

runCallback : IO ()
runCallback = foreign FFI_C "runCallback" (IO ())

export
run : IO ()
run = do
  ref <- newIORef 42
  setTestCallbackWithPtr ref
  runCallback
  ret <- readIORef ref
  putStrLn $ "IORef = " ++ show ret

#include "idris_clbks_test.h"

static TestCallback clbk = 0;
static void* idris_ref = NULL;

void
registerCallback(void* ref, TestCallback c)
{
  idris_ref = ref;  
  clbk = c;
}

void
runCallback()
{
  if (clbk)
  {
    clbk(idris_ref, 42);
  }
}

[melted]

GHC solves the problem by generating code on the fly: https://ghc.haskell.org/trac/ghc/wiki/Commentary/Rts/FFI

[thalerjonathan]

Am I correct in assuming that the 1.3.1 update prevents now such things already at compile-time? I tried to recompile my project with the new version and I am running into

idris: Can't wrap a closure as callback.
CallStack (from HasCallStack):
  error, called at src/IRTS/CodegenC.hs:903:22 in idris-1.3.1-2z8wIvUOZnAJWybETWlOEe:IRTS.CodegenC

[melted]

Yes, this can’t be supported on the C backend without run-time code generation.

29 okt. 2018 kl. 11:53 skrev IO.Nathan notifications@github.com:

Am I correct in assuming that the 1.3.1 update prevents now such things already at compile-time? I tried to recompile my project with the new version and I am running into

idris: Can't wrap a closure as callback. CallStack (from HasCallStack): error, called at src/IRTS/CodegenC.hs:903:22 in idris-1.3.1-2z8wIvUOZnAJWybETWlOEe:IRTS.CodegenC — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.