add option for reference-types #124
Conversation
|
I tried with my own code and could successfully initialize a Module that errors without the option. Since there is nothing actually implemented, I set the default to false and keep the feature disabled for fuzzers, I hope that is correct |
fitzgen
left a comment
fitzgen
left a comment
There was a problem hiding this comment.
Can you extend the wasm_validate function to disallow the following reference-types instructions:
ref.nullref.is_null- the typed variant of
select ref.functable.gettable.settable.sizetable.growtable.filltable.inittable.copy
?
And also please add the following tests:
- a table-modifying instruction is used when reference-types are enabled, but the wasm is still rejected
- reference-types are enabled and an instruction encoding that requires reference-types is used for an instruction which doesn't actually mutate tables or manipulate references, and therefore wizer succeeds (for example, a
call_indirectusing a non-zero table index)
Thanks!
Gentle
force-pushed
the
enable_reference_types
branch
from
b1921f0 to
5fca98b
Compare
|
I hope I added everything |
fitzgen
left a comment
fitzgen
left a comment
There was a problem hiding this comment.
Thanks! Two minor things and then we can merge this.
|
I hope I did it right, this seems to use the new call_indirect and the second table |
|
as expected the test now goes red if I disable reference-types, so it uses the right call now :) Edit: actually that statement is not neccessarily true, with reference types disabled it already rejects the existence of a second table, but still, the fact that call_indirect finds the second table means it should be the new call_indirect |
|
@fitzgen Is this still waiting on changes before it can be merged and released? We'd like to upgrade Rust but Wizer not supporting Rust 1.81+ wasm output is a blocker. |
|
I'd also like to depend on the npm packages again soon, is there anything else that would need to happen before a new wizer release? |
|
The tests are currently failing on Windows. @Gentle, is that something you can look into to unblock landing this PR? |
|
looking into it as soon as I get rust installed on the only PC with windows ;) |
|
@tschneidereit rebasing onto #127 (ie. updating wasmtime) fixes the tests on Windows, the crash was deep in wasmtime code which led me to updating first before debugging does anything speak against just bumping wasmtime instead of debugging an apparently fixed bug? |
|
I think updating wasmtime makes a lot of sense, yes. @fitzgen, any concerns? |
|
This can be rebased now to get CI passing since the wasmtime update merged -- thanks! |
only enables the wasmtime feature, which allows initializing Modules that were compiled with newer llvm versions but don't actually use reference_types Disabled by default
Co-authored-by: Nick Fitzgerald <fitzgen@gmail.com>
Gentle
force-pushed
the
enable_reference_types
branch
from
d2b1c77 to
27e0fd2
Compare
|
One thing I'm realizing belatedly: should this be enabled by default? I can't really think of a scenario in which I wouldn't want to enable it, since it's increasingly required to make any content work with Wizer at all. |
|
I thought the same, there is never a reason to have this set to false, realistically. I would love to change the default |
|
Happy to have a follow up to enable it and bulk-memory by default. |
* add option for reference_types only enables the wasmtime feature, which allows initializing Modules that were compiled with newer llvm versions but don't actually use reference_types Disabled by default * Update src/lib.rs Co-authored-by: Nick Fitzgerald <fitzgen@gmail.com> * reject reference-types related instructions * add setter for wasm_reference_types * bail instead of unreachable * tests for reference-types still rejecting tyble modifying instructions * better error messages * test indirect call with reference_types enabled * same docstring for library and cli * (hopefully) actually use the new call_indirect --------- Co-authored-by: Gentle <ramon.klass@gmail.com> Co-authored-by: Nick Fitzgerald <fitzgen@gmail.com>
* add option for reference_types only enables the wasmtime feature, which allows initializing Modules that were compiled with newer llvm versions but don't actually use reference_types Disabled by default * Update src/lib.rs Co-authored-by: Nick Fitzgerald <fitzgen@gmail.com> * reject reference-types related instructions * add setter for wasm_reference_types * bail instead of unreachable * tests for reference-types still rejecting tyble modifying instructions * better error messages * test indirect call with reference_types enabled * same docstring for library and cli * (hopefully) actually use the new call_indirect --------- Co-authored-by: Gentle <ramon.klass@gmail.com> Co-authored-by: Nick Fitzgerald <fitzgen@gmail.com>
4 participants
only enables the wasmtime feature, which allows initializing Modules that were compiled with newer llvm versions but don't actually use reference-types