wasm-interp: Fix catch handlers' value stack sizes (#2478)

Fixes the value stack size of the catch handler. There were two
(related) issues here:

- The previous code used `func_->locals.size()` as soon as the function
was available, but it hadn't processed the function's locals yet, so it
was always empty. (This might not matter in practice, as it's only used
by the "function-wide catch handler", which just rethrows.)
- The previous code didn't take the function's locals into account when
computing the value stack height (relative to the function frame) for a
try-catch block. So, it would drop the locals when catching an
exception.

Closes #2476 

(Split from #2470 )

Author: SoniEx2

src/interp/binary-reader-interp.cc

@@ -831,13 +831,6 @@ Result BinaryReaderInterp::BeginFunctionBody(Index index, Offset size) {
   // try-delegate instruction.
   PushLabel(LabelKind::Try, Istream::kInvalidOffset, Istream::kInvalidOffset,
             func_->handlers.size());
-  func_->handlers.push_back(HandlerDesc{HandlerKind::Catch,
-                                        istream_.end(),
-                                        Istream::kInvalidOffset,
-                                        {},
-                                        {Istream::kInvalidOffset},
-                                        static_cast<u32>(func_->locals.size()),
-                                        0});
   return Result::Ok;
 }
 
@@ -856,6 +849,17 @@ 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;
 }
 
@@ -1516,7 +1520,9 @@ Result BinaryReaderInterp::OnTryExpr(Type sig_type) {
   u32 exn_stack_height;
   CHECK_RESULT(
       validator_.GetCatchCount(label_stack_.size() - 1, &exn_stack_height));
-  u32 value_stack_height = validator_.type_stack_size();
+  // 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_;
   CHECK_RESULT(validator_.OnTry(GetLocation(), sig_type));
   // Push a label that tracks mapping of exn -> catch
   PushLabel(LabelKind::Try, Istream::kInvalidOffset, Istream::kInvalidOffset,

test/regress/interp-ehv3-locals.txt

@@ -0,0 +1,22 @@
+;;; TOOL: run-interp-spec
+;;; ARGS*: --enable-exceptions
+;;; NOTE: ref: issue-2476
+(module
+  (tag $e0)
+  (func (export "broken-local") (result i32)
+    (local $value i32)
+    (try $try
+      (do
+        (local.set $value (i32.const 1))
+        (throw $e0)
+      )
+      (catch $e0)
+    )
+    (local.get $value)
+  )
+)
+
+(assert_return (invoke "broken-local") (i32.const 1))
+(;; STDOUT ;;;
+2/2 tests passed.
+;;; STDOUT ;;)