update br_table validation rule + enforce module validity in spectest-interp #1895
Conversation
sbc100
left a comment
There was a problem hiding this comment.
Regarding the br_table validation rules I think we can but fairly sure its good if the spec tests all path.
lgtm % comment.
BTW, why not include modules in the tally.. then I would expect the see the overall number of tests go up in most files.. which would tell us just how much coverage we've been missing out on.
|
Done. If it's any solace, the corresponding wasm2c spec tests (for the same wast files) have been enforcing module validity all along, and that goes through basically the same codepath, so it hasn't exactly been missing out on this test coverage completely. (And of course the absence of an error message in the .txt file was also test coverage -- there were only two tests that had this issue, and now it's going to be zero.) |
sbc100
left a comment
There was a problem hiding this comment.
Wow.. we have quite a lack coverage there it seems.
br_tablewere relaxed after reference-types (leading to a test error message when wabt tried to enforce the old rule, but not a test failure because of the above)This PR updates the
br_tablevalidation and then modifiesspectest-interpto require success on "module" and "register" commands. (The only other test with this problem was just fixed in #1894.)(Disclaimer: I'm not 100% positive that this correctly captures the new
br_tablevalidation algorithm -- it seems somehow too simple. But it does seem to be what the appendix (https://webassembly.github.io/spec/core/appendix/algorithm.html#validation-of-opcode-sequences) is asking for, and it does pass all of the core spec tests.)