Skip to content

Inconsistency of implementation-defined limits #1195

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

Merged
merged 3 commits into from
May 25, 2020
Merged

Conversation

gahaas
Copy link
Collaborator

@gahaas gahaas commented May 20, 2020

This PR resolves an inconsistency of implementation-defined limits in the js-api specification, and execution limits in the core specification. The core specification requires under execution limits that implementations throw a runtime error if tables or memories grow beyond an implementation-defined limit. The js-api specification, however, required a compile error if the module defines limits beyond the implementation-defined limits.

This PR resolves this inconsistency by adjusting the js-api specification. For the maximum memory size and maximum table size, a RuntimeError is now required instead of a CompileError.

This PR resolves an inconsistency of implementation-defined limits in the js-api specification, and execution limits in the core specification. The core specification requires in https://webassembly.github.io/spec/core/appendix/implementation.html#execution that implementations throw a runtime error if tables or memories grow beyond an implementation-defined limit. The js-api specification, however, required a compile error if the module defines limits beyond the implementation-defined limits.

This PR resolves this inconsistency by adjusting the js-api specification. For the maximum memory size and maximum table size, a RuntimeError is now required instead of a CompileError.
Copy link
Collaborator

@Ms2ger Ms2ger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

@rossberg rossberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thanks!

@@ -1121,8 +1121,7 @@ Note: ECMAScript doesn't specify any sort of behavior on out-of-memory condition

The WebAssembly core specification allows an implementation to define limits on the syntactic structure of the module.
While each embedding of WebAssembly may choose to define its own limits, for predictability the standard WebAssembly JavaScript Interface described in this document defines the following exact limits.
An implementation must reject a module that exceeds these limits with a {{CompileError}}.
In practice, an implementation may run out of resources for valid modules below these limits.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we should keep this sentence here as well, since it may apply even to static limits.

@rossberg
Copy link
Member

Thanks!

@rossberg rossberg merged commit a7a1856 into master May 25, 2020
@rossberg rossberg deleted the gahaas-patch-1 branch May 25, 2020 10:47
mdesharnais pushed a commit to mdesharnais/spec that referenced this pull request Jun 2, 2020
This PR resolves an inconsistency of implementation-defined limits in the js-api specification, and execution limits in the core specification. The core specification requires in https://webassembly.github.io/spec/core/appendix/implementation.html#execution that implementations throw a runtime error if tables or memories grow beyond an implementation-defined limit. The js-api specification, however, required a compile error if the module defines limits beyond the implementation-defined limits.

This PR resolves this inconsistency by adjusting the js-api specification. For the maximum memory size and maximum table size, a RuntimeError is now required instead of a CompileError.
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 this pull request may close these issues.

3 participants