Ryan Levick (rylev) (Apr 19 2023 at 19:33): Ryan Levick (rylev) (Apr 19 2023 at 19:33):@Alex Crichton re: your request for a reproduction of the wasm component issue:
broken-composed.wasm
Alex Crichton (Apr 19 2023 at 19:53):@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
Alex Crichton (Apr 19 2023 at 20:17):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
Alex Crichton (Apr 19 2023 at 20:25):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.
Alex Crichton (Apr 19 2023 at 20:26):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
Ryan Levick (rylev) (Apr 19 2023 at 21:04):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:
Alex Crichton (Apr 19 2023 at 21:11):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.
Ryan Levick (rylev) (Apr 19 2023 at 21:15):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
Ryan Levick (rylev) (Apr 19 2023 at 21:15):Thanks for the explainer!
Alex Crichton (Apr 19 2023 at 21:18):If you've got the original source or core wasm modules I can test out a solution too
Alex Crichton (Apr 19 2023 at 21:18):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
Alex Crichton (Apr 19 2023 at 21:19):but re-running wit-component should be all that's necessary
Peter Huene (Apr 19 2023 at 21:19):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
Ryan Levick (rylev) (Apr 19 2023 at 21:46):@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?
Alex Crichton (Apr 19 2023 at 21:56):those look like they're both already components
Peter Huene (Apr 19 2023 at 21:59):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)
Alex Crichton (Apr 19 2023 at 22:00):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
Peter Huene (Apr 19 2023 at 22:00):a hack would be to manually depend your project on the bindings crate and do a cargo build --target wasm32...
Peter Huene (Apr 19 2023 at 22:02):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)
Ryan Levick (rylev) (Apr 19 2023 at 22:07):Luckily I'm using macOS so the core module was in the deps folder
Ryan Levick (rylev) (Apr 19 2023 at 22:07):I don't have easy access to the adapter core module but I can get that too if it makes life easier
Alex Crichton (Apr 20 2023 at 00:25):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
Alex Crichton (Apr 20 2023 at 18:17):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?
Ryan Levick (rylev) (Apr 21 2023 at 15:04):@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.
Ryan Levick (rylev) (Apr 21 2023 at 15:49):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).
Ryan Levick (rylev) (Apr 21 2023 at 15:50):I'm getting this error: func not valid to be used as import (at offset 0x24ae86)
Ryan Levick (rylev) (Apr 21 2023 at 15:50):Here's the component in question: component.wasm
Ryan Levick (rylev) (Apr 21 2023 at 15:50):(note this is before any composition - it's just during "componentization" - when we are taking core modules and making them components)
Ryan Levick (rylev) (Apr 21 2023 at 15:52):It would probably help if we could make validate_and_register_named_types return what is not valid instead of returning just a bool
Alex Crichton (Apr 21 2023 at 15:54):hm ok that may be a further bug, I'll try to dig in in a bit
Ryan Levick (rylev) (Apr 21 2023 at 15:57):Looks like there is a record definition for a "request" type that might not be exported?
Ryan Levick (rylev) (Apr 21 2023 at 15:59):At some point this check returns false.
Ryan Levick (rylev) (Apr 21 2023 at 16:03):(deleted)
Ryan Levick (rylev) (Apr 22 2023 at 09:24):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).
Ryan Levick (rylev) (Apr 27 2023 at 10:17):@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?
Ryan Levick (rylev) (Apr 27 2023 at 11:42):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...
Ryan Levick (rylev) (Apr 27 2023 at 11:42):Investigating...
Ryan Levick (rylev) (Apr 27 2023 at 11:47):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)
Ryan Levick (rylev) (Apr 27 2023 at 12:06):I think this too is a dangling dependency that has an old wasm-tools version :weary:
Ryan Levick (rylev) (Apr 27 2023 at 12:38):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!
Ryan Levick (rylev) (Apr 27 2023 at 12:39):Closed https://github.com/bytecodealliance/wasm-tools/issues/986
Ryan Levick (rylev) (Apr 27 2023 at 12:41):This may have uncovered an issue with wasm-compose though...
Ryan Levick (rylev) (Apr 27 2023 at 12:58):Nevermind. So many moving parts, but I got it working.
Ryan Levick (rylev) (Apr 27 2023 at 13:04):@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.
Alex Crichton (Apr 27 2023 at 14:06):Heh well I'm glad at least everything's working! I'll do a release today for all the various crates
Alex Crichton (Apr 27 2023 at 14:14):https://github.com/bytecodealliance/wasm-tools/pull/1002
Ryan Levick (rylev) (Apr 27 2023 at 14:38):I can send PRs to cargo-component, wit-bindgen, etc. once this lands if that would be helpful
Alex Crichton (Apr 27 2023 at 14:38):it would indeed!
Alex Crichton (Apr 27 2023 at 17:18):ok published!
Ryan Levick (rylev) (Apr 27 2023 at 17:44):First bump: https://github.com/bytecodealliance/wit-bindgen/pull/562
Ryan Levick (rylev) (Apr 27 2023 at 18:18):Tests are failing. I’ll update them in a bit
Ryan Levick (rylev) (Apr 27 2023 at 18:58):Pushed a fix. Hopefully that's enough to get the tests passing.
Ryan Levick (rylev) (Apr 27 2023 at 19:31):Tests pass :check:
Ryan Levick (rylev) (Apr 27 2023 at 19:57):Once you caught a new release for wit-bindgen I’ll update cargo-component.
Alex Crichton (Apr 27 2023 at 20:20):ok publishing now
Mossaka (Joe) (Apr 28 2023 at 22:26):Thanks @Ryan Levick (rylev) for sharing and contributing. That's awesome and tons of useful info.
Khelifa Saif eddine (Jul 10 2023 at 20:19):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: Oct 23 2024 at 20:03 UTC