Skip to content

Conversation

dschuff
Copy link
Member

@dschuff dschuff commented Sep 28, 2016

Fixes #708

@dschuff dschuff changed the title s2wasm: Do not add drops for non-void values s2wasm: Do not add drops for void values Sep 28, 2016
@kripken
Copy link
Member

kripken commented Sep 28, 2016

In #708 we also saw that the type of exit looks wrong (function type is void, but it has a return statement with a value) - is that an unrelated problem to the one this PR fixes?

Separately, can we enable drop validation with this PR? That is, replace the code in src/wasm-validator.h for visitDrop(), which currently checks an env var, with something like

void visitDrop(Drop* curr) {
  shouldBeTrue(isConcreteWasmType(curr->value->type) || curr->value->type == unreachable, curr, "can only drop a valid value");
}

@dschuff
Copy link
Member Author

dschuff commented Sep 28, 2016

I think the exit type being wrong is a problem with the .s file and not with s2wasm. Also there's one further failure in the wasm-opt tests before we can turn on drop validation:

.. vacuum.wast
executing:  bin/wasm-opt --vacuum split.wast --print
[wasm-validator error in function $if-drop] unexpected false: Dropping a none value, on 
(drop
  (if
    (call $if-drop)
    (call $int)
    (br $out)
  )
)
[wasm-validator error in function $if-drop] unexpected false: Dropping a none value, on 
(drop
  (if
    (call $if-drop)
    (br $out)
    (call $int)
  )
)
Fatal: error in validating input

@kripken
Copy link
Member

kripken commented Sep 28, 2016

Ok, thanks, lgtm. I'll investigate that drop validation problem.

@dschuff dschuff merged commit 0938940 into master Sep 28, 2016
@dschuff dschuff deleted the bad-drops branch September 29, 2016 22:19
@kripken kripken mentioned this pull request Sep 30, 2016
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.

2 participants