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.

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

wasm_encoder does not increase TypeSection num_added length proportionally to amount of types in recursive type #1393

Open
birbe opened this issue Jan 29, 2024 · 2 comments

Comments

@birbe
Copy link

birbe commented Jan 29, 2024

Is this the correct behavior? I'm generating a module in Rust but it does so incorrectly as I have a recursive type get added with multiple entries, and so .len() on the TypeSection is desynchronized. The type indices I use after the fact are then invalid and pointing to the wrong thing. pseudo-code-ish example of what I mean:

let types = TypeSection::new();
let funcs = FunctionSection::new();

types.rec([SubType { .. }, SubType { .. }]);

let func_type_id = types.len(); //Expected: 2; Got: 1
types.func([ValType::I32], [ValType::I32]);
funcs.function(func_type_id); //Invalid
@birbe birbe changed the title wasm_encoder does not increase type length according to amount of types specified in recursive type wasm_encoder does not increase TypeSection num_added length proportionally to amount of types in recursive type Jan 29, 2024
@fitzgen
Copy link
Member

fitzgen commented Jan 29, 2024

This is expected. If the type section's encoding says 5, then there can be five different rec groups that each have, say, ten items within them. In some sense a whole rec group is a single type, it just happens to introduce multiple entries into the types index space. This is actually pretty similar to how types canonicalization works. Anyways, wasm-encoder must keep track of the encoding index in order to encode properly, so if you need to keep track of the Wasm types index space (used to be 1:1 but now different) then you should implement your own counter and not rely on wasm-encoder's.

Alternatively, I think it could make sense to have a second counter that reflects the Wasm type index space, and exists just as a convenience for library users. If you want to make a PR, I could review it.

@birbe
Copy link
Author

birbe commented Jan 29, 2024

I see, thanks for the speedy response! I might make a PR soon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants