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

Memory alignment errors with the stack_restore and stack_checkpoint syscalls #4284

Open
Michael-F-Bryan opened this issue Oct 30, 2023 · 1 comment
Assignees
Labels
bug Something isn't working 📦 lib-wasi About wasmer-wasi 🕵️ needs investigation The issue/PR needs further investigation priority-high High priority issue
Milestone

Comments

@Michael-F-Bryan
Copy link
Contributor

Michael-F-Bryan commented Oct 30, 2023

Describe the bug

Programs trigger memory alignment errors when working with the stack_restore() and stack_checkpoint() syscalls.

$ wasmer -vV
wasmer 4.2.2 (b75bcc4 2023-10-04)
binary: wasmer-cli
commit-hash: b75bcc4d7510298d6e8699e126f9fb4c453b1db5
commit-date: 2023-10-04
host: aarch64-apple-darwin
compiler: singlepass,cranelift

$ rustc -vV
rustc 1.70.0 (90c541806 2023-05-31)
binary: rustc
commit-hash: 90c541806f23a127002de5b4038be731ba1458ca
commit-date: 2023-05-31
host: aarch64-apple-darwin
release: 1.70.0
LLVM version: 16.0.2

Steps to reproduce

The easiest way to reproduce this is using the sharrattj/dash package.

$ wasmer run sharrattj/dash --env MYENV1=VAL1 -- -s stdin
# echo hello $MYENV1 > /dev/tty; exit
$ echo $?
78

(note $ is the host machine and # is from the Dash shell)

If you run wasmer run with $RUST_LOG set to wasmer_wasix::syscalls=debug, this log message jumps out as suspicious:

2023-10-30T05:58:57.829423Z  WARN ThreadId(01) wasmer_wasix::syscalls::wasix::stack_restore: failed to read stack snapshot - memory access out of bounds

Here is the code emitting that log message and returning an error from the syscall:

let snapshot = match snapshot_ptr.read(&memory) {
Ok(a) => {
trace!("with_ret={}, hash={}, user={}", val, a.hash, a.user);
a
}
Err(err) => {
warn!("failed to read stack snapshot - {}", err);
return Err(WasiError::Exit(mem_error_to_wasi(err).into()));
}
};

Additional context

As part of #4192 we've switched the WasiRunner over to using wasmer_wasix::bin_factory::exec::spawn_exec() for running WASIX programs instead of doing WasiEnvBuilder::run(). After this change, several snapshot tests and the (previously ignored) os::console::tests::test_console_dash_tty_with_args_and_env test have started failing, saying the process completed with an exit code of 78 (memory violation) rather than the expected value.

I've been able to reproduce this error on Wasmer 4.2.2, which means it isn't related to the changes in WasiRunner.

For context, see the conversations on Slack about exit codes and a previous conversation about unaligned pointers being passed to stack_checkpoint.

I've also only seen issues with the sharrattj/dash package, so this may be a bug in the package rather than our WASIX implementation.

Michael-F-Bryan pushed a commit that referenced this issue Oct 30, 2023
@Michael-F-Bryan Michael-F-Bryan added bug Something isn't working 🕵️ needs investigation The issue/PR needs further investigation 📦 lib-wasi About wasmer-wasi priority-high High priority issue labels Oct 31, 2023
@Michael-F-Bryan Michael-F-Bryan added this to the v4.3 milestone Oct 31, 2023
@nikneym
Copy link

nikneym commented Dec 17, 2023

Hi, I get the following error when I try to use stack_checkpoint, program exits with 79. I feel like this might be relevant to this issue.

$ wasmer run --enable-all --net --enable-async-threads --http-client --verbose zig-out/bin/wasix-test.wasm
2023-12-16T17:03:30.517913Z  INFO ThreadId(01) execute_wasm: wasmer_cli::commands::run: enter
2023-12-16T17:03:30.517945Z  INFO ThreadId(01) execute_wasm:execute_wasi_module: wasmer_cli::commands::run: enter
2023-12-16T17:03:30.518415Z  WARN ThreadId(17) wasmer_wasix::syscalls: unable to get the memory stack - failed to save stack: not exported __stack_pointer global
2023-12-16T17:03:30.518533Z  INFO ThreadId(01) execute_wasm:execute_wasi_module: wasmer_cli::commands::run: close time.busy=585µs time.idle=2.33µs
2023-12-16T17:03:30.518548Z  INFO ThreadId(01) execute_wasm: wasmer_cli::commands::run: close time.busy=640µs time.idle=4.56µs

This is the code I've compiled to WASM (API is identical to wasix-libc but it's on Zig):

pub fn coro(snapshot: *wx.stack_snapshot_t, ptr: *wx.longsize_t) void {
    const res = wx.stack_checkpoint(snapshot, ptr);
}

pub fn main() !void {
    var snapshot: wx.stack_snapshot_t = undefined;
    var ptr: wx.longsize_t = undefined;

    coro(&snapshot, &ptr);
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working 📦 lib-wasi About wasmer-wasi 🕵️ needs investigation The issue/PR needs further investigation priority-high High priority issue
Projects
None yet
Development

No branches or pull requests

3 participants