Skip to content

Check all segment bounds beforehand #399

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 2 commits into from
Jan 19, 2017
Merged

Check all segment bounds beforehand #399

merged 2 commits into from
Jan 19, 2017

Conversation

rossberg
Copy link
Member

...and add tests that memories/tables remain unchanged in case of error. Addresses WebAssembly/design#897

@jfbastien
Copy link
Member

I would rather check bounds as we apply segments, instead of up front. IMO it's not a useful property to purposefully apply segments and rely on halfway failure, but it's faster to check each bounds while applying them versus doing two passes. I'd therefore do a single pass, and fail halfway. This optimizes for the useful case (everything succeeds) and leaves the useless case in a well-specified state.

@rossberg
Copy link
Member Author

@jfbastien, I agree that's simpler, but if you prefer that semantics then I suggest you create a PR to change JS.md.

jfbastien added a commit to WebAssembly/design that referenced this pull request Dec 12, 2016
@jfbastien
Copy link
Member

Done: WebAssembly/design#902

@gahaas
Copy link
Collaborator

gahaas commented Jan 19, 2017

lgtm

@rossberg
Copy link
Member Author

@jfbastien, are you fine with landing this for the time being? Until we have consensus for changing the design the spec should better match the current design.

@jfbastien
Copy link
Member

@rossberg-chromium with that rationale we could as well merge WebAssembly/design#902, no? I'm OK revisiting later, so feel free to merge as long as we discuss this.

@rossberg
Copy link
Member Author

@jfbastien, thanks. There doesn't seem to be consensus on the broader change from #902, but the overriding goal is to get design and spec into sync. We can always adjust the spec when #902 evolves.

@rossberg rossberg merged commit 9e0892d into master Jan 19, 2017
@rossberg rossberg deleted the init-checks branch January 19, 2017 16:55
ngzhian pushed a commit to ngzhian/spec that referenced this pull request Nov 4, 2021
dhil pushed a commit to dhil/webassembly-spec that referenced this pull request Oct 3, 2023
Fix WebAssembly#399 and a couple of other small bugs with conversion to/from JS numbers.
raoxiaojia pushed a commit to WasmCert/spec that referenced this pull request Apr 29, 2025
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