Fix safety issues of some LLVM components

- Remove dependency on utilities

Author: appcypher

Cargo.lock

@@ -2,11 +2,14 @@
 name = "wasmo_runtime"
 version = "0.1.0"
 edition = "2021"
+description = "The Wasmo Runtime"
+license-file = "../LICENSE"
+repository = "https://github.com/appcypher/wasmo"
 
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
 [dependencies]
-utilities = { path = "../../utilities" }
+anyhow = "1.0"
 wasmparser = "0.82.0"
 env_logger = "0.9.0"
 wat = "1.0.41"

runtime/Cargo.toml

@@ -3,7 +3,7 @@
 use super::Store;
 use crate::compiler::value::Value;
 use crate::{Imports, Module};
-use utilities::result::Result;
+use anyhow::Result;
 
 /// An Instance is a fully resolved wasm runtime context.
 /// External references (globals, functions, memories, tables) are resolved.

runtime/lib/api/instance.rs

@@ -1,8 +1,8 @@
 // Copyright 2022 the Gigamono authors. All rights reserved. GPL-3.0 License.
 
 use crate::{compiler::Compiler, Imports, Instance, Options};
+use anyhow::Result;
 use serde::{Deserialize, Serialize};
-use utilities::result::Result;
 
 /// A WebAssembly module with compiled code but with unresolved external references.
 /// Memories and tables are also not created yet.

runtime/lib/api/module.rs

@@ -5,7 +5,7 @@ use std::pin::Pin;
 use serde::{Deserialize, Serialize};
 
 use log::debug;
-use utilities::result::Result;
+use anyhow::Result;
 use wasmparser::{
     DataSectionReader, ElementSectionReader, ExportSectionReader, FunctionBody,
     FunctionSectionReader, GlobalSectionReader, ImportSectionEntryType, ImportSectionReader,

runtime/lib/compiler/compiler.rs

@@ -5,6 +5,5 @@ mod function;
 mod llvm;
 mod module;
 mod types;
-mod utils;
 
 pub(crate) use llvm::*;

runtime/lib/compiler/llvm.rs

@@ -1,18 +1,22 @@
 // Copyright 2022 the Gigamono authors. All rights reserved. GPL-3.0 License.
 
-use std::pin::Pin;
+use std::rc::Rc;
 
+use anyhow::Result;
 use llvm_sys::{
-    core::{
-        LLVMContextCreate, LLVMContextDispose, LLVMDoubleTypeInContext, LLVMFloatTypeInContext,
-        LLVMInt32TypeInContext, LLVMInt64TypeInContext,
-    },
+    core::{LLVMContextCreate, LLVMContextDispose},
     prelude::LLVMContextRef,
 };
 
-use super::types::{LLFunctionType, LLType, LLTypeRef};
+use super::{
+    module::LLModule,
+    types::{LLFunctionType, LLType, LLTypeKind},
+};
 
 /// This a wrapper for LLVM Context.
+///
+/// # Ownership
+/// Owns the LLVM Module.
 #[derive(Debug)]
 pub(crate) struct LLContext {
     context_ref: LLVMContextRef,
@@ -25,40 +29,36 @@ impl LLContext {
         }
     }
 
+    pub(crate) fn create_module(&self, name: &str) -> Result<LLModule> {
+        LLModule::new(name, self)
+    }
+
     pub(crate) fn as_ptr(&self) -> LLVMContextRef {
         self.context_ref
     }
 
     pub(crate) fn i64_type(&self) -> LLType {
-        LLType::I64(LLTypeRef::new(unsafe {
-            LLVMInt64TypeInContext(self.context_ref)
-        }))
+        LLType::new(self, LLTypeKind::I64)
     }
 
     pub(crate) fn i32_type(&self) -> LLType {
-        LLType::I64(LLTypeRef::new(unsafe {
-            LLVMInt32TypeInContext(self.context_ref)
-        }))
+        LLType::new(self, LLTypeKind::I32)
     }
 
     pub(crate) fn f64_type(&self) -> LLType {
-        LLType::I64(LLTypeRef::new(unsafe {
-            LLVMDoubleTypeInContext(self.context_ref)
-        }))
+        LLType::new(self, LLTypeKind::F64)
     }
 
     pub(crate) fn f32_type(&self) -> LLType {
-        LLType::I64(LLTypeRef::new(unsafe {
-            LLVMFloatTypeInContext(self.context_ref)
-        }))
+        LLType::new(self, LLTypeKind::F32)
     }
 
-    pub(crate) fn function_type<'t>(
+    pub(crate) fn function_type(
         &self,
-        params: &'t [LLType],
-        result: &'t LLType,
+        params: &[LLType],
+        result: &LLType,
         is_varargs: bool,
-    ) -> Pin<Box<LLFunctionType<'t>>> {
+    ) -> Rc<LLFunctionType> {
         LLFunctionType::new(params, result, is_varargs)
     }
 }

