@Alex Crichton re: your request for a reproduction of the wasm component issue:
broken-composed.wasm
@Ryan Levick (rylev) do you have the source modules here that I could play with? Either the source code or the input *.wasm
core wasm as input to wit-component
hm ok I think I see the issue, and it looks like this is unrelated to wasm-tools compose
but is instead a bug in wit-component
which requires nested components to expose, which only wasm-tools compose
builds
I think I have an idea of how to fix, but it'll have to wait until the resources PR is merged since it'll build on that.
this raw binary component will become invalid with that PR, however, but when made vaild (by re-running wit-component
) the new binary should work with the future PR I have in mind
Sounds good to me. If you have the energy to write up a sentence or two with more detail on the issue I'd love to hear it (trying to solidify my understanding of the binary format), but no worries if you don't get around to it :grinning:
oh sure!
The way that wit-component works is that it walks the type information as produced by wasmparser
's validation phase, notably it doesn't walk the component manually itself (that's what wasmparser
does during validation). The problem with nested components, however, is that the final exported item from the outer component in your example is referring to an imported type of an inner component. This means that during wit-component's walk over the type of the component itself it never actually encountered the inner types, meaning it never registered anything for them.
The solution I have in mind is that these types are supplied when the outer component is instantiating the inner component. During this process type-substitution is already happening for resource types and I figure it can be updated to happen for normal types as well. For example if you satisfy a type import with a type, then that type you provided should be used to substitute the final type of the component.
Given this when wit-component walks over the type information it'll find that http-types
, which is imported by the outer component, is the exact same type IDs that's exported by the result, meaning that it shouldn't "miss" anything due to it being internal to a component.
Or at least that's the theory! The type substitution support isn't there today in wasmparser but it's available with the resources PR, which I think should be relatively easy to piggy-back onto.
Nice. The explanation of the problem matches my mental model (which I'm very happy about). I'll check out the resources PR to see if I can grok what's happening there. The proposed solution sounds clear to me
Thanks for the explainer!
If you've got the original source or core wasm modules I can test out a solution too
I was thinking to test out my idea but it requires a new component due to the new validation rules which make the existing one invalid
but re-running wit-component
should be all that's necessary
i'm currently reviewing the resource types PR so hopefully we can get that merged in soon to help with a basis for a fix
@Alex Crichton here are the wasm components I'm trying to compose:
base.wasm
adapter.wasm
config.yml
wasm-tools compose --output broken-composed.wasm -c config.yml base.wasm
@Ryan Levick (rylev) do you have the pre-wit-component core wasm outputs?
those look like they're both already components
If you're using cargo-component
, there's no good way to tell it to not componentize the core module in-place (if you're on macOS, however, a quirk of how cargo copies rather than hardlinks the deps to the target directory will keep the module in the deps directory)
ah ok, in that case I'll keep poking, I'm trying to handwrite a minimal test case but am running into a few issues
a hack would be to manually depend your project on the bindings crate and do a cargo build --target wasm32...
if we might find it helpful for the future, can add a --keep-module
option or something so that it preserves the core module (perhaps copies it to foo.core.wasm
)
Luckily I'm using macOS so the core module was in the deps folder
I don't have easy access to the adapter core module but I can get that too if it makes life easier
ok thanks! I've discovered a fuzz bug (probably should have run this sooner with this branch) which I need to bottom out and may require larger changes to fix
Ok all the fuzz issues I think are now fixed and the output of wit-component
has changed even more. This additionally necessitated adding a test case which showcases the type-substitution behavior so now the type-substitution logic I was alluding to is now included in the PR.
I believe the basic tl;dr; is that your issue will be fixed with the PR as-is. I haven't fully tested though. Are you able to check out the PR and see that toolchain works for you?
@Alex Crichton I can give it a try, but I think I'll need to update Cargo component to use your latest changes. I will try that out and let you know if everything works.
Unfortunately, I'm getting an error, but I'm unsure if it's a mistake on my end (to reproduce the set up of my original issue, I have to update wasm-tools and wit-bindgen dependencies in many places).
I'm getting this error: func not valid to be used as import (at offset 0x24ae86)
Here's the component in question: component.wasm
(note this is before any composition - it's just during "componentization" - when we are taking core modules and making them components)
It would probably help if we could make validate_and_register_named_types
return what is not valid instead of returning just a bool
hm ok that may be a further bug, I'll try to dig in in a bit
Looks like there is a record definition for a "request" type that might not be exported?
At some point this check returns false.
(deleted)
Unfortunately it's going to take some time to update the tooling on my end to test all of this (we're on an old version of wit-bindgen and upgrading is not trivial).
@Alex Crichton I think I'm still seeing the same issue with everything upgrade to the latest version of wasm-tools including your recently merged resources PR and including transitive wasm-tools dependencies of wit-bindgen:
(type (;28;) (record (field "status" 26) (field "headers" 27) (field "body" 24)))
(type (;29;) (func (param "req" 25) (result 28)))
(alias core export 3 "inbound-http#handle-request" (core func (;54;)))
(alias core export 3 "cabi_post_inbound-http#handle-request" (core func (;55;)))
(func (;25;) (type 29) (canon lift (core func 54) (memory 0) (realloc 10) string-encoding=utf8 (post-return 55)))
(component (;0;)
(alias outer 1 29 (type (;0;)))
(import "import-handle-request" (func (;0;) (type 0)))
The "import-handle-request" has an aliased type to a non-exported type (the record type "28"). Would you expect this to be fixed already? Perhaps I'm missing some transitive dependency on wasm-tools somewhere?
Hmm... So it does indeed look like I was missing a transitive dependency on wasm-tools somewhere. I've updated that, and now I'm running into a different issue...
Investigating...
I'm now getting the following error:
missing component instantiation argument named `import-type-method` (at offset 0x247c7f)
But it seems fine to me when inspecting the wat
(type (;18;) (enum "get" "post" "put" "delete" "patch" "head" "options"))
;; ....
(instance (;10;) (instantiate 0
(with "import-func-handle-request" (func 25))
(with "import-type-method" (type 18)) ;; It seems to think this is missing
(with "import-type-uri" (type 19))
(with "import-type-headers" (type 21))
(with "import-type-params" (type 22))
(with "import-type-body" (type 23))
(with "import-type-request" (type 25))
(with "import-type-http-status" (type 26))
(with "import-type-response" (type 28))
)
)
(as a side note, this seems to only be happening when programmatically instantiating a component not when running wasm-tools validate
)
I think this too is a dangling dependency that has an old wasm-tools version :weary:
Please ignore everything above. I finally was able to update everything to the latest version of wasm-tools (including wit-bindgen, wasmtime, etc.), and it works!
Closed https://github.com/bytecodealliance/wasm-tools/issues/986
This may have uncovered an issue with wasm-compose though...
Nevermind. So many moving parts, but I got it working.
@Alex Crichton sorry for the ping again. This exercise has shown me that the full stack of tools seems to be working with the latest version of wasm-tools. Do we have a timeline for cutting a new release? I'd be happy to help update the world when the new version lands.
Heh well I'm glad at least everything's working! I'll do a release today for all the various crates
https://github.com/bytecodealliance/wasm-tools/pull/1002
I can send PRs to cargo-component, wit-bindgen, etc. once this lands if that would be helpful
it would indeed!
ok published!
First bump: https://github.com/bytecodealliance/wit-bindgen/pull/562
Tests are failing. I’ll update them in a bit
Pushed a fix. Hopefully that's enough to get the tests passing.
Tests pass :check:
Once you caught a new release for wit-bindgen I’ll update cargo-component.
ok publishing now
Thanks @Ryan Levick (rylev) for sharing and contributing. That's awesome and tons of useful info.
Hello everyone i have a question about wasi-nn i'm reading the code done by @Mossaka (Joe) @Radu Matei and i found that under crates/wasi-nn/rust/src/ you will find a generated.rs and it says inside this file : "DO NOT EDIT , To regenerate this file run the crates/witx-bindgen
command"
my question is how to generate this file again ? because i want to add a new backend like onnxruntime to wasi-nn and i don't know how to generate this file from witx specification can anyone help what cmd line i should write any resources anything ?
Last updated: Dec 23 2024 at 12:05 UTC