diff --git a/Cargo.lock b/Cargo.lock index da9b44fe2..9eb765a43 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -464,7 +464,7 @@ dependencies = [ "log", "smallvec", "thiserror", - "wasmparser 0.67.0", + "wasmparser 0.68.0", ] [[package]] @@ -1254,6 +1254,7 @@ dependencies = [ "lucetc", "tempfile", "wiggle", + "wiggle-borrow", "wiggle-test", ] @@ -2595,7 +2596,6 @@ dependencies = [ "libc", "thiserror", "tracing", - "wig", "wiggle", "winapi 0.3.8", "winx", @@ -2670,9 +2670,9 @@ checksum = "a950e6a618f62147fd514ff445b2a0b53120d382751960797f85f058c7eda9b9" [[package]] name = "wasmparser" -version = "0.67.0" +version = "0.68.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9f091cf3849e5fe76a60255bff169277459f2201435bc583b6656880553f0ad0" +checksum = "29a00e14eed9c2ecbbdbdd4fb284f49d21b6808965de24769a6379a13ec47d4c" [[package]] name = "wast" @@ -2694,23 +2694,20 @@ dependencies = [ ] [[package]] -name = "wig" +name = "wiggle" version = "0.21.0" dependencies = [ - "heck", - "proc-macro2 1.0.24", - "quote 1.0.6", + "thiserror", + "tracing", + "wiggle-macro", "witx", ] [[package]] -name = "wiggle" +name = "wiggle-borrow" version = "0.21.0" dependencies = [ - "thiserror", - "tracing", - "wiggle-macro", - "witx", + "wiggle", ] [[package]] @@ -2738,10 +2735,11 @@ dependencies = [ [[package]] name = "wiggle-test" -version = "0.19.0" +version = "0.21.0" dependencies = [ "proptest", "wiggle", + "wiggle-borrow", ] [[package]] diff --git a/lucet-wasi/src/runtime.rs b/lucet-wasi/src/runtime.rs index dc0a3b06a..a061ce947 100644 --- a/lucet-wasi/src/runtime.rs +++ b/lucet-wasi/src/runtime.rs @@ -44,9 +44,9 @@ impl<'a> types::GuestErrorConversion for LucetWasiCtx<'a> { } impl<'a> types::UserErrorConversion for LucetWasiCtx<'a> { - fn errno_from_error(&self, e: Error) -> types::Errno { + fn errno_from_error(&self, e: Error) -> Result { debug!("Error: {:?}", e); - e.into() + Ok(e.into()) } } diff --git a/lucet-wiggle/Cargo.toml b/lucet-wiggle/Cargo.toml index 6892821fd..377a59838 100644 --- a/lucet-wiggle/Cargo.toml +++ b/lucet-wiggle/Cargo.toml @@ -15,6 +15,7 @@ lucet-wiggle-macro = { path = "./macro", version = "0.7.0-dev" } lucet-wiggle-generate = { path = "./generate", version = "0.7.0-dev" } lucet-runtime = { path = "../lucet-runtime", version = "0.7.0-dev" } wiggle = { path = "../wasmtime/crates/wiggle", version = "0.21.0" } +wiggle-borrow = { path = "../wasmtime/crates/wiggle/borrow", version = "0.21.0" } [dev-dependencies] wiggle-test = { path = "../wasmtime/crates/wiggle/test-helpers" } diff --git a/lucet-wiggle/generate/src/lib.rs b/lucet-wiggle/generate/src/lib.rs index 9dbc0de31..ee5e435b2 100644 --- a/lucet-wiggle/generate/src/lib.rs +++ b/lucet-wiggle/generate/src/lib.rs @@ -69,7 +69,10 @@ pub fn generate( let mut ctx: #ctx_type = #ctx_constructor; let r = super::#mod_name::#method_name(&ctx, &memory, #(#call_args),*); { #post_hook } - r + match r { + Ok(r) => { r }, + Err(e) => { lucet_runtime::lucet_hostcall_terminate!(e); } + } } } }); diff --git a/lucet-wiggle/src/borrow.rs b/lucet-wiggle/src/borrow.rs deleted file mode 100644 index 5ed16e271..000000000 --- a/lucet-wiggle/src/borrow.rs +++ /dev/null @@ -1,188 +0,0 @@ -use std::cell::RefCell; -use std::collections::HashMap; -use wiggle::{BorrowHandle, GuestError, Region}; - -pub struct BorrowChecker { - bc: RefCell, -} - -impl BorrowChecker { - /// A `BorrowChecker` manages run-time validation of borrows from a `GuestMemory`. It keeps - /// track of regions of guest memory which are possible to alias with Rust references (via the - /// `GuestSlice` and `GuestStr` structs, which implement `std::ops::Deref` and - /// `std::ops::DerefMut`. It also enforces that `GuestPtr::read` and `GuestPtr::write` do not - /// access memory with an outstanding borrow. - pub fn new() -> Self { - BorrowChecker { - bc: RefCell::new(InnerBorrowChecker::new()), - } - } - /// Indicates whether any outstanding borrows are known to the `BorrowChecker`. This function - /// must be `false` in order for it to be safe to recursively call into a WebAssembly module, - /// or to manipulate the WebAssembly memory by any other means. - pub fn has_outstanding_borrows(&self) -> bool { - self.bc.borrow().has_outstanding_borrows() - } - - pub(crate) fn borrow(&self, r: Region) -> Result { - self.bc.borrow_mut().borrow(r) - } - pub(crate) fn unborrow(&self, h: BorrowHandle) { - self.bc.borrow_mut().unborrow(h) - } - pub(crate) fn is_borrowed(&self, r: Region) -> bool { - self.bc.borrow().is_borrowed(r) - } -} - -#[derive(Debug)] -/// This is a pretty naive way to account for borrows. This datastructure -/// could be made a lot more efficient with some effort. -struct InnerBorrowChecker { - /// Map from handle to region borrowed. A HashMap is probably not ideal - /// for this but it works. It would be more efficient if we could - /// check `is_borrowed` without an O(n) iteration, by organizing borrows - /// by an ordering of Region. - borrows: HashMap, - /// Handle to give out for the next borrow. This is the bare minimum of - /// bookkeeping of free handles, and in a pathological case we could run - /// out, hence [`GuestError::BorrowCheckerOutOfHandles`] - next_handle: BorrowHandle, -} - -impl InnerBorrowChecker { - fn new() -> Self { - InnerBorrowChecker { - borrows: HashMap::new(), - next_handle: BorrowHandle(0), - } - } - - fn has_outstanding_borrows(&self) -> bool { - !self.borrows.is_empty() - } - - fn is_borrowed(&self, r: Region) -> bool { - !self.borrows.values().all(|b| !b.overlaps(r)) - } - - fn new_handle(&mut self) -> Result { - // Reset handles to 0 if all handles have been returned. - if self.borrows.is_empty() { - self.next_handle = BorrowHandle(0); - } - let h = self.next_handle; - // Get the next handle. Since we don't recycle handles until all of - // them have been returned, there is a pathological case where a user - // may make a Very Large (usize::MAX) number of valid borrows and - // unborrows while always keeping at least one borrow outstanding, and - // we will run out of borrow handles. - self.next_handle = BorrowHandle( - h.0.checked_add(1) - .ok_or_else(|| GuestError::BorrowCheckerOutOfHandles)?, - ); - Ok(h) - } - - fn borrow(&mut self, r: Region) -> Result { - if self.is_borrowed(r) { - return Err(GuestError::PtrBorrowed(r)); - } - let h = self.new_handle()?; - self.borrows.insert(h, r); - Ok(h) - } - - fn unborrow(&mut self, h: BorrowHandle) { - let _ = self.borrows.remove(&h); - } -} - -#[cfg(test)] -mod test { - use super::*; - #[test] - fn nonoverlapping() { - let mut bs = InnerBorrowChecker::new(); - let r1 = Region::new(0, 10); - let r2 = Region::new(10, 10); - assert!(!r1.overlaps(r2)); - bs.borrow(r1).expect("can borrow r1"); - bs.borrow(r2).expect("can borrow r2"); - - let mut bs = InnerBorrowChecker::new(); - let r1 = Region::new(10, 10); - let r2 = Region::new(0, 10); - assert!(!r1.overlaps(r2)); - bs.borrow(r1).expect("can borrow r1"); - bs.borrow(r2).expect("can borrow r2"); - } - - #[test] - fn overlapping() { - let mut bs = InnerBorrowChecker::new(); - let r1 = Region::new(0, 10); - let r2 = Region::new(9, 10); - assert!(r1.overlaps(r2)); - bs.borrow(r1).expect("can borrow r1"); - assert!(bs.borrow(r2).is_err(), "cant borrow r2"); - - let mut bs = InnerBorrowChecker::new(); - let r1 = Region::new(0, 10); - let r2 = Region::new(2, 5); - assert!(r1.overlaps(r2)); - bs.borrow(r1).expect("can borrow r1"); - assert!(bs.borrow(r2).is_err(), "cant borrow r2"); - - let mut bs = InnerBorrowChecker::new(); - let r1 = Region::new(9, 10); - let r2 = Region::new(0, 10); - assert!(r1.overlaps(r2)); - bs.borrow(r1).expect("can borrow r1"); - assert!(bs.borrow(r2).is_err(), "cant borrow r2"); - - let mut bs = InnerBorrowChecker::new(); - let r1 = Region::new(2, 5); - let r2 = Region::new(0, 10); - assert!(r1.overlaps(r2)); - bs.borrow(r1).expect("can borrow r1"); - assert!(bs.borrow(r2).is_err(), "cant borrow r2"); - - let mut bs = InnerBorrowChecker::new(); - let r1 = Region::new(2, 5); - let r2 = Region::new(10, 5); - let r3 = Region::new(15, 5); - let r4 = Region::new(0, 10); - assert!(r1.overlaps(r4)); - bs.borrow(r1).expect("can borrow r1"); - bs.borrow(r2).expect("can borrow r2"); - bs.borrow(r3).expect("can borrow r3"); - assert!(bs.borrow(r4).is_err(), "cant borrow r4"); - } - - #[test] - fn unborrowing() { - let mut bs = InnerBorrowChecker::new(); - let r1 = Region::new(0, 10); - let r2 = Region::new(10, 10); - assert!(!r1.overlaps(r2)); - assert_eq!(bs.has_outstanding_borrows(), false, "start with no borrows"); - let h1 = bs.borrow(r1).expect("can borrow r1"); - assert_eq!(bs.has_outstanding_borrows(), true, "h1 is outstanding"); - let h2 = bs.borrow(r2).expect("can borrow r2"); - - assert!(bs.borrow(r2).is_err(), "can't borrow r2 twice"); - bs.unborrow(h2); - assert_eq!( - bs.has_outstanding_borrows(), - true, - "h1 is still outstanding" - ); - bs.unborrow(h1); - assert_eq!(bs.has_outstanding_borrows(), false, "no remaining borrows"); - - let _h3 = bs - .borrow(r2) - .expect("can borrow r2 again now that its been unborrowed"); - } -} diff --git a/lucet-wiggle/src/lib.rs b/lucet-wiggle/src/lib.rs index 6304d09ed..d1c3f5121 100644 --- a/lucet-wiggle/src/lib.rs +++ b/lucet-wiggle/src/lib.rs @@ -1,12 +1,10 @@ -mod borrow; - pub use lucet_wiggle_macro::from_witx; pub use wiggle::*; pub mod runtime { - use crate::borrow::BorrowChecker; use lucet_runtime::vmctx::Vmctx; use wiggle::{BorrowHandle, GuestError, GuestMemory, Region}; + use wiggle_borrow::BorrowChecker; pub struct LucetMemory<'a> { vmctx: &'a Vmctx, @@ -35,14 +33,23 @@ pub mod runtime { fn has_outstanding_borrows(&self) -> bool { self.bc.has_outstanding_borrows() } - fn is_borrowed(&self, r: Region) -> bool { - self.bc.is_borrowed(r) + fn is_mut_borrowed(&self, r: Region) -> bool { + self.bc.is_mut_borrowed(r) + } + fn is_shared_borrowed(&self, r: Region) -> bool { + self.bc.is_shared_borrowed(r) + } + fn mut_borrow(&self, r: Region) -> Result { + self.bc.mut_borrow(r) + } + fn shared_borrow(&self, r: Region) -> Result { + self.bc.shared_borrow(r) } - fn borrow(&self, r: Region) -> Result { - self.bc.borrow(r) + fn mut_unborrow(&self, h: BorrowHandle) { + self.bc.mut_unborrow(h) } - fn unborrow(&self, h: BorrowHandle) { - self.bc.unborrow(h) + fn shared_unborrow(&self, h: BorrowHandle) { + self.bc.shared_unborrow(h) } } } diff --git a/lucetc/src/module.rs b/lucetc/src/module.rs index 468f53f7a..e49893ce9 100644 --- a/lucetc/src/module.rs +++ b/lucetc/src/module.rs @@ -227,7 +227,7 @@ impl<'a> ModuleEnvironment<'a> for ModuleValidation<'a> { &mut self, sig_index: TypeIndex, module: &'a str, - field: &'a str, + field: Option<&'a str>, ) -> WasmResult<()> { debug_assert_eq!( self.info.functions.len(), @@ -235,6 +235,7 @@ impl<'a> ModuleEnvironment<'a> for ModuleValidation<'a> { "import functions are declared first" ); + let field = field.expect("lucet does not support optional `field` in imports"); let unique_fn_index = self .info .imported_funcs @@ -260,13 +261,15 @@ impl<'a> ModuleEnvironment<'a> for ModuleValidation<'a> { &mut self, global: Global, module: &'a str, - field: &'a str, + field: Option<&'a str>, ) -> WasmResult<()> { debug_assert_eq!( self.info.globals.len(), self.info.imported_globals.len(), "import globals are declared first" ); + + let field = field.expect("lucet does not support optional `field` in imports"); self.info.globals.push(Exportable::new(global)); self.info.imported_globals.push((module, field)); Ok(()) @@ -276,13 +279,15 @@ impl<'a> ModuleEnvironment<'a> for ModuleValidation<'a> { &mut self, table: Table, module: &'a str, - field: &'a str, + field: Option<&'a str>, ) -> WasmResult<()> { debug_assert_eq!( self.info.tables.len(), self.info.imported_tables.len(), "import tables are declared first" ); + + let field = field.expect("lucet does not support optional `field` in imports"); self.info.tables.push(Exportable::new(table)); self.info.imported_tables.push((module, field)); Ok(()) @@ -292,13 +297,15 @@ impl<'a> ModuleEnvironment<'a> for ModuleValidation<'a> { &mut self, memory: Memory, module: &'a str, - field: &'a str, + field: Option<&'a str>, ) -> WasmResult<()> { debug_assert_eq!( self.info.memories.len(), self.info.imported_memories.len(), "import memories are declared first" ); + + let field = field.expect("lucet does not support optional `field` in imports"); self.info .data_initializers .insert(MemoryIndex::new(self.info.memories.len()), vec![]); diff --git a/wasmtime b/wasmtime index bf971efa4..efe7f3754 160000 --- a/wasmtime +++ b/wasmtime @@ -1 +1 @@ -Subproject commit bf971efa428442e64a7b0963baaef67de020a85c +Subproject commit efe7f3754272abafe54f47391709b39d3f92f43d