V8 can't run tests due to binary version mismatch

[eholk]

We recently updated binaryen to a version that generates Wasm 0xC binaries. The version of V8 downloaded from http://wasm-stat.us unfortunately is still 0xB. We need to update V8.

I tried updating to a more recent snapshot, but that doesn't appear to be updated yet either.

It might be better to fetch and build V8 on our own. This would let us make sure we're running on a version we support, and also let us run V8 on other platforms as well.

Alternatively, maybe we should add an environment variable or something that points to a suitable d8, and leave finding an appropriate one up to person running the tests.

[eholk]

Oh, it looks like the build paths changed sometime. Once I got those fixed, I got a d8 that supports 0xC.

Now I'm getting validation errors, which are similar to the errors in #47.

[lqd]

What kind of validation errors, could you post them somewhere maybe ?
This is where having CI running the v8 tests would be very valuable on this project!

[eholk]

Here's the output from one of the tests that's failing: https://gist.github.com/eholk/f2748e1b0c98b4e39a56356b25e9aaea

That was from my work-in-progress V8 update branch. I've pushed the code to https://github.com/eholk/mir2wasm/tree/0xc if you want to see what's changed.

The issue seems to be that we don't have the right number of values on the stack at certain times. This in turn seems to be related to our handling of the existence or not of return values from functions.

When I read over the .wast code in question, everything looks okay to me. I wonder if binaryen or wabt are missing some of the control flow? If so, we might be able to work around this with clever placements of unreachable opcodes.

[eholk]

Also, CI is a great idea! I thought it wasn't really possible on this project for some reason, but it occurs to me I had no reason to think that and that this should be straightforward to do with Travis CI.

@brson - Do you think you could set us up with Travis (or something similar)? Alternatively, if you give me admin access on this repo then I could do it too.

[lqd]

hum.
$main:

(local $0 i32)
...
(local $12 i32)
(local $13 i32)

errors:

[wasm-validator error in function $main] 1 != 0: set_local type must match function, on 
[none] (set_local $12
  [none] (get_local $0)
)
[wasm-validator error in function $main] 1 != 0: set_local type must match function, on 
[none] (set_local $13
  [none] (get_local $12)
)

they're all i32s, does this type error look weird to you as well ?

[eholk]

I must be misinterpreting the error message, because my interpretation is blatantly contradicted by reality.

To me this means the validator is expecting $main to return a single value, but that the set_local does not have a return value, so it doesn't match the expectation.

There are a couple of problems. First, the type for $main does not have a return value. Second, it seems like for set_local to be the return value it should be in tail position, which it is not.

@kripken - Am I misunderstanding the error message?

[lqd]

@eholk I've tried running the wast in the gist with the wasm-shell from master and there are no validation errors. Are those errors in the gist from v8 or binaryen ?

[kripken]

About

 [wasm-validator error in function $main] 1 != 0: set_local type must match function, on 
[none] (set_local $12
  [none] (get_local $0)
)

The first question is why the get_local has type none. BinaryenGetLocal receives the type as the last parameter - perhaps we are giving it none or something else invalid?

[lqd]

@kripken ooooh, so that's why in-memory validation can be different from "text" validation ? (I was wondering what those new [type] were as well)

I guess most/all node creation related to locals, with a none type, are invalid in-memory but valid textually.

Another thing to add on our list to fix :)

[kripken]

It could be invalid in memory but valid textually, if we don't validate those, which is indeed a bug in binaryen, I now see. I'll add validation.