From 9900b52fd4c305e6e33ffc4ecafa6a6fb70579a9 Mon Sep 17 00:00:00 2001 From: HalidOdat Date: Tue, 22 Sep 2020 17:26:59 +0200 Subject: [PATCH] Fix panic when a self mutating function is constructing --- boa/src/builtins/function/tests.rs | 23 ++++++++++++++++++-- boa/src/object/gcobject.rs | 35 ++++++++++++++++++------------ 2 files changed, 42 insertions(+), 16 deletions(-) diff --git a/boa/src/builtins/function/tests.rs b/boa/src/builtins/function/tests.rs index 287734fa301..eaacbf4193c 100644 --- a/boa/src/builtins/function/tests.rs +++ b/boa/src/builtins/function/tests.rs @@ -2,7 +2,7 @@ use crate::{forward, forward_val, Context}; #[allow(clippy::float_cmp)] #[test] -fn check_arguments_object() { +fn arguments_object() { let mut engine = Context::new(); let init = r#" @@ -25,7 +25,7 @@ fn check_arguments_object() { } #[test] -fn check_self_mutating_func() { +fn self_mutating_function_when_calling() { let mut engine = Context::new(); let func = r#" function x() { @@ -42,3 +42,22 @@ fn check_self_mutating_func() { 3 ); } + +#[test] +fn self_mutating_function_when_constructing() { + let mut engine = Context::new(); + let func = r#" + function x() { + x.y = 3; + } + new x(); + "#; + eprintln!("{}", forward(&mut engine, func)); + let y = forward_val(&mut engine, "x.y").expect("value expected"); + assert_eq!(y.is_integer(), true); + assert_eq!( + y.to_i32(&mut engine) + .expect("Could not convert value to i32"), + 3 + ); +} diff --git a/boa/src/object/gcobject.rs b/boa/src/object/gcobject.rs index b7f5d439eea..aa6e262b2cc 100644 --- a/boa/src/object/gcobject.rs +++ b/boa/src/object/gcobject.rs @@ -190,16 +190,14 @@ impl GcObject { // #[track_caller] pub fn construct(&self, args: &[Value], ctx: &mut Context) -> Result { - let this = Object::create(self.borrow().get(&PROTOTYPE.into())).into(); + let this: Value = Object::create(self.borrow().get(&PROTOTYPE.into())).into(); let this_function_object = self.clone(); - let object = self.borrow(); - if let Some(function) = object.as_function() { + let body = if let Some(function) = self.borrow().as_function() { if function.is_constructable() { match function { Function::BuiltIn(BuiltInFunction(function), _) => { - function(&this, args, ctx)?; - Ok(this) + FunctionBody::BuiltIn(*function) } Function::Ordinary { body, @@ -211,7 +209,7 @@ impl GcObject { // let local_env = new_function_environment( this_function_object, - Some(this), + Some(this.clone()), Some(environment.clone()), // Arrow functions do not have a this binding https://tc39.es/ecma262/#sec-function-environment-records if flags.is_lexical_this_mode() { @@ -244,20 +242,29 @@ impl GcObject { ctx.realm_mut().environment.push(local_env); - // Call body should be set before reaching here - let _ = body.run(ctx); - - // local_env gets dropped here, its no longer needed - let binding = ctx.realm_mut().environment.get_this_binding(); - Ok(binding) + FunctionBody::Ordinary(body.clone()) } } } else { let name = this.get_field("name").display().to_string(); - ctx.throw_type_error(format!("{} is not a constructor", name)) + return ctx.throw_type_error(format!("{} is not a constructor", name)); } } else { - ctx.throw_type_error("not a function") + return ctx.throw_type_error("not a function"); + }; + + match body { + FunctionBody::BuiltIn(function) => { + function(&this, args, ctx)?; + Ok(this) + } + FunctionBody::Ordinary(body) => { + let _ = body.run(ctx); + + // local_env gets dropped here, its no longer needed + let binding = ctx.realm_mut().environment.get_this_binding(); + Ok(binding) + } } } }