Enable the new preview1 implementation by default (bytecodealliance#7365

) Currently Wasmtime has two implementations of the `wasi_snapshot_preview1` set of APIs. The now-historical implementation lives in the `wasi-common` crate and the more recent implementation lives in the `wasmtime-wasi` crate. The main difference is that the `wasmtime-wasi` implementation is based on the implementation of preview2, meaning that the preview1 implementation is a shim in that direction. Additionally currently the preview2 implementation of preview1 is accessible via the `-Spreview2` flag on the CLI. This commit updates the interpretation of the `-Spreview2` flag and the defaults around which implementation to choose. By default the preview1-built-on-preview2 implementation (the new `wasmtime-wasi` implementation) is selected now. This means that the `wasi-common` implementation is disabled by default. There are still two use cases to keep running the `wasi-common` implementation, however: * When running modules that import from `wasi_unstable`, the "snapshot" before `wasi_snapshot_preview1`, currently `wasi-common` is required. The shims to implement `wasi_unstable` have not yet been implemented in the `wasmtime-wasi` crate. * When running with WASI threads (`-Sthreads`) the preview2 implementation does not work. This is because the preview2 implementation expects mutable access to the table which is not granted when threads are enabled. Tests using `wasi_unstable` now pass `-Spreview2=n` to explicitly request the old `wasi-common` implementation. Additionally the `wasi-common` implementation is still selected by default when `-Sthreads` is passed (enabling the WASI threads proposal).
yuyang-ok · Oct 27, 2023 · 5ea563f · 5ea563f
1 parent 2340dff
commit 5ea563f
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 21 deletions.
diff --git a/crates/wasi/src/preview2/preview1.rs b/crates/wasi/src/preview2/preview1.rs
@@ -9,7 +9,7 @@ use crate::preview2::bindings::{
     io::{poll, streams},
 };
 use crate::preview2::{FsError, IsATTY, StreamError, StreamResult, TableError, WasiView};
-use anyhow::{anyhow, bail, Context};
+use anyhow::{bail, Context};
 use std::borrow::Borrow;
 use std::collections::{BTreeMap, HashSet};
 use std::mem::{self, size_of, size_of_val};
@@ -1131,6 +1131,9 @@ impl<
             .map_err(types::Error::trap)?;
         let mut fs_flags = types::Fdflags::empty();
         let mut fs_rights_base = types::Rights::all();
+        if let types::Filetype::Directory = fs_filetype {
+            fs_rights_base &= !types::Rights::FD_SEEK;
+        }
         if !flags.contains(filesystem::DescriptorFlags::READ) {
             fs_rights_base &= !types::Rights::FD_READ;
         }
