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

[Merged by Bors] - Handle __proto__ fields in object literals #2423

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
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
7 changes: 6 additions & 1 deletion boa_engine/src/builtins/json/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ use crate::{
value::IntegerOrInfinity,
Context, JsResult, JsString, JsValue,
};
use boa_parser::Parser;
use boa_profiler::Profiler;
use tap::{Conv, Pipe};

Expand Down Expand Up @@ -194,7 +195,11 @@ impl Json {
// 8. NOTE: The PropertyDefinitionEvaluation semantics defined in 13.2.5.5 have special handling for the above evaluation.
// 9. Let unfiltered be completion.[[Value]].
// 10. Assert: unfiltered is either a String, Number, Boolean, Null, or an Object that is defined by either an ArrayLiteral or an ObjectLiteral.
let unfiltered = context.eval(script_string.as_bytes())?;
let mut parser = Parser::new(script_string.as_bytes());
parser.set_json_parse();
let statement_list = parser.parse_all(context.interner_mut())?;
let code_block = context.compile_json_parse(&statement_list)?;
let unfiltered = context.execute(code_block)?;

// 11. If IsCallable(reviver) is true, then
if let Some(obj) = args.get_or_undefined(1).as_callable() {
Expand Down
1 change: 1 addition & 0 deletions boa_engine/src/bytecompiler/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ impl FunctionCompiler {
bindings_map: FxHashMap::default(),
jump_info: Vec::new(),
in_async_generator: self.generator && self.r#async,
json_parse: false,
context,
};

Expand Down
17 changes: 14 additions & 3 deletions boa_engine/src/bytecompiler/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,7 @@ pub struct ByteCompiler<'b> {
bindings_map: FxHashMap<BindingLocator, u32>,
jump_info: Vec<JumpControlInfo>,
in_async_generator: bool,
json_parse: bool,
context: &'b mut Context,
}

Expand All @@ -242,14 +243,15 @@ impl<'b> ByteCompiler<'b> {
const DUMMY_ADDRESS: u32 = u32::MAX;

#[inline]
pub fn new(name: Sym, strict: bool, context: &'b mut Context) -> Self {
pub fn new(name: Sym, strict: bool, json_parse: bool, context: &'b mut Context) -> Self {
Self {
code_block: CodeBlock::new(name, 0, strict),
literals_map: FxHashMap::default(),
names_map: FxHashMap::default(),
bindings_map: FxHashMap::default(),
jump_info: Vec::new(),
in_async_generator: false,
json_parse,
context,
}
}
Expand Down Expand Up @@ -1200,13 +1202,18 @@ impl<'b> ByteCompiler<'b> {
match property {
PropertyDefinition::IdentifierReference(ident) => {
let index = self.get_or_insert_name(*ident);
self.access_get(Access::Variable { name: *ident }, true)?;
self.emit(Opcode::DefineOwnPropertyByName, &[index]);
}
PropertyDefinition::Property(name, expr) => match name {
PropertyName::Literal(name) => {
self.compile_expr(expr, true)?;
let index = self.get_or_insert_name((*name).into());
self.emit(Opcode::DefineOwnPropertyByName, &[index]);
if *name == Sym::__PROTO__ && !self.json_parse {
self.emit_opcode(Opcode::SetPrototype);
} else {
self.emit(Opcode::DefineOwnPropertyByName, &[index]);
}
}
PropertyName::Computed(name_node) => {
self.compile_expr(name_node, true)?;
Expand Down Expand Up @@ -3225,6 +3232,7 @@ impl<'b> ByteCompiler<'b> {
bindings_map: FxHashMap::default(),
jump_info: Vec::new(),
in_async_generator: false,
json_parse: self.json_parse,
context: self.context,
};
compiler.context.push_compile_time_environment(true);
Expand Down Expand Up @@ -3467,6 +3475,7 @@ impl<'b> ByteCompiler<'b> {
bindings_map: FxHashMap::default(),
jump_info: Vec::new(),
in_async_generator: false,
json_parse: self.json_parse,
context: self.context,
};
field_compiler.context.push_compile_time_environment(true);
Expand Down Expand Up @@ -3498,6 +3507,7 @@ impl<'b> ByteCompiler<'b> {
bindings_map: FxHashMap::default(),
jump_info: Vec::new(),
in_async_generator: false,
json_parse: self.json_parse,
context: self.context,
};
field_compiler.context.push_compile_time_environment(true);
Expand Down Expand Up @@ -3554,7 +3564,8 @@ impl<'b> ByteCompiler<'b> {
}
ClassElement::StaticBlock(statement_list) => {
self.emit_opcode(Opcode::Dup);
let mut compiler = ByteCompiler::new(Sym::EMPTY_STRING, true, self.context);
let mut compiler =
ByteCompiler::new(Sym::EMPTY_STRING, true, false, self.context);
compiler.context.push_compile_time_environment(true);
compiler.create_decls(statement_list, false);
compiler.compile_statement_list(statement_list, false, false)?;
Expand Down
17 changes: 15 additions & 2 deletions boa_engine/src/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,20 @@ impl Context {
#[inline]
pub fn compile(&mut self, statement_list: &StatementList) -> JsResult<Gc<CodeBlock>> {
let _timer = Profiler::global().start_event("Compilation", "Main");
let mut compiler = ByteCompiler::new(Sym::MAIN, statement_list.strict(), self);
let mut compiler = ByteCompiler::new(Sym::MAIN, statement_list.strict(), false, self);
compiler.create_decls(statement_list, false);
compiler.compile_statement_list(statement_list, true, false)?;
Ok(Gc::new(compiler.finish()))
}

/// Compile the AST into a `CodeBlock` ready to be executed by the VM in a `JSON.parse` context.
#[inline]
pub fn compile_json_parse(
&mut self,
statement_list: &StatementList,
) -> JsResult<Gc<CodeBlock>> {
let _timer = Profiler::global().start_event("Compilation", "Main");
let mut compiler = ByteCompiler::new(Sym::MAIN, statement_list.strict(), true, self);
compiler.create_decls(statement_list, false);
compiler.compile_statement_list(statement_list, true, false)?;
Ok(Gc::new(compiler.finish()))
Expand All @@ -484,7 +497,7 @@ impl Context {
strict: bool,
) -> JsResult<Gc<CodeBlock>> {
let _timer = Profiler::global().start_event("Compilation", "Main");
let mut compiler = ByteCompiler::new(Sym::MAIN, statement_list.strict(), self);
let mut compiler = ByteCompiler::new(Sym::MAIN, statement_list.strict(), false, self);
compiler.compile_statement_list_with_new_declarative(statement_list, true, strict)?;
Ok(Gc::new(compiler.finish()))
}
Expand Down
1 change: 1 addition & 0 deletions boa_engine/src/vm/code_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,7 @@ impl CodeBlock {
| Opcode::NewSpread
| Opcode::SuperCallSpread
| Opcode::ForAwaitOfLoopIterate
| Opcode::SetPrototype
| Opcode::Nop => String::new(),
}
}
Expand Down
7 changes: 7 additions & 0 deletions boa_engine/src/vm/opcode/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,13 @@ generate_impl! {
/// Stack: home, function **=>** home, function
SetHomeObject,

/// Set the prototype of an object if the value is an object or null.
///
/// Operands:
///
/// Stack: object, value **=>**
SetPrototype,

/// Push an empty array value on the stack.
///
/// Operands:
Expand Down
2 changes: 2 additions & 0 deletions boa_engine/src/vm/opcode/set/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@ pub(crate) mod home_object;
pub(crate) mod name;
pub(crate) mod private;
pub(crate) mod property;
pub(crate) mod prototype;

pub(crate) use class_prototype::*;
pub(crate) use home_object::*;
pub(crate) use name::*;
pub(crate) use private::*;
pub(crate) use property::*;
pub(crate) use prototype::*;
36 changes: 36 additions & 0 deletions boa_engine/src/vm/opcode/set/prototype.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
use crate::{
vm::{opcode::Operation, ShouldExit},
Context, JsResult,
};

/// `SetPrototype` implements the Opcode Operation for `Opcode::SetPrototype`
///
/// Operation:
/// - Sets the prototype of an object.
#[derive(Debug, Clone, Copy)]
pub(crate) struct SetPrototype;

impl Operation for SetPrototype {
const NAME: &'static str = "SetPrototype";
const INSTRUCTION: &'static str = "INST - SetPrototype";

fn execute(context: &mut Context) -> JsResult<ShouldExit> {
let value = context.vm.pop();
let object = context.vm.pop();

let prototype = if let Some(prototype) = value.as_object() {
Some(prototype.clone())
} else if value.is_null() {
None
} else {
return Ok(ShouldExit::False);
};

let object = object.as_object().expect("object is not an object");
object
.__set_prototype_of__(prototype, context)
.expect("cannot fail per spec");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here it says that this cannot fail per the spec, but then I'm missing some link to the spec, or the spec text that says so, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will open is issue for this as it is currently a problem with all opcodes.


Ok(ShouldExit::False)
}
}
8 changes: 6 additions & 2 deletions boa_interner/src/sym.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,9 @@ impl Sym {
/// Symbol for the `"target"` string.
pub const TARGET: Self = unsafe { Self::new_unchecked(28) };

/// Symbol for the `"__proto__"` string.
pub const __PROTO__: Self = unsafe { Self::new_unchecked(29) };

/// Creates a new [`Sym`] from the provided `value`, or returns `None` if `index` is zero.
#[inline]
pub(super) fn new(value: usize) -> Option<Self> {
Expand Down Expand Up @@ -134,7 +137,7 @@ impl Sym {
}

macro_rules! create_static_strings {
( $( $s:literal ),+ ) => {
( $( $s:literal ),+$(,)? ) => {
/// Ordered set of commonly used static `UTF-8` strings.
///
/// # Note
Expand Down Expand Up @@ -193,5 +196,6 @@ create_static_strings! {
"false",
"async",
"of",
"target"
"target",
"__proto__",
}
16 changes: 16 additions & 0 deletions boa_parser/src/parser/cursor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ pub(super) struct Cursor<R> {

/// Tracks if the cursor is in a arrow function declaration.
arrow: bool,

/// Indicate if the cursor is used in `JSON.parse`.
json_parse: bool,
}

impl<R> Cursor<R>
Expand All @@ -44,6 +47,7 @@ where
buffered_lexer: Lexer::new(reader).into(),
private_environments_stack: Vec::new(),
arrow: false,
json_parse: false,
}
}

Expand Down Expand Up @@ -126,6 +130,18 @@ where
self.arrow = arrow;
}

/// Returns if the cursor is currently used in `JSON.parse`.
#[inline]
pub(super) fn json_parse(&self) -> bool {
self.json_parse
}

/// Set if the cursor is currently used in `JSON.parse`.
#[inline]
pub(super) fn set_json_parse(&mut self, json_parse: bool) {
self.json_parse = json_parse;
}

/// Push a new private environment.
#[inline]
pub(super) fn push_private_environment(&mut self) {
Expand Down
46 changes: 38 additions & 8 deletions boa_parser/src/parser/expression/primary/object_initializer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,15 +75,34 @@ where
let _timer = Profiler::global().start_event("ObjectLiteral", "Parsing");
let mut elements = Vec::new();

let mut has_proto = false;
let mut duplicate_proto_position = None;

loop {
if cursor.next_if(Punctuator::CloseBlock, interner)?.is_some() {
break;
}

elements.push(
PropertyDefinition::new(self.allow_yield, self.allow_await)
.parse(cursor, interner)?,
);
let position = cursor.peek(0, interner).or_abrupt()?.span().start();

let property = PropertyDefinition::new(self.allow_yield, self.allow_await)
.parse(cursor, interner)?;

if matches!(
property,
property::PropertyDefinition::Property(
property::PropertyName::Literal(Sym::__PROTO__),
_
)
) {
if has_proto && duplicate_proto_position.is_none() {
duplicate_proto_position = Some(position);
} else {
has_proto = true;
}
}

elements.push(property);

if cursor.next_if(Punctuator::CloseBlock, interner)?.is_some() {
break;
Expand All @@ -100,6 +119,20 @@ where
}
}

if let Some(position) = duplicate_proto_position {
if !cursor.json_parse()
&& match cursor.peek(0, interner)? {
Some(token) => token.kind() != &TokenKind::Punctuator(Punctuator::Assign),
None => true,
}
{
return Err(Error::general(
"Duplicate __proto__ fields are not allowed in object literals.",
position,
));
}
}

Ok(literal::ObjectLiteral::from(elements))
}
}
Expand Down Expand Up @@ -143,10 +176,7 @@ where
TokenKind::Punctuator(Punctuator::CloseBlock | Punctuator::Comma) => {
let ident = IdentifierReference::new(self.allow_yield, self.allow_await)
.parse(cursor, interner)?;
return Ok(property::PropertyDefinition::Property(
ident.sym().into(),
ident.into(),
));
return Ok(property::PropertyDefinition::IdentifierReference(ident));
}
TokenKind::Punctuator(Punctuator::Assign) => {
return CoverInitializedName::new(self.allow_yield, self.allow_await)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -284,9 +284,8 @@ fn check_object_short_function_set() {
fn check_object_shorthand_property_names() {
let interner = &mut Interner::default();

let object_properties = vec![PropertyDefinition::Property(
let object_properties = vec![PropertyDefinition::IdentifierReference(
interner.get_or_intern_static("a", utf16!("a")).into(),
Identifier::new(interner.get_or_intern_static("a", utf16!("a"))).into(),
)];

check_parser(
Expand Down Expand Up @@ -322,13 +321,11 @@ fn check_object_shorthand_multiple_properties() {
let interner = &mut Interner::default();

let object_properties = vec![
PropertyDefinition::Property(
PropertyDefinition::IdentifierReference(
interner.get_or_intern_static("a", utf16!("a")).into(),
Identifier::new(interner.get_or_intern_static("a", utf16!("a"))).into(),
),
PropertyDefinition::Property(
PropertyDefinition::IdentifierReference(
interner.get_or_intern_static("b", utf16!("b")).into(),
Identifier::new(interner.get_or_intern_static("b", utf16!("b"))).into(),
),
];

Expand Down
9 changes: 9 additions & 0 deletions boa_parser/src/parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,15 @@ impl<R> Parser<R> {
self.cursor.set_strict_mode(true);
}

/// Set the parser strict mode to true.
#[inline]
pub fn set_json_parse(&mut self)
where
R: Read,
{
self.cursor.set_json_parse(true);
}

/// Parse the full input as a [ECMAScript Script][spec] into the boa AST representation.
/// The resulting `StatementList` can be compiled into boa bytecode and executed in the boa vm.
///
Expand Down