From 37067cf3ca915a30d9981ddbd10bb601457a3506 Mon Sep 17 00:00:00 2001 From: Hendiadyoin1 Date: Wed, 28 Jun 2023 18:17:13 +0200 Subject: [PATCH] LibJS: Use the IdentifierTable for NewFunction and NewClass lhs names This makes them trivially copyable, which is an assumption multiple optimizations use when rebuilding the instruction stream. This fixes most optimized crashes in the test262 suite. --- Userland/Libraries/LibJS/AST.h | 5 ++-- .../Libraries/LibJS/Bytecode/ASTCodegen.cpp | 28 +++++++++---------- .../Libraries/LibJS/Bytecode/Generator.cpp | 4 +-- Userland/Libraries/LibJS/Bytecode/Generator.h | 4 +-- Userland/Libraries/LibJS/Bytecode/Op.cpp | 7 +++-- Userland/Libraries/LibJS/Bytecode/Op.h | 12 ++++---- 6 files changed, 32 insertions(+), 28 deletions(-) diff --git a/Userland/Libraries/LibJS/AST.h b/Userland/Libraries/LibJS/AST.h index 3a1d477868..f5110c032d 100644 --- a/Userland/Libraries/LibJS/AST.h +++ b/Userland/Libraries/LibJS/AST.h @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -738,7 +739,7 @@ public: virtual void dump(int indent) const override; virtual Bytecode::CodeGenerationErrorOr generate_bytecode(Bytecode::Generator&) const override; - virtual Bytecode::CodeGenerationErrorOr generate_bytecode_with_lhs_name(Bytecode::Generator&, Optional lhs_name) const; + virtual Bytecode::CodeGenerationErrorOr generate_bytecode_with_lhs_name(Bytecode::Generator&, Optional lhs_name) const; bool has_name() const { return !name().is_empty(); } @@ -1421,7 +1422,7 @@ public: virtual Completion execute(Interpreter&) const override; virtual void dump(int indent) const override; virtual Bytecode::CodeGenerationErrorOr generate_bytecode(Bytecode::Generator&) const override; - virtual Bytecode::CodeGenerationErrorOr generate_bytecode_with_lhs_name(Bytecode::Generator&, Optional lhs_name) const; + virtual Bytecode::CodeGenerationErrorOr generate_bytecode_with_lhs_name(Bytecode::Generator&, Optional lhs_name) const; bool has_name() const { return !m_name.is_empty(); } diff --git a/Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp b/Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp index 6fb424ed58..1693d8b3dc 100644 --- a/Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp +++ b/Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp @@ -400,7 +400,7 @@ Bytecode::CodeGenerationErrorOr AssignmentExpression::generate_bytecode(By // i. Let rref be the result of evaluating AssignmentExpression. // ii. Let rval be ? GetValue(rref). if (lhs->is_identifier()) { - TRY(generator.emit_named_evaluation_if_anonymous_function(*m_rhs, static_cast(*lhs).string())); + TRY(generator.emit_named_evaluation_if_anonymous_function(*m_rhs, generator.intern_identifier(static_cast(*lhs).string()))); } else { TRY(m_rhs->generate_bytecode(generator)); } @@ -495,7 +495,7 @@ Bytecode::CodeGenerationErrorOr AssignmentExpression::generate_bytecode(By generator.emit(lhs_reg); if (lhs->is_identifier()) - TRY(generator.emit_named_evaluation_if_anonymous_function(*m_rhs, static_cast(*lhs).string())); + TRY(generator.emit_named_evaluation_if_anonymous_function(*m_rhs, generator.intern_identifier(static_cast(*lhs).string()))); else TRY(m_rhs->generate_bytecode(generator)); @@ -907,7 +907,7 @@ Bytecode::CodeGenerationErrorOr ObjectExpression::generate_bytecode(Byteco if (property_kind == Bytecode::Op::PropertyKind::ProtoSetter) { TRY(property->value().generate_bytecode(generator)); } else if (property_kind != Bytecode::Op::PropertyKind::Spread) { - DeprecatedFlyString name = string_literal.value(); + auto name = generator.intern_identifier(string_literal.value()); TRY(generator.emit_named_evaluation_if_anonymous_function(property->value(), name)); } @@ -994,7 +994,7 @@ Bytecode::CodeGenerationErrorOr FunctionDeclaration::generate_bytecode(Byt return {}; } -Bytecode::CodeGenerationErrorOr FunctionExpression::generate_bytecode_with_lhs_name(Bytecode::Generator& generator, Optional lhs_name) const +Bytecode::CodeGenerationErrorOr FunctionExpression::generate_bytecode_with_lhs_name(Bytecode::Generator& generator, Optional lhs_name) const { bool has_name = !name().is_empty(); Optional name_identifier; @@ -1085,9 +1085,9 @@ static Bytecode::CodeGenerationErrorOr generate_object_binding_pattern_byt generator.switch_to_basic_block(if_undefined_block); if (auto const* alias_identifier = alias.get_pointer>()) { - TRY(generator.emit_named_evaluation_if_anonymous_function(*initializer, (*alias_identifier)->string())); + TRY(generator.emit_named_evaluation_if_anonymous_function(*initializer, generator.intern_identifier((*alias_identifier)->string()))); } else if (auto const* lhs = name.get_pointer>()) { - TRY(generator.emit_named_evaluation_if_anonymous_function(*initializer, (*lhs)->string())); + TRY(generator.emit_named_evaluation_if_anonymous_function(*initializer, generator.intern_identifier((*lhs)->string()))); } else { TRY(initializer->generate_bytecode(generator)); } @@ -1294,9 +1294,9 @@ static Bytecode::CodeGenerationErrorOr generate_array_binding_pattern_byte generator.switch_to_basic_block(value_is_undefined_block); if (auto const* alias_identifier = alias.get_pointer>()) { - TRY(generator.emit_named_evaluation_if_anonymous_function(*initializer, (*alias_identifier)->string())); + TRY(generator.emit_named_evaluation_if_anonymous_function(*initializer, generator.intern_identifier((*alias_identifier)->string()))); } else if (auto const* name_identifier = name.get_pointer>()) { - TRY(generator.emit_named_evaluation_if_anonymous_function(*initializer, (*name_identifier)->string())); + TRY(generator.emit_named_evaluation_if_anonymous_function(*initializer, generator.intern_identifier((*name_identifier)->string()))); } else { TRY(initializer->generate_bytecode(generator)); } @@ -1356,7 +1356,7 @@ Bytecode::CodeGenerationErrorOr VariableDeclaration::generate_bytecode(Byt for (auto& declarator : m_declarations) { if (declarator->init()) { if (auto const* lhs = declarator->target().get_pointer>()) { - TRY(generator.emit_named_evaluation_if_anonymous_function(*declarator->init(), (*lhs)->string())); + TRY(generator.emit_named_evaluation_if_anonymous_function(*declarator->init(), generator.intern_identifier((*lhs)->string()))); } else { TRY(declarator->init()->generate_bytecode(generator)); } @@ -2273,7 +2273,7 @@ Bytecode::CodeGenerationErrorOr ClassDeclaration::generate_bytecode(Byteco return {}; } -Bytecode::CodeGenerationErrorOr ClassExpression::generate_bytecode_with_lhs_name(Bytecode::Generator& generator, Optional lhs_name) const +Bytecode::CodeGenerationErrorOr ClassExpression::generate_bytecode_with_lhs_name(Bytecode::Generator& generator, Optional lhs_name) const { generator.emit(*this, lhs_name); return {}; @@ -2405,9 +2405,9 @@ static Bytecode::CodeGenerationErrorOr for_in_of_he auto& variable = variable_declaration.declarations().first(); if (variable->init()) { VERIFY(variable->target().has>()); - auto& binding_id = variable->target().get>()->string(); + auto binding_id = generator.intern_identifier(variable->target().get>()->string()); TRY(generator.emit_named_evaluation_if_anonymous_function(*variable->init(), binding_id)); - generator.emit(generator.intern_identifier(binding_id)); + generator.emit(binding_id); } } else { // 1. Let oldEnv be the running execution context's LexicalEnvironment. @@ -2751,7 +2751,7 @@ Bytecode::CodeGenerationErrorOr MetaProperty::generate_bytecode(Bytecode:: Bytecode::CodeGenerationErrorOr ClassFieldInitializerStatement::generate_bytecode(Bytecode::Generator& generator) const { - TRY(generator.emit_named_evaluation_if_anonymous_function(*m_expression, m_class_field_identifier_name)); + TRY(generator.emit_named_evaluation_if_anonymous_function(*m_expression, generator.intern_identifier(m_class_field_identifier_name))); generator.perform_needed_unwinds(); generator.emit(); return {}; @@ -2881,7 +2881,7 @@ Bytecode::CodeGenerationErrorOr ExportStatement::generate_bytecode(Bytecod // ExportDeclaration : export default AssignmentExpression ; VERIFY(is(*m_statement)); - TRY(generator.emit_named_evaluation_if_anonymous_function(static_cast(*m_statement), DeprecatedFlyString("default"sv))); + TRY(generator.emit_named_evaluation_if_anonymous_function(static_cast(*m_statement), generator.intern_identifier("default"sv))); generator.emit(generator.intern_identifier(ExportStatement::local_name_for_default), Bytecode::Op::SetVariable::InitializationMode::Initialize); return {}; } diff --git a/Userland/Libraries/LibJS/Bytecode/Generator.cpp b/Userland/Libraries/LibJS/Bytecode/Generator.cpp index fe8d891546..befaa7d90b 100644 --- a/Userland/Libraries/LibJS/Bytecode/Generator.cpp +++ b/Userland/Libraries/LibJS/Bytecode/Generator.cpp @@ -452,7 +452,7 @@ void Generator::pop_home_object() m_home_objects.take_last(); } -void Generator::emit_new_function(FunctionExpression const& function_node, Optional lhs_name) +void Generator::emit_new_function(FunctionExpression const& function_node, Optional lhs_name) { if (m_home_objects.is_empty()) emit(function_node, lhs_name); @@ -460,7 +460,7 @@ void Generator::emit_new_function(FunctionExpression const& function_node, Optio emit(function_node, lhs_name, m_home_objects.last()); } -CodeGenerationErrorOr Generator::emit_named_evaluation_if_anonymous_function(Expression const& expression, Optional lhs_name) +CodeGenerationErrorOr Generator::emit_named_evaluation_if_anonymous_function(Expression const& expression, Optional lhs_name) { if (is(expression)) { auto const& function_expression = static_cast(expression); diff --git a/Userland/Libraries/LibJS/Bytecode/Generator.h b/Userland/Libraries/LibJS/Bytecode/Generator.h index 2349f159ee..848e30475a 100644 --- a/Userland/Libraries/LibJS/Bytecode/Generator.h +++ b/Userland/Libraries/LibJS/Bytecode/Generator.h @@ -85,9 +85,9 @@ public: void push_home_object(Register); void pop_home_object(); - void emit_new_function(JS::FunctionExpression const&, Optional lhs_name); + void emit_new_function(JS::FunctionExpression const&, Optional lhs_name); - CodeGenerationErrorOr emit_named_evaluation_if_anonymous_function(Expression const&, Optional lhs_name); + CodeGenerationErrorOr emit_named_evaluation_if_anonymous_function(Expression const&, Optional lhs_name); void begin_continuable_scope(Label continue_target, Vector const& language_label_set); void end_continuable_scope(); diff --git a/Userland/Libraries/LibJS/Bytecode/Op.cpp b/Userland/Libraries/LibJS/Bytecode/Op.cpp index e48c4af61f..4c9c6957de 100644 --- a/Userland/Libraries/LibJS/Bytecode/Op.cpp +++ b/Userland/Libraries/LibJS/Bytecode/Op.cpp @@ -795,7 +795,10 @@ ThrowCompletionOr NewFunction::execute_impl(Bytecode::Interpreter& interpr auto& vm = interpreter.vm(); if (!m_function_node.has_name()) { - interpreter.accumulator() = m_function_node.instantiate_ordinary_function_expression(vm, m_lhs_name.value_or({})); + DeprecatedFlyString name = {}; + if (m_lhs_name.has_value()) + name = interpreter.current_executable().get_identifier(m_lhs_name.value()); + interpreter.accumulator() = m_function_node.instantiate_ordinary_function_expression(vm, name); } else { interpreter.accumulator() = ECMAScriptFunctionObject::create(interpreter.realm(), m_function_node.name(), m_function_node.source_text(), m_function_node.body(), m_function_node.parameters(), m_function_node.function_length(), vm.lexical_environment(), vm.running_execution_context().private_environment, m_function_node.kind(), m_function_node.is_strict_mode(), m_function_node.might_need_arguments_object(), m_function_node.contains_direct_call_to_eval(), m_function_node.is_arrow_function()); } @@ -1149,7 +1152,7 @@ ThrowCompletionOr NewClass::execute_impl(Bytecode::Interpreter& interprete ECMAScriptFunctionObject* class_object = nullptr; if (!m_class_expression.has_name() && m_lhs_name.has_value()) - class_object = TRY(m_class_expression.class_definition_evaluation(vm, {}, m_lhs_name.value())); + class_object = TRY(m_class_expression.class_definition_evaluation(vm, {}, interpreter.current_executable().get_identifier(m_lhs_name.value()))); else class_object = TRY(m_class_expression.class_definition_evaluation(vm, name, name.is_null() ? ""sv : name)); diff --git a/Userland/Libraries/LibJS/Bytecode/Op.h b/Userland/Libraries/LibJS/Bytecode/Op.h index 588ed81119..a7aaec1143 100644 --- a/Userland/Libraries/LibJS/Bytecode/Op.h +++ b/Userland/Libraries/LibJS/Bytecode/Op.h @@ -822,10 +822,10 @@ private: class NewClass final : public Instruction { public: - explicit NewClass(ClassExpression const& class_expression, Optional lhs_name) + explicit NewClass(ClassExpression const& class_expression, Optional lhs_name) : Instruction(Type::NewClass) , m_class_expression(class_expression) - , m_lhs_name(lhs_name.has_value() ? *lhs_name : Optional {}) + , m_lhs_name(lhs_name) { } @@ -836,15 +836,15 @@ public: private: ClassExpression const& m_class_expression; - Optional m_lhs_name; + Optional m_lhs_name; }; class NewFunction final : public Instruction { public: - explicit NewFunction(FunctionExpression const& function_node, Optional lhs_name, Optional home_object = {}) + explicit NewFunction(FunctionExpression const& function_node, Optional lhs_name, Optional home_object = {}) : Instruction(Type::NewFunction) , m_function_node(function_node) - , m_lhs_name(lhs_name.has_value() ? *lhs_name : Optional {}) + , m_lhs_name(lhs_name) , m_home_object(move(home_object)) { } @@ -856,7 +856,7 @@ public: private: FunctionExpression const& m_function_node; - Optional m_lhs_name; + Optional m_lhs_name; Optional m_home_object; };