From 1254b564b2d36cbf96ef8b8fe0c17fa2fa668ae3 Mon Sep 17 00:00:00 2001
From: Heejin Ahn <aheejin@gmail.com>
Date: Thu, 18 Feb 2021 04:36:56 +0900
Subject: [EH] Make rethrow's target a try label (#3568)

I was previously mistaken about `rethrow`'s argument rule and thought
it only counted `catch`'s depth. But it turns out it follows the same
rule `delegate`'s label: the immediate argument follows the same rule as
when computing branch labels, but it only can target `try` labels
(semantically it targets that `try`'s corresponding `catch`); otherwise
it will be a validation failure. Unlike `delegate`, `rethrow`'s label
denotes not where to rethrow, but which exception to rethrow. For
example,
```wasm
try $l0
catch ($l0)
  try $l1
  catch ($l1)
   rethrow $l0 ;; rethrow the exception caught by 'catch ($l0)'
  end
end
```

Refer to this comment for the more detailed informal semantics:
https://github.com/WebAssembly/exception-handling/issues/146#issuecomment-777714491

---

This also reverts some of `delegateTarget` -> `exceptionTarget` changes
done in #3562 in the validator. Label validation rules apply differently
for `delegate` and `rethrow` for try-catch. For example, this is valid:
```wasm
try $l0
  try
  delegate $l0
catch ($l0)
end
```
But this is NOT valid:
```wasm
try $l0
catch ($l0)
  try
  delegate $l0
end
```
So `try`'s label should be used within try-catch range (not catch-end
range) for `delegate`s.

But for the `rethrow` the rule is different. For example, this is valid:
```wasm
try $l0
catch ($l0)
  rethrow $l0
end
```
But this is NOT valid:
```wasm
try $l0
  rethrow $l0
catch ($l0)
end
```
So the `try`'s label should be used within catch-end range instead.
---
 src/wasm/wasm-s-parser.cpp | 29 +++--------------------------
 1 file changed, 3 insertions(+), 26 deletions(-)

(limited to 'src/wasm/wasm-s-parser.cpp')

diff --git a/src/wasm/wasm-s-parser.cpp b/src/wasm/wasm-s-parser.cpp
index 0ff4bfb9f..13b76801b 100644
--- a/src/wasm/wasm-s-parser.cpp
+++ b/src/wasm/wasm-s-parser.cpp
@@ -2067,8 +2067,8 @@ Expression* SExpressionWasmBuilder::makeTry(Element& s) {
     // We create a different name for the wrapping block, because try's name can
     // be used by internal delegates
     block->name = nameMapper.pushLabelName(sName);
-    // For simplicity, try's name canonly be targeted by delegates. Make the
-    // branches target the new wrapping block instead.
+    // For simplicity, try's name can only be targeted by delegates and
+    // rethrows. Make the branches target the new wrapping block instead.
     BranchUtils::replaceBranchTargets(ret, ret->name, block->name);
     block->list.push_back(ret);
     nameMapper.popLabelName(block->name);
@@ -2078,29 +2078,6 @@ Expression* SExpressionWasmBuilder::makeTry(Element& s) {
   return ret;
 }
 
-Expression*
-SExpressionWasmBuilder::makeTryOrCatchBody(Element& s, Type type, bool isTry) {
-  if (isTry && !elementStartsWith(s, "do")) {
-    throw ParseException("invalid try do clause", s.line, s.col);
-  }
-  if (!isTry && !elementStartsWith(s, "catch") &&
-      !elementStartsWith(s, "catch_all")) {
-    throw ParseException("invalid catch clause", s.line, s.col);
-  }
-  if (s.size() == 1) { // (do) / (catch) / (catch_all) without instructions
-    return makeNop();
-  }
-  auto ret = allocator.alloc<Block>();
-  for (size_t i = 1; i < s.size(); i++) {
-    ret->list.push_back(parseExpression(s[i]));
-  }
-  if (ret->list.size() == 1) {
-    return ret->list[0];
-  }
-  ret->finalize(type);
-  return ret;
-}
-
 Expression* SExpressionWasmBuilder::makeThrow(Element& s) {
   auto ret = allocator.alloc<Throw>();
   Index i = 1;
@@ -2118,7 +2095,7 @@ Expression* SExpressionWasmBuilder::makeThrow(Element& s) {
 
 Expression* SExpressionWasmBuilder::makeRethrow(Element& s) {
   auto ret = allocator.alloc<Rethrow>();
-  ret->depth = atoi(s[1]->str().c_str());
+  ret->target = getLabel(*s[1], LabelType::Exception);
   ret->finalize();
   return ret;
 }
-- 
cgit v1.2.3