From c55a4c7f30702bf4f1e95b483d91c0346143e33e Mon Sep 17 00:00:00 2001 From: Luke Wilde Date: Sun, 17 Jul 2022 19:06:44 +0100 Subject: [PATCH] LibJS/Bytecode: Create global variables before setting them This allows them to be accessed before assignment, and also prevents throwing in strict mode as we are trying to set a non-existent variable. --- .../Libraries/LibJS/Bytecode/ASTCodegen.cpp | 13 +++++++------ Userland/Libraries/LibJS/Bytecode/Op.cpp | 19 ++++++++++++++----- Userland/Libraries/LibJS/Bytecode/Op.h | 6 ++++-- 3 files changed, 25 insertions(+), 13 deletions(-) diff --git a/Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp b/Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp index 98c9872450..c55caa9e06 100644 --- a/Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp +++ b/Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp @@ -181,9 +181,7 @@ Bytecode::CodeGenerationErrorOr ScopeNode::generate_bytecode(Bytecode::Gen // ii. If declaredFunctionOrVarNames does not contain F, then if (!declared_function_names.contains(function_name) && !declared_var_names.contains(function_name)) { // i. Perform ? env.CreateGlobalVarBinding(F, false). - generator.emit(index, Bytecode::Op::EnvironmentMode::Var, false); - generator.emit(js_undefined()); - generator.emit(index, Bytecode::Op::SetVariable::InitializationMode::Initialize, Bytecode::Op::EnvironmentMode::Var); + generator.emit(index, Bytecode::Op::EnvironmentMode::Var, false, true); // ii. Append F to declaredFunctionOrVarNames. declared_function_names.set(function_name); @@ -238,9 +236,12 @@ Bytecode::CodeGenerationErrorOr ScopeNode::generate_bytecode(Bytecode::Gen } // 17. For each String vn of declaredVarNames, do - // a. Perform ? env.CreateGlobalVarBinding(vn, false). - for (auto& var_name : declared_var_names) - generator.register_binding(generator.intern_identifier(var_name), Bytecode::Generator::BindingMode::Var); + for (auto& var_name : declared_var_names) { + // a. Perform ? env.CreateGlobalVarBinding(vn, false). + auto index = generator.intern_identifier(var_name); + generator.register_binding(index, Bytecode::Generator::BindingMode::Var); + generator.emit(index, Bytecode::Op::EnvironmentMode::Var, false, true); + } } else { // Perform the steps of FunctionDeclarationInstantiation. generator.begin_variable_scope(Bytecode::Generator::BindingMode::Var, Bytecode::Generator::SurroundingScopeKind::Function); diff --git a/Userland/Libraries/LibJS/Bytecode/Op.cpp b/Userland/Libraries/LibJS/Bytecode/Op.cpp index 84e7d44358..c5d1497d74 100644 --- a/Userland/Libraries/LibJS/Bytecode/Op.cpp +++ b/Userland/Libraries/LibJS/Bytecode/Op.cpp @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -339,6 +340,8 @@ ThrowCompletionOr CreateVariable::execute_impl(Bytecode::Interpreter& inte auto const& name = interpreter.current_executable().get_identifier(m_identifier); if (m_mode == EnvironmentMode::Lexical) { + VERIFY(!m_is_global); + // Note: This is papering over an issue where "FunctionDeclarationInstantiation" creates these bindings for us. // Instead of crashing in there, we'll just raise an exception here. if (TRY(vm.lexical_environment()->has_binding(name))) @@ -349,10 +352,16 @@ ThrowCompletionOr CreateVariable::execute_impl(Bytecode::Interpreter& inte else vm.lexical_environment()->create_mutable_binding(interpreter.global_object(), name, vm.in_strict_mode()); } else { - if (m_is_immutable) - vm.variable_environment()->create_immutable_binding(interpreter.global_object(), name, vm.in_strict_mode()); - else - vm.variable_environment()->create_mutable_binding(interpreter.global_object(), name, vm.in_strict_mode()); + if (!m_is_global) { + if (m_is_immutable) + vm.variable_environment()->create_immutable_binding(interpreter.global_object(), name, vm.in_strict_mode()); + else + vm.variable_environment()->create_mutable_binding(interpreter.global_object(), name, vm.in_strict_mode()); + } else { + // NOTE: CreateVariable with m_is_global set to true is expected to only be used in GlobalDeclarationInstantiation currently, which only uses "false" for "can_be_deleted". + // The only area that sets "can_be_deleted" to true is EvalDeclarationInstantiation, which is currently fully implemented in C++ and not in Bytecode. + verify_cast(vm.variable_environment())->create_global_var_binding(name, false); + } } return {}; } @@ -882,7 +891,7 @@ String CreateEnvironment::to_string_impl(Bytecode::Executable const&) const String CreateVariable::to_string_impl(Bytecode::Executable const& executable) const { auto mode_string = m_mode == EnvironmentMode::Lexical ? "Lexical" : "Variable"; - return String::formatted("CreateVariable env:{} immutable:{} {} ({})", mode_string, m_is_immutable, m_identifier, executable.identifier_table->get(m_identifier)); + return String::formatted("CreateVariable env:{} immutable:{} global:{} {} ({})", mode_string, m_is_immutable, m_is_global, m_identifier, executable.identifier_table->get(m_identifier)); } String EnterObjectEnvironment::to_string_impl(Executable const&) const diff --git a/Userland/Libraries/LibJS/Bytecode/Op.h b/Userland/Libraries/LibJS/Bytecode/Op.h index 293ee76889..2217136147 100644 --- a/Userland/Libraries/LibJS/Bytecode/Op.h +++ b/Userland/Libraries/LibJS/Bytecode/Op.h @@ -317,11 +317,12 @@ public: class CreateVariable final : public Instruction { public: - explicit CreateVariable(IdentifierTableIndex identifier, EnvironmentMode mode, bool is_immutable) + explicit CreateVariable(IdentifierTableIndex identifier, EnvironmentMode mode, bool is_immutable, bool is_global = false) : Instruction(Type::CreateVariable) , m_identifier(identifier) , m_mode(mode) , m_is_immutable(is_immutable) + , m_is_global(is_global) { } @@ -332,7 +333,8 @@ public: private: IdentifierTableIndex m_identifier; EnvironmentMode m_mode; - bool m_is_immutable { false }; + bool m_is_immutable : 4 { false }; + bool m_is_global : 4 { false }; }; class SetVariable final : public Instruction {