Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix panic when a self mutating function is constructing an object #710

Merged
merged 1 commit into from
Sep 22, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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