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

[js-api] Memory and table allocation pre-conditions not validated #1792

Open
bvisness opened this issue Aug 23, 2024 · 3 comments · May be fixed by #1793
Open

[js-api] Memory and table allocation pre-conditions not validated #1792

bvisness opened this issue Aug 23, 2024 · 3 comments · May be fixed by #1793

Comments

@bvisness
Copy link
Contributor

The mem_alloc and table_alloc operations in the wasm embedding spec have pre-conditions asserting that the memory and table types are valid. The JS API spec does not actually ensure that these pre-conditions are met, which means that it is not clear which kind of error should be thrown if the type is invalid.

For example, mem_alloc has a pre-condition that the memtype is valid, which means that the limits must be valid within range 2^16. The JS API definition for Memory(descriptor) asserts that initial <= maximum, but not that initial <= 2^16 and maximum <= 2^16. What type of error should be thrown if the given memory type is invalid?

(table_alloc technically has the same problem, although EnforceRange makes it impossible to express an out-of-bounds size.)

@Ms2ger
Copy link
Collaborator

Ms2ger commented Aug 26, 2024

Thanks for pointing this out. Have you tested what browsers do?

@bvisness
Copy link
Contributor Author

Chrome, Firefox, and Safari all seem to throw RangeError, so I guess the best course of action would just be to add a line explicitly stating this in the JS API spec. I should be able to make a PR for this fairly easily in the next few days.

@Ms2ger
Copy link
Collaborator

Ms2ger commented Aug 26, 2024

Thanks!

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

Successfully merging a pull request may close this issue.

2 participants