From 3579e839e6e0c9cee6fa9119dbbd38ed5ea4d1bc Mon Sep 17 00:00:00 2001 From: Simon Warta Date: Thu, 5 Oct 2023 17:28:32 +0300 Subject: [PATCH 1/9] Fix test comments init -> instantiate --- packages/vm/src/cache.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/vm/src/cache.rs b/packages/vm/src/cache.rs index 7d853affa0..5a1546065a 100644 --- a/packages/vm/src/cache.rs +++ b/packages/vm/src/cache.rs @@ -854,7 +854,7 @@ mod tests { assert_eq!(cache.stats().hits_fs_cache, 1); assert_eq!(cache.stats().misses, 0); - // init + // instantiate let info = mock_info("creator", &coins(1000, "earth")); let msg = br#"{"verifier": "verifies", "beneficiary": "benefits"}"#; let res = @@ -873,7 +873,7 @@ mod tests { assert_eq!(cache.stats().hits_fs_cache, 1); assert_eq!(cache.stats().misses, 0); - // init + // instantiate let info = mock_info("creator", &coins(1000, "earth")); let msg = br#"{"verifier": "verifies", "beneficiary": "benefits"}"#; let res = @@ -894,7 +894,7 @@ mod tests { assert_eq!(cache.stats().hits_fs_cache, 2); assert_eq!(cache.stats().misses, 0); - // init + // instantiate let info = mock_info("creator", &coins(1000, "earth")); let msg = br#"{"verifier": "verifies", "beneficiary": "benefits"}"#; let res = @@ -919,7 +919,7 @@ mod tests { assert_eq!(cache.stats().hits_fs_cache, 1); assert_eq!(cache.stats().misses, 0); - // init + // instantiate let info = mock_info("creator", &coins(1000, "earth")); let msg = br#"{"verifier": "verifies", "beneficiary": "benefits"}"#; let response = @@ -947,7 +947,7 @@ mod tests { assert_eq!(cache.stats().hits_fs_cache, 1); assert_eq!(cache.stats().misses, 0); - // init + // instantiate let info = mock_info("creator", &coins(1000, "earth")); let msg = br#"{"verifier": "verifies", "beneficiary": "benefits"}"#; let response = @@ -977,7 +977,7 @@ mod tests { assert_eq!(cache.stats().hits_fs_cache, 2); assert_eq!(cache.stats().misses, 0); - // init + // instantiate let info = mock_info("creator", &coins(1000, "earth")); let msg = br#"{"verifier": "verifies", "beneficiary": "benefits"}"#; let response = @@ -1005,7 +1005,7 @@ mod tests { let backend1 = mock_backend(&[]); let backend2 = mock_backend(&[]); - // init instance 1 + // instantiate instance 1 let mut instance = cache .get_instance(&checksum, backend1, TESTING_OPTIONS) .unwrap(); @@ -1017,7 +1017,7 @@ mod tests { assert_eq!(msgs.len(), 0); let backend1 = instance.recycle().unwrap(); - // init instance 2 + // instantiate instance 2 let mut instance = cache .get_instance(&checksum, backend2, TESTING_OPTIONS) .unwrap(); From a440660a9b915bdf56d729afa2f58cfc3177bfaa Mon Sep 17 00:00:00 2001 From: Simon Warta Date: Thu, 5 Oct 2023 16:54:14 +0300 Subject: [PATCH 2/9] Add test reproducing the issue --- packages/vm/src/cache.rs | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/packages/vm/src/cache.rs b/packages/vm/src/cache.rs index 5a1546065a..070b624cf3 100644 --- a/packages/vm/src/cache.rs +++ b/packages/vm/src/cache.rs @@ -996,6 +996,42 @@ mod tests { } } + #[test] + fn call_execute_on_recompiled_contract() { + let options = make_testing_options(); + let cache = unsafe { Cache::new(options.clone()).unwrap() }; + let checksum = cache.save_wasm(CONTRACT).unwrap(); + + // Remove compiled module from disk + remove_dir_all(options.base_dir.join(CACHE_DIR).join(MODULES_DIR)).unwrap(); + + // Recompiles the Wasm (miss on all caches) + let backend = mock_backend(&[]); + let mut instance = cache + .get_instance(&checksum, backend, TESTING_OPTIONS) + .unwrap(); + assert_eq!(cache.stats().hits_pinned_memory_cache, 0); + assert_eq!(cache.stats().hits_memory_cache, 0); + assert_eq!(cache.stats().hits_fs_cache, 0); + assert_eq!(cache.stats().misses, 1); + + // instantiate + let info = mock_info("creator", &coins(1000, "earth")); + let msg = br#"{"verifier": "verifies", "beneficiary": "benefits"}"#; + let response = call_instantiate::<_, _, _, Empty>(&mut instance, &mock_env(), &info, msg) + .unwrap() + .unwrap(); + assert_eq!(response.messages.len(), 0); + + // execute + let info = mock_info("verifies", &coins(15, "earth")); + let msg = br#"{"release":{}}"#; + let response = call_execute::<_, _, _, Empty>(&mut instance, &mock_env(), &info, msg) + .unwrap() + .unwrap(); + assert_eq!(response.messages.len(), 1); + } + #[test] fn use_multiple_cached_instances_of_same_contract() { let cache = unsafe { Cache::new(make_testing_options()).unwrap() }; From d9e05924d660a4914c29e707299865603563e7d2 Mon Sep 17 00:00:00 2001 From: Simon Warta Date: Thu, 5 Oct 2023 16:57:34 +0300 Subject: [PATCH 3/9] Fix misuse of Module Before this fix, the compiling engine was attached to the Module instance (indirectly via the artifact). However, the Store used was created from a different engine. --- packages/vm/src/cache.rs | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/packages/vm/src/cache.rs b/packages/vm/src/cache.rs index 070b624cf3..dd62d96c3a 100644 --- a/packages/vm/src/cache.rs +++ b/packages/vm/src/cache.rs @@ -377,11 +377,22 @@ where // stored the old module format. let wasm = self.load_wasm_with_path(&cache.wasm_path, checksum)?; cache.stats.misses = cache.stats.misses.saturating_add(1); - // Module will run with a different engine, so we can set memory limit to None - let engine = make_compiling_engine(None); - let module = compile(&engine, &wasm)?; - let module_size = cache.fs_cache.store(checksum, &module)?; + { + // Module will run with a different engine, so we can set memory limit to None + let compiling_engine = make_compiling_engine(None); + // Note that module cannot be used directly as it was not created with the + // runtime engine. + let module = compile(&compiling_engine, &wasm)?; + cache.fs_cache.store(checksum, &module)?; + } + // This time we'll hit the file-system cache. + let Some((module, module_size)) = cache.fs_cache.load(checksum, &cache.runtime_engine)? + else { + return Err(VmError::generic_err( + "Can't load module from file system cache after storing it to file system cache", + )); + }; cache .memory_cache .store(checksum, module.clone(), module_size)?; From 949d82b267b220d9e9626f52a78aef359488c98d Mon Sep 17 00:00:00 2001 From: Simon Warta Date: Fri, 6 Oct 2023 11:57:46 +0300 Subject: [PATCH 4/9] Pull out test_hackatom_instance_execution --- packages/vm/src/cache.rs | 41 ++++++++++++++++++++++++---------------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/packages/vm/src/cache.rs b/packages/vm/src/cache.rs index dd62d96c3a..d04c88d854 100644 --- a/packages/vm/src/cache.rs +++ b/packages/vm/src/cache.rs @@ -543,6 +543,30 @@ mod tests { } } + /// Takes an instance and executes it + fn test_hackatom_instance_execution(instance: &mut Instance) + where + A: BackendApi + 'static, + S: Storage + 'static, + Q: Querier + 'static, + { + // instantiate + let info = mock_info("creator", &coins(1000, "earth")); + let msg = br#"{"verifier": "verifies", "beneficiary": "benefits"}"#; + let response = call_instantiate::<_, _, _, Empty>(instance, &mock_env(), &info, msg) + .unwrap() + .unwrap(); + assert_eq!(response.messages.len(), 0); + + // execute + let info = mock_info("verifies", &coins(15, "earth")); + let msg = br#"{"release":{}}"#; + let response = call_execute::<_, _, _, Empty>(instance, &mock_env(), &info, msg) + .unwrap() + .unwrap(); + assert_eq!(response.messages.len(), 1); + } + #[test] fn new_base_dir_will_be_created() { let my_base_dir = TempDir::new() @@ -1025,22 +1049,7 @@ mod tests { assert_eq!(cache.stats().hits_memory_cache, 0); assert_eq!(cache.stats().hits_fs_cache, 0); assert_eq!(cache.stats().misses, 1); - - // instantiate - let info = mock_info("creator", &coins(1000, "earth")); - let msg = br#"{"verifier": "verifies", "beneficiary": "benefits"}"#; - let response = call_instantiate::<_, _, _, Empty>(&mut instance, &mock_env(), &info, msg) - .unwrap() - .unwrap(); - assert_eq!(response.messages.len(), 0); - - // execute - let info = mock_info("verifies", &coins(15, "earth")); - let msg = br#"{"release":{}}"#; - let response = call_execute::<_, _, _, Empty>(&mut instance, &mock_env(), &info, msg) - .unwrap() - .unwrap(); - assert_eq!(response.messages.len(), 1); + test_hackatom_instance_execution(&mut instance); } #[test] From 85201719fb61776389cd2b47fc59e6bfce819025 Mon Sep 17 00:00:00 2001 From: Simon Warta Date: Fri, 6 Oct 2023 12:01:24 +0300 Subject: [PATCH 5/9] Execute instances in pin_unpin_works test --- packages/vm/src/cache.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/vm/src/cache.rs b/packages/vm/src/cache.rs index d04c88d854..57ee769c67 100644 --- a/packages/vm/src/cache.rs +++ b/packages/vm/src/cache.rs @@ -1286,13 +1286,14 @@ mod tests { // check not pinned let backend = mock_backend(&[]); - let _instance = cache + let mut instance = cache .get_instance(&checksum, backend, TESTING_OPTIONS) .unwrap(); assert_eq!(cache.stats().hits_pinned_memory_cache, 0); assert_eq!(cache.stats().hits_memory_cache, 0); assert_eq!(cache.stats().hits_fs_cache, 1); assert_eq!(cache.stats().misses, 0); + test_hackatom_instance_execution(&mut instance); // first pin hits file system cache cache.pin(&checksum).unwrap(); @@ -1310,26 +1311,28 @@ mod tests { // check pinned let backend = mock_backend(&[]); - let _instance = cache + let mut instance = cache .get_instance(&checksum, backend, TESTING_OPTIONS) .unwrap(); assert_eq!(cache.stats().hits_pinned_memory_cache, 1); assert_eq!(cache.stats().hits_memory_cache, 0); assert_eq!(cache.stats().hits_fs_cache, 2); assert_eq!(cache.stats().misses, 0); + test_hackatom_instance_execution(&mut instance); // unpin cache.unpin(&checksum).unwrap(); // verify unpinned let backend = mock_backend(&[]); - let _instance = cache + let mut instance = cache .get_instance(&checksum, backend, TESTING_OPTIONS) .unwrap(); assert_eq!(cache.stats().hits_pinned_memory_cache, 1); assert_eq!(cache.stats().hits_memory_cache, 1); assert_eq!(cache.stats().hits_fs_cache, 2); assert_eq!(cache.stats().misses, 0); + test_hackatom_instance_execution(&mut instance); // unpin again has no effect cache.unpin(&checksum).unwrap(); From 7c7b93d9c6a401ed36742d9f3da2e03f5adebc75 Mon Sep 17 00:00:00 2001 From: Simon Warta Date: Fri, 6 Oct 2023 12:03:38 +0300 Subject: [PATCH 6/9] Reproduce bug in pin implementation --- packages/vm/src/cache.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/vm/src/cache.rs b/packages/vm/src/cache.rs index 57ee769c67..69ac065d19 100644 --- a/packages/vm/src/cache.rs +++ b/packages/vm/src/cache.rs @@ -1361,13 +1361,14 @@ mod tests { // After the compilation in pin, the module can be used from pinned memory cache let backend = mock_backend(&[]); - let _ = cache + let mut instance = cache .get_instance(&checksum, backend, TESTING_OPTIONS) .unwrap(); assert_eq!(cache.stats().hits_pinned_memory_cache, 1); assert_eq!(cache.stats().hits_memory_cache, 0); assert_eq!(cache.stats().hits_fs_cache, 0); assert_eq!(cache.stats().misses, 1); + test_hackatom_instance_execution(&mut instance); } #[test] From b76c1e5bcb578da5359caa11b0cf114e0d086aca Mon Sep 17 00:00:00 2001 From: Simon Warta Date: Fri, 6 Oct 2023 12:09:38 +0300 Subject: [PATCH 7/9] Annotate compiled module consistenly --- packages/vm/src/cache.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/vm/src/cache.rs b/packages/vm/src/cache.rs index 69ac065d19..779fdc374f 100644 --- a/packages/vm/src/cache.rs +++ b/packages/vm/src/cache.rs @@ -202,6 +202,7 @@ where pub fn save_wasm_unchecked(&self, wasm: &[u8]) -> VmResult { // We need a new engine for each Wasm -> module compilation due to the metering middleware. let compiling_engine = make_compiling_engine(None); + // This module cannot be executed directly as it was not created with the runtime engine let module = compile(&compiling_engine, wasm)?; let mut cache = self.inner.lock().unwrap(); @@ -292,8 +293,9 @@ where let wasm = self.load_wasm_with_path(&cache.wasm_path, checksum)?; cache.stats.misses = cache.stats.misses.saturating_add(1); // Module will run with a different engine, so we can set memory limit to None - let engine = make_compiling_engine(None); - let module = compile(&engine, &wasm)?; + let compiling_engine = make_compiling_engine(None); + // This module cannot be executed directly as it was not created with the runtime engine + let module = compile(&compiling_engine, &wasm)?; // Store into the fs cache too let module_size = cache.fs_cache.store(checksum, &module)?; cache @@ -380,8 +382,7 @@ where { // Module will run with a different engine, so we can set memory limit to None let compiling_engine = make_compiling_engine(None); - // Note that module cannot be used directly as it was not created with the - // runtime engine. + // This module cannot be executed directly as it was not created with the runtime engine let module = compile(&compiling_engine, &wasm)?; cache.fs_cache.store(checksum, &module)?; } From e99425bdbd7f00b856e4841888ded7937f8056f0 Mon Sep 17 00:00:00 2001 From: Simon Warta Date: Fri, 6 Oct 2023 12:18:23 +0300 Subject: [PATCH 8/9] Fix bug for the pin implementation --- packages/vm/src/cache.rs | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/packages/vm/src/cache.rs b/packages/vm/src/cache.rs index 779fdc374f..7bf893bef2 100644 --- a/packages/vm/src/cache.rs +++ b/packages/vm/src/cache.rs @@ -292,12 +292,22 @@ where // Re-compile from original Wasm bytecode let wasm = self.load_wasm_with_path(&cache.wasm_path, checksum)?; cache.stats.misses = cache.stats.misses.saturating_add(1); - // Module will run with a different engine, so we can set memory limit to None - let compiling_engine = make_compiling_engine(None); - // This module cannot be executed directly as it was not created with the runtime engine - let module = compile(&compiling_engine, &wasm)?; - // Store into the fs cache too - let module_size = cache.fs_cache.store(checksum, &module)?; + { + // Module will run with a different engine, so we can set memory limit to None + let compiling_engine = make_compiling_engine(None); + // This module cannot be executed directly as it was not created with the runtime engine + let module = compile(&compiling_engine, &wasm)?; + cache.fs_cache.store(checksum, &module)?; + } + + // This time we'll hit the file-system cache. + let Some((module, module_size)) = cache.fs_cache.load(checksum, &cache.runtime_engine)? + else { + return Err(VmError::generic_err( + "Can't load module from file system cache after storing it to file system cache (pin)", + )); + }; + cache .pinned_memory_cache .store(checksum, module, module_size) @@ -391,7 +401,7 @@ where let Some((module, module_size)) = cache.fs_cache.load(checksum, &cache.runtime_engine)? else { return Err(VmError::generic_err( - "Can't load module from file system cache after storing it to file system cache", + "Can't load module from file system cache after storing it to file system cache (get_module)", )); }; cache From 7774c748e18c5dddf646f99240ba40e5bfcd60d0 Mon Sep 17 00:00:00 2001 From: Simon Warta Date: Sat, 7 Oct 2023 14:01:46 +0300 Subject: [PATCH 9/9] Add CHANGELOG entry for Wasmer engine regression fix --- CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1011c1a908..12280bdc57 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,14 @@ and this project adheres to ## [Unreleased] +## Fixed + +- cosmwasm-vm: Fix a 1.3.x -> 1.4.0 regression bug leading to a _Wasmer runtime + error: RuntimeError: out of bounds memory access_ in cases when the Wasm file + is re-compiled and used right away. ([#1907]) + +[#1907]: https://github.com/CosmWasm/cosmwasm/pull/1907 + ### Changed - cosmwasm-check: Use "=" for pinning the versions of cosmwasm-vm and