Skip to content

Commit

Permalink
Fix panic when a self mutating function is constructing an object (#710)
Browse files Browse the repository at this point in the history
  • Loading branch information
HalidOdat committed Sep 22, 2020
1 parent 38eaaf4 commit 3f6a8ef
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 16 deletions.
23 changes: 21 additions & 2 deletions boa/src/builtins/function/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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#"
Expand All @@ -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() {
Expand All @@ -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
);
}
35 changes: 21 additions & 14 deletions boa/src/object/gcobject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,16 +190,14 @@ impl GcObject {
// <https://tc39.es/ecma262/#sec-ecmascript-function-objects-construct-argumentslist-newtarget>
#[track_caller]
pub fn construct(&self, args: &[Value], ctx: &mut Context) -> Result<Value> {
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,
Expand All @@ -211,7 +209,7 @@ impl GcObject {
// <https://tc39.es/ecma262/#sec-prepareforordinarycall>
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() {
Expand Down Expand Up @@ -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)
}
}
}
}
Expand Down

0 comments on commit 3f6a8ef

Please sign in to comment.