Skip to content

Fix issues with stubbing WASI imports #185

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.

Sign up for GitHub

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

Merged
tschneidereit merged 1 commit into from
Feb 12, 2025
Merged

Fix issues with stubbing WASI imports #185

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 20 additions & 17 deletions crates/spidermonkey-embedding-splicer/src/stub_wasi.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ where
let mut builder = FunctionBuilder::new(params.as_slice(), results.as_slice());
let _args = stub(&mut builder)?;

println!("Warning: the component import '{full_import}#{name}' isn't listed in the target WIT world, and will abort execution when called.");
builder.replace_import_in_module(module, iid);

return Ok(Some(fid));
Expand Down Expand Up @@ -117,7 +118,7 @@ pub fn stub_wasi(
let mut target_world_imports = HashSet::new();

for (key, _) in &target_world.imports {
target_world_imports.insert(resolve.name_world_key(key));
target_world_imports.insert(resolve.name_canonicalized_world_key(key));
}

let mut module = Module::parse(wasm.as_slice(), false).unwrap();
Expand Down Expand Up @@ -174,22 +175,6 @@ fn target_world_requires_io(target_world_imports: &HashSet<String>) -> bool {

const PREVIEW1: &str = "wasi_snapshot_preview1";
fn stub_preview1(module: &mut Module) -> Result<()> {
Copy link

@guybedford guybedford Feb 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The assumption here is effectively that none of StarlingMonkey calls are needed at runtime, so in a sense this code path binds to the StarlingMonkey internals.

This was before we had full support for per-subsystem stubbing. Preview1 as special here because it is implicit in the C++ bindings and pulled in automatically over the explicit preview2 host API.

Now that we have better subsystem management I agree we can integrate this into the main stubbing routines, based on the single assumption that the filesystem usage from core starlingmonkey is disabled always by default (at least until we implement an FS..!).

Copy link

@guybedford guybedford Feb 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For binding custom builtins, if we wanted to better support FS, happy to discuss this further too.

Copy link
Member Author

@tschneidereit tschneidereit Feb 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Context for this PR is my work on debugging support in StarlingMonkey, which needs post-wizening access to env vars (to check if the debugger should be activated) and to socket APIs (to make debugging work at all.)

I think now that we have the subsystem management it makes sense to lean into it in this way, and allow custom builtins to make use of WASI APIs at their own pace, without a need to have changes applied to ComponentizeJS.

Copy link
Member Author

@tschneidereit tschneidereit Feb 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we wanted to better support FS, happy to discuss this further too.

I'm not sure we really need much else from ComponentizeJS, at least for now: as long as the stubs are only applied to things not in the target world, everything should work well

stub_import(module, PREVIEW1, "environ_get", unreachable_stub)?;
stub_import(module, PREVIEW1, "environ_sizes_get", unreachable_stub)?;
stub_import(module, PREVIEW1, "fd_close", unreachable_stub)?;
stub_import(module, PREVIEW1, "fd_fdstat_set_flags", unreachable_stub)?;
stub_import(module, PREVIEW1, "fd_prestat_get", unreachable_stub)?;
stub_import(module, PREVIEW1, "fd_readdir", unreachable_stub)?;
stub_import(module, PREVIEW1, "args_get", unreachable_stub)?;
stub_import(module, PREVIEW1, "args_sizes_get", unreachable_stub)?;
stub_import(module, PREVIEW1, "path_filestat_get", unreachable_stub)?;
stub_import(module, PREVIEW1, "fd_prestat_dir_name", unreachable_stub)?;
stub_import(module, PREVIEW1, "fd_read", unreachable_stub)?;
stub_import(module, PREVIEW1, "fd_seek", unreachable_stub)?;
stub_import(module, PREVIEW1, "path_open", unreachable_stub)?;
stub_import(module, PREVIEW1, "path_remove_directory", unreachable_stub)?;
stub_import(module, PREVIEW1, "path_unlink_file", unreachable_stub)?;
stub_import(module, PREVIEW1, "proc_exit", unreachable_stub)?;
// random comes from prevew2 only in StarlingMonkey
stub_import(module, PREVIEW1, "random_get", unreachable_stub)?;
Ok(())
Expand Down Expand Up @@ -1478,6 +1463,18 @@ fn stub_filesystem(module: &mut Module, world_imports: &HashSet<String>) -> Resu
"[resource-drop]directory-entry-stream",
unreachable_stub,
)?;

stub_import(module, PREVIEW1, "fd_close", unreachable_stub)?;
stub_import(module, PREVIEW1, "fd_fdstat_set_flags", unreachable_stub)?;
stub_import(module, PREVIEW1, "fd_prestat_get", unreachable_stub)?;
stub_import(module, PREVIEW1, "fd_readdir", unreachable_stub)?;
stub_import(module, PREVIEW1, "fd_prestat_dir_name", unreachable_stub)?;
stub_import(module, PREVIEW1, "fd_read", unreachable_stub)?;
stub_import(module, PREVIEW1, "fd_seek", unreachable_stub)?;
stub_import(module, PREVIEW1, "path_open", unreachable_stub)?;
stub_import(module, PREVIEW1, "path_filestat_get", unreachable_stub)?;
stub_import(module, PREVIEW1, "path_remove_directory", unreachable_stub)?;
stub_import(module, PREVIEW1, "path_unlink_file", unreachable_stub)?;
}

if !world_imports.contains("wasi:filesystem/preopens@0.2") {
Expand Down Expand Up @@ -1513,10 +1510,16 @@ fn stub_cli(module: &mut Module, world_imports: &HashSet<String>) -> Result<()>
"initial-cwd",
unreachable_stub,
)?;

stub_import(module, PREVIEW1, "args_get", unreachable_stub)?;
stub_import(module, PREVIEW1, "args_sizes_get", unreachable_stub)?;
stub_import(module, PREVIEW1, "environ_get", unreachable_stub)?;
stub_import(module, PREVIEW1, "environ_sizes_get", unreachable_stub)?;
}

if !world_imports.contains("wasi:cli/exit@0.2") {
stub_wasi_imports(module, "wasi:cli/exit", "exit", unreachable_stub)?;
stub_import(module, PREVIEW1, "proc_exit", unreachable_stub)?;
}

if !world_imports.contains("wasi:cli/terminal-stdin@0.2") {
Expand Down
Loading