From eaa1016305973fa905d850956ab2eec2691f6149 Mon Sep 17 00:00:00 2001 From: m0g3r <87276771+m0g3r@users.noreply.github.com> Date: Thu, 13 Aug 2026 16:41:15 +0000 Subject: [PATCH 1/8] Fix panic compiling an empty component with debug info (#14130) generate_simulated_dwarf unwrapped the first core-module translation to name its compilation unit, but a component with no core modules has no translations, so `wasmtime compile -D debug-info=y` panicked on a valid `(component)` input. Return early instead: with no translations there are no functions to describe. --- .../cranelift/src/debug/transform/simulate.rs | 9 ++++++++- tests/all/cli_tests.rs | 18 ++++++++++++++++++ tests/all/cli_tests/empty_component.wat | 1 + 3 files changed, 27 insertions(+), 1 deletion(-) create mode 100644 tests/all/cli_tests/empty_component.wat diff --git a/crates/cranelift/src/debug/transform/simulate.rs b/crates/cranelift/src/debug/transform/simulate.rs index dd39c2f54398..2504ea32653a 100644 --- a/crates/cranelift/src/debug/transform/simulate.rs +++ b/crates/cranelift/src/debug/transform/simulate.rs @@ -296,8 +296,15 @@ pub fn generate_simulated_dwarf( out_strings: &mut write::StringTable, isa: &dyn TargetIsa, ) -> Result<(), Error> { + // A component without any core modules has no functions to describe, and + // the compilation unit below names itself after the first translation's + // wasm file. There is nothing to simulate, so leave the DWARF empty. + let Some((_, first_translation)) = compilation.translations.iter().next() else { + return Ok(()); + }; + let (wasm_file, path) = { - let di = &compilation.translations.iter().next().unwrap().1.debuginfo; + let di = &first_translation.debuginfo; let path = di .wasm_file .path diff --git a/tests/all/cli_tests.rs b/tests/all/cli_tests.rs index fa3fee6c7429..bbdfba73d507 100644 --- a/tests/all/cli_tests.rs +++ b/tests/all/cli_tests.rs @@ -3651,3 +3651,21 @@ fn non_utf8_raises_error() -> Result<()> { } Ok(()) } + +#[test] +fn compile_empty_component_with_debug_info() -> Result<()> { + // A component with no core modules reached simulated-DWARF generation with + // nothing to describe, which used to panic instead of compiling. + let td = TempDir::new()?; + let cwasm = td.path().join("empty-component.cwasm"); + let stdout = run_wasmtime(&[ + "compile", + "-D", + "debug-info=y", + "tests/all/cli_tests/empty_component.wat", + "-o", + cwasm.to_str().unwrap(), + ])?; + assert_eq!(stdout, ""); + Ok(()) +} diff --git a/tests/all/cli_tests/empty_component.wat b/tests/all/cli_tests/empty_component.wat new file mode 100644 index 000000000000..e5627d1d0fc7 --- /dev/null +++ b/tests/all/cli_tests/empty_component.wat @@ -0,0 +1 @@ +(component) From da062a8cb02484d746e1ecbd8a3de64c5a386947 Mon Sep 17 00:00:00 2001 From: grandpig Date: Wed, 12 Aug 2026 16:02:27 +0000 Subject: [PATCH 2/8] Run linker callback finalizers on invalid names (#14126) --- crates/c-api/src/async.rs | 2 +- crates/c-api/src/linker.rs | 4 ++-- crates/c-api/tests/async.cc | 27 +++++++++++++++++++++++ crates/c-api/tests/linker.cc | 42 ++++++++++++++++++++++++++++++++++++ 4 files changed, 72 insertions(+), 3 deletions(-) diff --git a/crates/c-api/src/async.rs b/crates/c-api/src/async.rs index a3d079001949..6e7b68bbd7ee 100644 --- a/crates/c-api/src/async.rs +++ b/crates/c-api/src/async.rs @@ -288,9 +288,9 @@ pub unsafe extern "C" fn wasmtime_linker_define_async_func( finalizer: Option, ) -> Option> { let ty = ty.ty().ty(linker.linker.engine()); + let cb = c_async_callback_to_rust_fn(callback, data, finalizer); let module = to_str!(module, module_len); let name = to_str!(name, name_len); - let cb = c_async_callback_to_rust_fn(callback, data, finalizer); handle_result( linker.linker.func_new_async(module, name, ty, cb), diff --git a/crates/c-api/src/linker.rs b/crates/c-api/src/linker.rs index f7b1da6308bc..b21422a8bc56 100644 --- a/crates/c-api/src/linker.rs +++ b/crates/c-api/src/linker.rs @@ -79,9 +79,9 @@ pub unsafe extern "C" fn wasmtime_linker_define_func( finalizer: Option, ) -> Option> { let ty = ty.ty().ty(linker.linker.engine()); + let cb = crate::func::c_callback_to_rust_fn(callback, data, finalizer); let module = to_str!(module, module_len); let name = to_str!(name, name_len); - let cb = crate::func::c_callback_to_rust_fn(callback, data, finalizer); handle_result(linker.linker.func_new(module, name, ty, cb), |_linker| ()) } @@ -98,9 +98,9 @@ pub unsafe extern "C" fn wasmtime_linker_define_func_unchecked( finalizer: Option, ) -> Option> { let ty = ty.ty().ty(linker.linker.engine()); + let cb = crate::func::c_unchecked_callback_to_rust_fn(callback, data, finalizer); let module = to_str!(module, module_len); let name = to_str!(name, name_len); - let cb = crate::func::c_unchecked_callback_to_rust_fn(callback, data, finalizer); handle_result( linker.linker.func_new_unchecked(module, name, ty, cb), |_linker| (), diff --git a/crates/c-api/tests/async.cc b/crates/c-api/tests/async.cc index 7f396187835e..7785ed1664ac 100644 --- a/crates/c-api/tests/async.cc +++ b/crates/c-api/tests/async.cc @@ -6,6 +6,33 @@ using namespace wasmtime; +namespace { + +void async_callback(void *, wasmtime_caller_t *, const wasmtime_val_t *, size_t, + wasmtime_val_t *, size_t, wasm_trap_t **, + wasmtime_async_continuation_t *) {} + +void finalize(void *data) { *static_cast(data) = true; } + +} // namespace + +TEST(async, finalizes_callback_when_name_parsing_fails) { + Engine engine; + Linker linker(engine); + auto *ty = wasm_functype_new_0_0(); + const char invalid_utf8[] = {static_cast(0xff)}; + bool finalized = false; + + auto *error = wasmtime_linker_define_async_func( + linker.capi(), invalid_utf8, sizeof(invalid_utf8), "name", 4, ty, + async_callback, &finalized, finalize); + + ASSERT_NE(error, nullptr); + wasmtime_error_delete(error); + EXPECT_TRUE(finalized); + wasm_functype_delete(ty); +} + TEST(async, call_func_async) { Engine engine; Store store(engine); diff --git a/crates/c-api/tests/linker.cc b/crates/c-api/tests/linker.cc index 3f99af4f4d85..730c6b3a3ba7 100644 --- a/crates/c-api/tests/linker.cc +++ b/crates/c-api/tests/linker.cc @@ -1,9 +1,26 @@ #include +#include #include #include using namespace wasmtime; +namespace { + +wasm_trap_t *callback(void *, wasmtime_caller_t *, const wasmtime_val_t *, + size_t, wasmtime_val_t *, size_t) { + return nullptr; +} + +wasm_trap_t *unchecked_callback(void *, wasmtime_caller_t *, + wasmtime_val_raw_t *, size_t) { + return nullptr; +} + +void finalize(void *data) { *static_cast(data) = true; } + +} // namespace + TEST(Linker, Smoke) { Engine engine; Linker linker(engine); @@ -75,6 +92,31 @@ TEST(Linker, CallableCopy) { linker.func_new("a", "f", FuncType({}, {}), cf).unwrap(); } +TEST(Linker, FinalizesCallbacksWhenNameParsingFails) { + Engine engine; + Linker linker(engine); + auto *ty = wasm_functype_new_0_0(); + const char invalid_utf8[] = {static_cast(0xff)}; + + bool finalized = false; + auto *error = wasmtime_linker_define_func(linker.capi(), invalid_utf8, + sizeof(invalid_utf8), "name", 4, ty, + callback, &finalized, finalize); + ASSERT_NE(error, nullptr); + wasmtime_error_delete(error); + EXPECT_TRUE(finalized); + + finalized = false; + error = wasmtime_linker_define_func_unchecked( + linker.capi(), "module", 6, invalid_utf8, sizeof(invalid_utf8), ty, + unchecked_callback, &finalized, finalize); + ASSERT_NE(error, nullptr); + wasmtime_error_delete(error); + EXPECT_TRUE(finalized); + + wasm_functype_delete(ty); +} + TEST(Linker, DefineUnknownImportsAsTraps) { Engine engine; Linker linker(engine); From 44df43fa1fa52bfe21226213fb8a2a104dcb2f5e Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Thu, 13 Aug 2026 15:09:47 +0000 Subject: [PATCH 3/8] Shuffle more finalizers in the C API (#14133) * Shuffle more finalizers in the C API This implements a similar refactoring to #14126 but for the component linker as well. * Clang-format --- crates/c-api/src/component/linker.rs | 12 +++--- crates/c-api/tests/component/linker.cc | 57 ++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 6 deletions(-) diff --git a/crates/c-api/src/component/linker.rs b/crates/c-api/src/component/linker.rs index 3823b8b69324..e4714f769cf8 100644 --- a/crates/c-api/src/component/linker.rs +++ b/crates/c-api/src/component/linker.rs @@ -115,13 +115,13 @@ pub unsafe extern "C" fn wasmtime_component_linker_instance_add_func( data: *mut c_void, finalizer: Option, ) -> Option> { + let foreign = crate::ForeignData { data, finalizer }; + let name = unsafe { std::slice::from_raw_parts(name, name_len) }; let Ok(name) = std::str::from_utf8(name) else { return crate::bad_utf8(); }; - let foreign = crate::ForeignData { data, finalizer }; - let result = linker_instance .linker_instance .func_new(&name, move |ctx, ty, args, rets| { @@ -181,13 +181,13 @@ pub unsafe extern "C" fn wasmtime_component_linker_instance_add_func_async( data: *mut c_void, finalizer: Option, ) -> Option> { + let foreign = crate::ForeignData { data, finalizer }; + let name = unsafe { std::slice::from_raw_parts(name, name_len) }; let Ok(name) = std::str::from_utf8(name) else { return crate::bad_utf8(); }; - let foreign = crate::ForeignData { data, finalizer }; - let result = linker_instance .linker_instance @@ -322,13 +322,13 @@ pub unsafe extern "C" fn wasmtime_component_linker_instance_add_resource( data: *mut c_void, finalizer: Option, ) -> Option> { + let foreign = crate::ForeignData { data, finalizer }; + let name = unsafe { std::slice::from_raw_parts(name, name_len) }; let Ok(name) = std::str::from_utf8(name) else { return crate::bad_utf8(); }; - let foreign = crate::ForeignData { data, finalizer }; - let result = linker_instance .linker_instance .resource(name, ty.ty, move |ctx, rep| { diff --git a/crates/c-api/tests/component/linker.cc b/crates/c-api/tests/component/linker.cc index 88158a3c4763..c928aa799951 100644 --- a/crates/c-api/tests/component/linker.cc +++ b/crates/c-api/tests/component/linker.cc @@ -4,6 +4,63 @@ using namespace wasmtime::component; +static wasmtime_error_t *func_callback(void *, wasmtime_context_t *, + const wasmtime_component_func_type_t *, + wasmtime_component_val_t *, size_t, + wasmtime_component_val_t *, size_t) { + return nullptr; +} + +static void async_func_callback(void *, wasmtime_context_t *, + const wasmtime_component_func_type_t *, + wasmtime_component_val_t *, size_t, + wasmtime_component_val_t *, size_t, + wasmtime_error_t **, + wasmtime_async_continuation_t *) {} + +static wasmtime_error_t *resource_destructor(void *, wasmtime_context_t *, + uint32_t) { + return nullptr; +} + +static void finalize(void *data) { *static_cast(data) = true; } + +TEST(Linker, finalizes_callbacks_when_name_parsing_fails) { + wasmtime::Engine engine; + auto *raw = wasmtime_component_linker_new(engine.capi()); + auto *root = wasmtime_component_linker_root(raw); + const char invalid_utf8[] = {static_cast(0xff)}; + + bool finalized = false; + auto *error = wasmtime_component_linker_instance_add_func( + root, invalid_utf8, sizeof(invalid_utf8), func_callback, &finalized, + finalize); + ASSERT_NE(error, nullptr); + wasmtime_error_delete(error); + EXPECT_TRUE(finalized); + + finalized = false; + error = wasmtime_component_linker_instance_add_func_async( + root, invalid_utf8, sizeof(invalid_utf8), async_func_callback, &finalized, + finalize); + ASSERT_NE(error, nullptr); + wasmtime_error_delete(error); + EXPECT_TRUE(finalized); + + finalized = false; + auto *ty = wasmtime_component_resource_type_new_host(0); + error = wasmtime_component_linker_instance_add_resource( + root, invalid_utf8, sizeof(invalid_utf8), ty, resource_destructor, + &finalized, finalize); + ASSERT_NE(error, nullptr); + wasmtime_error_delete(error); + EXPECT_TRUE(finalized); + wasmtime_component_resource_type_delete(ty); + + wasmtime_component_linker_instance_delete(root); + wasmtime_component_linker_delete(raw); +} + TEST(Linker, allow_shadowing) { wasmtime::Engine engine; Linker linker(engine); From 6333aa6ee6e4e1c1a2060ce38da6ca46650f482d Mon Sep 17 00:00:00 2001 From: Eduardo de Moura Rodrigues <16357187+eduardomourar@users.noreply.github.com> Date: Tue, 11 Aug 2026 13:42:42 +0000 Subject: [PATCH 4/8] fix(wasmtime-cli): generic eio error thrown for wasip2 (#14107) --- .../src/bin/p2_cli_stdin_eisdir.rs | 36 ++++++++ .../src/bin/p2_cli_stdout_epipe.rs | 29 +++++++ crates/wasi/src/cli.rs | 85 ++++++++++++++++++- crates/wasi/src/cli/stdout.rs | 6 +- crates/wasi/src/cli/worker_thread_stdin.rs | 4 +- tests/all/cli_tests.rs | 65 ++++++++++++++ 6 files changed, 219 insertions(+), 6 deletions(-) create mode 100644 crates/test-programs/src/bin/p2_cli_stdin_eisdir.rs create mode 100644 crates/test-programs/src/bin/p2_cli_stdout_epipe.rs diff --git a/crates/test-programs/src/bin/p2_cli_stdin_eisdir.rs b/crates/test-programs/src/bin/p2_cli_stdin_eisdir.rs new file mode 100644 index 000000000000..0538db3f7c4d --- /dev/null +++ b/crates/test-programs/src/bin/p2_cli_stdin_eisdir.rs @@ -0,0 +1,36 @@ +//! Guest program that reads from stdin expecting an `IsADirectory` error. +//! +//! When stdin is redirected from a directory (e.g. `< /some/dir`), the host +//! should surface the error as `StreamError::LastOperationFailed` with the +//! original `io::Error` preserved, recoverable via `filesystem-error-code`. + +use test_programs::wasi::cli::stdin; +use test_programs::wasi::filesystem::types::{self as filesystem, ErrorCode}; +use test_programs::wasi::io::streams::StreamError; + +fn main() { + let stdin = stdin::get_stdin(); + + // Keep polling until data or an error is available. + loop { + stdin.subscribe().block(); + match stdin.read(1024) { + Ok(bytes) if bytes.is_empty() => continue, + Ok(_) => panic!("expected an error reading from a directory, got data"), + Err(StreamError::Closed) => { + panic!("expected LastOperationFailed(IsDirectory), got Closed") + } + Err(StreamError::LastOperationFailed(err)) => { + // Use filesystem-error-code to recover the specific error. + let code = filesystem::filesystem_error_code(&err); + assert_eq!( + code, + Some(ErrorCode::IsDirectory), + "expected IsDirectory, got {code:?}" + ); + eprintln!("got expected ErrorCode::IsDirectory"); + return; + } + } + } +} diff --git a/crates/test-programs/src/bin/p2_cli_stdout_epipe.rs b/crates/test-programs/src/bin/p2_cli_stdout_epipe.rs new file mode 100644 index 000000000000..3e52fda4855d --- /dev/null +++ b/crates/test-programs/src/bin/p2_cli_stdout_epipe.rs @@ -0,0 +1,29 @@ +//! Guest program that writes to stdout until the pipe is closed, then verifies +//! it gets `StreamError::Closed` (which maps to EPIPE) rather than a trap or +//! generic EIO. + +use test_programs::wasi::cli::stdout; +use test_programs::wasi::io::streams::StreamError; + +fn main() { + let stdout = stdout::get_stdout(); + let chunk = vec![b'x'; 4096]; + + loop { + match stdout.blocking_write_and_flush(&chunk) { + Ok(()) => continue, + Err(StreamError::Closed) => { + // This is the expected outcome: the pipe was closed by the + // reader, and wasmtime correctly reports it as Closed (EPIPE). + eprintln!("got expected StreamError::Closed"); + return; + } + Err(StreamError::LastOperationFailed(err)) => { + panic!( + "unexpected LastOperationFailed (should have been Closed): {}", + err.to_debug_string() + ); + } + } + } +} diff --git a/crates/wasi/src/cli.rs b/crates/wasi/src/cli.rs index 090a656c4c76..eff5089d1cc6 100644 --- a/crates/wasi/src/cli.rs +++ b/crates/wasi/src/cli.rs @@ -3,7 +3,7 @@ use std::pin::Pin; use std::sync::Arc; use tokio::io::{AsyncRead, AsyncWrite, empty}; use wasmtime::component::{HasData, ResourceTable}; -use wasmtime_wasi_io::streams::{InputStream, OutputStream}; +use wasmtime_wasi_io::streams::{InputStream, OutputStream, StreamError}; mod empty; mod file; @@ -15,6 +15,27 @@ mod worker_thread_stdin; pub use self::file::{InputFile, OutputFile}; pub use self::locked_async::{AsyncStdinStream, AsyncStdoutStream}; +/// Convert a host `io::Error` into a `StreamError`, matching the error-code +/// recovery that wasip1 performs via `filesystem::ErrorCode::from`. +/// +/// * `BrokenPipe` is mapped to `StreamError::Closed` so that downstream +/// consumers (e.g. wasi-libc) can recover `EPIPE` rather than falling back +/// to a generic `EIO`. +/// +/// * All other errors (including `IsADirectory`, permission errors, etc.) are +/// preserved as `LastOperationFailed` with the original `std::io::Error` +/// intact. This allows guests to recover the specific error code via the +/// `wasi:filesystem/types#filesystem-error-code` function, which downcasts +/// the error back to `std::io::Error` and maps it through +/// `ErrorCode::from`. +fn stream_error_from(e: std::io::Error) -> StreamError { + if e.kind() == std::io::ErrorKind::BrokenPipe { + StreamError::Closed + } else { + StreamError::LastOperationFailed(e.into()) + } +} + // Convenience reexport for stdio types so tokio doesn't have to be imported // itself. #[doc(no_inline)] @@ -366,4 +387,66 @@ mod test { s.write_ready().await?; Ok(()) } + + // Verify that the stdio OutputStream implementation reports a usable + // write permit and can successfully write + flush (exercises the full + // trait impl including the error conversion path). + #[test] + fn stdio_output_stream_write_flush() { + let mut stream: Box = + StdoutStream::p2_stream(&std::io::stderr()); + + let permit = stream.check_write().expect("check_write"); + assert!(permit > 0, "permit should be nonzero"); + + // Writing empty bytes must succeed. + stream + .write(Bytes::new()) + .expect("writing empty bytes should succeed"); + + // Flushing must succeed. + stream.flush().expect("flush should succeed"); + } + + #[test] + fn stream_error_from_broken_pipe_maps_to_closed() { + use std::io; + use wasmtime_wasi_io::streams::StreamError; + + let err = super::stream_error_from(io::Error::from(io::ErrorKind::BrokenPipe)); + assert!(matches!(err, StreamError::Closed)); + } + + #[test] + fn stream_error_from_preserves_io_error() { + use std::io; + use wasmtime_wasi_io::streams::StreamError; + + let err = super::stream_error_from(io::Error::from(io::ErrorKind::IsADirectory)); + match err { + StreamError::LastOperationFailed(e) => { + let io_err = e.downcast::().expect("should downcast"); + assert_eq!(io_err.kind(), io::ErrorKind::IsADirectory); + } + other => panic!("expected LastOperationFailed, got: {other:?}"), + } + } + + #[cfg(unix)] + #[test] + fn stream_error_from_raw_os_eisdir() { + use rustix::io::Errno; + use std::io; + use wasmtime_wasi_io::streams::StreamError; + + let err = + super::stream_error_from(io::Error::from_raw_os_error(Errno::ISDIR.raw_os_error())); + match err { + StreamError::LastOperationFailed(e) => { + let io_err = e.downcast::().expect("should downcast"); + assert_eq!(io_err.raw_os_error(), Some(Errno::ISDIR.raw_os_error())); + } + other => panic!("expected LastOperationFailed, got: {other:?}"), + } + } } diff --git a/crates/wasi/src/cli/stdout.rs b/crates/wasi/src/cli/stdout.rs index 9d0a213e528c..0ffe2ee976b3 100644 --- a/crates/wasi/src/cli/stdout.rs +++ b/crates/wasi/src/cli/stdout.rs @@ -1,4 +1,4 @@ -use crate::cli::{IsTerminal, StdoutStream}; +use crate::cli::{IsTerminal, StdoutStream, stream_error_from}; use crate::p2; use bytes::Bytes; use std::io::{self, Write}; @@ -78,7 +78,7 @@ impl OutputStream for StdioOutputStream { StdioOutputStream::Stdout => std::io::stdout().write_all(&bytes), StdioOutputStream::Stderr => std::io::stderr().write_all(&bytes), } - .map_err(|e| p2::StreamError::LastOperationFailed(wasmtime::format_err!(e))) + .map_err(|e| stream_error_from(e)) } fn flush(&mut self) -> p2::StreamResult<()> { @@ -86,7 +86,7 @@ impl OutputStream for StdioOutputStream { StdioOutputStream::Stdout => std::io::stdout().flush(), StdioOutputStream::Stderr => std::io::stderr().flush(), } - .map_err(|e| p2::StreamError::LastOperationFailed(wasmtime::format_err!(e))) + .map_err(|e| stream_error_from(e)) } fn check_write(&mut self) -> p2::StreamResult { diff --git a/crates/wasi/src/cli/worker_thread_stdin.rs b/crates/wasi/src/cli/worker_thread_stdin.rs index 6f92190ce1dd..06ee1cb07017 100644 --- a/crates/wasi/src/cli/worker_thread_stdin.rs +++ b/crates/wasi/src/cli/worker_thread_stdin.rs @@ -23,7 +23,7 @@ //! This module is one that's likely to change over time though as new systems //! are encountered along with preexisting bugs. -use crate::cli::{IsTerminal, StdinStream}; +use crate::cli::{IsTerminal, StdinStream, stream_error_from}; use bytes::{Bytes, BytesMut}; use std::io::Read; use std::mem; @@ -176,7 +176,7 @@ impl InputStream for WasiStdin { } StdinState::Error(e) => { *locked = StdinState::Closed; - Err(StreamError::LastOperationFailed(e.into())) + Err(stream_error_from(e)) } StdinState::Closed => { *locked = StdinState::Closed; diff --git a/tests/all/cli_tests.rs b/tests/all/cli_tests.rs index bbdfba73d507..9c2b8e9b208e 100644 --- a/tests/all/cli_tests.rs +++ b/tests/all/cli_tests.rs @@ -1205,6 +1205,71 @@ mod test_programs { Ok(()) } + #[test] + fn p2_cli_stdout_epipe() -> Result<()> { + let mut child = get_wasmtime_command()? + .args(&["run", "-Wcomponent-model", P2_CLI_STDOUT_EPIPE_COMPONENT]) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .stdin(Stdio::null()) + .spawn()?; + + // Read a small amount from stdout then drop it to close the pipe, + // which should cause the guest to receive StreamError::Closed (EPIPE). + let mut stdout = child.stdout.take().unwrap(); + let mut buf = [0u8; 64]; + let _ = stdout.read(&mut buf)?; + drop(stdout); + + let output = child.wait_with_output()?; + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + output.status.success(), + "guest should exit successfully after receiving Closed, stderr: {stderr}" + ); + assert!( + stderr.contains("got expected StreamError::Closed"), + "guest should have reported StreamError::Closed, stderr: {stderr}" + ); + Ok(()) + } + + #[cfg(unix)] + #[test] + fn p2_cli_stdin_eisdir() -> Result<()> { + let dir = tempfile::tempdir()?; + // Open the directory and transfer the fd to Stdio for use as stdin. + let dir_file = std::fs::File::open(dir.path())?; + let stdin_stdio: Stdio = dir_file.into(); + + let child = get_wasmtime_command()? + .args(&["run", "-Wcomponent-model", P2_CLI_STDIN_EISDIR_COMPONENT]) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .stdin(stdin_stdio) + .spawn()?; + + let output = child.wait_with_output()?; + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + output.status.success(), + "guest should exit successfully after receiving IsDirectory, stderr: {stderr}" + ); + assert!( + stderr.contains("got expected ErrorCode::IsDirectory"), + "guest should have reported ErrorCode::IsDirectory, stderr: {stderr}" + ); + Ok(()) + } + + // EISDIR is a Unix-specific concept; on Windows opening a directory for + // reading behaves differently, so this test only runs on Unix. + #[cfg(not(unix))] + #[test] + fn p2_cli_stdin_eisdir() -> Result<()> { + Ok(()) + } + #[test] fn p2_cli_env() -> Result<()> { run_wasmtime(&[ From 5f1c146073f8e6f4397b098a73eeda725831f7a4 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Mon, 10 Aug 2026 22:59:47 +0000 Subject: [PATCH 5/8] Cranelift: unwind last-store state after removing a dead store (#14111) Alias analysis's dead-store elimination removed the dead store's `mem_values` entry, but left the region's last-store slot naming the instruction it had just deleted. Leaving the removed-store meant that when we then reprocess the overwriting store, we keyed its lookup on a removed instruction, found nothing, and failed to notice that (for example) the overwriting store became idempotent and could also be removed. With this commit, each store now records the memory version it displaced, and eliminating a dead store rolls that version back, so a chain like v1 = load.i32 region0 v0 store region0 v2, v0 ;; dead store region0 v1, v0 ;; idempotent once the dead store is gone collapses in the single pass we actually make, rather than removing only one link in the chain and requiring that we do N passes to fully clean up a chain of N dead/idempotent stores. This code pattern the shape fused sync adapters emit around the `MAY_LEAVE` flag and the relevant disas tests each lose a store as a result. --- cranelift/codegen/src/alias_analysis.rs | 113 ++++++++++- .../alias/check-unset-reset-flag.clif | 2 +- .../dead-store-then-idempotent-store.clif | 181 ++++++++++++++++++ .../direct-adapter-calls-inlining.wat | 1 - .../direct-adapter-calls-x64.wat | 55 +++--- .../component-model/direct-adapter-calls.wat | 1 - .../component-model/sync-adapter-calls.wat | 1 - 7 files changed, 312 insertions(+), 42 deletions(-) create mode 100644 cranelift/filetests/filetests/alias/dead-store-then-idempotent-store.clif diff --git a/cranelift/codegen/src/alias_analysis.rs b/cranelift/codegen/src/alias_analysis.rs index c15769fd70f9..626455a778e8 100644 --- a/cranelift/codegen/src/alias_analysis.rs +++ b/cranelift/codegen/src/alias_analysis.rs @@ -424,6 +424,32 @@ impl LastStores { } } + /// Roll this state back to the memory version from just before `dead`, + /// which is a store being removed from the function by dead-store + /// elimination. + /// + /// `prev_last_store` must be the last-store instruction that immediately + /// preceded `dead`, as recorded when `dead` itself was processed. + /// + /// Only `dead`'s own alias-region slot is restored. A store with no alias + /// region is treated as a fence by `update`, which clears *every* region + /// slot, and we do not undo that; in that case, we leave this state + /// alone. Similarly, stores marked observed while processing `dead` stay + /// observed. + fn undo_store(&mut self, func: &Function, dead: Inst, prev_last_store: PackedOption) { + debug_assert!(func.dfg.insts[dead].opcode().can_store()); + + let Some(region) = func.dfg.insts[dead].alias_region(&func.dfg) else { + return; + }; + + // Only roll back if `dead` really is the current last store to its + // region. + if self.regions[region].expand() == Some(dead) { + self.regions[region] = prev_last_store; + } + } + /// Get the last-store instruction for the given `inst`'s alias region, if /// any. fn get_last_store(&self, func: &Function, inst: Inst) -> PackedOption { @@ -527,6 +553,26 @@ struct MemoryLoc { extending_opcode: Option, } +/// What is known to be in memory at an associated `MemoryLoc`. +#[derive(Clone, Copy, Debug)] +struct KnownValue { + /// The value held at the associated `MemoryLoc`. + value: Value, + + /// The instruction that produced `value`: either the load that read it out + /// of memory or the store that wrote it there. + /// + /// Kept around for quick dominance checks. + def_inst: Inst, + + /// When this entry was created by a store, the last-store instruction that + /// immediately preceded that store: that is, the memory version this + /// location was at just *before* `def_inst` overwrote it. + /// + /// `None` for entries created by loads. + prev_last_store: Option>, +} + /// The result of processing an instruction through alias analysis. pub enum OptResult { /// No optimization applied. @@ -576,9 +622,7 @@ pub struct AliasAnalysis<'a> { /// Known memory-value equivalences. This is the result of the /// analysis. This is a mapping from (last store, address /// expression, offset, type) to SSA `Value`. - /// - /// We keep the defining inst around for quick dominance checks. - mem_values: FxHashMap, + mem_values: FxHashMap, } impl<'a> AliasAnalysis<'a> { @@ -754,7 +798,36 @@ impl<'a> AliasAnalysis<'a> { ty, extending_opcode: get_ext_opcode(opcode), }; - self.mem_values.remove(&dead_loc); + let dead_entry = self.mem_values.remove(&dead_loc); + + // Roll our last-store state back to the memory version + // just before the dead store, so that `state` describes + // memory as if the dead store had never happened. + // + // Our callers remove the dead store from the layout and + // then reprocess this overwriting store. Without the + // rollback, that reprocessing keys its `mem_values` + // lookup on the instruction we just removed, finds + // nothing, and so fails to notice that the overwriter + // has now become an idempotent store. Chains like + // + // v1 = load.i32 region0 v0 + // store region0 v2, v0 ;; dead + // store region0 v1, v0 ;; idempotent, once the + // ;; dead store is gone + // + // would then need a whole additional pass over the + // function to collapse each link. + // + // A missing entry means we never processed the dead + // store as a store in this pass (it can come from a + // precomputed `block_input` snapshot, for a predecessor + // block we have not walked yet), so we have no previous + // version to roll back to and simply don't. + if let Some(prev) = dead_entry.and_then(|e| e.prev_last_store) { + state.undo_store(func, last_store, prev); + } + return OptResult::DeadStore { dead: last_store, overwriter: inst, @@ -769,7 +842,12 @@ impl<'a> AliasAnalysis<'a> { ty, extending_opcode: get_ext_opcode(opcode), }; - if let Some((def_inst, known_value)) = self.mem_values.get(&check_loc).cloned() { + if let Some(KnownValue { + def_inst, + value: known_value, + .. + }) = self.mem_values.get(&check_loc).cloned() + { // Check for idempotent stores, where we are // storing the exact same value back to a location // that already has that value. @@ -806,7 +884,14 @@ impl<'a> AliasAnalysis<'a> { extending_opcode: get_ext_opcode(opcode), }; trace!(" --> updating known values in memory: {mem_loc:?} = {store_data}"); - self.mem_values.insert(mem_loc, (inst, store_data)); + self.mem_values.insert( + mem_loc, + KnownValue { + def_inst: inst, + value: store_data, + prev_last_store: Some(last_store), + }, + ); OptResult::None } else if opcode.can_load() { @@ -831,8 +916,9 @@ impl<'a> AliasAnalysis<'a> { // load (stores will always dominate though if // their `last_store` survives through // meet-points to this use-site). - let aliased = if let Some((def_inst, value)) = - self.mem_values.get(&mem_loc).cloned() + let aliased = if let Some(KnownValue { + def_inst, value, .. + }) = self.mem_values.get(&mem_loc).cloned() { trace!(" see known value {value} from {def_inst}"); if self.domtree.dominates(def_inst, inst, &func.layout) { @@ -851,7 +937,16 @@ impl<'a> AliasAnalysis<'a> { // as a new equivalent value. if aliased.is_none() { trace!(" --> inserting load result {load_result} at loc {mem_loc:?}"); - self.mem_values.insert(mem_loc, (inst, load_result)); + self.mem_values.insert( + mem_loc, + KnownValue { + def_inst: inst, + value: load_result, + // A load does not advance the memory version, so + // there is no previous version to roll back to. + prev_last_store: None, + }, + ); } match aliased { diff --git a/cranelift/filetests/filetests/alias/check-unset-reset-flag.clif b/cranelift/filetests/filetests/alias/check-unset-reset-flag.clif index af926b0a0bf8..08188dd75cb8 100644 --- a/cranelift/filetests/filetests/alias/check-unset-reset-flag.clif +++ b/cranelift/filetests/filetests/alias/check-unset-reset-flag.clif @@ -21,7 +21,7 @@ block0(v0: i64, v1: i32): ; block0(v0: i64, v1: i32): ; v2 = load.i64 notrap aligned region0 v0 ; trapz v2, user42 -; store notrap aligned region0 v2, v0 ; v4 = iadd v1, v1 ; return v4 ; } + diff --git a/cranelift/filetests/filetests/alias/dead-store-then-idempotent-store.clif b/cranelift/filetests/filetests/alias/dead-store-then-idempotent-store.clif new file mode 100644 index 000000000000..02e5ecd5b8a5 --- /dev/null +++ b/cranelift/filetests/filetests/alias/dead-store-then-idempotent-store.clif @@ -0,0 +1,181 @@ +test optimize precise-output +set opt_level=speed +target x86_64 + +;; Removing a dead store must expose the *previous* memory version to the store +;; that overwrote it, so that a save/clear/restore sequence collapses entirely in +;; a single pass rather than one link per pass. +function %save_clear_restore(i64) { + region0 = 0 "flags" +block0(v0: i64): + v1 = load.i32 notrap aligned region0 v0 + v2 = iconst.i32 0 + store notrap aligned region0 v2, v0 + store notrap aligned region0 v1, v0 + return +} + +; function %save_clear_restore(i64) fast { +; region0 = 0 "flags" +; +; block0(v0: i64): +; v1 = load.i32 notrap aligned region0 v0 +; return +; } + +;; The same, but with several dead stores between the load and the restore. +function %save_clobber_many_restore(i64, i32, i32) { + region0 = 0 "flags" +block0(v0: i64, v1: i32, v2: i32): + v3 = load.i32 notrap aligned region0 v0 + store notrap aligned region0 v1, v0 + store notrap aligned region0 v2, v0 + store notrap aligned region0 v1, v0 + store notrap aligned region0 v3, v0 + return +} + +; function %save_clobber_many_restore(i64, i32, i32) fast { +; region0 = 0 "flags" +; +; block0(v0: i64, v1: i32, v2: i32): +; v3 = load.i32 notrap aligned region0 v0 +; return +; } + +;; Two independent flags, each in its own alias region, are both collapsed. +;; +;; Note that the accesses are interleaved: unwinding one region's dead store +;; must not disturb the other region's last-store state. +function %two_regions_interleaved(i64, i64) { + region0 = 0 "flags0" + region1 = 1 "flags1" +block0(v0: i64, v1: i64): + v2 = load.i32 notrap aligned region0 v0 + v3 = load.i32 notrap aligned region1 v1 + v4 = iconst.i32 0 + store notrap aligned region0 v4, v0 + store notrap aligned region1 v4, v1 + store notrap aligned region0 v2, v0 + store notrap aligned region1 v3, v1 + return +} + +; function %two_regions_interleaved(i64, i64) fast { +; region0 = 0 "flags0" +; region1 = 1 "flags1" +; +; block0(v0: i64, v1: i64): +; v2 = load.i32 notrap aligned region0 v0 +; v3 = load.i32 notrap aligned region1 v1 +; return +; } + +;; The restore is folded across intervening blocks, so long as nothing in them +;; observes the flag. +function %save_clear_restore_cross_block(i64) { + region0 = 0 "flags" +block0(v0: i64): + v1 = load.i32 notrap aligned region0 v0 + v2 = iconst.i32 0 + store notrap aligned region0 v2, v0 + jump block1 + +block1: + jump block2 + +block2: + store notrap aligned region0 v1, v0 + return +} + +; function %save_clear_restore_cross_block(i64) fast { +; region0 = 0 "flags" +; +; block0(v0: i64): +; v1 = load.i32 notrap aligned region0 v0 +; jump block1 +; +; block1: +; jump block2 +; +; block2: +; return +; } + +;; Negative test: a call between the clear and the restore observes the cleared +;; flag, so neither store may be removed. +function %call_observes_cleared_flag(i64) { + region0 = 0 "flags" + fn0 = %g(i64) +block0(v0: i64): + v1 = load.i32 notrap aligned region0 v0 + v2 = iconst.i32 0 + store notrap aligned region0 v2, v0 + call fn0(v0) + store notrap aligned region0 v1, v0 + return +} + +; function %call_observes_cleared_flag(i64) fast { +; region0 = 0 "flags" +; sig0 = (i64) fast +; fn0 = %g sig0 +; +; block0(v0: i64): +; v1 = load.i32 notrap aligned region0 v0 +; v2 = iconst.i32 0 +; store notrap aligned region0 v2, v0 ; v2 = 0 +; call fn0(v0) +; store notrap aligned region0 v1, v0 +; return +; } + +;; Negative test: the final store writes a value other than the saved one, so it +;; is not idempotent. Only the dead middle store is removed. +function %restore_wrong_value(i64, i32) { + region0 = 0 "flags" +block0(v0: i64, v1: i32): + v2 = load.i32 notrap aligned region0 v0 + v3 = iconst.i32 0 + store notrap aligned region0 v3, v0 + store notrap aligned region0 v1, v0 + return +} + +; function %restore_wrong_value(i64, i32) fast { +; region0 = 0 "flags" +; +; block0(v0: i64, v1: i32): +; v2 = load.i32 notrap aligned region0 v0 +; store notrap aligned region0 v1, v0 +; return +; } + +;; Negative test: rolling back to the previous memory version must not resurrect +;; knowledge across a store to a *different* address in the same region. The +;; region's last-store slot is per-region, not per-address, so after the store to +;; `v0+8` the analysis no longer knows what is at `v0`, and the final store to +;; `v0` cannot be proven idempotent. +function %same_region_different_address(i64, i32) { + region0 = 0 "flags" +block0(v0: i64, v1: i32): + v2 = load.i32 notrap aligned region0 v0 + v3 = iconst.i32 0 + store notrap aligned region0 v3, v0 + store notrap aligned region0 v1, v0+8 + store notrap aligned region0 v2, v0 + return +} + +; function %same_region_different_address(i64, i32) fast { +; region0 = 0 "flags" +; +; block0(v0: i64, v1: i32): +; v2 = load.i32 notrap aligned region0 v0 +; v3 = iconst.i32 0 +; store notrap aligned region0 v3, v0 ; v3 = 0 +; store notrap aligned region0 v1, v0+8 +; store notrap aligned region0 v2, v0 +; return +; } diff --git a/tests/disas/component-model/direct-adapter-calls-inlining.wat b/tests/disas/component-model/direct-adapter-calls-inlining.wat index 04435c56b5d6..d7900b80b5ba 100644 --- a/tests/disas/component-model/direct-adapter-calls-inlining.wat +++ b/tests/disas/component-model/direct-adapter-calls-inlining.wat @@ -104,7 +104,6 @@ ;; block9: ;; v11 = load.i64 notrap aligned readonly can_move region3 v3+112 ;; v12 = load.i32 notrap aligned region4 v11 -;; store notrap aligned region4 v12, v11 ;; jump block13 ;; ;; block13: diff --git a/tests/disas/component-model/direct-adapter-calls-x64.wat b/tests/disas/component-model/direct-adapter-calls-x64.wat index cfa4fead7efa..8831a79b6fc8 100644 --- a/tests/disas/component-model/direct-adapter-calls-x64.wat +++ b/tests/disas/component-model/direct-adapter-calls-x64.wat @@ -87,7 +87,7 @@ ;; movq 0x18(%r10), %r10 ;; addq $0x60, %r10 ;; cmpq %rsp, %r10 -;; ja 0x147 +;; ja 0x13e ;; 79: subq $0x50, %rsp ;; movq %rbx, 0x20(%rsp) ;; movq %r12, 0x28(%rsp) @@ -97,10 +97,10 @@ ;; movq %rdi, (%rsp) ;; movq (%rsp), %rdi ;; movq 0x88(%rdi), %rcx -;; movl (%rcx), %eax +;; movl (%rcx), %esi ;; movq %rcx, 0x10(%rsp) -;; testl %eax, %eax -;; movq %rax, 8(%rsp) +;; testl %esi, %esi +;; movq %rsi, 8(%rsp) ;; jne 0xd5 ;; b9: movq (%rsp), %rdi ;; movq 0x58(%rdi), %rax @@ -109,24 +109,19 @@ ;; movq (%rsp), %rsi ;; callq *%rax ;; ├─╼ exception frame offset: SP = FP - 0x50 -;; ╰─╼ exception handler: default handler, context at [SP+0x0], handler=0x132 -;; jmp 0x130 -;; d5: movq (%rsp), %rsi -;; movq 0x70(%rsi), %rax -;; movl (%rax), %ecx -;; movl %ecx, (%rax) -;; movq 0x48(%rsi), %rdi +;; ╰─╼ exception handler: default handler, context at [SP+0x0], handler=0x121 +;; jmp 0x11f +;; d5: movq (%rsp), %rcx +;; movq 0x70(%rcx), %rax +;; movl (%rax), %eax +;; movq 0x48(%rcx), %rdi +;; movq (%rsp), %rsi ;; callq 0 ;; ├─╼ exception frame offset: SP = FP - 0x50 -;; ╰─╼ exception handler: default handler, context at [SP+0x0], handler=0xef -;; jmp 0xf7 -;; ef: movq %rax, %rdx -;; jmp 0x132 -;; f7: movq %rax, %rdx -;; movq 8(%rsp), %rcx -;; movq 0x10(%rsp), %rax -;; movl %ecx, (%rax) -;; movq %rdx, %rax +;; ╰─╼ exception handler: default handler, context at [SP+0x0], handler=0x121 +;; movq 0x10(%rsp), %rcx +;; movq 8(%rsp), %rsi +;; movl %esi, (%rcx) ;; movq 0x20(%rsp), %rbx ;; movq 0x28(%rsp), %r12 ;; movq 0x30(%rsp), %r13 @@ -136,12 +131,14 @@ ;; movq %rbp, %rsp ;; popq %rbp ;; retq -;; 12b: jmp 0x132 -;; 130: ud2 -;; 132: movq (%rsp), %rsi -;; 136: movq 0x58(%rsi), %rcx -;; 13a: movq 0x68(%rsi), %rdi -;; 13e: movl $0x31, %edx -;; 143: callq *%rcx -;; 145: ud2 -;; 147: ud2 +;; 11a: jmp 0x121 +;; 11f: ud2 +;; 121: movq (%rsp), %rcx +;; 125: movq 0x58(%rcx), %rcx +;; 129: movq (%rsp), %rax +;; 12d: movq 0x68(%rax), %rdi +;; 131: movl $0x31, %edx +;; 136: movq (%rsp), %rsi +;; 13a: callq *%rcx +;; 13c: ud2 +;; 13e: ud2 diff --git a/tests/disas/component-model/direct-adapter-calls.wat b/tests/disas/component-model/direct-adapter-calls.wat index e3ce501b14d6..870554c73541 100644 --- a/tests/disas/component-model/direct-adapter-calls.wat +++ b/tests/disas/component-model/direct-adapter-calls.wat @@ -133,7 +133,6 @@ ;; block7: ;; @008e v11 = load.i64 notrap aligned readonly can_move region2 v0+112 ;; @008e v12 = load.i32 notrap aligned region3 v11 -;; @009a store notrap aligned region3 v12, v11 ;; @009c v16 = load.i64 notrap aligned readonly can_move region4 v0+72 ;; @009c try_call fn0(v16, v0, v2), sig1, block10(ret0), [ context v0, default: block6(exn0) ] ;; diff --git a/tests/disas/component-model/sync-adapter-calls.wat b/tests/disas/component-model/sync-adapter-calls.wat index 36536000f4cc..b59860e582a4 100644 --- a/tests/disas/component-model/sync-adapter-calls.wat +++ b/tests/disas/component-model/sync-adapter-calls.wat @@ -149,7 +149,6 @@ ;; store notrap aligned region5 v19, v20+136 ;; v26 = load.i64 notrap aligned readonly can_move region3 v3+176 ;; v27 = load.i32 notrap aligned region4 v26 -;; store notrap aligned region4 v27, v26 ;; jump block17 ;; ;; block17: From 0d648a2125fb193509dd96519debf78b09ca034a Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Thu, 13 Aug 2026 23:09:49 +0000 Subject: [PATCH 6/8] Alias analysis: do not restore the last-fence into a region slot (#14134) * Alias analysis: do not restore the last-fence into a region slot When we eliminate a dead store, we undo the effects that the dead store had on the `LastStore` state. However, querying the last store for a particular region falls back to the last fence, and we were incorrectly restoring that last fence into the region slot, rather than resetting the region slot to `None`. While technically incorrect, it was generally benign, but it did lead to "observing" instructions that we didn't mark observed during our initial observation pass, which ultimately led to debug assertion failures. Fixes #14131 * untrim whitespace in filetests --- cranelift/codegen/src/alias_analysis.rs | 55 +++++++++++++------ .../filetests/alias/issue-14131-atomic.clif | 40 ++++++++++++++ .../filetests/alias/issue-14131.clif | 38 +++++++++++++ 3 files changed, 116 insertions(+), 17 deletions(-) create mode 100644 cranelift/filetests/filetests/alias/issue-14131-atomic.clif create mode 100644 cranelift/filetests/filetests/alias/issue-14131.clif diff --git a/cranelift/codegen/src/alias_analysis.rs b/cranelift/codegen/src/alias_analysis.rs index 626455a778e8..f2a0261c4e61 100644 --- a/cranelift/codegen/src/alias_analysis.rs +++ b/cranelift/codegen/src/alias_analysis.rs @@ -424,19 +424,30 @@ impl LastStores { } } + /// Get the contents of `inst`'s own alias region's slot, without falling + /// back to the last fence. + /// + /// Returns `None` when `inst` has no alias region. + fn raw_region_slot(&self, func: &Function, inst: Inst) -> Option> { + let region = func.dfg.insts[inst].alias_region(&func.dfg)?; + Some(self.regions[region]) + } + /// Roll this state back to the memory version from just before `dead`, /// which is a store being removed from the function by dead-store /// elimination. /// - /// `prev_last_store` must be the last-store instruction that immediately - /// preceded `dead`, as recorded when `dead` itself was processed. + /// `prev_region_slot` must be what `dead`'s own alias-region slot held + /// immediately before `dead` overwrote it, as recorded by `region_slot` + /// when `dead` itself was processed (that is, it must not be the last-fence + /// fallback). /// /// Only `dead`'s own alias-region slot is restored. A store with no alias /// region is treated as a fence by `update`, which clears *every* region /// slot, and we do not undo that; in that case, we leave this state /// alone. Similarly, stores marked observed while processing `dead` stay /// observed. - fn undo_store(&mut self, func: &Function, dead: Inst, prev_last_store: PackedOption) { + fn undo_store(&mut self, func: &Function, dead: Inst, prev_region_slot: PackedOption) { debug_assert!(func.dfg.insts[dead].opcode().can_store()); let Some(region) = func.dfg.insts[dead].alias_region(&func.dfg) else { @@ -446,7 +457,7 @@ impl LastStores { // Only roll back if `dead` really is the current last store to its // region. if self.regions[region].expand() == Some(dead) { - self.regions[region] = prev_last_store; + self.regions[region] = prev_region_slot; } } @@ -565,12 +576,17 @@ struct KnownValue { /// Kept around for quick dominance checks. def_inst: Inst, - /// When this entry was created by a store, the last-store instruction that - /// immediately preceded that store: that is, the memory version this - /// location was at just *before* `def_inst` overwrote it. + /// When this entry was created by a store to a particular alias region, + /// whatever that region's last-store slot held just *before* `def_inst` + /// overwrote it, as given by `LastStores::region_slot`. + /// + /// `None` means either the entry was created by a load or by a store with + /// no alias region. Neither will ever undo `LastStores` state. /// - /// `None` for entries created by loads. - prev_last_store: Option>, + /// `Some(maybe_inst)` contains the alias region slot's previous value, so + /// that it can be restored if `def_inst` is a dead store that gets + /// eliminated. + prev_region_slot: Option>, } /// The result of processing an instruction through alias analysis. @@ -819,12 +835,13 @@ impl<'a> AliasAnalysis<'a> { // would then need a whole additional pass over the // function to collapse each link. // - // A missing entry means we never processed the dead - // store as a store in this pass (it can come from a - // precomputed `block_input` snapshot, for a predecessor - // block we have not walked yet), so we have no previous - // version to roll back to and simply don't. - if let Some(prev) = dead_entry.and_then(|e| e.prev_last_store) { + // A missing entry means we have no previous version to + // roll back to, and simply don't: either we never + // processed the dead store as a store in this pass (it + // can come from a precomputed `block_input` snapshot, + // for a predecessor block we have not walked yet) or it + // has no alias region and therefore no slot of its own. + if let Some(prev) = dead_entry.and_then(|e| e.prev_region_slot) { state.undo_store(func, last_store, prev); } @@ -889,7 +906,11 @@ impl<'a> AliasAnalysis<'a> { KnownValue { def_inst: inst, value: store_data, - prev_last_store: Some(last_store), + // NB: we use the raw region slot, without the + // last-fence fallback, because we don't want to move an + // instruction without a region into a region slot on + // DSE rollback. + prev_region_slot: state.raw_region_slot(func, inst), }, ); @@ -944,7 +965,7 @@ impl<'a> AliasAnalysis<'a> { value: load_result, // A load does not advance the memory version, so // there is no previous version to roll back to. - prev_last_store: None, + prev_region_slot: None, }, ); } diff --git a/cranelift/filetests/filetests/alias/issue-14131-atomic.clif b/cranelift/filetests/filetests/alias/issue-14131-atomic.clif new file mode 100644 index 000000000000..f0a68a79461b --- /dev/null +++ b/cranelift/filetests/filetests/alias/issue-14131-atomic.clif @@ -0,0 +1,40 @@ +test optimize precise-output +set opt_level=speed_and_size +target x86_64 + +;; Regression test for https://github.com/bytecodealliance/wasmtime/issues/14131 +;; +;; The last-store slot that dead-store elimination must not materialize into +;; `region0` holds an *atomic* store rather than a regionless store. +;; +;; The `atomic_store` has memory fence semantics, so it becomes the `last_fence` +;; while `region0`'s last-store slot stays empty. The first `region0` store +;; therefore sees the `last_fence` fallback, and is then made dead by the second +;; `region0` store. Unwinding the last-store state past the removed store must +;; restore `region0`'s empty slot rather than the last-fence fallback: otherwise +;; reprocessing the overwriting store "observes" the atomic store, an +;; observation the up-front observed-stores analysis never makes (the trailing +;; `fence` replaces the atomic in `last_fence` without observing it), ultimately +;; leading to an assertion failure. + +function %f(i64, i32) system_v { + region0 = 1 "table" + +block0(v0: i64, v1: i32): + atomic_store.i32 notrap v1, v0 + store notrap region0 v1, v0 + store notrap region0 v1, v0 + fence + return +} + +; function %f(i64, i32) system_v { +; region0 = 1 "table" +; +; block0(v0: i64, v1: i32): +; atomic_store notrap v1, v0 +; store notrap region0 v1, v0 +; fence +; return +; } + diff --git a/cranelift/filetests/filetests/alias/issue-14131.clif b/cranelift/filetests/filetests/alias/issue-14131.clif new file mode 100644 index 000000000000..e16a0833b6e2 --- /dev/null +++ b/cranelift/filetests/filetests/alias/issue-14131.clif @@ -0,0 +1,38 @@ +test optimize precise-output +set opt_level=speed_and_size +target x86_64 + +;; Regression test for https://github.com/bytecodealliance/wasmtime/issues/14131 +;; +;; The first store has no alias region, so it is treated as a fence and becomes +;; the `last_fence`, while `region0`'s last-store slot stays empty. The second +;; store is then a dead store, overwritten by the third. When dead-store +;; elimination unwinds the last-store state past the store it just removed, it +;; must not materialize the `last_fence` fallback into `region0`'s slot: +;; reprocessing the overwriting store would then "observe" the regionless store, +;; an observation that the up-front observed-stores analysis never made, which +;; leads to an assertion failure. + +function %f(i64, i64) system_v { + region0 = 1 "table" + +block0(v0: i64, v1: i64): + v2 = iconst.i32 0 + store notrap v2, v1 + store notrap region0 v2, v0 + store notrap region0 v2, v0 + fence + return +} + +; function %f(i64, i64) system_v { +; region0 = 1 "table" +; +; block0(v0: i64, v1: i64): +; v2 = iconst.i32 0 +; store notrap v2, v1 ; v2 = 0 +; store notrap region0 v2, v0 ; v2 = 0 +; fence +; return +; } + From 6b1b2c7cba21cd3d639e72c8db98bbbb270c44ac Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Fri, 14 Aug 2026 13:52:36 +0000 Subject: [PATCH 7/8] Return is-directory when a directory fd is used as a file (#14135) * wasip2: return is-directory when a directory fd is used as a file Descriptor::file() treated a directory as a bad descriptor. POSIX read/write on a directory is EISDIR, and wasi:filesystem already has is-directory. Preview1 guests still get EBADF (separate match and adapter). Signed-off-by: Sebastien Tardif * wasip2: map is-directory only on read-via-stream Descriptor::file() must stay bad-descriptor for directories. wasi-testsuite filesystem-advise expects that for advise. Return is-directory from read-via-stream only (p2 result, p3 result future) so a directory read matches POSIX EISDIR. Signed-off-by: Sebastien Tardif --------- Signed-off-by: Sebastien Tardif --- crates/test-programs/src/bin/p2_file_read_write.rs | 5 +++++ .../src/bin/p3_filesystem_file_read_write.rs | 11 ++++++++++- crates/wasi/src/filesystem.rs | 3 +++ crates/wasi/src/p2/host/filesystem.rs | 8 ++++++-- crates/wasi/src/p3/filesystem/host.rs | 13 +++++++++++-- 5 files changed, 35 insertions(+), 5 deletions(-) diff --git a/crates/test-programs/src/bin/p2_file_read_write.rs b/crates/test-programs/src/bin/p2_file_read_write.rs index 8c39d6de573c..0c2b82045c87 100644 --- a/crates/test-programs/src/bin/p2_file_read_write.rs +++ b/crates/test-programs/src/bin/p2_file_read_write.rs @@ -5,6 +5,11 @@ fn main() { let preopens = wasi::filesystem::preopens::get_directories(); let (dir, _) = &preopens[0]; + assert_eq!( + dir.read_via_stream(0).err(), + Some(wasi::filesystem::types::ErrorCode::IsDirectory) + ); + let filename = "test.txt"; let file = dir .open_at( diff --git a/crates/test-programs/src/bin/p3_filesystem_file_read_write.rs b/crates/test-programs/src/bin/p3_filesystem_file_read_write.rs index b4a9518cbeb9..2559a8f83bc9 100644 --- a/crates/test-programs/src/bin/p3_filesystem_file_read_write.rs +++ b/crates/test-programs/src/bin/p3_filesystem_file_read_write.rs @@ -1,5 +1,7 @@ use futures::join; -use test_programs::p3::wasi::filesystem::types::{DescriptorFlags, OpenFlags, PathFlags}; +use test_programs::p3::wasi::filesystem::types::{ + DescriptorFlags, ErrorCode, OpenFlags, PathFlags, +}; use test_programs::p3::{wasi, wit_stream}; struct Component; @@ -11,6 +13,13 @@ impl test_programs::p3::exports::wasi::cli::run::Guest for Component { let preopens = wasi::filesystem::preopens::get_directories(); let (dir, _) = &preopens[0]; + let (_data_rx, data_fut) = dir.read_via_stream(0); + let err = data_fut.await.expect_err("directory read should fail"); + assert!( + matches!(err, ErrorCode::IsDirectory), + "unexpected error: {err:?}" + ); + let filename = "test.txt"; { let file = dir diff --git a/crates/wasi/src/filesystem.rs b/crates/wasi/src/filesystem.rs index a8e90bf9895e..e7b9190bb68c 100644 --- a/crates/wasi/src/filesystem.rs +++ b/crates/wasi/src/filesystem.rs @@ -490,6 +490,9 @@ impl Descriptor { pub(crate) fn file(&self) -> Result<&File, ErrorCode> { match self { Descriptor::File(f) => Ok(f), + // File-only ops such as advise stay bad-descriptor on a dir + // (wasi-testsuite filesystem-advise). read-via-stream maps Dir + // to is-directory on its own. Descriptor::Dir(_) => Err(ErrorCode::BadDescriptor), } } diff --git a/crates/wasi/src/p2/host/filesystem.rs b/crates/wasi/src/p2/host/filesystem.rs index 3fc37a45ad07..579c74129a05 100644 --- a/crates/wasi/src/p2/host/filesystem.rs +++ b/crates/wasi/src/p2/host/filesystem.rs @@ -367,8 +367,12 @@ impl HostDescriptor for WasiFilesystemCtxView<'_> { fd: Resource, offset: types::Filesize, ) -> FsResult> { - // Trap if fd lookup fails: - let f = self.table.get(&fd)?.file()?; + // Trap if fd lookup fails. A directory is is-directory, not + // bad-descriptor (POSIX EISDIR on read). + let f = match self.table.get(&fd)? { + Descriptor::File(f) => f, + Descriptor::Dir(_) => return Err(ErrorCode::IsDirectory.into()), + }; // Create a stream view for it. let reader: DynInputStream = Box::new(FileInputStream::new(f, offset)); diff --git a/crates/wasi/src/p3/filesystem/host.rs b/crates/wasi/src/p3/filesystem/host.rs index 54437ce23fa0..8eade7cde2f5 100644 --- a/crates/wasi/src/p3/filesystem/host.rs +++ b/crates/wasi/src/p3/filesystem/host.rs @@ -521,8 +521,17 @@ impl types::HostDescriptorWithStore for WasiFilesystem { fd: Resource, offset: Filesize, ) -> wasmtime::Result<(StreamReader, FutureReader>)> { - let file = get_file(store.get().table, &fd)?; - let file = file.clone(); + let file = match get_descriptor(store.get().table, &fd)? { + Descriptor::File(file) => file.clone(), + Descriptor::Dir(_) => { + return Ok(( + StreamReader::new(&mut store, iter::empty())?, + FutureReader::new(&mut store, async move { + wasmtime::error::Ok(Err(ErrorCode::IsDirectory)) + })?, + )); + } + }; let (result_tx, result_rx) = oneshot::channel(); Ok(( StreamReader::new( From 715b457cafacc24b308137791d60004624ca8734 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 14 Aug 2026 07:45:27 -0700 Subject: [PATCH 8/8] Fix test expectations --- tests/disas/component-model/sync-adapter-calls.wat | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/disas/component-model/sync-adapter-calls.wat b/tests/disas/component-model/sync-adapter-calls.wat index b59860e582a4..c8836867ab6c 100644 --- a/tests/disas/component-model/sync-adapter-calls.wat +++ b/tests/disas/component-model/sync-adapter-calls.wat @@ -268,7 +268,6 @@ ;; @00f0 store notrap aligned region6 v19, v20+136 ;; @00f2 v26 = load.i64 notrap aligned readonly can_move region2 v0+176 ;; @00f2 v27 = load.i32 notrap aligned region3 v26 -;; @00fe store notrap aligned region3 v27, v26 ;; @0100 jump block15 ;; ;; block15: