From d20304ea212e0c7db55db362586ed975e0aa31f2 Mon Sep 17 00:00:00 2001 From: Haled Odat <8566042+HalidOdat@users.noreply.github.com> Date: Fri, 5 May 2023 18:36:16 +0200 Subject: [PATCH] Prevent allocation of field names (#2901) --- boa_engine/src/bytecompiler/mod.rs | 6 ++++-- boa_engine/src/vm/code_block.rs | 10 +++------- boa_engine/src/vm/flowgraph/mod.rs | 2 +- .../src/vm/opcode/define/class/getter.rs | 12 ++++-------- .../src/vm/opcode/define/class/method.rs | 14 +++++--------- .../src/vm/opcode/define/class/setter.rs | 12 ++++-------- .../src/vm/opcode/define/own_property.rs | 8 ++------ boa_engine/src/vm/opcode/delete/mod.rs | 11 ++++------- boa_engine/src/vm/opcode/get/property.rs | 9 ++++----- boa_engine/src/vm/opcode/set/property.rs | 19 +++++++------------ 10 files changed, 38 insertions(+), 65 deletions(-) diff --git a/boa_engine/src/bytecompiler/mod.rs b/boa_engine/src/bytecompiler/mod.rs index 5dfe0ab96d..dfd6787ea8 100644 --- a/boa_engine/src/bytecompiler/mod.rs +++ b/boa_engine/src/bytecompiler/mod.rs @@ -14,6 +14,7 @@ mod utils; use crate::{ builtins::function::ThisMode, environments::{BindingLocator, CompileTimeEnvironment}, + js_string, vm::{BindingOpcode, CodeBlock, Opcode}, Context, JsBigInt, JsString, JsValue, }; @@ -241,7 +242,7 @@ pub struct ByteCompiler<'ctx, 'host> { pub(crate) literals: Vec, /// Property field names. - pub(crate) names: Vec, + pub(crate) names: Vec, /// Private names. pub(crate) private_names: Vec, @@ -363,8 +364,9 @@ impl<'ctx, 'host> ByteCompiler<'ctx, 'host> { return *index; } + let string = self.interner().resolve_expect(name.sym()).utf16(); let index = self.names.len() as u32; - self.names.push(name); + self.names.push(js_string!(string)); self.names_map.insert(name, index); index } diff --git a/boa_engine/src/vm/code_block.rs b/boa_engine/src/vm/code_block.rs index 4e4da6e722..80e2d4ef4f 100644 --- a/boa_engine/src/vm/code_block.rs +++ b/boa_engine/src/vm/code_block.rs @@ -18,10 +18,7 @@ use crate::{ vm::CallFrame, Context, JsError, JsResult, JsString, JsValue, }; -use boa_ast::{ - expression::Identifier, - function::{FormalParameterList, PrivateName}, -}; +use boa_ast::function::{FormalParameterList, PrivateName}; use boa_gc::{Finalize, Gc, GcRefCell, Trace}; use boa_interner::Sym; use boa_profiler::Profiler; @@ -89,8 +86,7 @@ pub struct CodeBlock { pub(crate) literals: Box<[JsValue]>, /// Property field names. - #[unsafe_ignore_trace] - pub(crate) names: Box<[Identifier]>, + pub(crate) names: Box<[JsString]>, /// Private names. #[unsafe_ignore_trace] @@ -365,7 +361,7 @@ impl CodeBlock { *pc += size_of::(); format!( "{operand:04}: '{}'", - interner.resolve_expect(self.names[operand as usize].sym()), + self.names[operand as usize].to_std_string_escaped(), ) } Opcode::SetPrivateField diff --git a/boa_engine/src/vm/flowgraph/mod.rs b/boa_engine/src/vm/flowgraph/mod.rs index 3be31ed666..26e9bd27f8 100644 --- a/boa_engine/src/vm/flowgraph/mod.rs +++ b/boa_engine/src/vm/flowgraph/mod.rs @@ -444,7 +444,7 @@ impl CodeBlock { pc += size_of::(); let label = format!( "{opcode_str} '{}'", - interner.resolve_expect(self.names[operand as usize].sym()), + self.names[operand as usize].to_std_string_escaped(), ); graph.add_node(previous_pc, NodeShape::None, label.into(), Color::None); graph.add_edge(previous_pc, pc, None, Color::None, EdgeStyle::Line); diff --git a/boa_engine/src/vm/opcode/define/class/getter.rs b/boa_engine/src/vm/opcode/define/class/getter.rs index 66b969d748..0766d7f4af 100644 --- a/boa_engine/src/vm/opcode/define/class/getter.rs +++ b/boa_engine/src/vm/opcode/define/class/getter.rs @@ -22,10 +22,8 @@ impl Operation for DefineClassStaticGetterByName { let function = context.vm.pop(); let class = context.vm.pop(); let class = class.as_object().expect("class must be object"); - let key = context - .interner() - .resolve_expect(context.vm.frame().code_block.names[index as usize].sym()) - .into_common::(false) + let key = context.vm.frame().code_block.names[index as usize] + .clone() .into(); { let function_object = function @@ -74,10 +72,8 @@ impl Operation for DefineClassGetterByName { let function = context.vm.pop(); let class_proto = context.vm.pop(); let class_proto = class_proto.as_object().expect("class must be object"); - let key = context - .interner() - .resolve_expect(context.vm.frame().code_block.names[index as usize].sym()) - .into_common::(false) + let key = context.vm.frame().code_block.names[index as usize] + .clone() .into(); { let function_object = function diff --git a/boa_engine/src/vm/opcode/define/class/method.rs b/boa_engine/src/vm/opcode/define/class/method.rs index 5d3fb470ab..7601892cef 100644 --- a/boa_engine/src/vm/opcode/define/class/method.rs +++ b/boa_engine/src/vm/opcode/define/class/method.rs @@ -3,7 +3,7 @@ use crate::{ object::CONSTRUCTOR, property::PropertyDescriptor, vm::{opcode::Operation, CompletionType}, - Context, JsResult, JsString, + Context, JsResult, }; /// `DefineClassStaticMethodByName` implements the Opcode Operation for `Opcode::DefineClassStaticMethodByName` @@ -22,10 +22,8 @@ impl Operation for DefineClassStaticMethodByName { let function = context.vm.pop(); let class = context.vm.pop(); let class = class.as_object().expect("class must be object"); - let key = context - .interner() - .resolve_expect(context.vm.frame().code_block.names[index as usize].sym()) - .into_common::(false) + let key = context.vm.frame().code_block.names[index as usize] + .clone() .into(); { let function_object = function @@ -70,10 +68,8 @@ impl Operation for DefineClassMethodByName { let function = context.vm.pop(); let class_proto = context.vm.pop(); let class_proto = class_proto.as_object().expect("class must be object"); - let key = context - .interner() - .resolve_expect(context.vm.frame().code_block.names[index as usize].sym()) - .into_common::(false) + let key = context.vm.frame().code_block.names[index as usize] + .clone() .into(); { let function_object = function diff --git a/boa_engine/src/vm/opcode/define/class/setter.rs b/boa_engine/src/vm/opcode/define/class/setter.rs index b5c2937a75..43f605a9c6 100644 --- a/boa_engine/src/vm/opcode/define/class/setter.rs +++ b/boa_engine/src/vm/opcode/define/class/setter.rs @@ -22,10 +22,8 @@ impl Operation for DefineClassStaticSetterByName { let function = context.vm.pop(); let class = context.vm.pop(); let class = class.as_object().expect("class must be object"); - let key = context - .interner() - .resolve_expect(context.vm.frame().code_block.names[index as usize].sym()) - .into_common::(false) + let key = context.vm.frame().code_block.names[index as usize] + .clone() .into(); { let function_object = function @@ -75,10 +73,8 @@ impl Operation for DefineClassSetterByName { let function = context.vm.pop(); let class_proto = context.vm.pop(); let class_proto = class_proto.as_object().expect("class must be object"); - let key = context - .interner() - .resolve_expect(context.vm.frame().code_block.names[index as usize].sym()) - .into_common::(false) + let key = context.vm.frame().code_block.names[index as usize] + .clone() .into(); { let function_object = function diff --git a/boa_engine/src/vm/opcode/define/own_property.rs b/boa_engine/src/vm/opcode/define/own_property.rs index 6a9870b58d..30de98a2a4 100644 --- a/boa_engine/src/vm/opcode/define/own_property.rs +++ b/boa_engine/src/vm/opcode/define/own_property.rs @@ -1,7 +1,7 @@ use crate::{ property::PropertyDescriptor, vm::{opcode::Operation, CompletionType}, - Context, JsNativeError, JsResult, JsString, + Context, JsNativeError, JsResult, }; /// `DefineOwnPropertyByName` implements the Opcode Operation for `Opcode::DefineOwnPropertyByName` @@ -24,11 +24,7 @@ impl Operation for DefineOwnPropertyByName { } else { object.to_object(context)? }; - let name = context.vm.frame().code_block.names[index as usize]; - let name = context - .interner() - .resolve_expect(name.sym()) - .into_common::(false); + let name = context.vm.frame().code_block.names[index as usize].clone(); object.__define_own_property__( &name.into(), PropertyDescriptor::builder() diff --git a/boa_engine/src/vm/opcode/delete/mod.rs b/boa_engine/src/vm/opcode/delete/mod.rs index 5f6fefada4..d365a6ddcb 100644 --- a/boa_engine/src/vm/opcode/delete/mod.rs +++ b/boa_engine/src/vm/opcode/delete/mod.rs @@ -1,7 +1,7 @@ use crate::{ error::JsNativeError, vm::{opcode::Operation, CompletionType}, - Context, JsResult, JsString, + Context, JsResult, }; /// `DeletePropertyByName` implements the Opcode Operation for `Opcode::DeletePropertyByName` @@ -17,14 +17,11 @@ impl Operation for DeletePropertyByName { fn execute(context: &mut Context<'_>) -> JsResult { let index = context.vm.read::(); - let key = context.vm.frame().code_block.names[index as usize]; - let key = context - .interner() - .resolve_expect(key.sym()) - .into_common::(false) - .into(); let value = context.vm.pop(); let object = value.to_object(context)?; + let key = context.vm.frame().code_block.names[index as usize] + .clone() + .into(); let result = object.__delete__(&key, context)?; if !result && context.vm.frame().code_block.strict { return Err(JsNativeError::typ() diff --git a/boa_engine/src/vm/opcode/get/property.rs b/boa_engine/src/vm/opcode/get/property.rs index 16910dccb4..2a52983e36 100644 --- a/boa_engine/src/vm/opcode/get/property.rs +++ b/boa_engine/src/vm/opcode/get/property.rs @@ -1,5 +1,4 @@ use crate::{ - js_string, property::PropertyKey, vm::{opcode::Operation, CompletionType}, Context, JsResult, JsValue, @@ -26,8 +25,9 @@ impl Operation for GetPropertyByName { value.to_object(context)? }; - let name = context.vm.frame().code_block.names[index as usize]; - let key: PropertyKey = context.interner().resolve_expect(name.sym()).utf16().into(); + let key = context.vm.frame().code_block.names[index as usize] + .clone() + .into(); let result = object.__get__(&key, value, context)?; context.vm.push(result); @@ -93,8 +93,7 @@ impl Operation for GetMethod { fn execute(context: &mut Context<'_>) -> JsResult { let index = context.vm.read::(); - let name = context.vm.frame().code_block.names[index as usize]; - let key = js_string!(context.interner().resolve_expect(name.sym()).utf16()); + let key = context.vm.frame().code_block.names[index as usize].clone(); let value = context.vm.pop(); let method = value.get_method(key, context)?; diff --git a/boa_engine/src/vm/opcode/set/property.rs b/boa_engine/src/vm/opcode/set/property.rs index c330735888..f71eab4a0b 100644 --- a/boa_engine/src/vm/opcode/set/property.rs +++ b/boa_engine/src/vm/opcode/set/property.rs @@ -30,8 +30,9 @@ impl Operation for SetPropertyByName { object.to_object(context)? }; - let name = context.vm.frame().code_block.names[index as usize]; - let name: PropertyKey = context.interner().resolve_expect(name.sym()).utf16().into(); + let name: PropertyKey = context.vm.frame().code_block.names[index as usize] + .clone() + .into(); let succeeded = object.__set__(name.clone(), value.clone(), receiver, context)?; if !succeeded && context.vm.frame().code_block.strict { @@ -163,11 +164,8 @@ impl Operation for SetPropertyGetterByName { let value = context.vm.pop(); let object = context.vm.pop(); let object = object.to_object(context)?; - let name = context.vm.frame().code_block.names[index as usize]; - let name = context - .interner() - .resolve_expect(name.sym()) - .into_common::(false) + let name = context.vm.frame().code_block.names[index as usize] + .clone() .into(); let set = object .__get_own_property__(&name, context)? @@ -240,11 +238,8 @@ impl Operation for SetPropertySetterByName { let value = context.vm.pop(); let object = context.vm.pop(); let object = object.to_object(context)?; - let name = context.vm.frame().code_block.names[index as usize]; - let name = context - .interner() - .resolve_expect(name.sym()) - .into_common::(false) + let name = context.vm.frame().code_block.names[index as usize] + .clone() .into(); let get = object .__get_own_property__(&name, context)?