Clean up code, add documentation, implement pieces properly

Makefile

@@ -219,6 +219,7 @@ check: check-bench
 	# builds, test as many combined features as possible with each backend
 	# as default, and test a minimal set of features with only one backend
 	# at a time.
+	cargo check --manifest-path lib/runtime-core/Cargo.toml
 	cargo check --manifest-path lib/runtime/Cargo.toml
 	# Check some of the cases where deterministic execution could matter
 	cargo check --manifest-path lib/runtime/Cargo.toml --features "deterministic-execution"

lib/runtime-core/src/backend.rs

@@ -124,6 +124,15 @@ pub struct CompilerConfig {
     pub generate_debug_info: bool,
 }
 
+impl CompilerConfig {
+    /// Use this to check if we should be generating debug information.
+    /// This function takes into account the features that runtime-core was
+    /// compiled with in addition to the value of the `generate_debug_info` field.
+    pub(crate) fn should_generate_debug_info(&self) -> bool {
+        cfg!(feature = "generate-debug-information") && self.generate_debug_info
+    }
+}
+
 /// An exception table for a `RunnableModule`.
 #[derive(Clone, Debug, Default, Serialize, Deserialize)]
 pub struct ExceptionTable {
@@ -212,11 +221,6 @@ pub trait RunnableModule: Send + Sync {
         None
     }
 
-    /// TODO: document before shipppping
-    fn get_local_function_pointers_and_lengths(&self) -> Option<Vec<(*const u8, usize)>> {
-        None
-    }
-
     /// Returns the inline breakpoint size corresponding to an Architecture (None in case is not implemented)
     fn get_inline_breakpoint_size(&self, _arch: Architecture) -> Option<usize> {
         None

lib/runtime-core/src/codegen.rs

@@ -18,7 +18,6 @@ use std::fmt;
 use std::fmt::Debug;
 use std::marker::PhantomData;
 use std::sync::{Arc, RwLock};
-use wasm_debug::types::{CompiledFunctionData, ValueLabelsRangesInner};
 use wasmparser::{self, WasmDecoder};
 use wasmparser::{Operator, Type as WpType};
 
@@ -128,7 +127,19 @@ pub trait ModuleCodeGenerator<FCG: FunctionCodeGenerator<E>, RM: RunnableModule,
     unsafe fn from_cache(cache: Artifact, _: Token) -> Result<ModuleInner, CacheError>;
 }
 
-/// missing documentation!
+/// Mock item when compiling without debug info generation.
+#[cfg(not(feature = "generate-debug-information"))]
+type CompiledFunctionData = ();
+
+/// Mock item when compiling without debug info generation.
+#[cfg(not(feature = "generate-debug-information"))]
+type ValueLabelsRangesInner = ();
+
+#[cfg(feature = "generate-debug-information")]
+use wasm_debug::types::{CompiledFunctionData, ValueLabelsRangesInner};
+
+#[derive(Clone, Debug)]
+/// Useful information for debugging gathered by compiling a Wasm module.
 pub struct DebugMetadata {
     /// [`CompiledFunctionData`] in [`FuncIndex`] order
     pub func_info: Map<FuncIndex, CompiledFunctionData>,
@@ -262,55 +273,47 @@ impl<
         };
         let mut chain = (self.middleware_chain_generator)();
         let info = crate::parse::read_module(wasm, &mut mcg, &mut chain, &compiler_config)?;
-        let ((exec_context, debug_metadata), cache_gen) = mcg
+        let ((exec_context, compile_debug_info), cache_gen) =
-            .finalize(&info.read().unwrap())
+            mcg.finalize(&info.read().unwrap())
-            .map_err(|x| CompileError::InternalError {
+                .map_err(|x| CompileError::InternalError {
-                msg: format!("{:?}", x),
+                    msg: format!("{:?}", x),
-            })?;
+                })?;
 
-        use target_lexicon::{
+        #[cfg(feature = "generate-debug-information")]
-            Architecture, BinaryFormat, Environment, OperatingSystem, Triple, Vendor,
+        {
-        };
+            if compiler_config.should_generate_debug_info() {
-        const X86_64_OSX: Triple = Triple {
+                if let Some(dbg_info) = compile_debug_info {
-            architecture: Architecture::X86_64,
+                    let debug_info = wasm_debug::read_debuginfo(wasm);
-            vendor: Vendor::Apple,
+                    let extra_info = wasm_debug::types::ModuleVmctxInfo::new(
-            operating_system: OperatingSystem::Darwin,
+                        crate::vm::Ctx::offset_memory_base() as _,
-            environment: Environment::Unknown,
+                        std::mem::size_of::<crate::vm::Ctx>() as _,
-            binary_format: BinaryFormat::Macho,
+                        dbg_info.stack_slot_offsets.values(),
-        };
+                    );
+                    let compiled_fn_map =
+                        wasm_debug::types::create_module_address_map(dbg_info.func_info.values());
+                    let range_map =
+                        wasm_debug::types::build_values_ranges(dbg_info.inst_info.values());
+                    let raw_func_slice = &dbg_info.pointers;
 
-        if compiler_config.generate_debug_info {
+                    let debug_image = wasm_debug::emit_debugsections_image(
-            if let Some(debug_metadata) = debug_metadata {
+                        target_lexicon::HOST,
-                let debug_info = wasm_debug::read_debuginfo(wasm);
+                        std::mem::size_of::<usize>() as u8,
-                let extra_info = wasm_debug::types::ModuleVmctxInfo::new(
+                        &debug_info,
-                    14 * 8,
+                        &extra_info,
-                    debug_metadata.stack_slot_offsets.values(),
+                        &compiled_fn_map,
-                );
+                        &range_map,
-                let compiled_fn_map =
+                        raw_func_slice,
-                    wasm_debug::types::create_module_address_map(debug_metadata.func_info.values());
+                    )
-                let range_map =
+                    .expect("make debug image");
-                    wasm_debug::types::build_values_ranges(debug_metadata.inst_info.values());
-                let raw_func_slice = debug_metadata.pointers;
 
-                let debug_image = wasm_debug::emit_debugsections_image(
+                    let mut writer = info.write().unwrap();
-                    X86_64_OSX,
+                    writer
-                    std::mem::size_of::<usize>() as u8,
+                        .debug_info_manager
-                    &debug_info,
+                        .register_new_jit_code_entry(&debug_image);
-                    &extra_info,
+                }
-                    &compiled_fn_map,
-                    &range_map,
-                    &raw_func_slice,
-                )
-                .expect("make debug image");
-
-                crate::jit_debug::register_new_jit_code_entry(
-                    &debug_image,
-                    crate::jit_debug::JITAction::JIT_REGISTER_FN,
-                );
-            } else {
-                eprintln!("Failed to generate debug information!");
             }
         }
+
         Ok(ModuleInner {
             cache_gen,
             runnable_module: Arc::new(Box::new(exec_context)),

lib/runtime-core/src/instance.rs

@@ -79,18 +79,14 @@ impl Instance {
         unsafe {
             let backing = &mut *(&mut inner.backing as *mut _);
             let import_backing = &mut *(&mut inner.import_backing as *mut _);
-            let real_ctx = match imports.call_state_creator() {
+            let mut real_ctx = match imports.call_state_creator() {
                 Some((data, dtor)) => {
                     vm::Ctx::new_with_data(backing, import_backing, &module, data, dtor)
                 }
                 None => vm::Ctx::new(backing, import_backing, &module),
             };
+            real_ctx.internal.ctx = vmctx.as_mut_ptr();
             vmctx.as_mut_ptr().write(real_ctx);
-            for (_, memory) in backing.vm_memories.iter_mut() {
-                let mem: &mut vm::LocalMemory = &mut **memory;
-                // remaining left to do:
-                mem.vmctx = dbg!(vmctx.as_mut_ptr());
-            }
         };
         Box::leak(vmctx);
 

lib/runtime-core/src/jit_debug.rs

@@ -1,5 +1,6 @@
 #![allow(missing_docs)]
 use std::ptr;
+use std::sync::Arc;
 
 // =============================================================================
 // LLDB hook magic:
@@ -22,7 +23,7 @@ extern "C" fn __jit_debug_register_code() {
 #[allow(non_camel_case_types)]
 #[derive(Debug)]
 #[repr(u32)]
-pub enum JITAction {
+pub(crate) enum JITAction {
     JIT_NOACTION = 0,
     JIT_REGISTER_FN = 1,
     JIT_UNREGISTER_FN = 2,
@@ -30,11 +31,10 @@ pub enum JITAction {
 
 #[no_mangle]
 #[repr(C)]
-pub struct JITCodeEntry {
+pub(crate) struct JITCodeEntry {
     next: *mut JITCodeEntry,
     prev: *mut JITCodeEntry,
-    // TODO: use CStr here?
+    symfile_addr: *mut u8,
-    symfile_addr: *const u8,
     symfile_size: u64,
 }
 
@@ -43,7 +43,7 @@ impl Default for JITCodeEntry {
         Self {
             next: ptr::null_mut(),
             prev: ptr::null_mut(),
-            symfile_addr: ptr::null(),
+            symfile_addr: ptr::null_mut(),
             symfile_size: 0,
         }
     }
@@ -51,7 +51,7 @@ impl Default for JITCodeEntry {
 
 #[no_mangle]
 #[repr(C)]
-pub struct JitDebugDescriptor {
+pub(crate) struct JitDebugDescriptor {
     version: u32,
     action_flag: u32,
     relevant_entry: *mut JITCodeEntry,
@@ -60,7 +60,7 @@ pub struct JitDebugDescriptor {
 
 #[no_mangle]
 #[allow(non_upper_case_globals)]
-pub static mut __jit_debug_descriptor: JitDebugDescriptor = JitDebugDescriptor {
+pub(crate) static mut __jit_debug_descriptor: JitDebugDescriptor = JitDebugDescriptor {
     version: 1,
     action_flag: JITAction::JIT_NOACTION as _,
     relevant_entry: ptr::null_mut(),
@@ -70,8 +70,8 @@ pub static mut __jit_debug_descriptor: JitDebugDescriptor = JitDebugDescriptor {
 /// Prepend an item to the front of the `__jit_debug_descriptor` entry list
 ///
 /// # Safety
-/// - Pointer to [`JITCodeEntry`] should point to a valid entry and stay alive
+/// - Access to underlying global variable is unsynchronized.
-///   for the 'static lifetime
+/// - Pointer to [`JITCodeEntry`] should point to a valid entry.
 unsafe fn push_front(jce: *mut JITCodeEntry) {
     if __jit_debug_descriptor.first_entry.is_null() {
         __jit_debug_descriptor.first_entry = jce;
@@ -84,28 +84,90 @@ unsafe fn push_front(jce: *mut JITCodeEntry) {
     }
 }
 
-// deleted static (added and deleted by Mark): TODO:
+/// Removes an entry from the doubly linked list, updating both nodes that it's
-pub fn register_new_jit_code_entry(bytes: &[u8], action: JITAction) -> *mut JITCodeEntry {
+/// connected to.
-    let owned_bytes = bytes.iter().cloned().collect::<Vec<u8>>();
+///
-    let ptr = owned_bytes.as_ptr();
+/// # Safety
-    let len = owned_bytes.len();
+/// - Access to underlying global variable is unsynchronized.
-
+/// - Pointer must point to a valid `JitCodeEntry`.
-    std::mem::forget(bytes);
+unsafe fn remove_node(jce: *mut JITCodeEntry) {
-
+    if __jit_debug_descriptor.first_entry == jce {
-    let entry: *mut JITCodeEntry = Box::into_raw(Box::new(JITCodeEntry {
+        debug_assert!((*jce).prev.is_null());
-        symfile_addr: ptr,
+        __jit_debug_descriptor.first_entry = (*jce).next;
-        symfile_size: len as _,
+    }
-        ..JITCodeEntry::default()
+    if !(*jce).prev.is_null() {
-    }));
+        (*(*jce).prev).next = (*jce).next;
-
+    }
-    unsafe {
+    if !(*jce).next.is_null() {
-        push_front(entry);
+        (*(*jce).next).prev = (*jce).prev;
-        __jit_debug_descriptor.relevant_entry = entry;
+    }
-        __jit_debug_descriptor.action_flag = action as u32;
+}
-        __jit_debug_register_code();
+
-        __jit_debug_descriptor.relevant_entry = ptr::null_mut();
+/// Type for implementing Drop on the memory shared with the debugger.
-        __jit_debug_descriptor.action_flag = JITAction::JIT_NOACTION as _;
+#[derive(Debug)]
+struct JITCodeDebugInfoEntryHandleInner(*mut JITCodeEntry);
+
+/// Handle to debug info about JIT code registered with a debugger
+#[derive(Debug, Clone)]
+pub(crate) struct JITCodeDebugInfoEntryHandle(Arc<JITCodeDebugInfoEntryHandleInner>);
+
+impl Drop for JITCodeDebugInfoEntryHandleInner {
+    fn drop(&mut self) {
+        unsafe {
+            // unregister the function when dropping the JIT code entry
+            __jit_debug_descriptor.relevant_entry = self.0;
+            __jit_debug_descriptor.action_flag = JITAction::JIT_UNREGISTER_FN as u32;
+            __jit_debug_register_code();
+            __jit_debug_descriptor.relevant_entry = ptr::null_mut();
+            __jit_debug_descriptor.action_flag = JITAction::JIT_NOACTION as u32;
+            remove_node(self.0);
+            let entry: Box<JITCodeEntry> = Box::from_raw(self.0);
+            Vec::from_raw_parts(
+                entry.symfile_addr,
+                entry.symfile_size as _,
+                entry.symfile_size as _,
+            );
+        }
+    }
+}
+
+/// Manager of debug info registered with the debugger.
+#[derive(Debug, Clone, Default)]
+pub struct JITCodeDebugInfoManager {
+    inner: Vec<JITCodeDebugInfoEntryHandle>,
+}
+
+impl JITCodeDebugInfoManager {
+    pub(crate) fn register_new_jit_code_entry(
+        &mut self,
+        bytes: &[u8],
+    ) -> JITCodeDebugInfoEntryHandle {
+        let mut owned_bytes = bytes.iter().cloned().collect::<Vec<u8>>();
+        // ensure length == capacity to simplify memory freeing code
+        owned_bytes.shrink_to_fit();
+        let ptr = owned_bytes.as_mut_ptr();
+        let len = owned_bytes.len();
+
+        std::mem::forget(owned_bytes);
+
+        let entry: *mut JITCodeEntry = Box::into_raw(Box::new(JITCodeEntry {
+            symfile_addr: ptr,
+            symfile_size: len as _,
+            ..JITCodeEntry::default()
+        }));
+
+        unsafe {
+            push_front(entry);
+            __jit_debug_descriptor.relevant_entry = entry;
+            __jit_debug_descriptor.action_flag = JITAction::JIT_REGISTER_FN as u32;
+            __jit_debug_register_code();
+            __jit_debug_descriptor.relevant_entry = ptr::null_mut();
+            __jit_debug_descriptor.action_flag = JITAction::JIT_NOACTION as u32;
+        }
+
+        let handle = JITCodeDebugInfoEntryHandle(Arc::new(JITCodeDebugInfoEntryHandleInner(entry)));
+        self.inner.push(handle.clone());
+
+        handle
     }
-
-    entry
 }

lib/runtime-core/src/memory/mod.rs

@@ -227,7 +227,6 @@ impl UnsharedMemory {
             base: std::ptr::null_mut(),
             bound: 0,
             memory: std::ptr::null_mut(),
-            vmctx: std::ptr::null_mut(),
         };
 
         let storage = match desc.memory_type() {
@@ -315,7 +314,6 @@ impl SharedMemory {
             base: std::ptr::null_mut(),
             bound: 0,
             memory: std::ptr::null_mut(),
-            vmctx: std::ptr::null_mut(),
         };
 
         let memory = StaticMemory::new(desc, &mut local)?;

lib/runtime-core/src/module.rs

@@ -16,6 +16,7 @@ use crate::{
 };
 
 use crate::backend::CacheGen;
+use crate::jit_debug;
 use indexmap::IndexMap;
 use std::collections::HashMap;
 use std::sync::Arc;
@@ -79,8 +80,12 @@ pub struct ModuleInfo {
     pub custom_sections: HashMap<String, Vec<u8>>,
 
     /// Flag controlling whether or not debug information for use in a debugger
-    /// will be generated
+    /// will be generated.
     pub generate_debug_info: bool,
+
+    #[serde(skip)]
+    /// Resource manager of debug information being used by a debugger.
+    pub debug_info_manager: jit_debug::JITCodeDebugInfoManager,
 }
 
 impl ModuleInfo {

lib/runtime-core/src/parse.rs

@@ -91,7 +91,8 @@ pub fn read_module<
 
         custom_sections: HashMap::new(),
 
-        generate_debug_info: compiler_config.generate_debug_info,
+        generate_debug_info: compiler_config.should_generate_debug_info(),
+        debug_info_manager: Default::default(),
     }));
 
     let mut parser = wasmparser::ValidatingParser::new(

lib/runtime-core/src/vm.rs

@@ -8,7 +8,7 @@ use crate::{
     module::{ModuleInfo, ModuleInner},
     sig_registry::SigRegistry,
     structures::TypedIndex,
-    types::{LocalMemoryIndex, LocalOrImport, MemoryIndex, TableIndex, Value},
+    types::{LocalOrImport, MemoryIndex, TableIndex, Value},
     vmcalls,
 };
 use std::{
@@ -136,7 +136,7 @@ pub struct InternalCtx {
     pub interrupt_signal_mem: *mut u8,
 
     /// hmm
-    pub first_mem: *mut LocalMemory,
+    pub ctx: *mut Ctx,
 }
 
 static INTERNAL_FIELDS: AtomicUsize = AtomicUsize::new(0);
@@ -309,7 +309,7 @@ impl Ctx {
                 internals: &mut local_backing.internals.0,
 
                 interrupt_signal_mem: get_interrupt_signal_mem(),
-                first_mem: local_backing.vm_memories[LocalMemoryIndex::new(0)],
+                ctx: std::ptr::null_mut(), //local_backing.vm_memories[LocalMemoryIndex::new(0)],
             },
             local_functions: local_backing.local_functions.as_ptr(),
 
@@ -366,7 +366,8 @@ impl Ctx {
 
                 interrupt_signal_mem: get_interrupt_signal_mem(),
 
-                first_mem: local_backing.vm_memories[LocalMemoryIndex::new(0)],
+                ctx: std::ptr::null_mut(),
+                //first_mem: local_backing.vm_memories[LocalMemoryIndex::new(0)],
             },
             local_functions: local_backing.local_functions.as_ptr(),
 
@@ -674,9 +675,6 @@ pub struct LocalMemory {
     /// This is either `*mut DynamicMemory`, `*mut StaticMemory`,
     /// or `*mut SharedStaticMemory`.
     pub memory: *mut (),
-
-    /// wat
-    pub vmctx: *mut Ctx,
 }
 
 // manually implemented because LocalMemory contains raw pointers