From 503fb219878d533f8f1052d236f7549ac53e88d8 Mon Sep 17 00:00:00 2001 From: Mythri Date: Thu, 7 Feb 2019 14:35:24 +0000 Subject: [PATCH] [ic] Fallback to runtime from builtins to check if we throw on error When an error occurs when storing the properties we either need to throw or ignore the error depending on the language mode. We used to infer the language mode from the type feedback vector. This cl instead falls back to runtime to check and throw an error when needed. Bug: v8:8580 Change-Id: Iebeb3ca86d753157329dc1b5cfd1c07af2ff3dcd Reviewed-on: https://chromium-review.googlesource.com/c/1458220 Reviewed-by: Toon Verwaest Commit-Queue: Mythri Alle Cr-Commit-Position: refs/heads/master@{#59563} --- src/builtins/builtins-definitions.h | 2 +- src/builtins/builtins-proxy-gen.cc | 23 ++++-------- src/code-stub-assembler.cc | 51 -------------------------- src/code-stub-assembler.h | 5 --- src/ic/accessor-assembler.cc | 57 +---------------------------- src/ic/accessor-assembler.h | 4 -- src/ic/keyed-store-generic.cc | 43 +++++++--------------- src/runtime/runtime-internal.cc | 7 ++++ src/runtime/runtime.h | 1 + 9 files changed, 32 insertions(+), 161 deletions(-) diff --git a/src/builtins/builtins-definitions.h b/src/builtins/builtins-definitions.h index df8f136ec8..74a235b4b3 100644 --- a/src/builtins/builtins-definitions.h +++ b/src/builtins/builtins-definitions.h @@ -918,7 +918,7 @@ namespace internal { TFJ(ProxyRevoke, 0, kReceiver) \ TFS(ProxyGetProperty, kProxy, kName, kReceiverValue, kOnNonExistent) \ TFS(ProxyHasProperty, kProxy, kName) \ - TFS(ProxySetProperty, kProxy, kName, kValue, kReceiverValue, kLanguageMode) \ + TFS(ProxySetProperty, kProxy, kName, kValue, kReceiverValue) \ \ /* Reflect */ \ ASM(ReflectApply, Dummy) \ diff --git a/src/builtins/builtins-proxy-gen.cc b/src/builtins/builtins-proxy-gen.cc index 737ebb5b0a..5d915cbaf1 100644 --- a/src/builtins/builtins-proxy-gen.cc +++ b/src/builtins/builtins-proxy-gen.cc @@ -545,7 +545,6 @@ TF_BUILTIN(ProxySetProperty, ProxiesCodeStubAssembler) { Node* name = Parameter(Descriptor::kName); Node* value = Parameter(Descriptor::kValue); Node* receiver = Parameter(Descriptor::kReceiverValue); - TNode language_mode = CAST(Parameter(Descriptor::kLanguageMode)); CSA_ASSERT(this, IsJSProxy(proxy)); @@ -596,13 +595,10 @@ TF_BUILTIN(ProxySetProperty, ProxiesCodeStubAssembler) { BIND(&failure); { - Label if_throw(this, Label::kDeferred); - Branch(SmiEqual(language_mode, SmiConstant(LanguageMode::kStrict)), - &if_throw, &success); - - BIND(&if_throw); - ThrowTypeError(context, MessageTemplate::kProxyTrapReturnedFalsishFor, - HeapConstant(set_string), name); + CallRuntime(Runtime::kThrowTypeErrorIfStrict, context, + SmiConstant(MessageTemplate::kProxyTrapReturnedFalsishFor), + HeapConstant(set_string), name); + Goto(&success); } // 12. Return true. @@ -611,16 +607,11 @@ TF_BUILTIN(ProxySetProperty, ProxiesCodeStubAssembler) { BIND(&private_symbol); { - Label failure(this), throw_error(this, Label::kDeferred); + Label failure(this); - Branch(SmiEqual(language_mode, SmiConstant(LanguageMode::kStrict)), - &throw_error, &failure); - - BIND(&failure); + CallRuntime(Runtime::kThrowTypeErrorIfStrict, context, + SmiConstant(MessageTemplate::kProxyPrivate)); Return(UndefinedConstant()); - - BIND(&throw_error); - ThrowTypeError(context, MessageTemplate::kProxyPrivate); } BIND(&trap_undefined); diff --git a/src/code-stub-assembler.cc b/src/code-stub-assembler.cc index a98069b3f7..7c07b9d434 100644 --- a/src/code-stub-assembler.cc +++ b/src/code-stub-assembler.cc @@ -10028,57 +10028,6 @@ void CodeStubAssembler::UpdateFeedback(Node* feedback, Node* maybe_vector, BIND(&end); } -Node* CodeStubAssembler::GetLanguageMode( - TNode shared_function_info, Node* context) { - VARIABLE(var_language_mode, MachineRepresentation::kTaggedSigned, - SmiConstant(LanguageMode::kStrict)); - Label language_mode_determined(this), language_mode_sloppy(this); - - // Get the language mode from SFI - TNode closure_is_strict = - DecodeWord32(LoadObjectField( - shared_function_info, SharedFunctionInfo::kFlagsOffset, - MachineType::Uint32())); - // It is already strict, we need not check context's language mode. - GotoIf(closure_is_strict, &language_mode_determined); - - // SFI::LanguageMode is sloppy, check if context has a stricter mode. - TNode scope_info = - CAST(LoadObjectField(context, Context::kScopeInfoOffset)); - // If no flags field assume sloppy - GotoIf(SmiLessThanOrEqual(LoadFixedArrayBaseLength(scope_info), - SmiConstant(ScopeInfo::Fields::kFlags)), - &language_mode_sloppy); - TNode flags = CAST(LoadFixedArrayElement( - scope_info, SmiConstant(ScopeInfo::Fields::kFlags))); - TNode context_is_strict = - DecodeWord32(SmiToInt32(flags)); - GotoIf(context_is_strict, &language_mode_determined); - Goto(&language_mode_sloppy); - - // Both Context::ScopeInfo::LanguageMode and SFI::LanguageMode are sloppy. - BIND(&language_mode_sloppy); - var_language_mode.Bind(SmiConstant(LanguageMode::kSloppy)); - Goto(&language_mode_determined); - - BIND(&language_mode_determined); - return var_language_mode.value(); -} - -Node* CodeStubAssembler::GetLanguageMode(TNode closure, - Node* context) { - TNode sfi = - CAST(LoadObjectField(closure, JSFunction::kSharedFunctionInfoOffset)); - return GetLanguageMode(sfi, context); -} - -Node* CodeStubAssembler::GetLanguageMode(TNode vector, - Node* context) { - TNode sfi = - CAST(LoadObjectField(vector, FeedbackVector::kSharedFunctionInfoOffset)); - return GetLanguageMode(sfi, context); -} - void CodeStubAssembler::ReportFeedbackUpdate( SloppyTNode feedback_vector, SloppyTNode slot_id, const char* reason) { diff --git a/src/code-stub-assembler.h b/src/code-stub-assembler.h index 23262ec4f2..1fbdded2d3 100644 --- a/src/code-stub-assembler.h +++ b/src/code-stub-assembler.h @@ -2788,11 +2788,6 @@ class V8_EXPORT_PRIVATE CodeStubAssembler // Update the type feedback vector. void UpdateFeedback(Node* feedback, Node* feedback_vector, Node* slot_id); - // Returns the stricter of the Context::ScopeInfo::LanguageMode and - // the language mode on the SFI. - Node* GetLanguageMode(TNode sfi, Node* context); - Node* GetLanguageMode(TNode closure, Node* context); - Node* GetLanguageMode(TNode vector, Node* context); // Report that there was a feedback update, performing any tasks that should // be done after a feedback update. diff --git a/src/ic/accessor-assembler.cc b/src/ic/accessor-assembler.cc index ea3b9d9189..1ab4e824e1 100644 --- a/src/ic/accessor-assembler.cc +++ b/src/ic/accessor-assembler.cc @@ -1393,17 +1393,6 @@ void AccessorAssembler::HandleStoreICProtoHandler( } } -Node* AccessorAssembler::GetLanguageMode(Node* vector, Node* slot) { - VARIABLE(var_language_mode, MachineRepresentation::kTaggedSigned, - SmiConstant(LanguageMode::kStrict)); - Label language_mode_determined(this); - BranchIfStrictMode(vector, slot, &language_mode_determined); - var_language_mode.Bind(SmiConstant(LanguageMode::kSloppy)); - Goto(&language_mode_determined); - BIND(&language_mode_determined); - return var_language_mode.value(); -} - void AccessorAssembler::HandleStoreToProxy(const StoreICParameters* p, Node* proxy, Label* miss, ElementSupport support_elements) { @@ -1413,18 +1402,13 @@ void AccessorAssembler::HandleStoreToProxy(const StoreICParameters* p, Label if_index(this), if_unique_name(this), to_name_failed(this, Label::kDeferred); - // TODO(8580): Get the language mode lazily when required to avoid the - // computation of GetLanguageMode here. Also make the computation of - // language mode not dependent on vector. - Node* language_mode = GetLanguageMode(p->vector, p->slot); - if (support_elements == kSupportElements) { TryToName(p->name, &if_index, &var_index, &if_unique_name, &var_unique, &to_name_failed); BIND(&if_unique_name); CallBuiltin(Builtins::kProxySetProperty, p->context, proxy, - var_unique.value(), p->value, p->receiver, language_mode); + var_unique.value(), p->value, p->receiver); Return(p->value); // The index case is handled earlier by the runtime. @@ -1439,7 +1423,7 @@ void AccessorAssembler::HandleStoreToProxy(const StoreICParameters* p, } else { Node* name = CallBuiltin(Builtins::kToName, p->context, p->name); TailCallBuiltin(Builtins::kProxySetProperty, p->context, proxy, name, - p->value, p->receiver, language_mode); + p->value, p->receiver); } } @@ -1949,43 +1933,6 @@ void AccessorAssembler::NameDictionaryNegativeLookup(Node* object, BIND(&done); } -void AccessorAssembler::BranchIfStrictMode(Node* vector, Node* slot, - Label* if_strict) { - Node* sfi = - LoadObjectField(vector, FeedbackVector::kSharedFunctionInfoOffset); - TNode metadata = CAST(LoadObjectField( - sfi, SharedFunctionInfo::kOuterScopeInfoOrFeedbackMetadataOffset)); - Node* slot_int = SmiToInt32(slot); - - // See VectorICComputer::index(). - const int kItemsPerWord = FeedbackMetadata::VectorICComputer::kItemsPerWord; - Node* word_index = Int32Div(slot_int, Int32Constant(kItemsPerWord)); - Node* word_offset = Int32Mod(slot_int, Int32Constant(kItemsPerWord)); - - int32_t first_item = FeedbackMetadata::kHeaderSize - kHeapObjectTag; - Node* offset = - ElementOffsetFromIndex(ChangeInt32ToIntPtr(word_index), UINT32_ELEMENTS, - INTPTR_PARAMETERS, first_item); - - Node* data = Load(MachineType::Int32(), metadata, offset); - - // See VectorICComputer::decode(). - const int kBitsPerItem = FeedbackMetadata::kFeedbackSlotKindBits; - Node* shift = Int32Mul(word_offset, Int32Constant(kBitsPerItem)); - const int kMask = FeedbackMetadata::VectorICComputer::kMask; - Node* kind = Word32And(Word32Shr(data, shift), Int32Constant(kMask)); - - STATIC_ASSERT(FeedbackSlotKind::kStoreGlobalSloppy <= - FeedbackSlotKind::kLastSloppyKind); - STATIC_ASSERT(FeedbackSlotKind::kStoreKeyedSloppy <= - FeedbackSlotKind::kLastSloppyKind); - STATIC_ASSERT(FeedbackSlotKind::kStoreNamedSloppy <= - FeedbackSlotKind::kLastSloppyKind); - GotoIfNot(Int32LessThanOrEqual(kind, Int32Constant(static_cast( - FeedbackSlotKind::kLastSloppyKind))), - if_strict); -} - void AccessorAssembler::InvalidateValidityCellIfPrototype(Node* map, Node* bitfield2) { Label is_prototype(this), cont(this); diff --git a/src/ic/accessor-assembler.h b/src/ic/accessor-assembler.h index 452e10ac9e..bf93fa8199 100644 --- a/src/ic/accessor-assembler.h +++ b/src/ic/accessor-assembler.h @@ -123,8 +123,6 @@ class AccessorAssembler : public CodeStubAssembler { void JumpIfDataProperty(Node* details, Label* writable, Label* readonly); - void BranchIfStrictMode(Node* vector, Node* slot, Label* if_strict); - void InvalidateValidityCellIfPrototype(Node* map, Node* bitfield2 = nullptr); void OverwriteExistingFastDataProperty(Node* object, Node* object_map, @@ -274,8 +272,6 @@ class AccessorAssembler : public CodeStubAssembler { const OnFoundOnReceiver& on_found_on_receiver, Label* miss, ICMode ic_mode); - Node* GetLanguageMode(Node* vector, Node* slot); - Node* PrepareValueForStore(Node* handler_word, Node* holder, Representation representation, Node* value, Label* bailout); diff --git a/src/ic/keyed-store-generic.cc b/src/ic/keyed-store-generic.cc index d143f561b2..faeaf70b85 100644 --- a/src/ic/keyed-store-generic.cc +++ b/src/ic/keyed-store-generic.cc @@ -912,29 +912,21 @@ void KeyedStoreGenericAssembler::EmitGenericPropertyStore( BIND(¬_callable); { - bool handle_strict = true; - Label strict(this); LanguageMode language_mode; if (maybe_language_mode.To(&language_mode)) { if (language_mode == LanguageMode::kStrict) { - Goto(&strict); - } else { - handle_strict = false; - exit_point->Return(p->value); - } - } else { - BranchIfStrictMode(p->vector, p->slot, &strict); - exit_point->Return(p->value); - } - - if (handle_strict) { - BIND(&strict); - { exit_point->ReturnCallRuntime( Runtime::kThrowTypeError, p->context, SmiConstant(MessageTemplate::kNoSetterInCallback), p->name, var_accessor_holder.value()); + } else { + exit_point->Return(p->value); } + } else { + CallRuntime(Runtime::kThrowTypeErrorIfStrict, p->context, + SmiConstant(MessageTemplate::kNoSetterInCallback), + p->name, var_accessor_holder.value()); + exit_point->Return(p->value); } } } @@ -943,27 +935,20 @@ void KeyedStoreGenericAssembler::EmitGenericPropertyStore( if (!ShouldReconfigureExisting()) { BIND(&readonly); { - bool handle_strict = true; - Label strict(this); LanguageMode language_mode; if (maybe_language_mode.To(&language_mode)) { if (language_mode == LanguageMode::kStrict) { - Goto(&strict); - } else { - handle_strict = false; - exit_point->Return(p->value); - } - } else { - BranchIfStrictMode(p->vector, p->slot, &strict); - exit_point->Return(p->value); - } - if (handle_strict) { - BIND(&strict); - { Node* type = Typeof(p->receiver); ThrowTypeError(p->context, MessageTemplate::kStrictReadOnlyProperty, p->name, type, p->receiver); + } else { + exit_point->Return(p->value); } + } else { + CallRuntime(Runtime::kThrowTypeErrorIfStrict, p->context, + SmiConstant(MessageTemplate::kStrictReadOnlyProperty), + p->name, Typeof(p->receiver), p->receiver); + exit_point->Return(p->value); } } } diff --git a/src/runtime/runtime-internal.cc b/src/runtime/runtime-internal.cc index 9b72eeb86c..bb76f2a4c4 100644 --- a/src/runtime/runtime-internal.cc +++ b/src/runtime/runtime-internal.cc @@ -95,6 +95,13 @@ RUNTIME_FUNCTION(Runtime_ThrowTypeError) { THROW_ERROR(isolate, args, NewTypeError); } +RUNTIME_FUNCTION(Runtime_ThrowTypeErrorIfStrict) { + if (GetShouldThrow(isolate, Nothing()) == + ShouldThrow::kDontThrow) + return ReadOnlyRoots(isolate).undefined_value(); + THROW_ERROR(isolate, args, NewTypeError); +} + #undef THROW_ERROR namespace { diff --git a/src/runtime/runtime.h b/src/runtime/runtime.h index 94942edd42..35c5147941 100644 --- a/src/runtime/runtime.h +++ b/src/runtime/runtime.h @@ -248,6 +248,7 @@ namespace internal { F(ThrowSymbolIteratorInvalid, 0, 1) \ F(ThrowThrowMethodMissing, 0, 1) \ F(ThrowTypeError, -1 /* >= 1 */, 1) \ + F(ThrowTypeErrorIfStrict, -1 /* >= 1 */, 1) \ F(Typeof, 1, 1) \ F(UnwindAndFindExceptionHandler, 0, 1) \ F(FinalizationGroupCleanupJob, 1, 1)