Failed to instantiate a deserialized Module compiled with the Dylib engine #2434
Assignees
Labels
bug
Something isn't working
📦 lib-engine-dylib
About wasmer-engine-dylib
🕵️ needs investigation
The issue/PR needs further investigation
priority-high
High priority issue
project-terra
Projects
Milestone
Comments
|
Thank you for the bug report! We will investigate. |
|
I can reproduce the bug. It doesn't happen with the It's not related to let compiler = LLVM::default();
let store = Store::new(&Dylib::new(compiler).engine());
let module = Module::new(…);
let serialized_module = module.serialize()?;
let module = unsafe { Module::deserialize(&store, &serialized_module)? };
let instance = Instance::new(&module, &import_object)?;I've changed the compiler to Cranelift, it doesn't change. It's really related to the serialization and deserialization of the Dylib engine's output. |
Hywan
changed the title
|
Still triggering. Is there any workaround? |
|
I cannot reproduce this with the latest master from git. |
|
here is detail logs. |
|
I'm horribly unfamiliar with that code, but is it possible there is an off-by-one error in // First up assert that our chunk of jit functions doesn't collide with
// any other known chunks of jit functions...
if let Some((_, prev)) = info.ranges.range(max..).next() {
assert!(prev.start > max);
}Looking at the code, |
let me make fork and do test with |
|
^ unfortunately not helpful |
Can you output the values of |
okay, I tried to reproduce this error but met other out of resource error like maximum thread exceed. |
|
here is result tested on |
|
I just ran into this when executing cosmwasm_vm tests in the CI. This is the first time I see it without any blockchain code involved. The engine is Universal and Wasmer 2.1.1 is used. Seems like one test hits the assertion and all others are poisened after that. |
|
I can reproduce this error with updated test code of https://github.com/CosmWasm/cosmwasm/blob/be4d94a8d73ceab359226b8ea0a5b4be032a07e6/packages/vm/src/modules/in_memory_cache.rs#L123-L171 use std::thread;
use std::thread::JoinHandle;
#[test]
fn in_memory_cache_run() {
let mut handles: Vec<JoinHandle<()>> = vec![];
for _ in 1..10000 {
let handle = thread::spawn(move || {
let mut cache = InMemoryCache::new(Size::mebi(1));
// Create module
let wasm = wat::parse_str(
r#"(module
(type $t0 (func (param i32) (result i32)))
(func $add_one (export "add_one") (type $t0) (param $p0 i32) (result i32)
get_local $p0
i32.const 1
i32.add)
)"#,
)
.unwrap();
let checksum = Checksum::generate(&wasm);
// Module does not exist
let cache_entry = cache.load(&checksum).unwrap();
assert!(cache_entry.is_none());
// Compile module
let original = compile(&wasm, None).unwrap();
// Store module
let size = wasm.len() * TESTING_WASM_SIZE_FACTOR;
cache.store(&checksum, original, size).unwrap();
let cached = cache.load(&checksum).unwrap().unwrap();
let instance = WasmerInstance::new(&cached.module, &imports! {}).unwrap();
set_remaining_points(&instance, TESTING_GAS_LIMIT);
let add_one = instance.exports.get_function("add_one").unwrap();
let result = add_one.call(&[42.into()]).unwrap();
assert_eq!(result[0].unwrap_i32(), 43);
});
handles.push(handle);
}
for handle in handles {
handle.join().unwrap();
}
}backtrace |
|
other logs, when I change test to program |
|
I made new branch to let you easily reproduce error. https://github.com/terra-money/cosmwasm/tree/bug/frame_info cd packages/vm
# execute
RUST_BACKTRACE=full cargo run 2> output.txt
# test
RUST_BACKTRACE=1 cargo test -- --nocapture in_memory_cache_run 2> output.txtboth way will make same panic error |
This was referenced Feb 15, 2022
|
@YunSuk-Yeo Thanks for the reproducer, this was a huge help in tracking down this bug. I have a potential fix in #2806. |
bors bot
added a commit
that referenced
this issue
Feb 25, 2022
2805: Enable `experimental-io-devices` by default r=Amanieu a=Amanieu Fixes #2695 2806: Fix drop order for Module fields r=Amanieu a=Amanieu The field ordering here is actually significant because of the drop order: we want to drop the artifact before dropping the engine. The reason for this is that dropping the Artifact will de-register the trap handling metadata from the global registry. This must be done before the code memory for the artifact is freed (which happens when the store is dropped) since there is a chance that this memory could be reused by another module which will try to register its own trap information. Note that in Rust, the drop order for struct fields is from top to bottom: the opposite of C++. In the future, this code should be refactored to properly describe the ownership of the code and its metadata. Fixes #2434 Co-authored-by: Amanieu d'Antras <amanieu@gmail.com> Co-authored-by: ptitSeb <sebastien.chev@gmail.com>
yun-yeo
pushed a commit
to terra-money/wasmer
that referenced
this issue
Feb 28, 2022
The field ordering here is actually significant because of the drop order: we want to drop the artifact before dropping the engine. The reason for this is that dropping the Artifact will de-register the trap handling metadata from the global registry. This must be done before the code memory for the artifact is freed (which happens when the store is dropped) since there is a chance that this memory could be reused by another module which will try to register its own trap information. Note that in Rust, the drop order for struct fields is from top to bottom: the opposite of C++. In the future, this code should be refactored to properly describe the ownership of the code and its metadata. Fixes wasmerio#2434
|
Small update: we had a follow-up PR #2812 that was just merged into Please let us know if this issue repeats in the future so we can reopen and re-investigate the issue |
ptitSeb
pushed a commit
that referenced
this issue
Oct 20, 2022
The field ordering here is actually significant because of the drop order: we want to drop the artifact before dropping the engine. The reason for this is that dropping the Artifact will de-register the trap handling metadata from the global registry. This must be done before the code memory for the artifact is freed (which happens when the store is dropped) since there is a chance that this memory could be reused by another module which will try to register its own trap information. Note that in Rust, the drop order for struct fields is from top to bottom: the opposite of C++. In the future, this code should be refactored to properly describe the ownership of the code and its metadata. Fixes #2434
This was referenced Apr 21, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the bug
Instance::new panics when the module is deserialized from FileSystemCache and engine is native.
Steps to reproduce
Test wasm:
Reproducer:
Expected behavior
I would expect the Instance::new to succeed (or return an error).
Actual behavior
Additional context
The text was updated successfully, but these errors were encountered: