only pop try block at the end of catch

fixes #87

crates/dash_compiler/src/builder.rs

@@ -7,7 +7,7 @@ use dash_middle::compiler::instruction::Instruction;
 use dash_middle::parser::statement::ScopeId;
 
 use crate::jump_container::JumpContainer;
-use crate::{jump_container, FunctionCompiler};
+use crate::{FunctionCompiler, jump_container};
 
 #[derive(Hash, Eq, PartialEq, Debug, Clone, Copy)]
 pub enum Label {

crates/dash_compiler/src/instruction.rs

@@ -56,7 +56,7 @@ impl InstructionBuilder<'_, '_> {
         build_this Instruction::This,
         build_strict_eq Instruction::StrictEq,
         build_strict_ne Instruction::StrictNe,
-        build_try_end Instruction::TryEnd,
+        build_pop_try Instruction::PopTry,
         build_throw Instruction::Throw,
         build_yield Instruction::Yield,
         build_await Instruction::Await,

crates/dash_compiler/src/lib.rs

@@ -1466,6 +1466,7 @@ impl Visitor<Result<(), Error>> for FunctionCompiler<'_> {
         let tc_depth = ib.current_function().try_depth;
         ib.accept_expr(stmt)?;
         if let Some(finally) = finally {
+            ib.build_pop_try();
             ib.write_instr(Instruction::DelayedReturn);
             ib.build_jmp(finally, false);
         } else {
@@ -1855,6 +1856,61 @@ impl Visitor<Result<(), Error>> for FunctionCompiler<'_> {
         Ok(())
     }
 
+    /// A try block is currently compiled to the following bytecode:
+    /// ```ignore
+    /// try
+    /// <try code> -- jmp to #catch on exception
+    /// poptry
+    /// jmp tryend
+    ///
+    /// catch:
+    /// <catch code>
+    /// poptry (if finally block exists, due to re-pushing with catch=None in handle_rt_error)
+    ///
+    /// -- If there's a finally block
+    /// finally:
+    /// tryend:
+    /// <finally code>
+    /// finallyend (executes any delayed returns)
+    ///
+    /// -- If there's no finally block
+    /// tryend:
+    /// poptry
+    /// ```
+    ///
+    /// We first emit a `try` instruction which takes two ip operands,
+    /// the ip for the start of the catch block and optionally the ip for the finally block.
+    /// The vm will push that into a `try_blocks` stack that we will look at the top at (at runtime) when an exception occurs.
+    /// (Initially this is just a placeholder 0 ip but it gets patched when we actually register the label later)
+    ///
+    /// It's important that we always cleanup the pushed trycatch as we might end up looking at the stack at any point,
+    /// so there are a few scenarios to consider.
+    /// We also need to pop it again at specific times so that throwing an exception from within different parts of the try block
+    /// do or don't jump to the same try block. (E.g., throwing an exception in a catch block should look for a different enclosing try-catch,
+    /// but throwing an exception in the try needs the same to be there.)
+    ///
+    /// 1.) Try block finishes without exceptions: we pop the try block and jump to the tryend label,
+    /// which is either the end of the try-catch entirely or is also the start of the finally code.
+    /// No trycatch block will be on the stack; finally block also does not need it.
+    ///
+    /// 2.) Try block runs into an exception: this is almost entirely handled at runtime,
+    /// but we look at the top of the trycatch stack, pop **iff** it's part of the same frame and then
+    /// jump to either its catch or finally ip. If we immediately jump to a finally, we also register a delayed_return
+    /// for the exception so we delay-throw it at the end of the finally block.
+    /// If the trycatch block has a finally, then we re-push with catch:None so we won't jump to the catch again
+    /// but will jump to the finally block.
+    ///
+    /// 3.) Try block returns: this may need to jump to a finally block, so try to find an enclosing finally
+    /// (may or may not be the same try block we're in), jump to it and pop the current try block.
+    ///
+    /// 4.) Catch block finishes without further exceptions: if there's a finally block,
+    /// then we've pushed a `{catch:None,finally_ip:...}` trycatch, so execute a poptry if there's a finally block.
+    /// We also naturally flow into the finally code without needing any jumps, if that block exists.
+    ///
+    /// 5.) Catch block runs into an exception: if there's a finally block, then we will jump to that since we've re-pushed one.
+    /// If there isn't one, we will find another enclosing try-catch block.
+    ///
+    /// 6.) Finally block: at this point the current try block must have been popped. At the end, execute any delayed_returns.
     fn visit_try_catch(&mut self, span: Span, TryCatch { try_, catch, finally }: TryCatch) -> Result<(), Error> {
         let mut ib = InstructionBuilder::new(self);
 
@@ -1870,6 +1926,7 @@ impl Visitor<Result<(), Error>> for FunctionCompiler<'_> {
         ib.current_function_mut().try_depth -= 1;
         res?;
 
+        ib.build_pop_try();
         ib.build_jmp(Label::TryEnd, true);
 
         if catch.is_none() && finally.is_none() {
@@ -1894,22 +1951,24 @@ impl Visitor<Result<(), Error>> for FunctionCompiler<'_> {
             }
 
             ib.visit_block_statement(catch.body_span, catch.body)?;
+
+            if finally.is_some() {
+                ib.build_pop_try();
+            }
         }
         ib.current_function_mut().finally_labels.pop();
 
         if let Some((finally_id, finally)) = finally {
             ib.current_function_mut()
                 .add_global_label(Label::Finally { finally_id });
             ib.add_local_label(Label::TryEnd);
-            ib.build_try_end();
 
             ib.accept(*finally)?;
 
             ib.write_instr(Instruction::FinallyEnd);
             ib.writew(ib.current_function().try_depth);
         } else {
             ib.add_local_label(Label::TryEnd);
-            ib.build_try_end();
         }
 
         Ok(())

crates/dash_decompiler/src/decompiler.rs

@@ -204,13 +204,10 @@ impl<'interner, 'buf> FunctionDecompiler<'interner, 'buf> {
                 }
                 Instruction::Call => {
                     let meta = FunctionCallMetadata::from(self.read()?);
-                    self.handle_op_map_instr(
-                        "call",
-                        &[
-                            ("argc", &meta.value()),
-                            ("is_constructor_call", &meta.is_constructor_call()),
-                        ],
-                    );
+                    self.handle_op_map_instr("call", &[
+                        ("argc", &meta.value()),
+                        ("is_constructor_call", &meta.is_constructor_call()),
+                    ]);
                 }
                 Instruction::StaticPropAccess => {
                     let b = self.read_u16()?;
@@ -306,7 +303,7 @@ impl<'interner, 'buf> FunctionDecompiler<'interner, 'buf> {
                 Instruction::StrictEq => self.handle_opless_instr("stricteq"),
                 Instruction::StrictNe => self.handle_opless_instr("strictne"),
                 Instruction::Try => self.handle_incw_op_instr("try")?, // TODO: show @offset like in JMP
-                Instruction::TryEnd => self.handle_opless_instr("tryend"),
+                Instruction::PopTry => self.handle_opless_instr("poptry"),
                 Instruction::FinallyEnd => self.handle_opless_instr("finallyend"),
                 Instruction::Throw => self.handle_opless_instr("throw"),
                 Instruction::Yield => self.handle_opless_instr("yield"),

crates/dash_middle/src/compiler/instruction.rs

@@ -57,7 +57,7 @@ pub enum Instruction {
     StrictEq,
     StrictNe,
     Try,
-    TryEnd,
+    PopTry,
     FinallyEnd,
     Throw,
     Yield,

crates/dash_vm/src/dispatch.rs

@@ -1843,7 +1843,7 @@ mod handlers {
         Ok(None)
     }
 
-    pub fn try_end(mut cx: DispatchContext<'_>) -> Result<Option<HandleResult>, Unrooted> {
+    pub fn pop_try(mut cx: DispatchContext<'_>) -> Result<Option<HandleResult>, Unrooted> {
         cx.try_blocks.pop();
         Ok(None)
     }
@@ -2363,7 +2363,7 @@ pub fn handle(vm: &mut Vm, instruction: Instruction) -> Result<Option<HandleResu
         Instruction::StrictEq => handlers::strict_eq(cx),
         Instruction::StrictNe => handlers::strict_ne(cx),
         Instruction::Try => handlers::try_block(cx),
-        Instruction::TryEnd => handlers::try_end(cx),
+        Instruction::PopTry => handlers::pop_try(cx),
         Instruction::FinallyEnd => handlers::finally_end(cx),
         Instruction::Throw => handlers::throw(cx),
         Instruction::Yield => handlers::yield_(cx),

crates/dash_vm/src/lib.rs

@@ -1342,8 +1342,7 @@ impl Vm {
                 }
             } else if let Some(finally_ip) = finally_ip {
                 self.active_frame_mut().delayed_ret = Some(Err(err));
-                // `+ 1` because we need to jump over the `TryEnd` instruction since there won't be a try block to pop.
-                self.active_frame_mut().ip = finally_ip + 1;
+                self.active_frame_mut().ip = finally_ip;
             }
 
             Ok(())

crates/dash_vm/src/test/mod.rs

@@ -4,12 +4,12 @@ use std::ptr;
 use dash_middle::interner::sym;
 use dash_optimizer::OptLevel;
 
-use crate::gc::persistent::Persistent;
+use crate::Vm;
 use crate::gc::ObjectId;
+use crate::gc::persistent::Persistent;
 use crate::value::object::{NamedObject, Object, PropertyValue};
 use crate::value::primitive::{Null, Number, Symbol, Undefined};
 use crate::value::{Root, Unpack, Value, ValueKind};
-use crate::Vm;
 
 const INTERPRETER: &str = include_str!("interpreter.js");
 
@@ -614,6 +614,15 @@ simple_test!(
     })() === 5);
     assert(o === '135');
 
+    // Issue #87
+    try {
+        try {
+            throw 1;
+        } catch(e) { }
+    
+        throw 1;
+    } catch(e) { }
+
     "#,
     Value::undefined()
 );