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

Remove array optimization path that prevents usage of setters on prototypes #3744

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 1 addition & 45 deletions core/engine/src/vm/opcode/set/property.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
use boa_macros::utf16;

use crate::{
builtins::{function::set_function_name, Proxy},
builtins::function::set_function_name,
object::{internal_methods::InternalMethodContext, shape::slot::SlotAttributes},
property::{PropertyDescriptor, PropertyKey},
vm::{opcode::Operation, CompletionType},
Expand Down Expand Up @@ -145,8 +143,6 @@ impl Operation for SetPropertyByValue {
break 'fast_path;
}

let shape = object_borrowed.shape().clone();

if let Some(dense_elements) = object_borrowed
.properties_mut()
.dense_indexed_properties_mut()
Expand All @@ -156,46 +152,6 @@ impl Operation for SetPropertyByValue {
*element = value;
context.vm.push(element.clone());
return Ok(CompletionType::Normal);
} else if dense_elements.len() == index {
// Cannot use fast path if the [[prototype]] is a proxy object,
// because we have to the call prototypes [[set]] on non-existing property,
// and proxy objects can override [[set]].
let prototype = shape.prototype();
if prototype.map_or(false, |x| x.is::<Proxy>()) {
break 'fast_path;
}

dense_elements.push(value.clone());
context.vm.push(value);

let len = dense_elements.len() as u32;
let length_key = PropertyKey::from(utf16!("length"));
let length = object_borrowed
.properties_mut()
.get(&length_key)
.expect("Arrays must have length property");

if length.expect_writable() {
// We have to get the max of previous length and len(dense_elements) + 1,
// this is needed if user spacifies `new Array(n)` then adds properties from 0, 1, etc.
let len = length
.expect_value()
.to_u32(context)
.expect("length should have a u32 value")
.max(len);
object_borrowed.insert(
length_key,
PropertyDescriptor::builder()
.value(len)
.writable(true)
.enumerable(length.expect_enumerable())
.configurable(false)
.build(),
);
} else if context.vm.frame().code_block.strict() {
return Err(JsNativeError::typ().with_message("TypeError: Cannot assign to read only property 'length' of array object").into());
}
return Ok(CompletionType::Normal);
}
}
}
Expand Down
Loading