From a164e1224e2014cbd9bc094883450f5605382cdd Mon Sep 17 00:00:00 2001
From: Heejin Ahn <aheejin@gmail.com>
Date: Wed, 9 Oct 2019 01:30:11 -0700
Subject: [PATCH] Make try body with multiple instructions roundtrip

Previously we didn't print an additional block when there are multiple
instructions within a `try` body, so those wast files cannot be parsed
correctly, because the wast parser assumes there are two bodies within a
`try` scope: a try body and a catch body.

We don't need to print an additional block for a `catch` body because
`(catch ...)` itself serves as a scope.
---
 src/passes/Print.cpp                          |   2 +-
 src/wasm/wasm-s-parser.cpp                    |   2 +-
 test/exception-handling.wast                  |  16 +++++++++++
 test/exception-handling.wast.from-wast        |  21 +++++++++++++-
 test/exception-handling.wast.fromBinary       |   8 +++++-
 ...ption-handling.wast.fromBinary.noDebugInfo |   6 ++++
 test/try-body-multiple-insts.wasm             | Bin 0 -> 49 bytes
 test/try-body-multiple-insts.wasm.fromBinary  |  26 ++++++++++++++++++
 8 files changed, 77 insertions(+), 4 deletions(-)
 create mode 100644 test/try-body-multiple-insts.wasm
 create mode 100644 test/try-body-multiple-insts.wasm.fromBinary

diff --git a/src/passes/Print.cpp b/src/passes/Print.cpp
index a5b5e3c2b2d..95c6a056831 100644
--- a/src/passes/Print.cpp
+++ b/src/passes/Print.cpp
@@ -1768,7 +1768,7 @@ struct PrintSExpression : public OverriddenVisitor<PrintSExpression> {
     o << '(';
     PrintExpressionContents(currFunction, o).visit(curr);
     incIndent();
-    maybePrintImplicitBlock(curr->body, true);
+    maybePrintImplicitBlock(curr->body, false);
     doIndent(o, indent);
     o << "(catch";
     incIndent();
diff --git a/src/wasm/wasm-s-parser.cpp b/src/wasm/wasm-s-parser.cpp
index 72d262e6383..f666abc2575 100644
--- a/src/wasm/wasm-s-parser.cpp
+++ b/src/wasm/wasm-s-parser.cpp
@@ -1792,7 +1792,7 @@ Expression* SExpressionWasmBuilder::makeTry(Element& s) {
     ret->body = parseExpression(*s[i++]);
   }
   if (!elementStartsWith(*s[i], "catch")) {
-    throw ParseException("catch clause does not exist");
+    throw ParseException("catch clause does not exist", s[i]->line, s[i]->col);
   }
   ret->catchBody = makeCatch(*s[i++]);
   ret->finalize(type);
diff --git a/test/exception-handling.wast b/test/exception-handling.wast
index cb3aee0d933..275350c615e 100644
--- a/test/exception-handling.wast
+++ b/test/exception-handling.wast
@@ -5,6 +5,9 @@
     (local.get $0)
   )
 
+  (func $foo)
+  (func $bar)
+
   (func $eh_test (local $exn exnref)
     (try
       (throw $e0 (i32.const 0))
@@ -36,5 +39,18 @@
         (drop (exnref.pop))
       )
     )
+
+    ;; Multiple instructions within try and catch bodies
+    (try
+      (block
+        (call $foo)
+        (call $bar)
+      )
+      (catch
+        (drop (exnref.pop))
+        (call $foo)
+        (call $bar)
+      )
+    )
   )
 )
diff --git a/test/exception-handling.wast.from-wast b/test/exception-handling.wast.from-wast
index 7c431a89f73..e3209518ef0 100644
--- a/test/exception-handling.wast.from-wast
+++ b/test/exception-handling.wast.from-wast
@@ -6,7 +6,13 @@
  (func $exnref_test (; 0 ;) (type $FUNCSIG$ee) (param $0 exnref) (result exnref)
   (local.get $0)
  )
- (func $eh_test (; 1 ;) (type $FUNCSIG$v)
+ (func $foo (; 1 ;) (type $FUNCSIG$v)
+  (nop)
+ )
+ (func $bar (; 2 ;) (type $FUNCSIG$v)
+  (nop)
+ )
+ (func $eh_test (; 3 ;) (type $FUNCSIG$v)
   (local $exn exnref)
   (try
    (throw $e0
@@ -43,5 +49,18 @@
     )
    )
   )
+  (try
+   (block $block
+    (call $foo)
+    (call $bar)
+   )
+   (catch
+    (drop
+     (exnref.pop)
+    )
+    (call $foo)
+    (call $bar)
+   )
+  )
  )
 )
diff --git a/test/exception-handling.wast.fromBinary b/test/exception-handling.wast.fromBinary
index 1cff3d21577..3151d1ea8e9 100644
--- a/test/exception-handling.wast.fromBinary
+++ b/test/exception-handling.wast.fromBinary
@@ -6,7 +6,13 @@
  (func $exnref_test (; 0 ;) (type $0) (param $0 exnref) (result exnref)
   (local.get $0)
  )
- (func $eh_test (; 1 ;) (type $1)
+ (func $foo (; 1 ;) (type $1)
+  (nop)
+ )
+ (func $bar (; 2 ;) (type $1)
+  (nop)
+ )
+ (func $eh_test (; 3 ;) (type $1)
   (local $0 exnref)
   (try
    (throw $event$0
diff --git a/test/exception-handling.wast.fromBinary.noDebugInfo b/test/exception-handling.wast.fromBinary.noDebugInfo
index 23ba7946c1f..3ef95033c61 100644
--- a/test/exception-handling.wast.fromBinary.noDebugInfo
+++ b/test/exception-handling.wast.fromBinary.noDebugInfo
@@ -7,6 +7,12 @@
   (local.get $0)
  )
  (func $1 (; 1 ;) (type $1)
+  (nop)
+ )
+ (func $2 (; 2 ;) (type $1)
+  (nop)
+ )
+ (func $3 (; 3 ;) (type $1)
   (local $0 exnref)
   (try
    (throw $event$0
diff --git a/test/try-body-multiple-insts.wasm b/test/try-body-multiple-insts.wasm
new file mode 100644
index 0000000000000000000000000000000000000000..dd67f3e529b9b858bcca0b8dca265f160acf7240
GIT binary patch
literal 49
ycmZQbEY4+QU|?WmVN76PU}j+kvbm(0nHd<lL0FKHF@w!PfI)zfT?#~Ua{~Zi&;v67

literal 0
HcmV?d00001

diff --git a/test/try-body-multiple-insts.wasm.fromBinary b/test/try-body-multiple-insts.wasm.fromBinary
new file mode 100644
index 00000000000..f52a1a3dcac
--- /dev/null
+++ b/test/try-body-multiple-insts.wasm.fromBinary
@@ -0,0 +1,26 @@
+(module
+ (type $0 (func))
+ (func $0 (; 0 ;) (type $0)
+  (nop)
+ )
+ (func $1 (; 1 ;) (type $0)
+  (nop)
+ )
+ (func $2 (; 2 ;) (type $0)
+  (local $0 exnref)
+  (try
+   (block
+    (call $0)
+    (call $1)
+   )
+   (catch
+    (drop
+     (exnref.pop)
+    )
+    (call $0)
+    (call $1)
+   )
+  )
+ )
+)
+