runtime/lib/compiler/llvm/context.rs

@@ -1,32 +1,44 @@
 // Copyright 2022 the Gigamono authors. All rights reserved. GPL-3.0 License.
 
-use std::marker::PhantomData;
-use utilities::result::Result;
+use anyhow::Result;
+use std::{ffi::CString, rc::Rc};
 
 use llvm_sys::{core::LLVMAddFunction, prelude::LLVMValueRef};
 
-use super::{module::LLModule, types::LLFunctionType, utils::LLString};
+use super::{module::LLModule, types::LLFunctionType};
 
-pub(crate) struct LLFunction<'m, 's, 't> {
+/// This is a wrapper for LLVM Function.
+///
+/// # Safety
+/// It is unsafe to use the reference of `LLFunctionType` because its params can be independently freed.
+/// Holding an `Rc` to it ensures that that does not happen.
+pub(crate) struct LLFunction {
     function_ref: LLVMValueRef,
-    _module: PhantomData<&'m ()>,
-    _name: PhantomData<&'s ()>,
-    _signature: PhantomData<&'t ()>,
+    function_type: Rc<LLFunctionType>,
 }
 
-impl<'m, 's, 't> LLFunction<'m, 's, 't> {
-    pub(crate) fn attach(
-        module: &'m LLModule,
-        name: &'s LLString,
-        signature: &'t LLFunctionType,
+impl LLFunction {
+    /// Creates a new LLVM function.
+    ///
+    /// # Safety
+    /// The use of temporary `CString` expecting that value will be copied in `LLVMAddFunction` might not be safe.
+    ///
+    /// - https://llvm.org/doxygen/Twine_8h_source.html#l00477
+    /// - https://llvm.org/doxygen/Value_8cpp_source.html#l00315
+    pub(crate) fn new(
+        name: &str,
+        module: &LLModule,
+        function_type: Rc<LLFunctionType>,
     ) -> Result<Self> {
         Ok(Self {
             function_ref: unsafe {
-                LLVMAddFunction(module.as_ptr(), name.as_ptr(), signature.as_ptr())
+                LLVMAddFunction(
+                    module.as_ptr(),
+                    CString::new(name)?.as_ptr(),
+                    function_type.as_ptr(),
+                )
             },
-            _module: PhantomData,
-            _name: PhantomData,
-            _signature: PhantomData,
+            function_type,
         })
     }
 }

runtime/lib/compiler/llvm/function.rs

@@ -2,9 +2,9 @@
 
 use std::pin::Pin;
 
-use super::{context::LLContext, module::LLModule, utils::LLString};
+use super::{context::LLContext, module::LLModule};
 use llvm_sys::core::LLVMShutdown;
-use utilities::result::Result;
+use anyhow::Result;
 
 /// Converts WebAssembly semantics to LLVM code, handles materialization.
 ///
@@ -44,8 +44,8 @@ use utilities::result::Result;
 /// - loading important values like memory address into registers
 #[derive(Debug)]
 pub(crate) struct LLVM {
-    pub(crate) module: Option<Pin<Box<LLModule>>>,
-    context: LLContext,
+    pub(crate) context: LLContext,
+    pub(crate) module: Option<LLModule>,
 }
 
 impl LLVM {
@@ -59,10 +59,7 @@ impl LLVM {
         });
 
         // The module field references the context field so this is self-referential.
-        this.module = Some(LLModule::new(
-            LLString::try_from("main module")?,
-            this.context.as_ptr(),
-        )?);
+        this.module = Some(LLModule::new("initial", &this.context)?);
 
         Ok(this)
     }

runtime/lib/compiler/llvm/llvm.rs

@@ -1,15 +1,15 @@
 // Copyright 2022 the Gigamono authors. All rights reserved. GPL-3.0 License.
 
-use std::pin::Pin;
+use std::{ffi::CString, rc::Rc};
 
-use utilities::result::Result;
+use anyhow::Result;
 
 use llvm_sys::{
     core::{LLVMDumpModule, LLVMModuleCreateWithNameInContext},
-    prelude::{LLVMContextRef, LLVMModuleRef},
+    prelude::LLVMModuleRef,
 };
 
-use super::{function::LLFunction, types::LLFunctionType, utils::LLString};
+use super::{context::LLContext, function::LLFunction, types::LLFunctionType};
 
 /// A wrapper for LLVM Module.
 ///
@@ -18,36 +18,48 @@ use super::{function::LLFunction, types::LLFunctionType, utils::LLString};
 /// When a Module references a Context, the Context frees it when it gets dropped.
 ///
 /// We leverage this behavior by not disposing the Module explicitly on drop, letting associated Context do the job.
-/// This is safe because we can only create a Module from a Context.
+///
+/// WARNING: This is safe only if we can only create a Module from a Context.
 ///
 /// NOTE: We can't use lifetime parameter since it leads to unresolvable self-referential structs when an `LLModule` is stored in the same struct as the associated `LLContext`.
 ///
-/// https://lists.llvm.org/pipermail/llvm-dev/2018-September/126134.html
+/// - https://lists.llvm.org/pipermail/llvm-dev/2018-September/126134.html
+/// - https://llvm.org/doxygen/Module_8cpp_source.html#l00079
+/// - https://llvm.org/doxygen/LLVMContextImpl_8cpp_source.html#l00052
+/// 
+/// # Ownership
+/// ???
+///
 #[derive(Debug)]
 pub(crate) struct LLModule {
-    name: Pin<Box<LLString>>,
     module_ref: LLVMModuleRef,
 }
 
 impl LLModule {
-    /// This is the only way to create an LLModule to ensure it has an associated Context.
-    pub(crate) fn new(name: Pin<Box<LLString>>, context: LLVMContextRef) -> Result<Pin<Box<Self>>> {
-        let mut this = Self {
-            name,
-            module_ref: std::ptr::null_mut(),
-        };
-
-        this.module_ref = unsafe { LLVMModuleCreateWithNameInContext(this.name.as_ptr(), context) };
-
-        Ok(Box::pin(this))
+    /// This is the only way to create an LLModule to ensure it has an associated Context that can dispose it.
+    ///
+    /// # Safety
+    /// A temporary `CString` name is safe to use here because it is copied into the LLVM Module.
+    ///
+    /// - https://llvm.org/doxygen/Module_8cpp_source.html#l00072
+    pub(crate) fn new(name: &str, context: &LLContext) -> Result<Self> {
+        Ok(Self {
+            module_ref: unsafe {
+                LLVMModuleCreateWithNameInContext(CString::new(name)?.as_ptr(), context.as_ptr())
+            },
+        })
     }
 
-    pub(crate) fn add_function<'m, 's, 't>(
-        &'m self,
-        name: &'s LLString,
-        signature: &'t LLFunctionType,
-    ) -> Result<LLFunction<'m, 's, 't>> {
-        LLFunction::attach(self, name, signature)
+    /// Adds a function to the module.
+    ///
+    /// # Safety
+    /// TODO(appcypher): Investigate safety properly.
+    pub(crate) fn add_function(
+        &self,
+        name: &str,
+        signature: Rc<LLFunctionType>,
+    ) -> Result<LLFunction> {
+        LLFunction::new(name, self, signature)
     }
 
     pub(crate) fn as_ptr(&self) -> LLVMModuleRef {