@@ -2258,14 +2261,11 @@ impl<
 
     #[instrument(skip(self))]
     fn proc_exit(&mut self, status: types::Exitcode) -> anyhow::Error {
-        let status = match status {
-            0 => Ok(()),
-            _ => Err(()),
-        };
-        match self.exit(status) {
-            Err(e) => e,
-            Ok(()) => anyhow!("`exit` did not return an error"),
+        // Check that the status is within WASI's range.
+        if status >= 126 {
+            return anyhow::Error::msg("exit with invalid exit status outside of [0..126)");
         }
+        crate::preview2::I32Exit(status as i32).into()
     }
 
     #[instrument(skip(self))]

diff --git a/src/commands/run.rs b/src/commands/run.rs
@@ -584,14 +584,22 @@ impl RunCommand {
         if self.run.common.wasi.common != Some(false) {
             match linker {
                 CliLinker::Core(linker) => {
-                    if self.run.common.wasi.preview2 == Some(true) {
-                        preview2::preview1::add_to_linker_sync(linker)?;
-                        self.set_preview2_ctx(store)?;
-                    } else {
-                        wasmtime_wasi::add_to_linker(linker, |host| {
-                            host.preview1_ctx.as_mut().unwrap()
-                        })?;
-                        self.set_preview1_ctx(store)?;
+                    match (self.run.common.wasi.preview2, self.run.common.wasi.threads) {
+                        // If preview2 is explicitly disabled, or if threads
+                        // are enabled, then use the historical preview1
+                        // implementation.
+                        (Some(false), _) | (None, Some(true)) => {
+                            wasmtime_wasi::add_to_linker(linker, |host| {
+                                host.preview1_ctx.as_mut().unwrap()
+                            })?;
+                            self.set_preview1_ctx(store)?;
+                        }
+                        // If preview2 was explicitly requested, always use it.
+                        // Otherwise use it so long as threads are disabled.
+                        (Some(true), _) | (None, Some(false) | None) => {
+                            preview2::preview1::add_to_linker_sync(linker)?;
+                            self.set_preview2_ctx(store)?;
+                        }
                     }
                 }
                 #[cfg(feature = "component-model")]

diff --git a/tests/all/cli_tests.rs b/tests/all/cli_tests.rs
@@ -184,7 +184,7 @@ fn run_wasmtime_unreachable_wat() -> Result<()> {
 #[test]
 fn hello_wasi_snapshot0() -> Result<()> {
     let wasm = build_wasm("tests/all/cli_tests/hello_wasi_snapshot0.wat")?;
-    let stdout = run_wasmtime(&["-Ccache=n", wasm.path().to_str().unwrap()])?;
+    let stdout = run_wasmtime(&["-Ccache=n", "-Spreview2=n", wasm.path().to_str().unwrap()])?;
     assert_eq!(stdout, "Hello, world!\n");
     Ok(())
 }
@@ -248,7 +248,10 @@ fn timeout_in_invoke() -> Result<()> {
 #[test]
 fn exit2_wasi_snapshot0() -> Result<()> {
     let wasm = build_wasm("tests/all/cli_tests/exit2_wasi_snapshot0.wat")?;
-    let output = run_wasmtime_for_output(&["-Ccache=n", wasm.path().to_str().unwrap()], None)?;
+    let output = run_wasmtime_for_output(
+        &["-Ccache=n", "-Spreview2=n", wasm.path().to_str().unwrap()],
+        None,
+    )?;
     assert_eq!(output.status.code().unwrap(), 2);
     Ok(())
 }
@@ -266,7 +269,10 @@ fn exit2_wasi_snapshot1() -> Result<()> {
 #[test]
 fn exit125_wasi_snapshot0() -> Result<()> {
     let wasm = build_wasm("tests/all/cli_tests/exit125_wasi_snapshot0.wat")?;
-    let output = run_wasmtime_for_output(&["-Ccache=n", wasm.path().to_str().unwrap()], None)?;
+    let output = run_wasmtime_for_output(
+        &["-Ccache=n", "-Spreview2=n", wasm.path().to_str().unwrap()],
+        None,
+    )?;
     if cfg!(windows) {
         assert_eq!(output.status.code().unwrap(), 1);
     } else {
@@ -292,7 +298,10 @@ fn exit125_wasi_snapshot1() -> Result<()> {
 #[test]
 fn exit126_wasi_snapshot0() -> Result<()> {
     let wasm = build_wasm("tests/all/cli_tests/exit126_wasi_snapshot0.wat")?;
-    let output = run_wasmtime_for_output(&["-Ccache=n", wasm.path().to_str().unwrap()], None)?;
+    let output = run_wasmtime_for_output(
+        &["-Ccache=n", "-Spreview2=n", wasm.path().to_str().unwrap()],
+        None,
+    )?;
     assert_eq!(output.status.code().unwrap(), 1);
     assert!(output.stdout.is_empty());
     assert!(String::from_utf8_lossy(&output.stderr).contains("invalid exit status"));
@@ -439,7 +448,7 @@ fn hello_wasi_snapshot0_from_stdin() -> Result<()> {
     let wasm = build_wasm("tests/all/cli_tests/hello_wasi_snapshot0.wat")?;
     let stdout = {
         let path = wasm.path();
-        let args: &[&str] = &["-Ccache=n", "-"];
+        let args: &[&str] = &["-Ccache=n", "-Spreview2=n", "-"];
         let output = run_wasmtime_for_output(args, Some(path))?;
         if !output.status.success() {
             bail!(