Bug 1728897 - Update Wasm try-delegate semantics to match spec r=rhunt

The semantics of the try-delegate Wasm exception handling instruction was recently changed in this spec discussion: WebAssembly/exception-handling#176 This patch adjusts compilation and validation to match the new semantics, which allows delegate to target any block. Differential Revision: https://phabricator.services.mozilla.com/D124424
aosmond · Sep 9, 2021 · d5eb3ec · d5eb3ec
1 parent e9afada
commit d5eb3ec
Show file tree

Hide file tree

Showing 7 changed files with 119 additions and 33 deletions.
diff --git a/js/src/jit-test/tests/wasm/exceptions/instructions.js b/js/src/jit-test/tests/wasm/exceptions/instructions.js
@@ -1136,7 +1136,7 @@ assertEq(
   1
 );
 
-// Test delegation to function body.
+// Test delegation to function body and blocks.
 assertEq(
   wasmEvalText(
     `(module
@@ -1149,6 +1149,46 @@ assertEq(
   1
 );
 
+assertEq(
+  wasmEvalText(
+    `(module
+       (tag $exn (param i32))
+       (func (export "f") (result i32)
+         try (result i32)
+           block
+             try
+               i32.const 1
+               throw $exn
+             delegate 0
+           end
+           i32.const 0
+         catch $exn
+         end))`
+  ).exports.f(),
+  1
+);
+
+assertEq(
+  wasmEvalText(
+    `(module
+       (tag $exn (param))
+       (func (export "f") (result i32)
+         try (result i32)
+           try
+             throw $exn
+           catch $exn
+             try
+               throw $exn
+             delegate 0
+           end
+           i32.const 0
+         catch_all
+           i32.const 1
+         end))`
+  ).exports.f(),
+  1
+);
+
 assertEq(
   wasmEvalText(
     `(module

diff --git a/js/src/jit-test/tests/wasm/exceptions/throw-to-js.js b/js/src/jit-test/tests/wasm/exceptions/throw-to-js.js
@@ -72,6 +72,46 @@ assertWasmThrowsExn(() =>
   ).exports.f()
 );
 
+assertWasmThrowsExn(() =>
+  wasmEvalText(
+    `(module
+       (tag $exn (param))
+       (func (export "f")
+         block
+           try
+             throw $exn
+           delegate 0
+         end))`
+  ).exports.f()
+);
+
+assertWasmThrowsExn(() =>
+  wasmEvalText(
+    `(module
+       (tag $exn (param))
+       (func (export "f")
+         loop
+           try
+             throw $exn
+           delegate 0
+         end))`
+  ).exports.f()
+);
+
+assertWasmThrowsExn(() =>
+  wasmEvalText(
+    `(module
+       (tag $exn (param))
+       (func (export "f")
+         (i32.const 1)
+         if
+           try
+             throw $exn
+           delegate 0
+         end))`
+  ).exports.f()
+);
+
 // Test throwing simple empty exceptions to JS.
 assertWasmThrowsExn(() =>
   wasmEvalText(

diff --git a/js/src/jit-test/tests/wasm/exceptions/validation.js b/js/src/jit-test/tests/wasm/exceptions/validation.js
@@ -576,6 +576,29 @@ function testValidateDelegate() {
          end))`
   );
 
+  wasmValidateText(
+    `(module
+       (tag $exn (param))
+       (func
+         block
+           try
+             throw $exn
+           delegate 0
+         end))`
+  );
+
+  wasmValidateText(
+    `(module
+       (tag $exn (param))
+       (func
+         try
+         catch $exn
+           try
+             throw $exn
+           delegate 0
+         end))`
+  );
+
   wasmFailValidateText(
     `(module
        (tag $exn (param))
@@ -611,31 +634,6 @@ function testValidateDelegate() {
     /delegate depth exceeds current nesting level/
   );
 
-  wasmFailValidateText(
-    `(module
-       (tag $exn (param))
-       (func
-         block
-           try
-             throw $exn
-           delegate 0
-         end))`,
-    /delegate target was not a try or function body/
-  );
-
-  wasmFailValidateText(
-    `(module
-       (tag $exn (param))
-       (func
-         try
-         catch $exn
-           try
-             throw $exn
-           delegate 0
-         end))`,
-    /delegate target was not a try or function body/
-  );
-
   wasmFailValidateText(
     `(module (func delegate 0))`,
     /delegate can only be used within a try/

diff --git a/js/src/wasm/WasmBCClass.h b/js/src/wasm/WasmBCClass.h
@@ -867,6 +867,7 @@ struct BaseCompiler final {
   inline Control& controlItem();
   inline Control& controlItem(uint32_t relativeDepth);
   inline Control& controlOutermost();
+  inline LabelKind controlKind(uint32_t relativeDepth);
 
   ////////////////////////////////////////////////////////////
   //

diff --git a/js/src/wasm/WasmBCRegMgmt-inl.h b/js/src/wasm/WasmBCRegMgmt-inl.h
@@ -443,6 +443,10 @@ Control& BaseCompiler::controlItem(uint32_t relativeDepth) {
 
 Control& BaseCompiler::controlOutermost() { return iter_.controlOutermost(); }
 
+LabelKind BaseCompiler::controlKind(uint32_t relativeDepth) {
+  return iter_.controlKind(relativeDepth);
+}
+
 }  // namespace wasm
 }  // namespace js
 

diff --git a/js/src/wasm/WasmBaselineCompile.cpp b/js/src/wasm/WasmBaselineCompile.cpp
@@ -3744,7 +3744,6 @@ bool BaseCompiler::emitDelegate() {
   }
 
   Control& tryDelegate = controlItem();
-  Control& target = controlItem(relativeDepth);
 
   // End the try branch like a plain catch block without exception ref handling.
   if (deadCode_) {
@@ -3778,6 +3777,15 @@ bool BaseCompiler::emitDelegate() {
   tryNote.entryPoint = tryNote.end;
   tryNote.framePushed = masm.framePushed();
 
+  // If the target block is a non-try block, skip over it and find the next
+  // try block or the very last block (to re-throw out of the function).
+  Control& lastBlock = controlOutermost();
+  while (controlKind(relativeDepth) != LabelKind::Try &&
+         &controlItem(relativeDepth) != &lastBlock) {
+    relativeDepth++;
+  }
+  Control& target = controlItem(relativeDepth);
+
   popBlockResults(ResultType::Empty(), target.stackHeight,
                   ContinuationKind::Jump);
   masm.jump(&target.otherLabel);
@@ -3799,7 +3807,7 @@ bool BaseCompiler::endTryCatch(ResultType type) {
   uint32_t lineOrBytecode = readCallSiteLineOrBytecode();
 
   Control& tryCatch = controlItem();
-  LabelKind tryKind = iter_.controlKind(0);
+  LabelKind tryKind = controlKind(0);
 
   if (deadCode_) {
     fr.resetStackHeight(tryCatch.stackHeight, type);

diff --git a/js/src/wasm/WasmOpIter.h b/js/src/wasm/WasmOpIter.h
@@ -1626,11 +1626,6 @@ inline bool OpIter<Policy>::readDelegate(uint32_t* relativeDepth,
     return fail("delegate depth exceeds current nesting level");
   }
 
-  LabelKind kind = controlKind(*relativeDepth);
-  if (kind != LabelKind::Try && kind != LabelKind::Body) {
-    return fail("delegate target was not a try or function body");
-  }
-
   // Because `delegate` acts like `end` and ends the block, we will check
   // the stack here.
   return checkStackAtEndOfBlock(resultType, tryResults);