Fix panics and refactor code

core/engine/src/bytecompiler/mod.rs

@@ -1512,13 +1512,18 @@ impl<'ctx> ByteCompiler<'ctx> {
 
     fn optimize(bytecode: &mut Vec<u8>, flags: CodeBlockFlags) {
         // only perform Tail Call Optimisation in strict mode
-        if flags.contains(CodeBlockFlags::STRICT) {
+        if flags.contains(CodeBlockFlags::STRICT) && flags.is_ordinary() {
             // if a sequence of Call, SetReturnValue, CheckReturn, Return is found
             // replace Call with TailCall to allow frame elimination
             for i in 0..bytecode.len() {
                 let opcode: Opcode = bytecode[i].into();
-                if matches!(opcode, Opcode::Call) {
-                    if bytecode.len() > i + 5 {
+                if !matches!(opcode, Opcode::Call) {
+                    continue;
+                }
+                if bytecode.len() <= i + 5 {
+                    return;
+                }
+
                 let second = bytecode[i + 2].into();
                 let third = bytecode[i + 3].into();
                 let fourth = bytecode[i + 4].into();
@@ -1527,10 +1532,6 @@ impl<'ctx> ByteCompiler<'ctx> {
                 {
                     bytecode[i] = Opcode::TailCall as u8;
                 }
-                    } else {
-                        return;
-                    }
-                }
             }
         }
     }

core/engine/src/vm/code_block.rs

@@ -76,6 +76,47 @@ bitflags! {
     }
 }
 
+impl CodeBlockFlags {
+    /// Check if the function is traced.
+    #[cfg(feature = "trace")]
+    pub(crate) fn traceable(&self) -> bool {
+        self.contains(CodeBlockFlags::TRACEABLE)
+    }
+    /// Check if the function is a class constructor.
+    pub(crate) fn is_class_constructor(&self) -> bool {
+        self.contains(CodeBlockFlags::IS_CLASS_CONSTRUCTOR)
+    }
+    /// Check if the function is in strict mode.
+    pub(crate) fn strict(&self) -> bool {
+        self.contains(CodeBlockFlags::STRICT)
+    }
+    /// Indicates if the function is an expression and has a binding identifier.
+    pub(crate) fn has_binding_identifier(&self) -> bool {
+        self.contains(CodeBlockFlags::HAS_BINDING_IDENTIFIER)
+    }
+    /// Does this function have the `[[ClassFieldInitializerName]]` internal slot set to non-empty value.
+    pub(crate) fn in_class_field_initializer(&self) -> bool {
+        self.contains(CodeBlockFlags::IN_CLASS_FIELD_INITIALIZER)
+    }
+    /// Returns true if this function is a derived constructor.
+    pub(crate) fn is_derived_constructor(&self) -> bool {
+        self.contains(CodeBlockFlags::IS_DERIVED_CONSTRUCTOR)
+    }
+    /// Returns true if this function an async function.
+    pub(crate) fn is_async(self) -> bool {
+        self.contains(CodeBlockFlags::IS_ASYNC)
+    }
+
+    /// Returns true if this function an generator function.
+    pub(crate) fn is_generator(self) -> bool {
+        self.contains(CodeBlockFlags::IS_GENERATOR)
+    }
+    /// Returns true if this function an async function.
+    pub(crate) fn is_ordinary(self) -> bool {
+        !self.is_async() && !self.is_generator()
+    }
+}
+
 // SAFETY: Nothing in CodeBlockFlags needs tracing, so this is safe.
 unsafe impl Trace for CodeBlockFlags {
     empty_trace!();
@@ -232,7 +273,7 @@ impl CodeBlock {
     /// Check if the function is traced.
     #[cfg(feature = "trace")]
     pub(crate) fn traceable(&self) -> bool {
-        self.flags.get().contains(CodeBlockFlags::TRACEABLE)
+        self.flags.get().traceable()
     }
     /// Enable or disable instruction tracing to `stdout`.
     #[cfg(feature = "trace")]
@@ -245,50 +286,42 @@ impl CodeBlock {
 
     /// Check if the function is a class constructor.
     pub(crate) fn is_class_constructor(&self) -> bool {
-        self.flags
-            .get()
-            .contains(CodeBlockFlags::IS_CLASS_CONSTRUCTOR)
+        self.flags.get().is_class_constructor()
     }
 
     /// Check if the function is in strict mode.
     pub(crate) fn strict(&self) -> bool {
-        self.flags.get().contains(CodeBlockFlags::STRICT)
+        self.flags.get().strict()
     }
 
     /// Indicates if the function is an expression and has a binding identifier.
     pub(crate) fn has_binding_identifier(&self) -> bool {
-        self.flags
-            .get()
-            .contains(CodeBlockFlags::HAS_BINDING_IDENTIFIER)
+        self.flags.get().has_binding_identifier()
     }
 
     /// Does this function have the `[[ClassFieldInitializerName]]` internal slot set to non-empty value.
     pub(crate) fn in_class_field_initializer(&self) -> bool {
-        self.flags
-            .get()
-            .contains(CodeBlockFlags::IN_CLASS_FIELD_INITIALIZER)
+        self.flags.get().in_class_field_initializer()
     }
 
     /// Returns true if this function is a derived constructor.
     pub(crate) fn is_derived_constructor(&self) -> bool {
-        self.flags
-            .get()
-            .contains(CodeBlockFlags::IS_DERIVED_CONSTRUCTOR)
+        self.flags.get().is_derived_constructor()
     }
 
     /// Returns true if this function an async function.
     pub(crate) fn is_async(&self) -> bool {
-        self.flags.get().contains(CodeBlockFlags::IS_ASYNC)
+        self.flags.get().is_async()
     }
 
     /// Returns true if this function an generator function.
     pub(crate) fn is_generator(&self) -> bool {
-        self.flags.get().contains(CodeBlockFlags::IS_GENERATOR)
+        self.flags.get().is_generator()
     }
 
     /// Returns true if this function an async function.
     pub(crate) fn is_ordinary(&self) -> bool {
-        !self.is_async() && !self.is_generator()
+        self.flags.get().is_ordinary()
     }
 
     /// Returns true if this function has the `"prototype"` property when function object is created.

core/engine/src/vm/opcode/call/mod.rs

@@ -3,7 +3,7 @@ use crate::{
     error::JsNativeError,
     module::{ModuleKind, Referrer},
     object::FunctionObjectBuilder,
-    vm::{opcode::Operation, CompletionType},
+    vm::{opcode::Operation, CallFrame, CompletionType},
     Context, JsObject, JsResult, JsValue, NativeFunction,
 };
 
@@ -210,7 +210,7 @@ pub(crate) struct TailCall;
 impl TailCall {
     fn operation(context: &mut Context, argument_count: usize) -> JsResult<CompletionType> {
         let at = context.vm.stack.len() - argument_count;
-        let func = &context.vm.stack[at - 1];
+        let func = context.vm.stack[at - 1].clone();
 
         let Some(object) = func.as_object() else {
             return Err(JsNativeError::typ()
@@ -218,39 +218,25 @@ impl TailCall {
                 .into());
         };
 
-        let is_ordinary_function = object.is::<OrdinaryFunction>();
-        object.__call__(argument_count).resolve(context)?;
-
-        // only tail call for ordinary functions
-        // don't tail call on the main script
-        // TODO: the 3 needs to be reviewed
-        if is_ordinary_function && context.vm.frames.len() > 3 {
-            // check that caller is also ordinary function
-            let frames = &context.vm.frames;
-            let caller_frame = &frames[frames.len() - 2];
-            let caller_function = caller_frame
-                .function(&context.vm)
-                .expect("there must be a caller function");
-            if caller_function.is::<OrdinaryFunction>() {
-                // remove caller's CallFrame
-                let frames = &mut context.vm.frames;
-                let caller_frame = frames.swap_remove(frames.len() - 2);
-
-                // remove caller's prologue from stack
-                // this + func + arguments
-                let to_remove = 1 + 1 + caller_frame.argument_count as usize;
+        let mut early_exit = false;
+        if object.is::<OrdinaryFunction>() {
+            if let Some(caller) = context.vm.pop_frame() {
+                let fp = caller.fp as usize;
                 context
                     .vm
                     .stack
-                    .drain((caller_frame.fp as usize)..(caller_frame.fp as usize + to_remove));
-
-                // update invoked function's fp
-                let frames = &mut context.vm.frames;
-                let invoked_frame = frames.last_mut().expect("invoked frame must exist");
-                invoked_frame.set_exit_early(caller_frame.exit_early());
-                invoked_frame.fp -= to_remove as u32;
+                    .drain(fp..(at - CallFrame::FUNCTION_PROLOGUE));
+                early_exit = caller.exit_early();
             }
         }
+
+        object.__call__(argument_count).resolve(context)?;
+
+        // If the caller is early exit, since it has been poped from the frames
+        // we have to mark the current frame as early exit.
+        if early_exit {
+            context.vm.frame_mut().set_exit_early(true);
+        }
         Ok(CompletionType::Normal)
     }
 }