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.

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

Jump to bottom

wasmtime-wasi only preopening the first and last directory set in WasiCtxBuilder #10058

Closed
justingaffney opened this issue Jan 21, 2025 · 4 comments · Fixed by #10064
Closed

wasmtime-wasi only preopening the first and last directory set in WasiCtxBuilder #10058

justingaffney opened this issue Jan 21, 2025 · 4 comments · Fixed by #10064
Labels
bug Incorrect behavior in the current implementation that needs fixing

Comments

@justingaffney
Copy link

Test Case

Clone example repo here: https://github.com/justingaffney/wasmtime-preopen-bug

Steps to Reproduce

  • Build component
    • cd component
    • cargo component build --release
  • Run host
    • cd ..
    • cargo run

Expected Results

The following output:

Reading /a: Success
Reading /b: Success
Reading /c: Success
Reading /d: Success
Reading /e: Success

Actual Results

The following output:

Reading /a: Success
Reading /b: Failed. Error: Some(Custom { kind: Uncategorized, error: "failed to find a pre-opened file descriptor through which \"/b\" could be opened" })
Reading /c: Failed. Error: Some(Custom { kind: Uncategorized, error: "failed to find a pre-opened file descriptor through which \"/c\" could be opened" })
Reading /d: Failed. Error: Some(Custom { kind: Uncategorized, error: "failed to find a pre-opened file descriptor through which \"/d\" could be opened" })
Reading /e: Success

Versions and Environment

Wasmtime version or commit: 28.0.1, 29.0.0

Operating system: Windows 11

Architecture: x86_64
@justingaffney justingaffney added the bug Incorrect behavior in the current implementation that needs fixing label Jan 21, 2025
@justingaffney
Copy link
Author

I just tried it on Ubuntu 24.04.1 and have the same issue

alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue Jan 21, 2025
@alexcrichton
Fix a missing increment in p1-to-p2 adapter
d3afa6d 
This commit fixes a bug in the WASIp1-to-WASIp2 adapter during
`fd_prestat_dir_name` where an iterator variable was forgotten to be
incremented. That means that getting the path for anything other than
the first preopen didn't work correctly.

Closes bytecodealliance#10058
@alexcrichton alexcrichton mentioned this issue Jan 21, 2025
Merged
@alexcrichton
Copy link
Member

Thanks for the report! This should be fixed in #10064. I'll note though that this is a bug in the "adapter" which will take a bit of time to propagate to toolchains. In the meantime working around this will be difficult as the adapter is pretty deep inside tooling and difficult to update-on-a-dime.

