From 6eb6752c4cb8bc339bdd5af51d3673ea6d100300 Mon Sep 17 00:00:00 2001 From: Matthew Olsson Date: Sat, 3 Oct 2020 17:02:43 -0700 Subject: [PATCH] LibJS: Strict mode is now handled by Functions and Programs, not Blocks Since blocks can't be strict by themselves, it makes no sense for them to store whether or not they are strict. Strict-ness is now stored in the Program and FunctionNode ASTNodes. Fixes issue #3641 --- Libraries/LibJS/AST.cpp | 7 +++++- Libraries/LibJS/AST.h | 23 ++++++++++++------ Libraries/LibJS/Interpreter.cpp | 19 ++++++++++----- Libraries/LibJS/Interpreter.h | 15 ++++++------ Libraries/LibJS/Parser.cpp | 27 ++++++++++++++------- Libraries/LibJS/Parser.h | 1 + Libraries/LibJS/Runtime/ScriptFunction.cpp | 9 ++++--- Libraries/LibJS/Runtime/ScriptFunction.h | 5 ++-- Libraries/LibJS/Runtime/VM.h | 1 + Libraries/LibJS/Tests/strict-mode-blocks.js | 22 +++++++++++++++++ 10 files changed, 92 insertions(+), 37 deletions(-) create mode 100644 Libraries/LibJS/Tests/strict-mode-blocks.js diff --git a/Libraries/LibJS/AST.cpp b/Libraries/LibJS/AST.cpp index a8a1b44cdcc..ccdb818d1b6 100644 --- a/Libraries/LibJS/AST.cpp +++ b/Libraries/LibJS/AST.cpp @@ -89,6 +89,11 @@ Value ScopeNode::execute(Interpreter& interpreter, GlobalObject& global_object) return interpreter.execute_statement(global_object, *this); } +Value Program::execute(Interpreter& interpreter, GlobalObject& global_object) const +{ + return interpreter.execute_statement(global_object, *this, {}, ScopeType::Block, m_is_strict_mode); +} + Value FunctionDeclaration::execute(Interpreter&, GlobalObject&) const { return js_undefined(); @@ -96,7 +101,7 @@ Value FunctionDeclaration::execute(Interpreter&, GlobalObject&) const Value FunctionExpression::execute(Interpreter& interpreter, GlobalObject& global_object) const { - return ScriptFunction::create(global_object, name(), body(), parameters(), function_length(), interpreter.current_environment(), m_is_arrow_function); + return ScriptFunction::create(global_object, name(), body(), parameters(), function_length(), interpreter.current_environment(), is_strict_mode(), m_is_arrow_function); } Value ExpressionStatement::execute(Interpreter& interpreter, GlobalObject& global_object) const diff --git a/Libraries/LibJS/AST.h b/Libraries/LibJS/AST.h index 057162b43fa..cdefaf6f78b 100644 --- a/Libraries/LibJS/AST.h +++ b/Libraries/LibJS/AST.h @@ -129,8 +129,6 @@ public: void add_functions(NonnullRefPtrVector); const NonnullRefPtrVector& variables() const { return m_variables; } const NonnullRefPtrVector& functions() const { return m_functions; } - bool in_strict_mode() const { return m_strict_mode; } - void set_strict_mode() { m_strict_mode = true; } protected: ScopeNode() { } @@ -140,14 +138,20 @@ private: NonnullRefPtrVector m_children; NonnullRefPtrVector m_variables; NonnullRefPtrVector m_functions; - bool m_strict_mode { false }; }; class Program final : public ScopeNode { public: Program() { } + virtual Value execute(Interpreter&, GlobalObject&) const override; + + bool is_strict_mode() const { return m_is_strict_mode; } + void set_strict_mode() { m_is_strict_mode = true; } + private: + bool m_is_strict_mode { false }; + virtual bool is_program() const override { return true; } virtual const char* class_name() const override { return "Program"; } }; @@ -180,14 +184,16 @@ public: const Statement& body() const { return *m_body; } const Vector& parameters() const { return m_parameters; }; i32 function_length() const { return m_function_length; } + bool is_strict_mode() const { return m_is_strict_mode; } protected: - FunctionNode(const FlyString& name, NonnullRefPtr body, Vector parameters, i32 function_length, NonnullRefPtrVector variables) + FunctionNode(const FlyString& name, NonnullRefPtr body, Vector parameters, i32 function_length, NonnullRefPtrVector variables, bool is_strict_mode) : m_name(name) , m_body(move(body)) , m_parameters(move(parameters)) , m_variables(move(variables)) , m_function_length(function_length) + , m_is_strict_mode(is_strict_mode) { } @@ -201,6 +207,7 @@ private: const Vector m_parameters; NonnullRefPtrVector m_variables; const i32 m_function_length; + bool m_is_strict_mode; }; class FunctionDeclaration final @@ -209,8 +216,8 @@ class FunctionDeclaration final public: static bool must_have_name() { return true; } - FunctionDeclaration(const FlyString& name, NonnullRefPtr body, Vector parameters, i32 function_length, NonnullRefPtrVector variables) - : FunctionNode(name, move(body), move(parameters), function_length, move(variables)) + FunctionDeclaration(const FlyString& name, NonnullRefPtr body, Vector parameters, i32 function_length, NonnullRefPtrVector variables, bool is_strict_mode = false) + : FunctionNode(name, move(body), move(parameters), function_length, move(variables), is_strict_mode) { } @@ -227,8 +234,8 @@ class FunctionExpression final public: static bool must_have_name() { return false; } - FunctionExpression(const FlyString& name, NonnullRefPtr body, Vector parameters, i32 function_length, NonnullRefPtrVector variables, bool is_arrow_function = false) - : FunctionNode(name, move(body), move(parameters), function_length, move(variables)) + FunctionExpression(const FlyString& name, NonnullRefPtr body, Vector parameters, i32 function_length, NonnullRefPtrVector variables, bool is_strict_mode, bool is_arrow_function = false) + : FunctionNode(name, move(body), move(parameters), function_length, move(variables), is_strict_mode) , m_is_arrow_function(is_arrow_function) { } diff --git a/Libraries/LibJS/Interpreter.cpp b/Libraries/LibJS/Interpreter.cpp index 2f4e80be376..a1351e7c649 100644 --- a/Libraries/LibJS/Interpreter.cpp +++ b/Libraries/LibJS/Interpreter.cpp @@ -91,15 +91,15 @@ const GlobalObject& Interpreter::global_object() const return static_cast(*m_global_object.cell()); } -void Interpreter::enter_scope(const ScopeNode& scope_node, ArgumentVector arguments, ScopeType scope_type, GlobalObject& global_object) +void Interpreter::enter_scope(const ScopeNode& scope_node, ArgumentVector arguments, ScopeType scope_type, GlobalObject& global_object, bool is_strict) { for (auto& declaration : scope_node.functions()) { - auto* function = ScriptFunction::create(global_object, declaration.name(), declaration.body(), declaration.parameters(), declaration.function_length(), current_environment()); + auto* function = ScriptFunction::create(global_object, declaration.name(), declaration.body(), declaration.parameters(), declaration.function_length(), current_environment(), is_strict); vm().set_variable(declaration.name(), function, global_object); } if (scope_type == ScopeType::Function) { - m_scope_stack.append({ scope_type, scope_node, false }); + push_scope({ scope_type, scope_node, false, is_strict }); return; } @@ -130,7 +130,7 @@ void Interpreter::enter_scope(const ScopeNode& scope_node, ArgumentVector argume pushed_lexical_environment = true; } - m_scope_stack.append({ scope_type, scope_node, pushed_lexical_environment }); + push_scope({ scope_type, scope_node, pushed_lexical_environment, is_strict }); } void Interpreter::exit_scope(const ScopeNode& scope_node) @@ -148,13 +148,20 @@ void Interpreter::exit_scope(const ScopeNode& scope_node) vm().unwind(ScopeType::None); } -Value Interpreter::execute_statement(GlobalObject& global_object, const Statement& statement, ArgumentVector arguments, ScopeType scope_type) +void Interpreter::push_scope(ScopeFrame frame) +{ + if (in_strict_mode()) + frame.is_strict_mode = true; + m_scope_stack.append(move(frame)); +} + +Value Interpreter::execute_statement(GlobalObject& global_object, const Statement& statement, ArgumentVector arguments, ScopeType scope_type, bool is_strict) { if (!statement.is_scope_node()) return statement.execute(*this, global_object); auto& block = static_cast(statement); - enter_scope(block, move(arguments), scope_type, global_object); + enter_scope(block, move(arguments), scope_type, global_object, is_strict); if (block.children().is_empty()) vm().set_last_value({}, js_undefined()); diff --git a/Libraries/LibJS/Interpreter.h b/Libraries/LibJS/Interpreter.h index afdc9a667a4..40c4cc42c44 100644 --- a/Libraries/LibJS/Interpreter.h +++ b/Libraries/LibJS/Interpreter.h @@ -88,10 +88,9 @@ public: bool in_strict_mode() const { - // FIXME: This implementation is bogus; strict mode is per-file or per-function, not per-block! if (m_scope_stack.is_empty()) - return true; - return m_scope_stack.last().scope_node->in_strict_mode(); + return false; + return m_scope_stack.last().is_strict_mode; } size_t argument_count() const { return vm().argument_count(); } @@ -100,10 +99,10 @@ public: LexicalEnvironment* current_environment() { return vm().current_environment(); } const CallFrame& call_frame() { return vm().call_frame(); } - void enter_scope(const ScopeNode&, ArgumentVector, ScopeType, GlobalObject&); + void enter_scope(const ScopeNode&, ArgumentVector, ScopeType, GlobalObject&, bool is_strict = false); void exit_scope(const ScopeNode&); - Value execute_statement(GlobalObject&, const Statement&, ArgumentVector = {}, ScopeType = ScopeType::Block); + Value execute_statement(GlobalObject&, const Statement&, ArgumentVector = {}, ScopeType = ScopeType::Block, bool is_strict = false); private: explicit Interpreter(VM&); @@ -113,11 +112,13 @@ private: return vm().call(function, this_value, move(arguments)); } + void push_scope(ScopeFrame frame); + + Vector m_scope_stack; + NonnullRefPtr m_vm; Handle m_global_object; - - Vector m_scope_stack; }; template<> diff --git a/Libraries/LibJS/Parser.cpp b/Libraries/LibJS/Parser.cpp index 6de165e6f41..7383277f3f1 100644 --- a/Libraries/LibJS/Parser.cpp +++ b/Libraries/LibJS/Parser.cpp @@ -390,10 +390,12 @@ RefPtr Parser::try_parse_arrow_function_expression(bool expe if (function_length == -1) function_length = parameters.size(); - auto function_body_result = [this]() -> RefPtr { + bool is_strict = false; + + auto function_body_result = [&]() -> RefPtr { if (match(TokenType::CurlyOpen)) { // Parse a function body with statements - return parse_block_statement(); + return parse_block_statement(is_strict); } if (match_expression()) { // Parse a function body which returns a single expression @@ -414,7 +416,7 @@ RefPtr Parser::try_parse_arrow_function_expression(bool expe if (!function_body_result.is_null()) { state_rollback_guard.disarm(); auto body = function_body_result.release_nonnull(); - return create_ast_node("", move(body), move(parameters), function_length, m_parser_state.m_var_scopes.take_last(), true); + return create_ast_node("", move(body), move(parameters), function_length, m_parser_state.m_var_scopes.take_last(), is_strict, true); } return nullptr; @@ -562,9 +564,9 @@ NonnullRefPtr Parser::parse_class_expression(bool expect_class_ constructor_body->append(create_ast_node(move(super_call))); constructor_body->add_variables(m_parser_state.m_var_scopes.last()); - constructor = create_ast_node(class_name, move(constructor_body), Vector { FunctionNode::Parameter { "args", nullptr, true } }, 0, NonnullRefPtrVector()); + constructor = create_ast_node(class_name, move(constructor_body), Vector { FunctionNode::Parameter { "args", nullptr, true } }, 0, NonnullRefPtrVector(), true); } else { - constructor = create_ast_node(class_name, move(constructor_body), Vector {}, 0, NonnullRefPtrVector()); + constructor = create_ast_node(class_name, move(constructor_body), Vector {}, 0, NonnullRefPtrVector(), true); } } @@ -1203,6 +1205,12 @@ NonnullRefPtr Parser::parse_return_statement() } NonnullRefPtr Parser::parse_block_statement() +{ + bool dummy = false; + return parse_block_statement(dummy); +} + +NonnullRefPtr Parser::parse_block_statement(bool& is_strict) { ScopePusher scope(*this, ScopePusher::Let); auto block = create_ast_node(); @@ -1212,7 +1220,7 @@ NonnullRefPtr Parser::parse_block_statement() bool initial_strict_mode_state = m_parser_state.m_strict_mode; if (initial_strict_mode_state) { m_parser_state.m_use_strict_directive = UseStrictDirectiveState::None; - block->set_strict_mode(); + is_strict = true; } else { m_parser_state.m_use_strict_directive = UseStrictDirectiveState::Looking; } @@ -1225,7 +1233,7 @@ NonnullRefPtr Parser::parse_block_statement() if (first && !initial_strict_mode_state) { if (m_parser_state.m_use_strict_directive == UseStrictDirectiveState::Found) { - block->set_strict_mode(); + is_strict = true; m_parser_state.m_strict_mode = true; } m_parser_state.m_use_strict_directive = UseStrictDirectiveState::None; @@ -1292,10 +1300,11 @@ NonnullRefPtr Parser::parse_function_node(bool check_for_funct if (function_length == -1) function_length = parameters.size(); - auto body = parse_block_statement(); + bool is_strict = false; + auto body = parse_block_statement(is_strict); body->add_variables(m_parser_state.m_var_scopes.last()); body->add_functions(m_parser_state.m_function_scopes.last()); - return create_ast_node(name, move(body), move(parameters), function_length, NonnullRefPtrVector()); + return create_ast_node(name, move(body), move(parameters), function_length, NonnullRefPtrVector(), is_strict); } NonnullRefPtr Parser::parse_variable_declaration(bool with_semicolon) diff --git a/Libraries/LibJS/Parser.h b/Libraries/LibJS/Parser.h index f6d45adca80..e3ee817c643 100644 --- a/Libraries/LibJS/Parser.h +++ b/Libraries/LibJS/Parser.h @@ -50,6 +50,7 @@ public: NonnullRefPtr parse_statement(); NonnullRefPtr parse_block_statement(); + NonnullRefPtr parse_block_statement(bool& is_strict); NonnullRefPtr parse_return_statement(); NonnullRefPtr parse_variable_declaration(bool with_semicolon = true); NonnullRefPtr parse_for_statement(); diff --git a/Libraries/LibJS/Runtime/ScriptFunction.cpp b/Libraries/LibJS/Runtime/ScriptFunction.cpp index aea79317e87..76a001c131f 100644 --- a/Libraries/LibJS/Runtime/ScriptFunction.cpp +++ b/Libraries/LibJS/Runtime/ScriptFunction.cpp @@ -47,18 +47,19 @@ static ScriptFunction* typed_this(VM& vm, GlobalObject& global_object) return static_cast(this_object); } -ScriptFunction* ScriptFunction::create(GlobalObject& global_object, const FlyString& name, const Statement& body, Vector parameters, i32 m_function_length, LexicalEnvironment* parent_environment, bool is_arrow_function) +ScriptFunction* ScriptFunction::create(GlobalObject& global_object, const FlyString& name, const Statement& body, Vector parameters, i32 m_function_length, LexicalEnvironment* parent_environment, bool is_strict, bool is_arrow_function) { - return global_object.heap().allocate(global_object, global_object, name, body, move(parameters), m_function_length, parent_environment, *global_object.function_prototype(), is_arrow_function); + return global_object.heap().allocate(global_object, global_object, name, body, move(parameters), m_function_length, parent_environment, *global_object.function_prototype(), is_strict, is_arrow_function); } -ScriptFunction::ScriptFunction(GlobalObject& global_object, const FlyString& name, const Statement& body, Vector parameters, i32 m_function_length, LexicalEnvironment* parent_environment, Object& prototype, bool is_arrow_function) +ScriptFunction::ScriptFunction(GlobalObject& global_object, const FlyString& name, const Statement& body, Vector parameters, i32 m_function_length, LexicalEnvironment* parent_environment, Object& prototype, bool is_strict, bool is_arrow_function) : Function(prototype, is_arrow_function ? vm().this_value(global_object) : Value(), {}) , m_name(name) , m_body(body) , m_parameters(move(parameters)) , m_parent_environment(parent_environment) , m_function_length(m_function_length) + , m_is_strict(is_strict) , m_is_arrow_function(is_arrow_function) { } @@ -140,7 +141,7 @@ Value ScriptFunction::call() arguments.append({ parameter.name, value }); vm().current_environment()->set(global_object(), parameter.name, { value, DeclarationKind::Var }); } - return interpreter->execute_statement(global_object(), m_body, arguments, ScopeType::Function); + return interpreter->execute_statement(global_object(), m_body, arguments, ScopeType::Function, m_is_strict); } Value ScriptFunction::construct(Function&) diff --git a/Libraries/LibJS/Runtime/ScriptFunction.h b/Libraries/LibJS/Runtime/ScriptFunction.h index 3f0b2276eda..335ca441d5b 100644 --- a/Libraries/LibJS/Runtime/ScriptFunction.h +++ b/Libraries/LibJS/Runtime/ScriptFunction.h @@ -35,9 +35,9 @@ class ScriptFunction final : public Function { JS_OBJECT(ScriptFunction, Function); public: - static ScriptFunction* create(GlobalObject&, const FlyString& name, const Statement& body, Vector parameters, i32 m_function_length, LexicalEnvironment* parent_environment, bool is_arrow_function = false); + static ScriptFunction* create(GlobalObject&, const FlyString& name, const Statement& body, Vector parameters, i32 m_function_length, LexicalEnvironment* parent_environment, bool is_strict, bool is_arrow_function = false); - ScriptFunction(GlobalObject&, const FlyString& name, const Statement& body, Vector parameters, i32 m_function_length, LexicalEnvironment* parent_environment, Object& prototype, bool is_arrow_function = false); + ScriptFunction(GlobalObject&, const FlyString& name, const Statement& body, Vector parameters, i32 m_function_length, LexicalEnvironment* parent_environment, Object& prototype, bool is_strict, bool is_arrow_function = false); virtual void initialize(GlobalObject&) override; virtual ~ScriptFunction(); @@ -63,6 +63,7 @@ private: const Vector m_parameters; LexicalEnvironment* m_parent_environment { nullptr }; i32 m_function_length; + bool m_is_strict; bool m_is_arrow_function; }; diff --git a/Libraries/LibJS/Runtime/VM.h b/Libraries/LibJS/Runtime/VM.h index a602febae78..045318e60b1 100644 --- a/Libraries/LibJS/Runtime/VM.h +++ b/Libraries/LibJS/Runtime/VM.h @@ -50,6 +50,7 @@ struct ScopeFrame { ScopeType type; NonnullRefPtr scope_node; bool pushed_environment { false }; + bool is_strict_mode { false }; }; struct CallFrame { diff --git a/Libraries/LibJS/Tests/strict-mode-blocks.js b/Libraries/LibJS/Tests/strict-mode-blocks.js new file mode 100644 index 00000000000..c1fe1a7d24a --- /dev/null +++ b/Libraries/LibJS/Tests/strict-mode-blocks.js @@ -0,0 +1,22 @@ +test("Issue #3641, strict mode should be function- or program-level, not block-level", () => { + function func() { + expect(isStrictMode()).toBeFalse(); + + { + "use strict"; + expect(isStrictMode()).toBeFalse(); + } + + if (true) { + "use strict"; + expect(isStrictMode()).toBeFalse(); + } + + do { + "use strict"; + expect(isStrictMode()).toBeFalse(); + } while (false); + } + + func(); +});