Make C calls be safe points. Move arguments to thread locals. Eventua…

…lly need to get rid of that whole complicated system

.vscode/launch.json

@@ -22,7 +22,7 @@
                     "kind": "bin"
                 }
             },
-            "args": ["resources/ffi_test.bg"],
+            "args": ["resources/thread.bg"],
             "cwd": "${workspaceFolder}"
         },
         {

TODO.md

@@ -22,5 +22,4 @@
 * Builtins need better type checking setup
 * Need to make the tests start a process so if one crashes, we still know the total results
 * I've got some stuff going on with malloc and free that is making rust not happy. In the case I saw, it looks like namsepace bindings are causing issues. I can find it by running the sdl example.
-* We need to say that a thread is at a safe point when it makes a c call
-* And then check if we are in gc when we return from one
+* I need to manage chunks of code better. I think right now I am marking things as mutable, but then code is running. If I just manage writing code in chunks rather than one big region that I keep mutating and then re-execing, this shouldn't be a problem

resources/beagle.ffi.bg

@@ -1,5 +1,7 @@
 namespace beagle.ffi
 
+import "beagle.builtin" as builtin
+
 
 // Probably should move this stuff to an ffi namespace
 struct Library {
@@ -36,6 +38,9 @@ enum Type {
 fn __create_ffi_function(ffi_info) {
     // TODO: Hack, but trying to get things to work
     fn(a1, a2, a3, a4, a5, a6) {
-        call_ffi_info(ffi_info, a1, a2, a3, a4, a5, a6)
+        builtin/__register_c_call();
+        let result = call_ffi_info(ffi_info, a1, a2, a3, a4, a5, a6)
+        builtin/__unregister_c_call()
+        result
     }
 }

src/builtins.rs

@@ -249,6 +249,16 @@ pub unsafe extern "C" fn __pause<Alloc: Allocator>(
     BuiltInTypes::null_value() as usize
 }
 
+fn pause_current_thread<Alloc: Allocator>(stack_pointer: usize, runtime: &mut Runtime<Alloc>) {
+    let thread_state = runtime.thread_state.clone();
+    let (lock, condvar) = &*thread_state;
+    let mut state = lock.lock().unwrap();
+    let stack_base = runtime.get_stack_base();
+    state.pause((stack_base, stack_pointer));
+    condvar.notify_one();
+    drop(state);
+}
+
 fn unpause_current_thread<Alloc: Allocator>(runtime: &mut Runtime<Alloc>) {
     let thread_state = runtime.thread_state.clone();
     let (lock, condvar) = &*thread_state;
@@ -257,14 +267,32 @@ fn unpause_current_thread<Alloc: Allocator>(runtime: &mut Runtime<Alloc>) {
     condvar.notify_one();
 }
 
-fn pause_current_thread<Alloc: Allocator>(stack_pointer: usize, runtime: &mut Runtime<Alloc>) {
+pub extern "C" fn register_c_call<Alloc: Allocator>(
+    runtime: *mut Runtime<Alloc>,
+    stack_pointer: usize,
+) -> usize {
+    let runtime = unsafe { &mut *runtime };
     let thread_state = runtime.thread_state.clone();
     let (lock, condvar) = &*thread_state;
     let mut state = lock.lock().unwrap();
     let stack_base = runtime.get_stack_base();
-    state.pause((stack_base, stack_pointer));
+    state.register_c_call((stack_base, stack_pointer));
     condvar.notify_one();
-    drop(state);
+    BuiltInTypes::null_value() as usize
+}
+
+pub extern "C" fn unregister_c_call<Alloc: Allocator>(runtime: *mut Runtime<Alloc>) -> usize {
+    let runtime = unsafe { &mut *runtime };
+    let thread_state = runtime.thread_state.clone();
+    let (lock, condvar) = &*thread_state;
+    let mut state = lock.lock().unwrap();
+    state.unregister_c_call();
+    condvar.notify_one();
+    while runtime.is_paused() {
+        // Park can unpark itself even if I haven't called unpark
+        thread::park();
+    }
+    BuiltInTypes::null_value() as usize
 }
 
 pub unsafe fn call_fn_1<Alloc: Allocator>(
@@ -514,35 +542,31 @@ pub unsafe extern "C" fn call_ffi_info<Alloc: Allocator>(
                     panic!("Expected string, got {:?}", ffi_type);
                 }
                 let string = runtime.compiler.get_string(*argument);
-                let pointer = runtime.memory.native_arguments.write_c_string(string);
-                argument_pointers.push(arg(&pointer));
+                let string = runtime.memory.write_c_string(string);
+                argument_pointers.push(arg(&string));
             }
             BuiltInTypes::Int => match ffi_type {
                 FFIType::U8 => {
                     let pointer = runtime
                         .memory
-                        .native_arguments
                         .write_u8(BuiltInTypes::untag(*argument) as u8);
                     argument_pointers.push(arg(pointer));
                 }
                 FFIType::U16 => {
                     let pointer = runtime
                         .memory
-                        .native_arguments
                         .write_u16(BuiltInTypes::untag(*argument) as u16);
                     argument_pointers.push(arg(pointer));
                 }
                 FFIType::U32 => {
                     let pointer = runtime
                         .memory
-                        .native_arguments
                         .write_u32(BuiltInTypes::untag(*argument) as u32);
                     argument_pointers.push(arg(pointer));
                 }
                 FFIType::I32 => {
                     let pointer = runtime
                         .memory
-                        .native_arguments
                         .write_i32(BuiltInTypes::untag(*argument) as i32);
                     argument_pointers.push(arg(pointer));
                 }
@@ -558,7 +582,7 @@ pub unsafe extern "C" fn call_ffi_info<Alloc: Allocator>(
                 // TODO: Make this type safe
                 let heap_object = HeapObject::from_tagged(*argument);
                 let buffer = BuiltInTypes::untag(heap_object.get_field(0));
-                let pointer = runtime.memory.native_arguments.write_pointer(buffer);
+                let pointer = runtime.memory.write_pointer(buffer);
                 argument_pointers.push(arg(pointer));
             }
             _ => {
@@ -603,7 +627,7 @@ pub unsafe extern "C" fn call_ffi_info<Alloc: Allocator>(
             todo!()
         }
     };
-    runtime.memory.native_arguments.clear();
+    runtime.memory.clear_native_arguments();
     return_value
 }
 
@@ -914,6 +938,18 @@ impl<Alloc: Allocator> Runtime<Alloc> {
             true,
         )?;
 
+        self.compiler.add_builtin_function(
+            "beagle.builtin/__register_c_call",
+            register_c_call::<Alloc> as *const u8,
+            true,
+        )?;
+
+        self.compiler.add_builtin_function(
+            "beagle.builtin/__unregister_c_call",
+            unregister_c_call::<Alloc> as *const u8,
+            false,
+        )?;
+
         self.compiler.add_builtin_function(
             "beagle.builtin/update_binding",
             update_binding::<Alloc> as *const u8,

src/runtime.rs

@@ -26,6 +26,8 @@ use crate::{
     CommandLineArguments, Data, Message,
 };
 
+use std::cell::RefCell;
+
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct Function {
     name: String,
@@ -158,6 +160,9 @@ pub struct FFIInfo {
 pub struct ThreadState {
     pub paused_threads: usize,
     pub stack_pointers: Vec<(usize, usize)>,
+    // TODO: I probably don't want to do this here. This requires taking a mutex
+    // not really ideal for c calls.
+    pub c_calling_stack_pointers: HashMap<ThreadId, (usize, usize)>,
 }
 
 impl ThreadState {
@@ -170,6 +175,17 @@ impl ThreadState {
         self.paused_threads -= 1;
     }
 
+    pub fn register_c_call(&mut self, stack_pointer: (usize, usize)) {
+        let thread_id = thread::current().id();
+        self.c_calling_stack_pointers
+            .insert(thread_id, stack_pointer);
+    }
+
+    pub fn unregister_c_call(&mut self) {
+        let thread_id = thread::current().id();
+        self.c_calling_stack_pointers.remove(&thread_id);
+    }
+
     pub fn clear(&mut self) {
         self.stack_pointers.clear();
     }
@@ -237,60 +253,64 @@ impl MMapMutWithOffset {
         unsafe { &*(self.mmap.as_ptr().add(start) as *const u16) }
     }
 
-    pub fn write_u32(&mut self, value: u32) -> &u32 {
+    pub fn write_u32(&mut self, value: u32) -> *const u32 {
         let start = self.offset;
         let bytes = value.to_le_bytes();
         for byte in bytes {
             self.mmap[self.offset] = byte;
             self.offset += 1;
         }
-        unsafe { &*(self.mmap.as_ptr().add(start) as *const u32) }
+        unsafe { self.mmap.as_ptr().add(start) as *const u32 }
     }
 
-    pub fn write_i32(&mut self, value: i32) -> &i32 {
+    pub fn write_i32(&mut self, value: i32) -> *const i32 {
         let start = self.offset;
         let bytes = value.to_le_bytes();
         for byte in bytes {
             self.mmap[self.offset] = byte;
             self.offset += 1;
         }
 
-        (unsafe { &*(self.mmap.as_ptr().add(start) as *const i32) }) as _
+        (unsafe { self.mmap.as_ptr().add(start) as *const i32 }) as _
     }
 
-    pub fn write_u8(&mut self, argument: u8) -> &u8 {
+    pub fn write_u8(&mut self, argument: u8) -> *const u8 {
         let start = self.offset;
         self.mmap[start] = argument;
         // We need to make sure we keep correct alignment
         self.offset += 2;
-        unsafe { &*(self.mmap.as_ptr().add(start)) }
+        unsafe { self.mmap.as_ptr().add(start) }
     }
 
-    pub fn write_pointer(&mut self, value: usize) -> &*mut c_void {
+    pub fn write_pointer(&mut self, value: usize) -> *const *mut c_void {
         let start = self.offset;
         let bytes = value.to_le_bytes();
         for byte in bytes {
             self.mmap[self.offset] = byte;
             self.offset += 1;
         }
-        unsafe { &*(self.mmap.as_ptr().add(start) as *const *mut c_void) }
+        unsafe { self.mmap.as_ptr().add(start) as *const *mut c_void }
     }
 
     pub fn clear(&mut self) {
         self.offset = 0;
     }
 }
 
+thread_local! {
+    pub static NATIVE_ARGUMENTS : RefCell<MMapMutWithOffset> = RefCell::new(MMapMutWithOffset::new());
+}
+
 pub struct Memory<Alloc: Allocator> {
     heap: Alloc,
     stacks: Vec<(ThreadId, MmapMut)>,
-    pub native_arguments: MMapMutWithOffset,
     pub join_handles: Vec<JoinHandle<u64>>,
     pub threads: Vec<Thread>,
     pub stack_map: StackMap,
     #[allow(unused)]
     command_line_arguments: CommandLineArguments,
 }
+
 impl<Alloc: Allocator> Memory<Alloc> {
     fn active_threads(&mut self) -> usize {
         let mut completed_threads = vec![];
@@ -325,6 +345,46 @@ impl<Alloc: Allocator> Memory<Alloc> {
         heap_object.write_fields(bytes);
         Ok(BuiltInTypes::HeapObject.tagged(pointer as usize))
     }
+
+    pub fn write_c_string(&mut self, string: String) -> *mut i8 {
+        let mut result: *mut i8 = std::ptr::null_mut();
+        NATIVE_ARGUMENTS.with(|memory| result = memory.borrow_mut().write_c_string(string));
+        result
+    }
+
+    pub fn write_pointer(&mut self, value: usize) -> &*mut c_void {
+        let mut result: *const *mut c_void = std::ptr::null_mut();
+        NATIVE_ARGUMENTS.with(|memory| result = memory.borrow_mut().write_pointer(value));
+        unsafe { &*result }
+    }
+
+    pub fn write_u32(&mut self, value: u32) -> &u32 {
+        let mut result: *const u32 = &0;
+        NATIVE_ARGUMENTS.with(|memory| result = memory.borrow_mut().write_u32(value));
+        unsafe { &*result }
+    }
+
+    pub fn write_i32(&mut self, value: i32) -> &i32 {
+        let mut result: *const i32 = &0;
+        NATIVE_ARGUMENTS.with(|memory| result = memory.borrow_mut().write_i32(value));
+        unsafe { &*result }
+    }
+
+    pub fn write_u8(&mut self, value: u8) -> &u8 {
+        let mut result: *const u8 = &0;
+        NATIVE_ARGUMENTS.with(|memory| result = memory.borrow_mut().write_u8(value));
+        unsafe { &*result }
+    }
+
+    pub fn write_u16(&mut self, value: u16) -> &u16 {
+        let mut result: *const u16 = &0;
+        NATIVE_ARGUMENTS.with(|memory| result = memory.borrow_mut().write_u16(value));
+        unsafe { &*result }
+    }
+
+    pub fn clear_native_arguments(&self) {
+        NATIVE_ARGUMENTS.with(|memory| memory.borrow_mut().clear());
+    }
 }
 
 impl<Alloc: Allocator> Allocator for Memory<Alloc> {
@@ -1366,7 +1426,6 @@ impl<Alloc: Allocator> Runtime<Alloc> {
                     std::thread::current().id(),
                     MmapOptions::new(STACK_SIZE).unwrap().map_mut().unwrap(),
                 )],
-                native_arguments: MMapMutWithOffset::new(),
                 join_handles: vec![],
                 threads: vec![std::thread::current()],
                 command_line_arguments,
@@ -1379,6 +1438,7 @@ impl<Alloc: Allocator> Runtime<Alloc> {
                 Mutex::new(ThreadState {
                     paused_threads: 0,
                     stack_pointers: vec![],
+                    c_calling_stack_pointers: HashMap::new(),
                 }),
                 Condvar::new(),
             )),
@@ -1564,7 +1624,9 @@ impl<Alloc: Allocator> Runtime<Alloc> {
 
         let (lock, cvar) = &*self.thread_state;
         let mut thread_state = lock.lock().unwrap();
-        while thread_state.paused_threads < self.memory.active_threads() {
+        while thread_state.paused_threads + thread_state.c_calling_stack_pointers.len()
+            < self.memory.active_threads()
+        {
             thread_state = cvar.wait(thread_state).unwrap();
         }