Merge pull request #1693 from CosmWasm/w3-follow-up

Wasmer 3 follow up

CHANGELOG.md

@@ -32,6 +32,7 @@ and this project adheres to
 - cosmwasm-vm: Add `Instance::set_debug_handler`/`unset_debug_handler` to allow
   customizing the handling of debug messages emitted by the contract ([#1667]).
 - cosmwasm-vm: Add `.wasm` extension to stored wasm files ([#1686]).
+- cosmwasm-vm: Upgrade Wasmer to version 3.3.0.
 - cosmwasm-check: Update clap dependency to version 4 ([#1677])
 
 [#1511]: https://github.com/CosmWasm/cosmwasm/issues/1511

packages/vm/src/cache.rs

@@ -243,10 +243,12 @@ where
 
     /// Pins a Module that was previously stored via save_wasm.
     ///
-    /// The module is lookup first in the memory cache, and then in the file system cache.
-    /// If not found, the code is loaded from the file system, compiled, and stored into the
+    /// The module is lookup first in the file system cache. If not found,
+    /// the code is loaded from the file system, compiled, and stored into the
     /// pinned cache.
-    /// If the given ID is not found, or the content does not match the hash (=ID), an error is returned.
+    ///
+    /// If the given contract for the given checksum is not found, or the content
+    /// does not match the checksum, an error is returned.
     pub fn pin(&self, checksum: &Checksum) -> VmResult<()> {
         let mut cache = self.inner.lock().unwrap();
         if cache.pinned_memory_cache.has(checksum) {
@@ -255,7 +257,7 @@ where
 
         // We don't load from the memory cache because we had to create new store here and
         // serialize/deserialize the artifact to get a full clone. Could be done but adds some code
-        // for a no-so-relevant use case.
+        // for a not-so-relevant use case.
 
         // Try to get module from file system cache
         let engine = Engine::headless();
@@ -730,7 +732,7 @@ mod tests {
         let backend5 = mock_backend(&[]);
 
         // from file system
-        let _instance1: Instance<MockApi, MockStorage, MockQuerier> = cache
+        let _instance1 = cache
             .get_instance(&checksum, backend1, TESTING_OPTIONS)
             .unwrap();
         assert_eq!(cache.stats().hits_pinned_memory_cache, 0);

packages/vm/src/errors/communication_error.rs

@@ -2,6 +2,7 @@ use std::fmt::Debug;
 use thiserror::Error;
 
 use super::region_validation_error::RegionValidationError;
+use crate::memory::Region;
 
 /// An error in the communcation between contract and host. Those happen around imports and exports.
 #[derive(Error, Debug)]
@@ -32,6 +33,12 @@ pub enum CommunicationError {
     RegionLengthTooBig { length: usize, max_length: usize },
     #[error("Region too small. Got {}, required {}", size, required)]
     RegionTooSmall { size: usize, required: usize },
+    #[error("Tried to access memory of region {:?} in Wasm memory of size {} bytes. This typically happens when the given Region pointer does not point to a proper Region struct.", region, memory_size)]
+    RegionAccessErr {
+        region: Region,
+        /// Current size of the linear memory in bytes
+        memory_size: usize,
+    },
     #[error("Got a zero Wasm address")]
     ZeroAddress {},
 }
@@ -64,6 +71,13 @@ impl CommunicationError {
         CommunicationError::RegionTooSmall { size, required }
     }
 
+    pub(crate) fn region_access_err(region: Region, memory_size: usize) -> Self {
+        CommunicationError::RegionAccessErr {
+            region,
+            memory_size,
+        }
+    }
+
     pub(crate) fn zero_address() -> Self {
         CommunicationError::ZeroAddress {}
     }

packages/vm/src/instance.rs

@@ -602,18 +602,21 @@ mod tests {
         let (engine, module) = compile(&wasm, &[]).unwrap();
         let mut store = make_store_with_engine(engine, memory_limit);
 
+        let called = Arc::new(AtomicBool::new(false));
+
         #[derive(Clone)]
         struct MyEnv {
             // This can be mutated across threads safely. We initialize it as `false`
             // and let our imported fn switch it to `true` to confirm it works.
             called: Arc<AtomicBool>,
         }
 
-        let my_env = MyEnv {
-            called: Arc::new(AtomicBool::new(false)),
-        };
-
-        let fe = FunctionEnv::new(&mut store, my_env);
+        let fe = FunctionEnv::new(
+            &mut store,
+            MyEnv {
+                called: called.clone(),
+            },
+        );
 
         let fun =
             Function::new_typed_with_env(&mut store, &fe, move |fe_mut: FunctionEnvMut<MyEnv>| {
@@ -636,8 +639,7 @@ mod tests {
 
         instance.call_function0("main", &[]).unwrap();
 
-        // TODO: allow inspecting env
-        // assert!(fe.as_ref(&store).called.load(Ordering::Relaxed));
+        assert!(called.load(Ordering::Relaxed));
     }
 
     #[test]

packages/vm/src/memory.rs

@@ -17,7 +17,7 @@ use crate::errors::{
 /// but defined here to allow Wasmer specific implementation.
 #[repr(C)]
 #[derive(Default, Clone, Copy, Debug)]
-struct Region {
+pub struct Region {
     /// The beginning of the region expressed as bytes from the beginning of the linear memory
     pub offset: u32,
     /// The number of bytes available in this region
@@ -47,12 +47,9 @@ pub fn read_region(memory: &wasmer::MemoryView, ptr: u32, max_length: usize) ->
     }
 
     let mut result = vec![0u8; region.length as usize];
-    // TODO: double check error handling
-    memory.read(region.offset as u64, &mut result).map_err(|_err| CommunicationError::deref_err(region.offset, format!(
-                 "Tried to access memory of region {:?} in wasm memory of size {} bytes. This typically happens when the given Region pointer does not point to a proper Region struct.",
-                 region,
-                 memory.size().bytes().0
-    )))?;
+    memory
+        .read(region.offset as u64, &mut result)
+        .map_err(|_err| CommunicationError::region_access_err(region, memory.size().bytes().0))?;
     Ok(result)
 }
 
@@ -82,25 +79,21 @@ pub fn write_region(memory: &wasmer::MemoryView, ptr: u32, data: &[u8]) -> VmRes
         return Err(CommunicationError::region_too_small(region_capacity, data.len()).into());
     }
 
-    // TODO: double check error handling
-    memory.write(region.offset as u64, data).map_err(|_err| CommunicationError::deref_err(region.offset, format!(
-                 "Tried to access memory of region {:?} in wasm memory of size {} bytes. This typically happens when the given Region pointer does not point to a proper Region struct.",
-                 region,
-                 memory.size().bytes().0
-             )))?;
+    memory
+        .write(region.offset as u64, data)
+        .map_err(|_err| CommunicationError::region_access_err(region, memory.size().bytes().0))?;
 
     region.length = data.len() as u32;
     set_region(memory, ptr, region)?;
 
     Ok(())
 }
 
-/// Reads in a Region at ptr in wasm memory and returns a copy of it
-fn get_region(memory: &wasmer::MemoryView, ptr: u32) -> CommunicationResult<Region> {
-    let wptr = WasmPtr::<Region>::new(ptr);
-    // TODO: double check error handling
+/// Reads in a Region at offset in Wasm memory and returns a copy of it
+fn get_region(memory: &wasmer::MemoryView, offset: u32) -> CommunicationResult<Region> {
+    let wptr = WasmPtr::<Region>::new(offset);
     let region = wptr.deref(memory).read().map_err(|_err| {
-        CommunicationError::deref_err(ptr, "Could not dereference this pointer to a Region")
+        CommunicationError::deref_err(offset, "Could not dereference this pointer to a Region")
     })?;
     validate_region(&region)?;
     Ok(region)
@@ -127,13 +120,11 @@ fn validate_region(region: &Region) -> RegionValidationResult<()> {
     Ok(())
 }
 
-/// Overrides a Region at ptr in wasm memory with data
-fn set_region(memory: &wasmer::MemoryView, ptr: u32, data: Region) -> CommunicationResult<()> {
-    let wptr = WasmPtr::<Region>::new(ptr);
-
-    // TODO: double check error handling
+/// Overrides a Region at offset in Wasm memory
+fn set_region(memory: &wasmer::MemoryView, offset: u32, data: Region) -> CommunicationResult<()> {
+    let wptr = WasmPtr::<Region>::new(offset);
     wptr.deref(memory).write(data).map_err(|_err| {
-        CommunicationError::deref_err(ptr, "Could not dereference this pointer to a Region")
+        CommunicationError::deref_err(offset, "Could not dereference this pointer to a Region")
     })?;
     Ok(())
 }

packages/vm/src/modules/in_memory_cache.rs

@@ -3,7 +3,7 @@ use std::collections::hash_map::RandomState;
 use std::num::NonZeroUsize;
 use wasmer::{Engine, Module};
 
-use super::sized_module::CachedModule;
+use super::cached_module::CachedModule;
 use crate::{Checksum, Size, VmError, VmResult};
 
 // Minimum module size.

packages/vm/src/modules/mod.rs

@@ -1,11 +1,11 @@
+mod cached_module;
 mod file_system_cache;
 mod in_memory_cache;
 mod pinned_memory_cache;
-mod sized_module;
 mod versioning;
 
+pub use cached_module::CachedModule;
 pub use file_system_cache::{FileSystemCache, NewFileSystemCacheError};
 pub use in_memory_cache::InMemoryCache;
 pub use pinned_memory_cache::PinnedMemoryCache;
-pub use sized_module::CachedModule;
 pub use versioning::current_wasmer_module_version;

packages/vm/src/modules/pinned_memory_cache.rs

@@ -1,7 +1,7 @@
 use std::collections::HashMap;
 use wasmer::{Engine, Module};
 
-use super::sized_module::CachedModule;
+use super::cached_module::CachedModule;
 use crate::{Checksum, VmResult};
 
 /// An pinned in memory module cache

packages/vm/src/wasm_backend/store.rs

@@ -37,26 +37,17 @@ pub fn make_engine(middlewares: &[Arc<dyn ModuleMiddleware>]) -> Engine {
     let metering = Arc::new(Metering::new(gas_limit, cost));
 
     #[cfg(feature = "cranelift")]
-    {
-        let mut compiler = Cranelift::default();
-        for middleware in middlewares {
-            compiler.push_middleware(middleware.clone());
-        }
-        compiler.push_middleware(deterministic);
-        compiler.push_middleware(metering);
-        compiler.into()
-    }
+    let mut compiler = Cranelift::default();
 
     #[cfg(not(feature = "cranelift"))]
-    {
-        let mut compiler = Singlepass::default();
-        for middleware in middlewares {
-            compiler.push_middleware(middleware.clone());
-        }
-        compiler.push_middleware(deterministic);
-        compiler.push_middleware(metering);
-        compiler.into()
+    let mut compiler = Singlepass::default();
+
+    for middleware in middlewares {
+        compiler.push_middleware(middleware.clone());
     }
+    compiler.push_middleware(deterministic);
+    compiler.push_middleware(metering);
+    compiler.into()
 }
 
 /// Created a store with no compiler and the given memory limit (in bytes)