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] Ambiguous and inconsistent handling of implementation limits #1863

Open
bvisness opened this issue Jan 13, 2025 · 2 comments
Open

Comments

@bvisness
Copy link
Contributor

bvisness commented Jan 13, 2025

The JS API's implementation limits were introduced in #873 and modified in #1195 to include runtime limits. However, the JS API spec's current handling of implementation limits still has many issues:

  • Lack of clarity on which limits should be checked during WebAssembly.validate, new WebAssembly.Module (and other "compilation" paths), and new WebAssembly.Instance (and other "instantiation" paths).
  • Lack of clarity on which limits are enforced when doing new WebAssembly.Memory, new WebAssembly.Table, or otherwise constructing a wasm object directly.
  • Lack of clarity on which errors are thrown for grow operations (the method specs say RangeError, but the Limits section says RuntimeError).
  • Redundant and imprecise conditions throughout the limits section:
    • The CompileError section has both "The maximum size of a table is 10,000,000" and "The maximum number of table entries in any table initialization is 10,000,000". What is the difference, and how can any "table initialization" exist at compile time?
    • The "maximum size of a table" condition is present in both the CompileError and RuntimeError sections, leading to confusion about which error should be thrown.
    • What does "size" mean anyway? Are we referring to the table type's initial (min) size? To the table type's max size? To the actual runtime size of the table?
  • Limits that are impossible to exceed (e.g. "The maximum number of pages of a memory is 65,536").

This has led to confusion in these other issues:

And this is actually implemented quite differently across engines:

  • V8 treats modules with oversize tables as invalid and will not compile them, but SpiderMonkey will compile the module and fail on instantiation.
  • V8 throws RangeError when constructing an oversize table directly, but SpiderMonkey throws RuntimeError.

I'm sure there are other inconsistencies too.

Overall I would suggest the following changes:

  • Remove the prescribed CompileError and RuntimeError from the Implementation Limits section, and instead describe each list as "syntactic limits" and "execution limits" in keeping with the core spec. Move the specific types of errors into the descriptions of the methods that can trigger them.
    • Update the "compile a WebAssembly module" section to explicitly refer to the syntactic limits and return error if they are violated. This avoids the ambiguity of whether a valid wasm module is actually "valid" under the JS API (as WebAssembly.validate triggers this compilation anyway).
    • Change Memory.grow and Table.grow to explicitly refer to the execution limits and throw RangeError like they do for any other kind of failed grow. (To my knowledge, a grow is the only way to trigger these runtime conditions.)
  • Remove the line about "table initialization".
  • Remove limits asserting that the size of a 32-bit memory must be less than 4GiB.
  • Update any notes about memory or table size in the syntactic limits section to explicitly refer to initial size.
  • Update any notes about memory or table size in the execution limits section to explicitly refer to the size of a memory or table instance's data.

I can put this work together at some point if people agree that this is the way to go.

cc @backes, @evicy

@Ms2ger
Copy link
Collaborator

Ms2ger commented Jan 14, 2025

I imagine something like this would make sense. I probably won't have time in the near future to investigate the details of what should be checked where. We should also make sure to extend the test suite.

@backes
Copy link
Member

backes commented Jan 14, 2025

Thanks, Ben, for compiling this surprisingly large list of problems. I agree to everything, so feel free to go ahead and propose the actual change.

About the "table initialization" line: Is this supposed to limit the size of element segments? It would still make sense to limit their size, because table.init only reads a subsection of them. So I would propose to replace this by "The maximum size of any element segment is 10,000,000.".

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

3 participants