Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stack var overlapped for multithread #2856

Closed
kvinwang opened this issue Apr 23, 2022 · 1 comment · Fixed by #2892
Closed

Stack var overlapped for multithread #2856

kvinwang opened this issue Apr 23, 2022 · 1 comment · Fixed by #2892
Assignees
Labels
bug Something isn't working priority-medium Medium priority issue
Milestone

Comments

@kvinwang
Copy link

kvinwang commented Apr 23, 2022

Describe the bug

Given the following guest rust code and host-runtime implementation:

extern "C" {
    fn ocall(thread: i32, nth: i32, p: i32);
}

#[inline(never)]
fn use_more_stack(n: i32) -> i32 {
    let mut bug_buffer = [n; 32];
    unsafe {
        ocall(n, 2, bug_buffer.as_mut_ptr() as i32);
    }
    bug_buffer[0]
}

#[no_mangle]
extern "C" fn entry(thread: i32) {
    let mut b = [thread; 1];
    unsafe {
        ocall(thread, 1, b.as_mut_ptr() as i32);
    }
    let bug_thread_num = use_more_stack(thread);
    unsafe {
        ocall(thread, 3, bug_thread_num);
    }
}
use wasmer::{imports, Function, Instance, Module, Store};

fn main() -> Result<(), Box<dyn std::error::Error>> {
    let wasm_bytes = include_bytes!("../guest/target/wasm32-unknown-unknown/release/guest.wasm");
    let store = Store::default();

    println!("Compiling module...");
    let module = Module::new(&store, wasm_bytes)?;

    fn ocall(thread: i32, nth: i32, p: i32) {
        println!("thread {}, {}-nth ocall, p={}", thread, nth, p);
        std::thread::sleep(std::time::Duration::from_millis(50));
    }

    let import_object = imports! {
        "env" => {
            "ocall" => Function::new_native(&store, ocall),
        }
    };

    println!("Instantiating module...");
    let instance = Instance::new(&module, &import_object)?;

    let mut threads = vec![];
    for i in 0..3 {
        let instance = instance.clone();
        let handle = std::thread::spawn(move || {
            let entry = instance
                .exports
                .get_function("entry")
                .unwrap()
                .native::<i32, ()>()
                .unwrap();
            entry.call(i).unwrap();
        });
        threads.push(handle);
    }
    for handle in threads {
        handle.join().unwrap();
    }
    Ok(())
}

cargo run usually output as expected:

$ cargo run --release
    Finished release [optimized] target(s) in 0.10s
     Running `target/release/play-wasmer`
Compiling module...
Instantiating module...
thread 0, 1-nth ocall, p=1048572
thread 1, 1-nth ocall, p=1048556
thread 2, 1-nth ocall, p=1048540
thread 0, 2-nth ocall, p=1048400
thread 1, 2-nth ocall, p=1048272
thread 2, 2-nth ocall, p=1048144
thread 0, 3-nth ocall, p=0
thread 2, 3-nth ocall, p=2
thread 1, 3-nth ocall, p=1

But sometimes, the output is somthing like:

$ cargo run --release
    Finished release [optimized] target(s) in 0.04s
     Running `target/release/play-wasmer`
Compiling module...
Instantiating module...
thread 0, 1-nth ocall, p=1048572
thread 2, 1-nth ocall, p=1048556
thread 1, 1-nth ocall, p=1048540
thread 0, 2-nth ocall, p=1048400
thread 2, 2-nth ocall, p=1048272
thread 1, 2-nth ocall, p=1048272
thread 0, 3-nth ocall, p=0
thread 2, 3-nth ocall, p=1
thread 1, 3-nth ocall, p=1

Which means the bug_buffer in guest get overlapped memory for thread 1 and thread 2.

$ echo "`wasmer -V` | `rustc -V` | `uname -m`"
wasmer: command not found
 | rustc 1.60.0 (7737e0b5c 2022-04-04) | x86_64

wasmer in Cargo.lock:

[[package]]
name = "wasmer"
version = "2.1.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "23f0188c23fc1b7de9bd7f8b834d0b1cd5edbe66e287452e8ce36d24418114f7"

Steps to reproduce

git clone https://github.com/kvinwang/wasmer-stack-overlap-poc.git
cd wasmer-stack-overlap-poc/guest
cargo build --release --target wasm32-unknown-unknown
cd ..
cargo run # again and again until the issue appears.
@kvinwang kvinwang added the bug Something isn't working label Apr 23, 2022
@heyjdp
Copy link

heyjdp commented Apr 27, 2022

Thanks for the bug report. It seems that the instance is called from multiple threads. The solution is to create a separate instance per thread. This API is being improved, it will be released as part of Wasmer 3.x

@syrusakbary syrusakbary added this to the v3.0 milestone Apr 27, 2022
@heyjdp heyjdp modified the milestones: v3.0, v3.x Apr 27, 2022
@heyjdp heyjdp added the priority-medium Medium priority issue label Apr 27, 2022
@bors bors bot closed this as completed in #2892 Jul 19, 2022
@bors bors bot closed this as completed in 06474c1 Jul 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority-medium Medium priority issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants