diff --git a/Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp b/Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp index beb358aaf16..fa73d46786a 100644 --- a/Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp +++ b/Userland/Libraries/LibJS/Bytecode/ASTCodegen.cpp @@ -1540,7 +1540,8 @@ static Bytecode::CodeGenerationErrorOr get_base_and_value_from_mem base, generator.intern_identifier(verify_cast(member_expression.property()).string())); } else { - generator.emit_get_by_id(value, base, generator.intern_identifier(verify_cast(member_expression.property()).string())); + auto base_identifier = generator.intern_identifier_for_expression(member_expression.object()); + generator.emit_get_by_id(value, base, generator.intern_identifier(verify_cast(member_expression.property()).string()), move(base_identifier)); } return BaseAndValue { base, value }; diff --git a/Userland/Libraries/LibJS/Bytecode/CommonImplementations.h b/Userland/Libraries/LibJS/Bytecode/CommonImplementations.h index d5a1fcc4bc1..d9e2b55e1f7 100644 --- a/Userland/Libraries/LibJS/Bytecode/CommonImplementations.h +++ b/Userland/Libraries/LibJS/Bytecode/CommonImplementations.h @@ -58,7 +58,30 @@ inline void fast_typed_array_set_element(TypedArrayBase& typed_array, u32 index, *slot = value; } -ALWAYS_INLINE ThrowCompletionOr> base_object_for_get(VM& vm, Value base_value) +template +ALWAYS_INLINE Completion throw_null_or_undefined_property_access(VM& vm, Value base_value, BaseType const& base_identifier, PropertyType const& property_identifier) +{ + VERIFY(base_value.is_nullish()); + + bool has_base_identifier = true; + bool has_property_identifier = true; + + if constexpr (requires { base_identifier.has_value(); }) + has_base_identifier = base_identifier.has_value(); + if constexpr (requires { property_identifier.has_value(); }) + has_property_identifier = property_identifier.has_value(); + + if (has_base_identifier && has_property_identifier) + return vm.throw_completion(ErrorType::ToObjectNullOrUndefinedWithPropertyAndName, property_identifier, base_value, base_identifier); + if (has_property_identifier) + return vm.throw_completion(ErrorType::ToObjectNullOrUndefinedWithProperty, property_identifier, base_value); + if (has_base_identifier) + return vm.throw_completion(ErrorType::ToObjectNullOrUndefinedWithName, base_identifier, base_value); + return vm.throw_completion(ErrorType::ToObjectNullOrUndefined); +} + +template +ALWAYS_INLINE ThrowCompletionOr> base_object_for_get(VM& vm, Value base_value, BaseType const& base_identifier, PropertyType property_identifier) { if (base_value.is_object()) [[likely]] return base_value.as_object(); @@ -77,10 +100,10 @@ ALWAYS_INLINE ThrowCompletionOr> base_object_for_get(VM& vm return realm.intrinsics().symbol_prototype(); // NOTE: At this point this is guaranteed to throw (null or undefined). - return base_value.to_object(vm); + return throw_null_or_undefined_property_access(vm, base_value, base_identifier, property_identifier); } -inline ThrowCompletionOr get_by_id(VM& vm, DeprecatedFlyString const& property, Value base_value, Value this_value, PropertyLookupCache& cache) +inline ThrowCompletionOr get_by_id(VM& vm, Optional const& base_identifier, DeprecatedFlyString const& property, Value base_value, Value this_value, PropertyLookupCache& cache) { if (base_value.is_string()) { auto string_value = TRY(base_value.as_string().get(vm, property)); @@ -88,7 +111,7 @@ inline ThrowCompletionOr get_by_id(VM& vm, DeprecatedFlyString const& pro return *string_value; } - auto base_obj = TRY(base_object_for_get(vm, base_value)); + auto base_obj = TRY(base_object_for_get(vm, base_value, base_identifier, property)); // OPTIMIZATION: Fast path for the magical "length" property on Array objects. if (base_obj->has_magical_length_property() && property == vm.names.length.as_string()) { @@ -112,7 +135,7 @@ inline ThrowCompletionOr get_by_id(VM& vm, DeprecatedFlyString const& pro return value; } -inline ThrowCompletionOr get_by_value(VM& vm, Value base_value, Value property_key_value) +inline ThrowCompletionOr get_by_value(VM& vm, Optional const& base_identifier, Value base_value, Value property_key_value) { // OPTIMIZATION: Fast path for simple Int32 indexes in array-like objects. if (base_value.is_object() && property_key_value.is_int32() && property_key_value.as_i32() >= 0) { @@ -169,7 +192,7 @@ inline ThrowCompletionOr get_by_value(VM& vm, Value base_value, Value pro } } - auto object = TRY(base_object_for_get(vm, base_value)); + auto object = TRY(base_object_for_get(vm, base_value, base_identifier, property_key_value)); auto property_key = TRY(property_key_value.to_property_key(vm)); diff --git a/Userland/Libraries/LibJS/Bytecode/Executable.h b/Userland/Libraries/LibJS/Bytecode/Executable.h index 150e9bc6f5f..e593ccd851c 100644 --- a/Userland/Libraries/LibJS/Bytecode/Executable.h +++ b/Userland/Libraries/LibJS/Bytecode/Executable.h @@ -74,6 +74,13 @@ public: ByteString const& get_string(StringTableIndex index) const { return string_table->get(index); } DeprecatedFlyString const& get_identifier(IdentifierTableIndex index) const { return identifier_table->get(index); } + Optional get_identifier(Optional const& index) const + { + if (!index.has_value()) + return {}; + return get_identifier(*index); + } + void dump() const; private: diff --git a/Userland/Libraries/LibJS/Bytecode/Generator.cpp b/Userland/Libraries/LibJS/Bytecode/Generator.cpp index 388607084f4..122a6c9b549 100644 --- a/Userland/Libraries/LibJS/Bytecode/Generator.cpp +++ b/Userland/Libraries/LibJS/Bytecode/Generator.cpp @@ -253,13 +253,16 @@ CodeGenerationErrorOr Generator::emit_load_from_re super_reference.loaded_value = dst; return super_reference; } + auto base = TRY(expression.object().generate_bytecode(*this)).value(); + auto base_identifier = intern_identifier_for_expression(expression.object()); + if (expression.is_computed()) { auto property = TRY(expression.property().generate_bytecode(*this)).value(); auto saved_property = Operand(allocate_register()); emit(saved_property, property); auto dst = preferred_dst.has_value() ? preferred_dst.value() : Operand(allocate_register()); - emit(dst, base, property); + emit(dst, base, property, move(base_identifier)); return ReferenceOperands { .base = base, .referenced_name = saved_property, @@ -270,7 +273,7 @@ CodeGenerationErrorOr Generator::emit_load_from_re if (expression.property().is_identifier()) { auto identifier_table_ref = intern_identifier(verify_cast(expression.property()).string()); auto dst = preferred_dst.has_value() ? preferred_dst.value() : Operand(allocate_register()); - emit_get_by_id(dst, base, identifier_table_ref); + emit_get_by_id(dst, base, identifier_table_ref, move(base_identifier)); return ReferenceOperands { .base = base, .referenced_identifier = identifier_table_ref, @@ -443,6 +446,39 @@ void Generator::emit_set_variable(JS::Identifier const& identifier, Operand valu } } +static Optional expression_identifier(Expression const& expression) +{ + if (expression.is_identifier()) { + auto const& identifier = static_cast(expression); + return identifier.string(); + } + + if (expression.is_member_expression()) { + auto const& member_expression = static_cast(expression); + StringBuilder builder; + + if (auto identifer = expression_identifier(member_expression.object()); identifer.has_value()) + builder.append(*identifer); + + if (auto identifer = expression_identifier(member_expression.property()); identifer.has_value()) { + if (!builder.is_empty()) + builder.append('.'); + builder.append(*identifer); + } + + return builder.to_byte_string(); + } + + return {}; +} + +Optional Generator::intern_identifier_for_expression(Expression const& expression) +{ + if (auto identifer = expression_identifier(expression); identifer.has_value()) + return intern_identifier(identifer.release_value()); + return {}; +} + void Generator::generate_scoped_jump(JumpType type) { TemporaryChange temp { m_current_unwind_context, m_current_unwind_context }; @@ -596,9 +632,9 @@ CodeGenerationErrorOr> Generator::emit_named_evaluation_if_ano return expression.generate_bytecode(*this, preferred_dst); } -void Generator::emit_get_by_id(Operand dst, Operand base, IdentifierTableIndex id) +void Generator::emit_get_by_id(Operand dst, Operand base, IdentifierTableIndex property_identifier, Optional base_identifier) { - emit(dst, base, id, m_next_property_lookup_cache++); + emit(dst, base, property_identifier, move(base_identifier), m_next_property_lookup_cache++); } void Generator::emit_get_by_id_with_this(Operand dst, Operand base, IdentifierTableIndex id, Operand this_value) diff --git a/Userland/Libraries/LibJS/Bytecode/Generator.h b/Userland/Libraries/LibJS/Bytecode/Generator.h index 79d0302b297..1b9b81ae69c 100644 --- a/Userland/Libraries/LibJS/Bytecode/Generator.h +++ b/Userland/Libraries/LibJS/Bytecode/Generator.h @@ -183,6 +183,8 @@ public: return m_identifier_table->insert(move(string)); } + Optional intern_identifier_for_expression(Expression const& expression); + bool is_in_generator_or_async_function() const { return m_enclosing_function_kind == FunctionKind::Async || m_enclosing_function_kind == FunctionKind::Generator || m_enclosing_function_kind == FunctionKind::AsyncGenerator; } bool is_in_generator_function() const { return m_enclosing_function_kind == FunctionKind::Generator || m_enclosing_function_kind == FunctionKind::AsyncGenerator; } bool is_in_async_function() const { return m_enclosing_function_kind == FunctionKind::Async || m_enclosing_function_kind == FunctionKind::AsyncGenerator; } @@ -247,7 +249,7 @@ public: m_boundaries.take_last(); } - void emit_get_by_id(Operand dst, Operand base, IdentifierTableIndex); + void emit_get_by_id(Operand dst, Operand base, IdentifierTableIndex property_identifier, Optional base_identifier = {}); void emit_get_by_id_with_this(Operand dst, Operand base, IdentifierTableIndex, Operand this_value); diff --git a/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp b/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp index 96b512d7433..249a826975f 100644 --- a/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp +++ b/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp @@ -1083,9 +1083,13 @@ ThrowCompletionOr SetLocal::execute_impl(Bytecode::Interpreter&) const ThrowCompletionOr GetById::execute_impl(Bytecode::Interpreter& interpreter) const { + auto base_identifier = interpreter.current_executable().get_identifier(m_base_identifier); + auto const& property_identifier = interpreter.current_executable().get_identifier(m_property); + auto base_value = interpreter.get(base()); auto& cache = interpreter.current_executable().property_lookup_caches[m_cache_index]; - interpreter.set(dst(), TRY(get_by_id(interpreter.vm(), interpreter.current_executable().get_identifier(m_property), base_value, base_value, cache))); + + interpreter.set(dst(), TRY(get_by_id(interpreter.vm(), base_identifier, property_identifier, base_value, base_value, cache))); return {}; } @@ -1094,7 +1098,7 @@ ThrowCompletionOr GetByIdWithThis::execute_impl(Bytecode::Interpreter& int auto base_value = interpreter.get(m_base); auto this_value = interpreter.get(m_this_value); auto& cache = interpreter.current_executable().property_lookup_caches[m_cache_index]; - interpreter.set(dst(), TRY(get_by_id(interpreter.vm(), interpreter.current_executable().get_identifier(m_property), base_value, this_value, cache))); + interpreter.set(dst(), TRY(get_by_id(interpreter.vm(), {}, interpreter.current_executable().get_identifier(m_property), base_value, this_value, cache))); return {}; } @@ -1499,7 +1503,9 @@ ThrowCompletionOr Await::execute_impl(Bytecode::Interpreter& interpreter) ThrowCompletionOr GetByValue::execute_impl(Bytecode::Interpreter& interpreter) const { - interpreter.set(dst(), TRY(get_by_value(interpreter.vm(), interpreter.get(m_base), interpreter.get(m_property)))); + auto base_identifier = interpreter.current_executable().get_identifier(m_base_identifier); + + interpreter.set(dst(), TRY(get_by_value(interpreter.vm(), base_identifier, interpreter.get(m_base), interpreter.get(m_property)))); return {}; } diff --git a/Userland/Libraries/LibJS/Bytecode/Op.h b/Userland/Libraries/LibJS/Bytecode/Op.h index 4f79de991f9..a38d11180b1 100644 --- a/Userland/Libraries/LibJS/Bytecode/Op.h +++ b/Userland/Libraries/LibJS/Bytecode/Op.h @@ -629,11 +629,12 @@ private: class GetById final : public Instruction { public: - GetById(Operand dst, Operand base, IdentifierTableIndex property, u32 cache_index) + GetById(Operand dst, Operand base, IdentifierTableIndex property, Optional base_identifier, u32 cache_index) : Instruction(Type::GetById, sizeof(*this)) , m_dst(dst) , m_base(base) , m_property(property) + , m_base_identifier(move(base_identifier)) , m_cache_index(cache_index) { } @@ -650,6 +651,7 @@ private: Operand m_dst; Operand m_base; IdentifierTableIndex m_property; + Optional m_base_identifier; u32 m_cache_index { 0 }; }; @@ -874,11 +876,12 @@ private: class GetByValue final : public Instruction { public: - explicit GetByValue(Operand dst, Operand base, Operand property) + GetByValue(Operand dst, Operand base, Operand property, Optional base_identifier = {}) : Instruction(Type::GetByValue, sizeof(*this)) , m_dst(dst) , m_base(base) , m_property(property) + , m_base_identifier(move(base_identifier)) { } @@ -889,10 +892,13 @@ public: Operand base() const { return m_base; } Operand property() const { return m_property; } + Optional base_identifier(Bytecode::Interpreter const&) const; + private: Operand m_dst; Operand m_base; Operand m_property; + Optional m_base_identifier; }; class GetByValueWithThis final : public Instruction { diff --git a/Userland/Libraries/LibJS/Runtime/ErrorTypes.h b/Userland/Libraries/LibJS/Runtime/ErrorTypes.h index 828233085b5..5bfc6600705 100644 --- a/Userland/Libraries/LibJS/Runtime/ErrorTypes.h +++ b/Userland/Libraries/LibJS/Runtime/ErrorTypes.h @@ -302,6 +302,9 @@ M(ThisHasNotBeenInitialized, "|this| has not been initialized") \ M(ThisIsAlreadyInitialized, "|this| is already initialized") \ M(ToObjectNullOrUndefined, "ToObject on null or undefined") \ + M(ToObjectNullOrUndefinedWithName, "\"{}\" is {}") \ + M(ToObjectNullOrUndefinedWithProperty, "Cannot access property \"{}\" on {} object") \ + M(ToObjectNullOrUndefinedWithPropertyAndName, "Cannot access property \"{}\" on {} object \"{}\"") \ M(TopLevelVariableAlreadyDeclared, "Redeclaration of top level variable '{}'") \ M(ToPrimitiveReturnedObject, "Can't convert {} to primitive with hint \"{}\", its @@toPrimitive method returned an object") \ M(TypedArrayContentTypeMismatch, "Can't create {} from {}") \ diff --git a/Userland/Libraries/LibJS/Tests/null-or-undefined-access.js b/Userland/Libraries/LibJS/Tests/null-or-undefined-access.js new file mode 100644 index 00000000000..2c167ceee89 --- /dev/null +++ b/Userland/Libraries/LibJS/Tests/null-or-undefined-access.js @@ -0,0 +1,36 @@ +test("null/undefined object", () => { + [null, undefined].forEach(value => { + let foo = value; + + expect(() => { + foo.bar; + }).toThrowWithMessage(TypeError, `Cannot access property "bar" on ${value} object "foo"`); + + expect(() => { + foo[0]; + }).toThrowWithMessage(TypeError, `Cannot access property "0" on ${value} object "foo"`); + }); +}); + +test("null/undefined object key", () => { + [null, undefined].forEach(value => { + let foo = { bar: value }; + + expect(() => { + foo.bar.baz; + }).toThrowWithMessage( + TypeError, + `Cannot access property "baz" on ${value} object "foo.bar"` + ); + }); +}); + +test("null/undefined array index", () => { + [null, undefined].forEach(value => { + let foo = [value]; + + expect(() => { + foo[0].bar; + }).toThrowWithMessage(TypeError, `Cannot access property "bar" on ${value} object`); + }); +});