[wasm] Inspect right control frames for unreachable flag

[wasm] Inspect right control frames for unreachable flag

We were looking at the unreachable flag or stack_depth of the target frame
instead of the current one in a couple of places (most notably BreakTo).
This change fixes these bugs and makes us pass the latest spec tests for
br_table validation. Also need to ensure that br_table targets have consistent
types, which is not implied if the stack is polymorphic.

R=titzer@chromium.org
BUG=6001
Review-Url: https://codereview.chromium.org/2696813002
Cr-Commit-Position: refs/heads/master@{#43250}
Committed: https://chromium.googlesource.com/v8/v8/+/e2b83fbbfdf403903d3b4e5bd9276d38151789c3

[titzer, 2017-02-14 13:30:27]

Can we also add a cctest as a regression test for this case?

https://codereview.chromium.org/2696813002/diff/1/src/wasm/function-body-deco...
File src/wasm/function-body-decoder.cc (right):

https://codereview.chromium.org/2696813002/diff/1/src/wasm/function-body-deco...
src/wasm/function-body-decoder.cc:978: if (control_.back().unreachable) {
At first glance this logic seems best to put into BreakTo itself.

https://codereview.chromium.org/2696813002/diff/1/src/wasm/function-body-deco...
src/wasm/function-body-decoder.cc:984: error(pos, pos, "inconsistent arity in
br_table taret %d"
s/taret/target/

[rossberg, 2017-02-14 15:11:17]

Added redundant tests from spec suite.

And since it was the third time that forgetting a spread has cost me half an
hour of misguided debugging, added sanity checking of the input array to the
builder's addBody method. Guess what, a couple of other tests were broken, too.

Also, discovered that we need to check consistent label arity for br_table even
in reachable code, to catch examples like

(block (block i32 ... (br_table 0 1 (i32.const 0) (i32.const 0)) ...) ...)

https://codereview.chromium.org/2696813002/diff/1/src/wasm/function-body-deco...
File src/wasm/function-body-decoder.cc (right):

https://codereview.chromium.org/2696813002/diff/1/src/wasm/function-body-deco...
src/wasm/function-body-decoder.cc:978: if (control_.back().unreachable) {
On 2017/02/14 13:30:27, titzer wrote:
> At first glance this logic seems best to put into BreakTo itself.

Unfortunately, no, because it compares across targets (with the label type
remembered in `merge`).

https://codereview.chromium.org/2696813002/diff/1/src/wasm/function-body-deco...
src/wasm/function-body-decoder.cc:984: error(pos, pos, "inconsistent arity in
br_table taret %d"
On 2017/02/14 13:30:27, titzer wrote:
> s/taret/target/

Done.

[titzer, 2017-02-16 12:57:52]

LGTM after online discussion

[titzer, 2017-02-16 12:58:13]

On 2017/02/16 12:57:52, titzer wrote:
> LGTM after online discussion

offline rather

[rossberg, 2017-02-16 13:37:50]

The CQ bit was checked by rossberg@chromium.org

[commit-bot: I haz the power, 2017-02-16 13:37:53]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-16 13:42:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-16 13:42:07]

Try jobs failed on following builders:
  v8_linux64_gyp_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng/build...)

[rossberg, 2017-02-16 13:49:20]

The CQ bit was checked by rossberg@chromium.org

[rossberg, 2017-02-16 13:49:21]

The patchset sent to the CQ was uploaded after l-g-t-m from titzer@chromium.org
Link to the patchset: https://codereview.chromium.org/2696813002/#ps40001
(title: "Avoid conversion warning")

[commit-bot: I haz the power, 2017-02-16 13:49:24]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-16 14:12:02]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1487252960870100,
"parent_rev": "8990399dc7c2f36ba4f566a415a0823d229dff21", "commit_rev":
"e2b83fbbfdf403903d3b4e5bd9276d38151789c3"}

[commit-bot: I haz the power, 2017-02-16 14:12:07]

Description was changed from

==========
[wasm] Inspect right control frames for unreachable flag

We were looking at the unreachable flag or stack_depth of the target frame
instead of the current one in a couple of places (most notably BreakTo).
This change fixes these bugs and makes us pass the latest spec tests for
br_table validation. Also need to ensure that br_table targets have consistent
types, which is not implied if the stack is polymorphic.

R=titzer@chromium.org
BUG=
==========

to

==========
[wasm] Inspect right control frames for unreachable flag

We were looking at the unreachable flag or stack_depth of the target frame
instead of the current one in a couple of places (most notably BreakTo).
This change fixes these bugs and makes us pass the latest spec tests for
br_table validation. Also need to ensure that br_table targets have consistent
types, which is not implied if the stack is polymorphic.

R=titzer@chromium.org
BUG=

Review-Url: https://codereview.chromium.org/2696813002
Cr-Commit-Position: refs/heads/master@{#43250}
Committed:
https://chromium.googlesource.com/v8/v8/+/e2b83fbbfdf403903d3b4e5bd9276d38151...
==========

[commit-bot: I haz the power, 2017-02-16 14:12:08]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/v8/v8/+/e2b83fbbfdf403903d3b4e5bd9276d38151...

[bradn, 2017-02-22 15:20:11]

Description was changed from

==========
[wasm] Inspect right control frames for unreachable flag

We were looking at the unreachable flag or stack_depth of the target frame
instead of the current one in a couple of places (most notably BreakTo).
This change fixes these bugs and makes us pass the latest spec tests for
br_table validation. Also need to ensure that br_table targets have consistent
types, which is not implied if the stack is polymorphic.

R=titzer@chromium.org
BUG=

Review-Url: https://codereview.chromium.org/2696813002
Cr-Commit-Position: refs/heads/master@{#43250}
Committed:
https://chromium.googlesource.com/v8/v8/+/e2b83fbbfdf403903d3b4e5bd9276d38151...
==========

to

==========
[wasm] Inspect right control frames for unreachable flag

We were looking at the unreachable flag or stack_depth of the target frame
instead of the current one in a couple of places (most notably BreakTo).
This change fixes these bugs and makes us pass the latest spec tests for
br_table validation. Also need to ensure that br_table targets have consistent
types, which is not implied if the stack is polymorphic.

R=titzer@chromium.org
BUG=6001

Review-Url: https://codereview.chromium.org/2696813002
Cr-Commit-Position: refs/heads/master@{#43250}
Committed:
https://chromium.googlesource.com/v8/v8/+/e2b83fbbfdf403903d3b4e5bd9276d38151...
==========