wasm-interp: Fix catch handlers correctly (#2483)

local decl count != local count

8 files changed, 40 insertions, 16 deletions

diff --git a/include/wabt/binary-reader-logging.h b/include/wabt/binary-reader-logging.h
index 4bb16deb..fee211f9 100644
--- a/include/wabt/binary-reader-logging.h
+++ b/include/wabt/binary-reader-logging.h
@@ -130,6 +130,7 @@ class BinaryReaderLogging : public BinaryReaderDelegate {
   Result BeginFunctionBody(Index index, Offset size) override;
   Result OnLocalDeclCount(Index count) override;
   Result OnLocalDecl(Index decl_index, Index count, Type type) override;
+  Result EndLocalDecls() override;
 
   Result OnOpcode(Opcode opcode) override;
   Result OnOpcodeBare() override;
diff --git a/include/wabt/binary-reader-nop.h b/include/wabt/binary-reader-nop.h
index ffa3a4b7..c7ec78b1 100644
--- a/include/wabt/binary-reader-nop.h
+++ b/include/wabt/binary-reader-nop.h
@@ -172,6 +172,7 @@ class BinaryReaderNop : public BinaryReaderDelegate {
   Result OnLocalDecl(Index decl_index, Index count, Type type) override {
     return Result::Ok;
   }
+  Result EndLocalDecls() override { return Result::Ok; }
 
   /* Function expressions; called between BeginFunctionBody and
    EndFunctionBody */
diff --git a/include/wabt/binary-reader.h b/include/wabt/binary-reader.h
index de7c4e8f..3d48f574 100644
--- a/include/wabt/binary-reader.h
+++ b/include/wabt/binary-reader.h
@@ -188,6 +188,7 @@ class BinaryReaderDelegate {
   virtual Result BeginFunctionBody(Index index, Offset size) = 0;
   virtual Result OnLocalDeclCount(Index count) = 0;
   virtual Result OnLocalDecl(Index decl_index, Index count, Type type) = 0;
+  virtual Result EndLocalDecls() = 0;
 
   /* Function expressions; called between BeginFunctionBody and
    EndFunctionBody */
diff --git a/src/binary-reader-logging.cc b/src/binary-reader-logging.cc
index 653e1e48..ea427391 100644
--- a/src/binary-reader-logging.cc
+++ b/src/binary-reader-logging.cc
@@ -795,6 +795,7 @@ DEFINE_BEGIN(BeginCodeSection)
 DEFINE_INDEX(OnFunctionBodyCount)
 DEFINE_INDEX(EndFunctionBody)
 DEFINE_INDEX(OnLocalDeclCount)
+DEFINE0(EndLocalDecls)
 DEFINE_LOAD_STORE_OPCODE(OnAtomicLoadExpr);
 DEFINE_LOAD_STORE_OPCODE(OnAtomicRmwExpr);
 DEFINE_LOAD_STORE_OPCODE(OnAtomicRmwCmpxchgExpr);
diff --git a/src/binary-reader.cc b/src/binary-reader.cc
index b4ad5b0c..b68562dd 100644
--- a/src/binary-reader.cc
+++ b/src/binary-reader.cc
@@ -2815,6 +2815,7 @@ Result BinaryReader::ReadCodeSection(Offset section_size) {
       ERROR_UNLESS(IsConcreteType(local_type), "expected valid local type");
       CALLBACK(OnLocalDecl, k, num_local_types, local_type);
     }
+    CALLBACK(EndLocalDecls);
 
     if (options_.skip_function_bodies) {
       state_.offset = end_offset;
diff --git a/src/interp/binary-reader-interp.cc b/src/interp/binary-reader-interp.cc
index f4d05a19..dc7ec410 100644
--- a/src/interp/binary-reader-interp.cc
+++ b/src/interp/binary-reader-interp.cc
@@ -147,6 +147,7 @@ class BinaryReaderInterp : public BinaryReaderNop {
   Result BeginFunctionBody(Index index, Offset size) override;
   Result OnLocalDeclCount(Index count) override;
   Result OnLocalDecl(Index decl_index, Index count, Type type) override;
+  Result EndLocalDecls() override;
 
   Result OnOpcode(Opcode Opcode) override;
   Result OnAtomicLoadExpr(Opcode opcode,
@@ -849,17 +850,6 @@ Result BinaryReaderInterp::EndFunctionBody(Index index) {
 Result BinaryReaderInterp::OnLocalDeclCount(Index count) {
   local_decl_count_ = count;
   local_count_ = 0;
-  // Continuation of the implicit func label, used for exception handling. (See
-  // BeginFunctionBody.)
-  // We need the local count for this, so we must do it here.
-  // NOTE: we don't count the parameters, as they're not part of the frame.
-  func_->handlers.push_back(HandlerDesc{HandlerKind::Catch,
-                                        istream_.end(),
-                                        Istream::kInvalidOffset,
-                                        {},
-                                        {Istream::kInvalidOffset},
-                                        static_cast<u32>(local_decl_count_),
-                                        0});
   return Result::Ok;
 }
 
@@ -870,10 +860,26 @@ Result BinaryReaderInterp::OnLocalDecl(Index decl_index,
 
   local_count_ += count;
   func_->locals.push_back(LocalDesc{type, count, local_count_});
+  return Result::Ok;
+}
 
-  if (decl_index == local_decl_count_ - 1) {
+Result BinaryReaderInterp::EndLocalDecls() {
+  if (local_count_ != 0) {
     istream_.Emit(Opcode::InterpAlloca, local_count_);
   }
+  // Continuation of the implicit func label, used for exception handling. (See
+  // BeginFunctionBody.)
+  // We need the local count for this, which is only available after processing
+  // all local decls.
+  // NOTE: we don't count the parameters, as they're not part of the frame.
+  func_->handlers.push_back(HandlerDesc{HandlerKind::Catch,
+                                        istream_.end(),
+                                        Istream::kInvalidOffset,
+                                        {},
+                                        {Istream::kInvalidOffset},
+                                        static_cast<u32>(local_count_),
+                                        0});
+
   return Result::Ok;
 }
 
@@ -1522,7 +1528,7 @@ Result BinaryReaderInterp::OnTryExpr(Type sig_type) {
       validator_.GetCatchCount(label_stack_.size() - 1, &exn_stack_height));
   // NOTE: *NOT* GetLocalCount. we don't count the parameters, as they're not
   // part of the frame.
-  u32 value_stack_height = validator_.type_stack_size() + local_decl_count_;
+  u32 value_stack_height = validator_.type_stack_size() + local_count_;
   CHECK_RESULT(validator_.OnTry(GetLocation(), sig_type));
   // Push a label that tracks mapping of exn -> catch
   PushLabel(LabelKind::Try, Istream::kInvalidOffset, Istream::kInvalidOffset,
diff --git a/test/interp/basic-logging.txt b/test/interp/basic-logging.txt
index d175d1c4..f62baea6 100644
--- a/test/interp/basic-logging.txt
+++ b/test/interp/basic-logging.txt
@@ -62,6 +62,7 @@ BeginModule(version: 1)
     OnFunctionBodyCount(1)
     BeginFunctionBody(0, size:5)
     OnLocalDeclCount(0)
+    EndLocalDecls
     OnI32ConstExpr(42 (0x2a))
     OnReturnExpr
     OnEndExpr
diff --git a/test/regress/interp-ehv3-locals.txt b/test/regress/interp-ehv3-locals.txt
index e4f05c83..316563ab 100644
--- a/test/regress/interp-ehv3-locals.txt
+++ b/test/regress/interp-ehv3-locals.txt
@@ -3,7 +3,7 @@
 ;;; NOTE: ref: issue-2476
 (module
   (tag $e0)
-  (func (export "broken-local") (result i32)
+  (func (export "set-local") (result i32)
     (local $value i32)
     (try $try
       (do
@@ -14,9 +14,21 @@
     )
     (local.get $value)
   )
+  (func (export "multiple-locals") (result i32)
+    (local $a i32)
+    (local $b i32)
+    (try $try
+      (do
+        (throw $e0)
+      )
+      (catch $e0)
+    )
+    (local.get $a)
+  )
 )
 
-(assert_return (invoke "broken-local") (i32.const 1))
+(assert_return (invoke "set-local") (i32.const 1))
+(assert_return (invoke "multiple-locals") (i32.const 0))
 (;; STDOUT ;;;
-2/2 tests passed.
+3/3 tests passed.
 ;;; STDOUT ;;)