github-merge-queue bot pushed a commit that referenced this issue Jan 21, 2025
@alexcrichton
Fix a missing increment in p1-to-p2 adapter (#10064)
7ba0347 
This commit fixes a bug in the WASIp1-to-WASIp2 adapter during
`fd_prestat_dir_name` where an iterator variable was forgotten to be
incremented. That means that getting the path for anything other than
the first preopen didn't work correctly.

Closes #10058
alexcrichton added a commit to alexcrichton/wasmtime that referenced this issue Jan 21, 2025
@alexcrichton
Fix a missing increment in p1-to-p2 adapter (bytecodealliance#10064)
ce4a5eb 
This commit fixes a bug in the WASIp1-to-WASIp2 adapter during
`fd_prestat_dir_name` where an iterator variable was forgotten to be
incremented. That means that getting the path for anything other than
the first preopen didn't work correctly.

Closes bytecodealliance#10058
alexcrichton added a commit that referenced this issue Jan 21, 2025
@alexcrichton @saulecabrera
[29.0.x]: Fix a missing increment in p1-to-p2 adapter (#10067)
e304726 
* Add audit for `wasmtime-math` (#10059)

I noticed that CI is failing given that an audit and policy for
`wasmtime-math` is missing.

`wasmtime-math` was introduced in
https://github.com/bytecodealliance/wasmtime/pull/9808/files.

I followed a similar approach to what it's used for all the other
`wasmtime-*` crates.

* Fix a missing increment in p1-to-p2 adapter (#10064)

This commit fixes a bug in the WASIp1-to-WASIp2 adapter during
`fd_prestat_dir_name` where an iterator variable was forgotten to be
incremented. That means that getting the path for anything other than
the first preopen didn't work correctly.

Closes #10058

* Downgrade `wasip2` dep to 0.13.0

Compat with 1.81.0 MSRV

---------

Co-authored-by: Saúl Cabrera <saulecabrera@gmail.com>
alexcrichton added a commit to alexcrichton/cargo-component that referenced this issue Jan 21, 2025
@alexcrichton
Update the adapter binaries cargo-component uses
67eb350 
This pulls in a fix for bytecodealliance/wasmtime#10058
@alexcrichton alexcrichton mentioned this issue Jan 21, 2025
Merged
alexcrichton added a commit to bytecodealliance/wasm-component-ld that referenced this issue Jan 21, 2025
@alexcrichton
Update adapter builds
93dd0eb 
Pulls in a fix for bytecodealliance/wasmtime#10058
@alexcrichton alexcrichton mentioned this issue Jan 21, 2025
Merged
github-merge-queue bot pushed a commit to bytecodealliance/wasm-component-ld that referenced this issue Jan 21, 2025
@alexcrichton
Update adapter builds (#56)
84cb761 
Pulls in a fix for bytecodealliance/wasmtime#10058
alexcrichton added a commit to alexcrichton/wasi-sdk that referenced this issue Jan 21, 2025
@alexcrichton
Update wasm-component-ld
bce9cf4 
Pulls in a fix for bytecodealliance/wasmtime#10058 in the adapters that
are used by default.
alexcrichton added a commit to alexcrichton/wasi-sdk that referenced this issue Jan 21, 2025
@alexcrichton
Update wasm-component-ld
36eef69 
Pulls in a fix for bytecodealliance/wasmtime#10058 in the adapters that
are used by default.
This was referenced Jan 21, 2025
Merged
Merged
alexcrichton added a commit to bytecodealliance/cargo-component that referenced this issue Jan 21, 2025
@alexcrichton
Update the adapter binaries cargo-component uses (#370)
241996c 
* Update the adapter binaries cargo-component uses

This pulls in a fix for bytecodealliance/wasmtime#10058

* Update wasmtime used in CI
jieyouxu pushed a commit to jieyouxu/rust that referenced this issue Jan 22, 2025
@alexcrichton
Update the wasm-component-ld tool
8600694 
This commit updates the `wasm-component-ld` tool from 0.5.11 to 0.5.12.
This pulls in a fix for the binary adapters that are included with this
tool for an issue described in bytecodealliance/wasmtime#10058. Some
other dependencies have additionally been updated in the meantime of
`wasm-component-ld` but there should otherwise be no major changes.
jieyouxu added a commit to jieyouxu/rust that referenced this issue Jan 22, 2025
@jieyouxu
Rollup merge of rust-lang#135850 - alexcrichton:update-wasm-component…
22ed6a8 
…-ld, r=jieyouxu

Update the `wasm-component-ld` tool

This commit updates the `wasm-component-ld` tool from 0.5.11 to 0.5.12. This pulls in a fix for the binary adapters that are included with this tool for an issue described in bytecodealliance/wasmtime#10058. Some other dependencies have additionally been updated in the meantime of `wasm-component-ld` but there should otherwise be no major changes.
@justingaffney
Copy link
Author

Thanks for the fast response!

It looks like the adapter is used in quite a few places, and I'm not sure how it all fits together yet. In the case of my example repo would I just need the next release of cargo-component, that contains your merged PR updating wasi-preview1-component-adapter-provider? Then I'd just need to run cargo component build again to include the new adapter version in the produced .wasm file?

@alexcrichton
Copy link
Member

I believe that should work yeah!

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jan 22, 2025
@matthiaskrgr
Rollup merge of rust-lang#135850 - alexcrichton:update-wasm-component…
b0bb6e3 
…-ld, r=jieyouxu

Update the `wasm-component-ld` tool

This commit updates the `wasm-component-ld` tool from 0.5.11 to 0.5.12. This pulls in a fix for the binary adapters that are included with this tool for an issue described in bytecodealliance/wasmtime#10058. Some other dependencies have additionally been updated in the meantime of `wasm-component-ld` but there should otherwise be no major changes.
alexcrichton added a commit to WebAssembly/wasi-sdk that referenced this issue Jan 22, 2025
@alexcrichton
Update wasm-component-ld (#506)
67283cc 
* Update wasm-component-ld

Pulls in a fix for bytecodealliance/wasmtime#10058 in the adapters that
are used by default.

* Update wasmtime installed in CI

* Downgrade the build-only-sysroot check step

Looks like this is failing on Ubuntu 24.04, the new default of
`ubuntu-latest`, so downgrade it to have it get fixed in a separate PR.

* Try downgrading Wasmtime version again

* Update base Linux images to Ubuntu 20.04

* Update Wasmtime back to 29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Incorrect behavior in the current implementation that needs fixing
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix a missing increment in p1-to-p2 adapter alexcrichton/wasmtime
2 participants
@alexcrichton @justingaffney