From 4419e6df04b91ae14e150b3b2f2f17c030eec7ad Mon Sep 17 00:00:00 2001 From: raskad <32105367+raskad@users.noreply.github.com> Date: Tue, 4 Jun 2024 04:16:09 +0100 Subject: [PATCH] Fix base objects in `with` statements (#3870) --- core/engine/src/builtins/eval/mod.rs | 3 ++ core/engine/src/builtins/function/mod.rs | 2 + core/engine/src/builtins/json/mod.rs | 2 + core/engine/src/bytecompiler/class.rs | 5 ++ core/engine/src/bytecompiler/declarations.rs | 2 + core/engine/src/bytecompiler/function.rs | 9 ++++ core/engine/src/bytecompiler/mod.rs | 21 +++++++- .../engine/src/bytecompiler/statement/with.rs | 3 ++ core/engine/src/environments/runtime/mod.rs | 51 +++++++++++++++++++ core/engine/src/module/source.rs | 1 + core/engine/src/module/synthetic.rs | 1 + core/engine/src/script.rs | 1 + core/engine/src/vm/code_block.rs | 4 +- core/engine/src/vm/flowgraph/mod.rs | 4 +- core/engine/src/vm/opcode/environment/mod.rs | 39 ++++++++++++++ core/engine/src/vm/opcode/mod.rs | 9 +++- 16 files changed, 150 insertions(+), 7 deletions(-) diff --git a/core/engine/src/builtins/eval/mod.rs b/core/engine/src/builtins/eval/mod.rs index 8982c716c0..1555473f6f 100644 --- a/core/engine/src/builtins/eval/mod.rs +++ b/core/engine/src/builtins/eval/mod.rs @@ -246,6 +246,8 @@ impl Eval { context, )?; + let in_with = context.vm.environments.has_object_environment(); + let mut compiler = ByteCompiler::new( js_string!("
"), body.strict(), @@ -253,6 +255,7 @@ impl Eval { var_env.clone(), lex_env.clone(), context.interner_mut(), + in_with, ); compiler.current_open_environments_count += 1; diff --git a/core/engine/src/builtins/function/mod.rs b/core/engine/src/builtins/function/mod.rs index 1bf76b5e82..8970df508a 100644 --- a/core/engine/src/builtins/function/mod.rs +++ b/core/engine/src/builtins/function/mod.rs @@ -626,10 +626,12 @@ impl BuiltInFunctionObject { body }; + let in_with = context.vm.environments.has_object_environment(); let code = FunctionCompiler::new() .name(js_string!("anonymous")) .generator(generator) .r#async(r#async) + .in_with(in_with) .compile( ¶meters, &body, diff --git a/core/engine/src/builtins/json/mod.rs b/core/engine/src/builtins/json/mod.rs index 52e8debde2..06e047ea7c 100644 --- a/core/engine/src/builtins/json/mod.rs +++ b/core/engine/src/builtins/json/mod.rs @@ -113,6 +113,7 @@ impl Json { parser.set_json_parse(); let script = parser.parse_script(context.interner_mut())?; let code_block = { + let in_with = context.vm.environments.has_object_environment(); let mut compiler = ByteCompiler::new( js_string!("
"), script.strict(), @@ -120,6 +121,7 @@ impl Json { context.realm().environment().compile_env(), context.realm().environment().compile_env(), context.interner_mut(), + in_with, ); compiler.compile_statement_list(script.statements(), true, false); Gc::new(compiler.finish()) diff --git a/core/engine/src/bytecompiler/class.rs b/core/engine/src/bytecompiler/class.rs index ca43318bd6..f18b9f6f9e 100644 --- a/core/engine/src/bytecompiler/class.rs +++ b/core/engine/src/bytecompiler/class.rs @@ -55,6 +55,7 @@ impl ByteCompiler<'_> { self.variable_environment.clone(), self.lexical_environment.clone(), self.interner, + self.in_with, ); compiler.code_block_flags |= CodeBlockFlags::IS_CLASS_CONSTRUCTOR; @@ -288,6 +289,7 @@ impl ByteCompiler<'_> { self.variable_environment.clone(), self.lexical_environment.clone(), self.interner, + self.in_with, ); // Function environment @@ -316,6 +318,7 @@ impl ByteCompiler<'_> { self.variable_environment.clone(), self.lexical_environment.clone(), self.interner, + self.in_with, ); let _ = field_compiler.push_compile_environment(true); if let Some(node) = field { @@ -354,6 +357,7 @@ impl ByteCompiler<'_> { self.variable_environment.clone(), self.lexical_environment.clone(), self.interner, + self.in_with, ); let _ = field_compiler.push_compile_environment(true); if let Some(node) = field { @@ -388,6 +392,7 @@ impl ByteCompiler<'_> { self.variable_environment.clone(), self.lexical_environment.clone(), self.interner, + self.in_with, ); let _ = compiler.push_compile_environment(true); diff --git a/core/engine/src/bytecompiler/declarations.rs b/core/engine/src/bytecompiler/declarations.rs index b8b4fde7a0..793f8a25d4 100644 --- a/core/engine/src/bytecompiler/declarations.rs +++ b/core/engine/src/bytecompiler/declarations.rs @@ -586,6 +586,7 @@ impl ByteCompiler<'_> { .generator(generator) .r#async(r#async) .strict(self.strict()) + .in_with(self.in_with) .binding_identifier(Some(name.sym().to_js_string(self.interner()))) .compile( parameters, @@ -954,6 +955,7 @@ impl ByteCompiler<'_> { .generator(generator) .r#async(r#async) .strict(self.strict()) + .in_with(self.in_with) .binding_identifier(Some(name.sym().to_js_string(self.interner()))) .compile( parameters, diff --git a/core/engine/src/bytecompiler/function.rs b/core/engine/src/bytecompiler/function.rs index 2a09a497b9..fceb90ce13 100644 --- a/core/engine/src/bytecompiler/function.rs +++ b/core/engine/src/bytecompiler/function.rs @@ -22,6 +22,7 @@ pub(crate) struct FunctionCompiler { strict: bool, arrow: bool, method: bool, + in_with: bool, binding_identifier: Option, } @@ -35,6 +36,7 @@ impl FunctionCompiler { strict: false, arrow: false, method: false, + in_with: false, binding_identifier: None, } } @@ -85,6 +87,12 @@ impl FunctionCompiler { self } + /// Indicate if the function is in a `with` statement. + pub(crate) const fn in_with(mut self, in_with: bool) -> Self { + self.in_with = in_with; + self + } + /// Compile a function statement list and it's parameters into bytecode. pub(crate) fn compile( mut self, @@ -105,6 +113,7 @@ impl FunctionCompiler { variable_environment, lexical_environment, interner, + self.in_with, ); compiler.length = length; compiler diff --git a/core/engine/src/bytecompiler/mod.rs b/core/engine/src/bytecompiler/mod.rs index c1bbe380f9..bad92dad8e 100644 --- a/core/engine/src/bytecompiler/mod.rs +++ b/core/engine/src/bytecompiler/mod.rs @@ -304,6 +304,9 @@ pub struct ByteCompiler<'ctx> { pub(crate) async_handler: Option, json_parse: bool, + /// Whether the function is in a `with` statement. + pub(crate) in_with: bool, + pub(crate) interner: &'ctx mut Interner, #[cfg(feature = "annex-b")] @@ -324,6 +327,7 @@ impl<'ctx> ByteCompiler<'ctx> { variable_environment: Rc, lexical_environment: Rc, interner: &'ctx mut Interner, + in_with: bool, ) -> ByteCompiler<'ctx> { let mut code_block_flags = CodeBlockFlags::empty(); code_block_flags.set(CodeBlockFlags::STRICT, strict); @@ -356,6 +360,7 @@ impl<'ctx> ByteCompiler<'ctx> { #[cfg(feature = "annex-b")] annex_b_function_names: Vec::new(), + in_with, } } @@ -1320,6 +1325,7 @@ impl<'ctx> ByteCompiler<'ctx> { .r#async(r#async) .strict(self.strict()) .arrow(arrow) + .in_with(self.in_with) .binding_identifier(binding_identifier) .compile( parameters, @@ -1395,6 +1401,7 @@ impl<'ctx> ByteCompiler<'ctx> { .strict(self.strict()) .arrow(arrow) .method(true) + .in_with(self.in_with) .binding_identifier(binding_identifier) .compile( parameters, @@ -1442,6 +1449,7 @@ impl<'ctx> ByteCompiler<'ctx> { .strict(true) .arrow(arrow) .method(true) + .in_with(self.in_with) .binding_identifier(binding_identifier) .compile( parameters, @@ -1481,8 +1489,19 @@ impl<'ctx> ByteCompiler<'ctx> { if *ident == Sym::EVAL { kind = CallKind::CallEval; } + + if self.in_with { + let name = self.resolve_identifier_expect(*ident); + let binding = self.lexical_environment.get_identifier_reference(name); + let index = self.get_or_insert_binding(binding.locator()); + self.emit_with_varying_operand(Opcode::ThisForObjectEnvironmentName, index); + } else { + self.emit_opcode(Opcode::PushUndefined); + } + } else { + self.emit_opcode(Opcode::PushUndefined); } - self.emit_opcode(Opcode::PushUndefined); + self.compile_expr(expr, true); } expr => { diff --git a/core/engine/src/bytecompiler/statement/with.rs b/core/engine/src/bytecompiler/statement/with.rs index ec90b75b17..db05b3d4dd 100644 --- a/core/engine/src/bytecompiler/statement/with.rs +++ b/core/engine/src/bytecompiler/statement/with.rs @@ -10,7 +10,10 @@ impl ByteCompiler<'_> { let _ = self.push_compile_environment(false); self.emit_opcode(Opcode::PushObjectEnvironment); + let in_with = self.in_with; + self.in_with = true; self.compile_stmt(with.statement(), use_expr, true); + self.in_with = in_with; self.pop_compile_environment(); self.lexical_environment = old_lex_env; diff --git a/core/engine/src/environments/runtime/mod.rs b/core/engine/src/environments/runtime/mod.rs index 05bdb0e9a5..59cc5a0aeb 100644 --- a/core/engine/src/environments/runtime/mod.rs +++ b/core/engine/src/environments/runtime/mod.rs @@ -411,6 +411,13 @@ impl EnvironmentStack { } names } + + /// Indicate if the current environment stack has an object environment. + pub(crate) fn has_object_environment(&self) -> bool { + self.stack + .iter() + .any(|env| matches!(env, Environment::Object(_))) + } } /// A binding locator contains all information about a binding that is needed to resolve it at runtime. @@ -541,6 +548,50 @@ impl Context { Ok(()) } + /// Finds the object environment that contains the binding and returns the `this` value of the object environment. + pub(crate) fn this_from_object_environment_binding( + &mut self, + locator: &BindingLocator, + ) -> JsResult> { + let current = self.vm.environments.current(); + if let Some(env) = current.as_declarative() { + if !env.with() { + return Ok(None); + } + } + + for env_index in (locator.environment_index..self.vm.environments.stack.len() as u32).rev() + { + match self.environment_expect(env_index) { + Environment::Declarative(env) => { + if env.poisoned() { + let compile = env.compile_env(); + if compile.is_function() && compile.get_binding(locator.name()).is_some() { + break; + } + } else if !env.with() { + break; + } + } + Environment::Object(o) => { + let o = o.clone(); + let key = locator.name().clone(); + if o.has_property(key.clone(), self)? { + if let Some(unscopables) = o.get(JsSymbol::unscopables(), self)?.as_object() + { + if unscopables.get(key.clone(), self)?.to_boolean() { + continue; + } + } + return Ok(Some(o)); + } + } + } + } + + Ok(None) + } + /// Checks if the binding pointed by `locator` is initialized. /// /// # Panics diff --git a/core/engine/src/module/source.rs b/core/engine/src/module/source.rs index 2062028e09..f136b3c47c 100644 --- a/core/engine/src/module/source.rs +++ b/core/engine/src/module/source.rs @@ -1433,6 +1433,7 @@ impl SourceTextModule { env.clone(), env.clone(), context.interner_mut(), + false, ); compiler.code_block_flags |= CodeBlockFlags::IS_ASYNC; diff --git a/core/engine/src/module/synthetic.rs b/core/engine/src/module/synthetic.rs index 734b1e9dd9..81446866bf 100644 --- a/core/engine/src/module/synthetic.rs +++ b/core/engine/src/module/synthetic.rs @@ -287,6 +287,7 @@ impl SyntheticModule { module_compile_env.clone(), module_compile_env.clone(), context.interner_mut(), + false, ); // 4. For each String exportName in module.[[ExportNames]], do diff --git a/core/engine/src/script.rs b/core/engine/src/script.rs index 457b731704..31152ee889 100644 --- a/core/engine/src/script.rs +++ b/core/engine/src/script.rs @@ -135,6 +135,7 @@ impl Script { self.inner.realm.environment().compile_env(), self.inner.realm.environment().compile_env(), context.interner_mut(), + false, ); #[cfg(feature = "annex-b")] diff --git a/core/engine/src/vm/code_block.rs b/core/engine/src/vm/code_block.rs index a1c2699219..66f8682690 100644 --- a/core/engine/src/vm/code_block.rs +++ b/core/engine/src/vm/code_block.rs @@ -611,6 +611,7 @@ impl CodeBlock { | Instruction::Exception | Instruction::MaybeException | Instruction::This + | Instruction::ThisForObjectEnvironmentName { .. } | Instruction::Super | Instruction::CheckReturn | Instruction::Return @@ -719,8 +720,7 @@ impl CodeBlock { | Instruction::Reserved50 | Instruction::Reserved51 | Instruction::Reserved52 - | Instruction::Reserved53 - | Instruction::Reserved54 => unreachable!("Reserved opcodes are unrechable"), + | Instruction::Reserved53 => unreachable!("Reserved opcodes are unrechable"), } } } diff --git a/core/engine/src/vm/flowgraph/mod.rs b/core/engine/src/vm/flowgraph/mod.rs index 0c931d5afe..5ed0326f59 100644 --- a/core/engine/src/vm/flowgraph/mod.rs +++ b/core/engine/src/vm/flowgraph/mod.rs @@ -400,6 +400,7 @@ impl CodeBlock { | Instruction::ToPropertyKey | Instruction::ToBoolean | Instruction::This + | Instruction::ThisForObjectEnvironmentName { .. } | Instruction::Super | Instruction::IncrementLoopIteration | Instruction::CreateForInIterator @@ -515,8 +516,7 @@ impl CodeBlock { | Instruction::Reserved50 | Instruction::Reserved51 | Instruction::Reserved52 - | Instruction::Reserved53 - | Instruction::Reserved54 => unreachable!("Reserved opcodes are unrechable"), + | Instruction::Reserved53 => unreachable!("Reserved opcodes are unrechable"), } } diff --git a/core/engine/src/vm/opcode/environment/mod.rs b/core/engine/src/vm/opcode/environment/mod.rs index 2d04092ec7..31f61ce8f1 100644 --- a/core/engine/src/vm/opcode/environment/mod.rs +++ b/core/engine/src/vm/opcode/environment/mod.rs @@ -35,6 +35,45 @@ impl Operation for This { } } +/// `ThisForObjectEnvironmentName` implements the Opcode Operation for `Opcode::ThisForObjectEnvironmentName` +/// +/// Operation: +/// - Pushes `this` value that is related to the object environment of the given binding. +#[derive(Debug, Clone, Copy)] +pub(crate) struct ThisForObjectEnvironmentName; + +impl ThisForObjectEnvironmentName { + fn operation(context: &mut Context, index: usize) -> JsResult { + let binding_locator = context.vm.frame().code_block.bindings[index].clone(); + let this = context + .this_from_object_environment_binding(&binding_locator)? + .map_or(JsValue::undefined(), Into::into); + context.vm.push(this); + Ok(CompletionType::Normal) + } +} + +impl Operation for ThisForObjectEnvironmentName { + const NAME: &'static str = "ThisForObjectEnvironmentName"; + const INSTRUCTION: &'static str = "INST - ThisForObjectEnvironmentName"; + const COST: u8 = 1; + + fn execute(context: &mut Context) -> JsResult { + let index = context.vm.read::(); + Self::operation(context, index as usize) + } + + fn execute_with_u16_operands(context: &mut Context) -> JsResult { + let index = context.vm.read::() as usize; + Self::operation(context, index) + } + + fn execute_with_u32_operands(context: &mut Context) -> JsResult { + let index = context.vm.read::(); + Self::operation(context, index as usize) + } +} + /// `Super` implements the Opcode Operation for `Opcode::Super` /// /// Operation: diff --git a/core/engine/src/vm/opcode/mod.rs b/core/engine/src/vm/opcode/mod.rs index 3f54a0080f..15a362159d 100644 --- a/core/engine/src/vm/opcode/mod.rs +++ b/core/engine/src/vm/opcode/mod.rs @@ -1614,6 +1614,13 @@ generate_opcodes! { /// Stack: **=>** this This, + /// Pushes `this` value that is related to the object environment of the given binding + /// + /// Operands: index: `VaryingOperand` + /// + /// Stack: **=>** value + ThisForObjectEnvironmentName { index: VaryingOperand }, + /// Pushes the current `super` value to the stack. /// /// Operands: @@ -2252,8 +2259,6 @@ generate_opcodes! { Reserved52 => Reserved, /// Reserved [`Opcode`]. Reserved53 => Reserved, - /// Reserved [`Opcode`]. - Reserved54 => Reserved, } /// Specific opcodes for bindings.