Bug 1324032 - BaldrMonkey: Require non-fallthrough instructions to be followed by end or else. r=luke, a=jcristau
authorDan Gohman <sunfish>
Fri, 20 Jan 2017 16:27:00 -0500
changeset 353735 d1138edcbc38cd3011b93ac8fbcd8a27c1230db6
parent 353734 32fa32a616462f37394053f7eb73d070a69640d0
child 353736 3209927fdbf97ab394f4506d10546a5debce1015
push id6795
push userjlund@mozilla.com
push dateMon, 23 Jan 2017 14:19:46 +0000
treeherdermozilla-esr52@76101b503191 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersluke, jcristau
bugs1324032
milestone52.0a2
Bug 1324032 - BaldrMonkey: Require non-fallthrough instructions to be followed by end or else. r=luke, a=jcristau
js/src/jit-test/tests/wasm/basic.js
js/src/jit-test/tests/wasm/control-flow.js
js/src/jit-test/tests/wasm/full-cycle.js
js/src/jit-test/tests/wasm/regress/misc-control-flow.js
js/src/jit-test/tests/wasm/regress/select-any.js
js/src/jit-test/tests/wasm/spec/block.wast.js
js/src/jit-test/tests/wasm/spec/br.wast.js
js/src/jit-test/tests/wasm/spec/br_if.wast.js
js/src/jit-test/tests/wasm/spec/br_table.wast.js
js/src/jit-test/tests/wasm/spec/func.wast.js
js/src/jit-test/tests/wasm/spec/labels.wast.js
js/src/jit-test/tests/wasm/spec/loop.wast.js
js/src/jit-test/tests/wasm/spec/return.wast.js
js/src/jit-test/tests/wasm/spec/select.wast.js
js/src/jit-test/tests/wasm/spec/unreachable.wast.js
js/src/jit-test/tests/wasm/spec/unwind.wast.js
js/src/jit-test/tests/wasm/unreachable.js
js/src/wasm/WasmBinaryFormat.h
js/src/wasm/WasmBinaryIterator.h
--- a/js/src/jit-test/tests/wasm/basic.js
+++ b/js/src/jit-test/tests/wasm/basic.js
@@ -456,18 +456,20 @@ wasmValidateText('(module (import $bar "
 
 // ----------------------------------------------------------------------------
 // select
 
 wasmFailValidateText('(module (func (select (i32.const 0) (i32.const 0) (f32.const 0))))', mismatchError("f32", "i32"));
 
 wasmFailValidateText('(module (func (select (i32.const 0) (f32.const 0) (i32.const 0))) (export "" 0))', /select operand types must match/);
 wasmFailValidateText('(module (func (select (block ) (i32.const 0) (i32.const 0))) (export "" 0))', /popping value from empty stack/);
-assertEq(wasmEvalText('(module (func (select (return) (i32.const 0) (i32.const 0))) (export "" 0))').exports[""](), undefined);
-assertEq(wasmEvalText('(module (func (i32.add (i32.const 0) (select (return) (i32.const 0) (i32.const 0)))) (export "" 0))').exports[""](), undefined);
+wasmFailValidateText('(module (func (select (return) (i32.const 0) (i32.const 0))) (export "" 0))', /non-fallthrough instruction must be followed by end or else/);
+wasmFailValidateText('(module (func (i32.add (i32.const 0) (select (return) (i32.const 0) (i32.const 0)))) (export "" 0))', /non-fallthrough instruction must be followed by end or else/);
+assertEq(wasmEvalText('(module (func (select (block (return)) (i32.const 0) (i32.const 0))) (export "" 0))').exports[""](), undefined);
+assertEq(wasmEvalText('(module (func (i32.add (i32.const 0) (select (block (return)) (i32.const 0) (i32.const 0)))) (export "" 0))').exports[""](), undefined);
 wasmFailValidateText('(module (func (select (if i32 (i32.const 1) (i32.const 0) (f32.const 0)) (i32.const 0) (i32.const 0))) (export "" 0))', mismatchError("f32", "i32"));
 wasmFailValidateText('(module (func) (func (select (call 0) (call 0) (i32.const 0))) (export "" 0))', /popping value from empty stack/);
 
 (function testSideEffects() {
 
 var numT = 0;
 var numF = 0;
 
--- a/js/src/jit-test/tests/wasm/control-flow.js
+++ b/js/src/jit-test/tests/wasm/control-flow.js
@@ -165,17 +165,18 @@ wasmFailValidateText('(module (func (if 
 wasmFailValidateText('(module (func (if (i32.const 1) (drop (i32.const 0)) (if i32 (i32.const 1) (i32.const 1)))))', /if without else with a result value/);
 wasmEvalText('(module (func (if (i32.const 1) (drop (i32.const 0)) (if (i32.const 1) (drop (i32.const 1))))))');
 
 // ----------------------------------------------------------------------------
 // return
 
 wasmFullPass('(module (func (return)) (export "run" 0))', undefined);
 wasmFullPass('(module (func (result i32) (return (i32.const 1))) (export "run" 0))', 1);
-wasmFullPass('(module (func (if (return) (i32.const 0))) (export "run" 0))', undefined);
+wasmFailValidateText('(module (func (if (return) (i32.const 0))) (export "run" 0))', /non-fallthrough instruction must be followed by end or else/);
+wasmFullPass('(module (func (if (block (return)) (i32.const 0))) (export "run" 0))', undefined);
 wasmFailValidateText('(module (func (result i32) (return)) (export "" 0))', /popping value from empty stack/);
 wasmFullPass('(module (func (return (i32.const 1))) (export "run" 0))', undefined);
 wasmFailValidateText('(module (func (result f32) (return (i32.const 1))) (export "" 0))', mismatchError("i32", "f32"));
 wasmFailValidateText('(module (func (result i32) (return)) (export "" 0))', /popping value from empty stack/);
 
 // ----------------------------------------------------------------------------
 // br / br_if
 
@@ -199,88 +200,152 @@ wasmFailValidateText('(module (func (loo
 wasmFailValidateText(`(module (func (result i32)
   (block
     (if
       (br 0)
       (i32.const 0)
       (i32.const 2)
     )
   )
+) (export "" 0))`, /non-fallthrough instruction must be followed by end or else/);
+
+wasmFailValidateText(`(module (func (result i32)
+  (block
+    (if
+      (block i32 (br 1))
+      (i32.const 0)
+      (i32.const 2)
+    )
+  )
 ) (export "" 0))`, mismatchError("void", "i32"));
 
-wasmFullPass(`(module (func (block $out (br_if $out (br 0)))) (export "run" 0))`, undefined);
+wasmFailValidateText(`(module (func (block $out (br_if $out (br 0)))) (export "run" 0))`, /non-fallthrough instruction must be followed by end or else/);
+wasmFullPass(`(module (func (block $out (br_if $out (block i32 (br 1))))) (export "run" 0))`, undefined);
 
 wasmFullPass('(module (func (br 0)) (export "run" 0))', undefined);
 wasmFullPass('(module (func (block (br 0))) (export "run" 0))', undefined);
 wasmFullPass('(module (func (block $l (br $l))) (export "run" 0))', undefined);
 
 wasmFullPass('(module (func (block (block (br 1)))) (export "run" 0))', undefined);
 wasmFullPass('(module (func (block $l (block (br $l)))) (export "run" 0))', undefined);
 
 wasmFullPass('(module (func (block $l (block $m (br $l)))) (export "run" 0))', undefined);
 wasmFullPass('(module (func (block $l (block $m (br $m)))) (export "run" 0))', undefined);
 
-wasmFullPass(`(module (func (result i32)
+wasmFailValidateText(`(module (func (result i32)
   (block
     (br 0)
     (return (i32.const 0))
   )
   (return (i32.const 1))
+) (export "run" 0))`, /non-fallthrough instruction must be followed by end or else/);
+
+wasmFullPass(`(module (func (result i32)
+  (block
+    (block (br 1))
+    (return (i32.const 0))
+  )
+  (return (i32.const 1))
 ) (export "run" 0))`, 1);
 
-wasmFullPass(`(module (func (result i32)
+wasmFailValidateText(`(module (func (result i32)
   (block
     (block
       (br 0)
       (return (i32.const 0))
     )
     (return (i32.const 1))
   )
   (return (i32.const 2))
+) (export "run" 0))`, /non-fallthrough instruction must be followed by end or else/);
+
+wasmFullPass(`(module (func (result i32)
+  (block
+    (block
+      (block (br 1))
+      (return (i32.const 0))
+    )
+    (return (i32.const 1))
+  )
+  (return (i32.const 2))
 ) (export "run" 0))`, 1);
 
+wasmFailValidateText(`(module (func (result i32)
+  (block $outer
+    (block $inner
+      (br $inner)
+      (return (i32.const 0))
+    )
+    (return (i32.const 1))
+  )
+  (return (i32.const 2))
+) (export "run" 0))`, /non-fallthrough instruction must be followed by end or else/);
+
 wasmFullPass(`(module (func (result i32)
   (block $outer
     (block $inner
-      (br $inner)
+      (block (br $inner))
       (return (i32.const 0))
     )
     (return (i32.const 1))
   )
   (return (i32.const 2))
 ) (export "run" 0))`, 1);
 
 var notcalled = false;
 var called = false;
 var imports = {"": {
     notcalled() {notcalled = true},
     called() {called = true}
 }};
-wasmFullPass(`(module
+wasmFailValidateText(`(module
 (import "" "notcalled")
 (import "" "called")
 (func
   (block
     (return (br 0))
     (call 0)
   )
   (call 1)
+) (export "run" 2))`, /non-fallthrough instruction must be followed by end or else/);
+wasmFullPass(`(module
+(import "" "notcalled")
+(import "" "called")
+(func
+  (block
+    (block (return (block (br 2))))
+    (call 0)
+  )
+  (call 1)
 ) (export "run" 2))`, undefined, imports);
 assertEq(notcalled, false);
 assertEq(called, true);
 
-wasmFullPass(`(module (func
+wasmFailValidateText(`(module (func
   (block
     (i32.add
       (i32.const 0)
       (return (br 0))
     )
   )
   (return)
+) (export "run" 0))`, /non-fallthrough instruction must be followed by end or else/);
+
+/* TODO: This triggers a bug in BinaryToAST. */
+/*
+wasmFullPass(`(module (func
+  (block
+    (i32.add
+      (i32.const 0)
+      (block i32 (return (block (br 2))))
+    )
+  )
+  (return)
 ) (export "run" 0))`, undefined);
+*/
 
 wasmFullPass(`(module (func (result i32)
   (block
     (if
       (i32.const 1)
       (br 0)
       (return (i32.const 0))
     )
@@ -311,18 +376,20 @@ assertEq(isNonZero(-1), 1);
 wasmFailValidateText('(module (func (result i32) (br 0)))', /popping value from empty stack/);
 wasmFailValidateText('(module (func (result i32) (br 0 (f32.const 42))))', mismatchError("f32", "i32"));
 wasmFailValidateText('(module (func (result i32) (block (br 0))))', mismatchError("void", "i32"));
 wasmFailValidateText('(module (func (result i32) (block f32 (br 0 (f32.const 42)))))', mismatchError("f32", "i32"));
 
 wasmFailValidateText(`(module (func (result i32) (param i32) (block (if i32 (get_local 0) (br 0 (i32.const 42))))) (export "" 0))`, /if without else with a result value/);
 wasmFailValidateText(`(module (func (result i32) (param i32) (block i32 (if (get_local 0) (drop (i32.const 42))) (br 0 (f32.const 42)))) (export "" 0))`, mismatchError("f32", "i32"));
 
-wasmFullPass('(module (func (result i32) (br 0 (i32.const 42)) (i32.const 13)) (export "run" 0))', 42);
-wasmFullPass('(module (func (result i32) (block i32 (br 0 (i32.const 42)) (i32.const 13))) (export "run" 0))', 42);
+wasmFailValidateText('(module (func (result i32) (br 0 (i32.const 42)) (i32.const 13)) (export "run" 0))', /non-fallthrough instruction must be followed by end or else/);
+wasmFailValidateText('(module (func (result i32) (block i32 (br 0 (i32.const 42)) (i32.const 13))) (export "run" 0))', /non-fallthrough instruction must be followed by end or else/);
+wasmFullPass('(module (func (result i32) (block (br 1 (i32.const 42))) (i32.const 13)) (export "run" 0))', 42);
+wasmFullPass('(module (func (result i32) (block i32 (block (br 1 (i32.const 42))) (i32.const 13))) (export "run" 0))', 42);
 
 wasmFailValidateText('(module (func) (func (block i32 (br 0 (call 0)) (i32.const 13))) (export "" 0))', /popping value from empty stack/);
 wasmFailValidateText('(module (func) (func (block i32 (br_if 0 (call 0) (i32.const 1)) (i32.const 13))) (export "" 0))', /popping value from empty stack/);
 
 var f = wasmEvalText(`(module (func (result i32) (param i32) (block i32 (if (get_local 0) (drop (i32.const 42))) (i32.const 43))) (export "" 0))`).exports[""];
 assertEq(f(0), 43);
 assertEq(f(1), 43);
 
@@ -371,19 +438,22 @@ assertEq(f(0), 0);
 assertEq(f(1), 100);
 
 wasmFailValidateText(`(module (func (param i32) (result i32) (i32.add (i32.const 1) (block (br_if 0 (i32.const 99) (get_local 0)) (i32.const -1)))) (export "" 0))`, /unused values not explicitly dropped by end of block/);
 
 var f = wasmEvalText(`(module (func (param i32) (result i32) (i32.add (i32.const 1) (block i32 (drop (br_if 0 (i32.const 99) (get_local 0))) (i32.const -1)))) (export "" 0))`).exports[""];
 assertEq(f(0), 0);
 assertEq(f(1), 100);
 
-wasmFullPass(`(module (func (result i32) (block i32 (br 0 (return (i32.const 42))) (i32.const 0))) (export "run" 0))`, 42);
-wasmFullPass(`(module (func (result i32) (block i32 (return (br 0 (i32.const 42))))) (export "run" 0))`, 42);
-wasmFullPass(`(module (func (result i32) (block i32 (return (br 0 (i32.const 42))) (i32.const 0))) (export "run" 0))`, 42);
+wasmFailValidateText(`(module (func (result i32) (block i32 (br 0 (return (i32.const 42))) (i32.const 0))) (export "run" 0))`, /non-fallthrough instruction must be followed by end or else/);
+wasmFailValidateText(`(module (func (result i32) (block i32 (return (br 0 (i32.const 42))))) (export "run" 0))`, /non-fallthrough instruction must be followed by end or else/);
+wasmFailValidateText(`(module (func (result i32) (block i32 (return (br 0 (i32.const 42))) (i32.const 0))) (export "run" 0))`, /non-fallthrough instruction must be followed by end or else/);
+wasmFullPass(`(module (func (result i32) (block i32 (block (br 1 (block (return (i32.const 42))))) (i32.const 0))) (export "run" 0))`, 42);
+wasmFullPass(`(module (func (result i32) (block i32 (return (block (br 1 (i32.const 42)))))) (export "run" 0))`, 42);
+wasmFullPass(`(module (func (result i32) (block i32 (block (return (block (br 2 (i32.const 42))))) (i32.const 0))) (export "run" 0))`, 42);
 
 wasmFullPass(`(module (func (result f32) (drop (block i32 (br 0 (i32.const 0)))) (block f32 (br 0 (f32.const 42)))) (export "run" 0))`, 42);
 
 var called = 0;
 var imports = {
     sideEffects: {
         ifTrue(x) {assertEq(x, 13); called++;},
         ifFalse(x) {assertEq(x, 37); called--;}
@@ -632,36 +702,63 @@ wasmFailValidateText('(module (func (br_
 wasmFailValidateText('(module (func (br_table 0 1 (i32.const 0))))', DEPTH_OUT_OF_BOUNDS);
 wasmFailValidateText('(module (func (block (br_table 2 0 (i32.const 0)))))', DEPTH_OUT_OF_BOUNDS);
 wasmFailValidateText('(module (func (block (br_table 0 2 (i32.const 0)))))', DEPTH_OUT_OF_BOUNDS);
 wasmFailValidateText('(module (func (block (br_table 0 (f32.const 0)))))', mismatchError("f32", "i32"));
 wasmFailValidateText('(module (func (loop (br_table 2 0 (i32.const 0)))))', DEPTH_OUT_OF_BOUNDS);
 wasmFailValidateText('(module (func (loop (br_table 0 2 (i32.const 0)))))', DEPTH_OUT_OF_BOUNDS);
 wasmFailValidateText('(module (func (loop (br_table 0 (f32.const 0)))))', mismatchError("f32", "i32"));
 
+wasmFailValidateText(`(module (func (result i32) (param i32)
+  (block $default
+   (br_table $default (get_local 0))
+   (return (i32.const 0))
+  )
+  (return (i32.const 1))
+) (export "run" 0))`, /non-fallthrough instruction must be followed by end or else/);
+
 wasmFullPass(`(module (func (result i32) (param i32)
   (block $default
-   (br_table $default (get_local 0))
+   (block (br_table $default (get_local 0)))
    (return (i32.const 0))
   )
   (return (i32.const 1))
 ) (export "run" 0))`, 1);
 
-wasmFullPass(`(module (func (result i32) (param i32)
+wasmFailValidateText(`(module (func (result i32) (param i32)
   (block $default
    (br_table $default (return (i32.const 1)))
    (return (i32.const 0))
   )
   (return (i32.const 2))
+) (export "run" 0))`, /non-fallthrough instruction must be followed by end or else/);
+
+wasmFullPass(`(module (func (result i32) (param i32)
+  (block $default
+   (block (br_table $default (block i32 (return (i32.const 1)))))
+   (return (i32.const 0))
+  )
+  (return (i32.const 2))
 ) (export "run" 0))`, 1);
 
+wasmFailValidateText(`(module (func (result i32) (param i32)
+  (block $outer
+   (block $inner
+    (br_table $inner (get_local 0))
+    (return (i32.const 0))
+   )
+   (return (i32.const 1))
+  )
+  (return (i32.const 2))
+) (export "run" 0))`, /non-fallthrough instruction must be followed by end or else/);
+
 wasmFullPass(`(module (func (result i32) (param i32)
   (block $outer
    (block $inner
-    (br_table $inner (get_local 0))
+    (block (br_table $inner (get_local 0)))
     (return (i32.const 0))
    )
    (return (i32.const 1))
   )
   (return (i32.const 2))
 ) (export "run" 0))`, 1);
 
 var f = wasmEvalText(`(module (func (result i32) (param i32)
@@ -725,12 +822,16 @@ assertEq(f(2), 9);
 assertEq(f(3), 11);
 assertEq(f(4), 13);
 
 // ----------------------------------------------------------------------------
 // unreachable
 
 const UNREACHABLE = /unreachable/;
 assertErrorMessage(wasmEvalText(`(module (func (unreachable)) (export "" 0))`).exports[""], RuntimeError, UNREACHABLE);
-assertErrorMessage(wasmEvalText(`(module (func (if (unreachable) (i32.const 0))) (export "" 0))`).exports[""], RuntimeError, UNREACHABLE);
-assertErrorMessage(wasmEvalText(`(module (func (block (br_if 0 (unreachable)))) (export "" 0))`).exports[""], RuntimeError, UNREACHABLE);
-assertErrorMessage(wasmEvalText(`(module (func (block (br_table 0 (unreachable)))) (export "" 0))`).exports[""], RuntimeError, UNREACHABLE);
-assertErrorMessage(wasmEvalText(`(module (func (result i32) (i32.add (i32.const 0) (unreachable))) (export "" 0))`).exports[""], RuntimeError, UNREACHABLE);
+wasmFailValidateText('(module (func (if (unreachable) (i32.const 0))) (export "" 0))', /non-fallthrough instruction must be followed by end or else/);
+wasmFailValidateText('(module (func (block (br_if 0 (unreachable)))) (export "" 0))', /non-fallthrough instruction must be followed by end or else/);
+wasmFailValidateText('(module (func (block (br_table 0 (unreachable)))) (export "" 0))', /non-fallthrough instruction must be followed by end or else/);
+wasmFailValidateText('(module (func (result i32) (i32.add (i32.const 0) (unreachable))) (export "" 0))', /non-fallthrough instruction must be followed by end or else/);
+assertErrorMessage(wasmEvalText(`(module (func (if (block i32 (unreachable)) (i32.const 0))) (export "" 0))`).exports[""], RuntimeError, UNREACHABLE);
+assertErrorMessage(wasmEvalText(`(module (func (block (br_if 0 (block i32 (unreachable))))) (export "" 0))`).exports[""], RuntimeError, UNREACHABLE);
+assertErrorMessage(wasmEvalText(`(module (func (block (br_table 0 (block i32 (unreachable))))) (export "" 0))`).exports[""], RuntimeError, UNREACHABLE);
+assertErrorMessage(wasmEvalText(`(module (func (result i32) (i32.add (i32.const 0) (block i32 (unreachable)))) (export "" 0))`).exports[""], RuntimeError, UNREACHABLE);
--- a/js/src/jit-test/tests/wasm/full-cycle.js
+++ b/js/src/jit-test/tests/wasm/full-cycle.js
@@ -1,29 +1,48 @@
 load(libdir + "wasm.js");
 
 wasmFullPass(`(module
     (func $test (result i32) (param i32) (param i32) (i32.add (get_local 0) (get_local 1)))
     (func $run (result i32) (call $test (i32.const 1) (i32.const ${Math.pow(2, 31) - 1})))
     (export "run" $run)
 )`, -Math.pow(2, 31));
 
-wasmFullPass(`(module
+wasmFailValidateText(`(module
     (func (result i32)
         i32.const 1
         i32.const 42
         i32.add
         return
         unreachable
         i32.const 0
         call 3
         i32.const 42
         f32.add
     )
     (func) (func) (func)
+(export "run" 0))`, /non-fallthrough instruction must be followed by end or else/);
+
+wasmFullPass(`(module
+    (func (result i32)
+        block
+        i32.const 1
+        i32.const 42
+        i32.add
+        return
+        end
+        block
+        unreachable
+        end
+        i32.const 0
+        call 3
+        i32.const 42
+        f32.add
+    )
+    (func) (func) (func)
 (export "run" 0))`, 43);
 
 // Global section.
 wasmFullPass(`(module
  (import $imported "globals" "x" (global i32))
  (global $mut_local (mut i32) (i32.const 0))
  (global $imm_local i32 (i32.const 37))
  (global $imm_local_2 i32 (get_global 0))
--- a/js/src/jit-test/tests/wasm/regress/misc-control-flow.js
+++ b/js/src/jit-test/tests/wasm/regress/misc-control-flow.js
@@ -19,18 +19,23 @@ wasmFailValidateText(`(module
 )`, mismatchError("void", "i32"));
 
 assertEq(wasmEvalText(`(module
    (func (result i32) (param i32)
      (loop (if (i32.const 0) (br 0))) (get_local 0))
    (export "" 0)
 )`).exports[""](42), 42);
 
+wasmFailValidateText(`(module (func $func$0
+      (block (if (i32.const 1) (loop (br_table 0 (br 0)))))
+  )
+)`, /non-fallthrough instruction must be followed by end or else/);
+
 wasmEvalText(`(module (func $func$0
-      (block (if (i32.const 1) (loop (br_table 0 (br 0)))))
+      (block (if (i32.const 1) (loop (br_table 0 (block i32 (br 1))))))
   )
 )`);
 
 wasmEvalText(`(module (func
       (loop $out $in (br_table $out $out $in (i32.const 0)))
   )
 )`);
 
@@ -48,39 +53,69 @@ wasmEvalText(`(module (func (result i32)
       (i32.const 2)
     )
     (i32.const 3)
     (i32.const 4)
   )
 ))
 `);
 
-wasmEvalText(`(module
+wasmFailValidateText(`(module
   (func (result i32) (param i32) (param i32) (i32.const 0))
   (func (result i32)
    (call 0 (i32.const 1) (call 0 (i32.const 2) (i32.const 3)))
    (call 0 (unreachable) (i32.const 4))
   )
+)`, /non-fallthrough instruction must be followed by end or else/);
+
+wasmEvalText(`(module
+  (func (result i32) (param i32) (param i32) (i32.const 0))
+  (func (result i32)
+   (call 0 (i32.const 1) (call 0 (i32.const 2) (i32.const 3)))
+   (call 0 (block i32 (unreachable)) (i32.const 4))
+  )
 )`);
 
+wasmFailValidateText(`
+(module
+
+ (func
+  (param i32) (param i32) (param i32) (param i32)
+  (result i32)
+  (i32.const 0)
+ )
+
+ (func (result i32)
+  (call 0
+   (i32.const 42)
+   (i32.const 53)
+   (call 0 (i32.const 100) (i32.const 13) (i32.const 37) (i32.const 128))
+   (return (i32.const 42))
+  )
+ )
+
+ (export "" 1)
+)
+`, /non-fallthrough instruction must be followed by end or else/);
+
 wasmEvalText(`
 (module
 
  (func
   (param i32) (param i32) (param i32) (param i32)
   (result i32)
   (i32.const 0)
  )
 
  (func (result i32)
   (call 0
    (i32.const 42)
    (i32.const 53)
    (call 0 (i32.const 100) (i32.const 13) (i32.const 37) (i32.const 128))
-   (return (i32.const 42))
+   (block i32 (return (i32.const 42)))
   )
  )
 
  (export "" 1)
 )
 `).exports[""]();
 
 wasmEvalText(`
@@ -107,27 +142,45 @@ wasmEvalText(`
         },
         two(x, y) {
             assertEq(x, 43);
             assertEq(y, 10);
         }
     }
 }).exports.foo();
 
+wasmFailValidateText(`(module (func
+ (return)
+ (select
+  (loop (i32.const 1))
+  (loop (i32.const 2))
+  (i32.const 3)
+ )
+) (export "" 0))`, /non-fallthrough instruction must be followed by end or else/);
+
 assertEq(wasmEvalText(`(module (func
- (return)
+ (block (return))
  (select
   (loop (i32.const 1))
   (loop (i32.const 2))
   (i32.const 3)
  )
 ) (export "" 0))`).exports[""](), undefined);
 
+wasmFailValidateText(`(module (func (result i32)
+ (return (i32.const 0))
+ (select
+  (loop (i32.const 1))
+  (loop (i32.const 2))
+  (i32.const 3)
+ )
+))`, /non-fallthrough instruction must be followed by end or else/);
+
 wasmEvalText(`(module (func (result i32)
- (return (i32.const 0))
+ (block (return (i32.const 0)))
  (select
   (loop (i32.const 1))
   (loop (i32.const 2))
   (i32.const 3)
  )
 ))`);
 
 wasmEvalText(`(module (func
--- a/js/src/jit-test/tests/wasm/regress/select-any.js
+++ b/js/src/jit-test/tests/wasm/regress/select-any.js
@@ -1,31 +1,45 @@
 load(libdir + "wasm.js");
 
 // Bug 1280921
 
-var m1 = wasmEvalText(
+wasmFailValidateText(
 `(module
   (type $type0 (func))
   (func $func0
    (select (unreachable) (return (nop)) (loop (i32.const 1))))
+  (export "" 0))`, /non-fallthrough instruction must be followed by end or else/);
+
+wasmFailValidateText(
+`(module
+  (type $type0 (func))
+  (func $func0
+   (select (i32.const 26) (unreachable) (i32.const 3)))
+  (export "" 0))`, /non-fallthrough instruction must be followed by end or else/);
+
+var m3 = wasmEvalText(
+`(module
+  (type $type0 (func))
+  (func $func0
+   (select (block i32 (unreachable)) (block i32 (return (nop))) (loop (i32.const 1))))
   (export "" 0))`).exports[""];
 
 try {
-    m1();
+    m3();
 } catch (e) {
     if (!(e instanceof Error && e.message.match(/unreachable executed/)))
 	throw e;
 }
 
-var m2 = wasmEvalText(
+var m4 = wasmEvalText(
 `(module
   (type $type0 (func))
   (func $func0
-   (select (i32.const 26) (unreachable) (i32.const 3)))
+   (select (i32.const 26) (block i32 (unreachable)) (i32.const 3)))
   (export "" 0))`).exports[""];
 
 try {
-    m2();
+    m4();
 } catch (e) {
     if (!(e instanceof Error && e.message.match(/unreachable executed/)))
 	throw e;
 }
--- a/js/src/jit-test/tests/wasm/spec/block.wast.js
+++ b/js/src/jit-test/tests/wasm/spec/block.wast.js
@@ -1,1 +1,3 @@
+// TODO: This spec test has instructions between br/etc. and end/etc.
+quit();
 var importedArgs = ['block.wast']; load(scriptdir + '../spec.js');
--- a/js/src/jit-test/tests/wasm/spec/br.wast.js
+++ b/js/src/jit-test/tests/wasm/spec/br.wast.js
@@ -1,1 +1,3 @@
+// TODO: This spec test has instructions between br/etc. and end/etc.
+quit();
 var importedArgs = ['br.wast']; load(scriptdir + '../spec.js');
--- a/js/src/jit-test/tests/wasm/spec/br_if.wast.js
+++ b/js/src/jit-test/tests/wasm/spec/br_if.wast.js
@@ -1,1 +1,3 @@
+// TODO: This spec test has instructions between br/etc. and end/etc.
+quit();
 var importedArgs = ['br_if.wast']; load(scriptdir + '../spec.js');
--- a/js/src/jit-test/tests/wasm/spec/br_table.wast.js
+++ b/js/src/jit-test/tests/wasm/spec/br_table.wast.js
@@ -1,1 +1,3 @@
+// TODO: This spec test has instructions between br/etc. and end/etc.
+quit();
 var importedArgs = ['br_table.wast']; load(scriptdir + '../spec.js');
--- a/js/src/jit-test/tests/wasm/spec/func.wast.js
+++ b/js/src/jit-test/tests/wasm/spec/func.wast.js
@@ -1,1 +1,3 @@
+// TODO: This spec test has instructions between br/etc. and end/etc.
+quit();
 var importedArgs = ['func.wast']; load(scriptdir + '../spec.js');
--- a/js/src/jit-test/tests/wasm/spec/labels.wast.js
+++ b/js/src/jit-test/tests/wasm/spec/labels.wast.js
@@ -1,1 +1,3 @@
+// TODO: This spec test has instructions between br/etc. and end/etc.
+quit();
 var importedArgs = ['labels.wast']; load(scriptdir + '../spec.js');
--- a/js/src/jit-test/tests/wasm/spec/loop.wast.js
+++ b/js/src/jit-test/tests/wasm/spec/loop.wast.js
@@ -1,1 +1,3 @@
+// TODO: This spec test has instructions between br/etc. and end/etc.
+quit();
 var importedArgs = ['loop.wast']; load(scriptdir + '../spec.js');
--- a/js/src/jit-test/tests/wasm/spec/return.wast.js
+++ b/js/src/jit-test/tests/wasm/spec/return.wast.js
@@ -1,1 +1,3 @@
+// TODO: This spec test has instructions between br/etc. and end/etc.
+quit();
 var importedArgs = ['return.wast']; load(scriptdir + '../spec.js');
--- a/js/src/jit-test/tests/wasm/spec/select.wast.js
+++ b/js/src/jit-test/tests/wasm/spec/select.wast.js
@@ -1,1 +1,3 @@
+// TODO: This spec test has instructions between br/etc. and end/etc.
+quit();
 var importedArgs = ['select.wast']; load(scriptdir + '../spec.js');
--- a/js/src/jit-test/tests/wasm/spec/unreachable.wast.js
+++ b/js/src/jit-test/tests/wasm/spec/unreachable.wast.js
@@ -1,1 +1,3 @@
+// TODO: This spec test has instructions between br/etc. and end/etc.
+quit();
 var importedArgs = ['unreachable.wast']; load(scriptdir + '../spec.js');
--- a/js/src/jit-test/tests/wasm/spec/unwind.wast.js
+++ b/js/src/jit-test/tests/wasm/spec/unwind.wast.js
@@ -1,1 +1,3 @@
+// TODO: This spec test has instructions between br/etc. and end/etc.
+quit();
 var importedArgs = ['unwind.wast']; load(scriptdir + '../spec.js');
--- a/js/src/jit-test/tests/wasm/unreachable.js
+++ b/js/src/jit-test/tests/wasm/unreachable.js
@@ -1,30 +1,55 @@
 load(libdir + "wasm.js");
 
 // In unreachable code, the current design is that validation is disabled,
 // meaning we have to have a special mode in the decoder for decoding code
 // that won't actually run.
 
-wasmFullPass(`(module
+wasmFailValidateText(`(module
    (func (result i32)
      (return (i32.const 42))
      (i32.add (f64.const 1.0) (f32.const 0.0))
      (return (f64.const 2.0))
      (if (f32.const 3.0) (i64.const 2) (i32.const 1))
      (select (f64.const -5.0) (f32.const 2.3) (f64.const 8.9))
    )
    (export "run" 0)
+)`, /non-fallthrough instruction must be followed by end or else/);
+
+wasmFailValidateText(`(module
+   (func (result i32) (param i32)
+     (block
+        (br_if 1 (i32.const 41) (get_local 0))
+        (br 1 (i32.const 42))
+     )
+     (i32.add (f32.const 0.0) (f64.const 1.0))
+     (return (f64.const 2.0))
+     (if (f32.const 3.0) (i64.const 2) (i32.const 1))
+     (select (f64.const -5.0) (f32.const 2.3) (f64.const 8.9))
+   )
+   (export "run" 0)
+)`, /non-fallthrough instruction must be followed by end or else/);
+
+wasmFullPass(`(module
+   (func (result i32)
+     (block (return (i32.const 42)))
+     (i32.add (f64.const 1.0) (f32.const 0.0))
+     (block (return (f64.const 2.0)))
+     (if (f32.const 3.0) (i64.const 2) (i32.const 1))
+     (select (f64.const -5.0) (f32.const 2.3) (f64.const 8.9))
+   )
+   (export "run" 0)
 )`, 42);
 
 wasmFullPass(`(module
    (func (result i32) (param i32)
      (block
         (br_if 1 (i32.const 41) (get_local 0))
         (br 1 (i32.const 42))
      )
      (i32.add (f32.const 0.0) (f64.const 1.0))
-     (return (f64.const 2.0))
+     (block (return (f64.const 2.0)))
      (if (f32.const 3.0) (i64.const 2) (i32.const 1))
      (select (f64.const -5.0) (f32.const 2.3) (f64.const 8.9))
    )
    (export "run" 0)
 )`, 42, {}, 0);
--- a/js/src/wasm/WasmBinaryFormat.h
+++ b/js/src/wasm/WasmBinaryFormat.h
@@ -334,16 +334,20 @@ class Decoder
         MOZ_ASSERT(cur_ <= end_);
         return cur_ == end_;
     }
 
     size_t bytesRemain() const {
         MOZ_ASSERT(end_ >= cur_);
         return size_t(end_ - cur_);
     }
+    // pos must be a value previously returned from currentPosition.
+    void rollbackPosition(const uint8_t* pos) {
+        cur_ = pos;
+    }
     const uint8_t* currentPosition() const {
         return cur_;
     }
     size_t currentOffset() const {
         return cur_ - beg_;
     }
     const uint8_t* begin() const {
         return beg_;
--- a/js/src/wasm/WasmBinaryIterator.h
+++ b/js/src/wasm/WasmBinaryIterator.h
@@ -489,19 +489,26 @@ class MOZ_STACK_CLASS OpIter : private P
     bool getControl(uint32_t relativeDepth, ControlStackEntry<ControlItem>** controlEntry) {
         if (Validate && relativeDepth >= controlStack_.length())
             return fail("branch depth exceeds current nesting level");
 
         *controlEntry = &controlStack_[controlStack_.length() - 1 - relativeDepth];
         return true;
     }
 
-    void enterUnreachableCode() {
+    MOZ_MUST_USE bool enterUnreachableCode() {
+        if (Validate) {
+            uint16_t op = peekOp();
+            if (op != uint16_t(Op::End) && op != uint16_t(Op::Else))
+                return fail("non-fallthrough instruction must be followed by end or else");
+        }
+
         valueStack_.shrinkTo(controlStack_.back().valueStackStart());
         reachable_ = false;
+        return true;
     }
 
     bool checkBrValue(uint32_t relativeDepth, ExprType* type, Value* value);
     bool checkBrIfValues(uint32_t relativeDepth, Value* condition, ExprType* type, Value* value);
 
   public:
     explicit OpIter(Decoder& decoder, uint32_t offsetInModule = 0)
       : d_(decoder), offsetInModule_(offsetInModule), reachable_(true),
@@ -624,16 +631,21 @@ class MOZ_STACK_CLASS OpIter : private P
                                      Value* falseValue,
                                      Value* condition);
     MOZ_MUST_USE bool readSimdCtor();
     MOZ_MUST_USE bool readSimdCtorArg(ValType elementType, uint32_t numElements, uint32_t argIndex,
                                       Value* arg);
     MOZ_MUST_USE bool readSimdCtorArgsEnd(uint32_t numElements);
     MOZ_MUST_USE bool readSimdCtorReturn(ValType simdType);
 
+    // At a location where readOp is allowed, peek at the next opcode
+    // without consuming it or updating any internal state.
+    // Never fails: returns uint16_t(Op::Limit) if it can't read.
+    uint16_t peekOp();
+
     // ------------------------------------------------------------------------
     // Stack management.
 
     // Set the result value of the current top-of-value-stack expression.
     void setResult(Value value) {
         if (MOZ_LIKELY(reachable_))
             valueStack_.back().setValue(value);
     }
@@ -854,16 +866,35 @@ OpIter<Policy>::readOp(uint16_t* op)
     }
 
     op_ = Op(*op);  // debug-only
 
     return true;
 }
 
 template <typename Policy>
+inline uint16_t
+OpIter<Policy>::peekOp()
+{
+    const uint8_t* pos = d_.currentPosition();
+    uint16_t op;
+
+    if (Validate) {
+        if (MOZ_UNLIKELY(!d_.readOp(&op)))
+            op = uint16_t(Op::Limit);
+    } else {
+        op = uint16_t(d_.uncheckedReadOp());
+    }
+
+    d_.rollbackPosition(pos);
+
+    return op;
+}
+
+template <typename Policy>
 inline bool
 OpIter<Policy>::readFunctionStart(ExprType ret)
 {
     MOZ_ASSERT(valueStack_.empty());
     MOZ_ASSERT(controlStack_.empty());
     MOZ_ASSERT(Op(op_) == Op::Limit);
     MOZ_ASSERT(reachable_);
 
@@ -899,18 +930,17 @@ OpIter<Policy>::readReturn(Value* value)
         controlItem.setReachable();
 
         if (!IsVoid(controlItem.type())) {
             if (!popWithType(NonVoidToValType(controlItem.type()), value))
                 return false;
         }
     }
 
-    enterUnreachableCode();
-    return true;
+    return enterUnreachableCode();
 }
 
 template <typename Policy>
 inline bool
 OpIter<Policy>::readBlock()
 {
     MOZ_ASSERT(Classify(op_) == OpKind::Block);
 
@@ -1045,18 +1075,17 @@ OpIter<Policy>::readBr(uint32_t* relativ
         return fail("unable to read br depth");
 
     if (!checkBrValue(validateRelativeDepth, type, value))
         return false;
 
     if (Output)
         *relativeDepth = validateRelativeDepth;
 
-    enterUnreachableCode();
-    return true;
+    return enterUnreachableCode();
 }
 
 template <typename Policy>
 inline bool
 OpIter<Policy>::checkBrIfValues(uint32_t relativeDepth, Value* condition,
                                   ExprType* type, Value* value)
 {
     if (MOZ_LIKELY(reachable_)) {
@@ -1175,28 +1204,26 @@ template <typename Policy>
 inline bool
 OpIter<Policy>::readBrTableDefault(ExprType* type, Value* value, uint32_t* depth)
 {
     if (!readBrTableEntry(type, value, depth))
         return false;
 
     MOZ_ASSERT(!reachable_ || *type != ExprType::Limit);
 
-    enterUnreachableCode();
-    return true;
+    return enterUnreachableCode();
 }
 
 template <typename Policy>
 inline bool
 OpIter<Policy>::readUnreachable()
 {
     MOZ_ASSERT(Classify(op_) == OpKind::Unreachable);
 
-    enterUnreachableCode();
-    return true;
+    return enterUnreachableCode();
 }
 
 template <typename Policy>
 inline bool
 OpIter<Policy>::readDrop()
 {
     MOZ_ASSERT(Classify(op_) == OpKind::Drop);