Skip to content
This repository was archived by the owner on Jun 16, 2020. It is now read-only.

Fix OperatorValidator for floating-point (non-deterministic) opcodes #203

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

camilbancioiu
Copy link

@camilbancioiu camilbancioiu commented Mar 5, 2020

This pull request fixes an inverted if in check_non_deterministic_enabled(), which was allowing floating-point opcodes even when deterministic_only was set to true.

Also renames check_non_deterministic_enabled() to check_deterministic_only(), to make it clearer.

On the master branch, Cargo.toml defines the deterministic feature, which seems to be enabled during the CI tests (is it?). The test which parses the file float_memory.0.wasm doesn't expect an error, and fails with the fixed OperatorValidator. Is the test correct?

camilbancioiu and others added 5 commits December 4, 2019 11:36
…k_deterministic_only(), and fix it to return Err properly
Rename OperatorValidator.check_non_deterministic_enabled() into .check_deterministic_only(), and fix it to return Err properly
@yurydelendik
Copy link
Collaborator

On the master branch, Cargo.toml defines the deterministic feature, which seems to be enabled during the CI tests (is it?)

It is defined, but not enabled (as default features)

The test which parses the file float_memory.0.wasm doesn't expect an error, and fails with the fixed OperatorValidator. Is the test correct?

The test is correct.

@camilbancioiu
Copy link
Author

@yurydelendik Thank you for your quick reply. I'll look deeper into this, it might be an issue somewhere else (we're using Wasmer, which in turn relies on Wasmparser).

@alexcrichton
Copy link
Member

This repository has now moved to a subfolder within https://github.com/bytecodealliance/wasm-tools. This current repository will be marked as archived in the future after we've sorted out active PRs, so if you'd like to still merge this then it will need to be opened as a new PR against the new repository. If you have any trouble in doing so, though, please let me know and I can try to help out. now

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

Successfully merging this pull request may close these issues.

3 participants