From 188297160d2b82a4e2a206ebbddfc21dd99a9d8d Mon Sep 17 00:00:00 2001 From: verwaest Date: Tue, 12 May 2015 08:11:18 -0700 Subject: [PATCH] Mark internal AccessorInfo properties as "special data properties" to ensure correct strict-mode handling. BUG= Review URL: https://codereview.chromium.org/1123163005 Cr-Commit-Position: refs/heads/master@{#28369} --- src/accessors.cc | 38 +--------------------------------- src/lookup.cc | 4 +++- src/objects-inl.h | 10 +++++++++ src/objects.cc | 15 ++++++++++---- src/objects.h | 6 +++++- test/mjsunit/array-length.js | 6 ++++++ test/mjsunit/es6/arguments-iterator.js | 5 +---- 7 files changed, 37 insertions(+), 47 deletions(-) diff --git a/src/accessors.cc b/src/accessors.cc index 9b24ee3..50df0dd 100644 --- a/src/accessors.cc +++ b/src/accessors.cc @@ -32,6 +32,7 @@ Handle Accessors::MakeAccessor( info->set_property_attributes(attributes); info->set_all_can_read(false); info->set_all_can_write(false); + info->set_is_special_data_property(true); info->set_name(*name); Handle get = v8::FromCData(isolate, getter); Handle set = v8::FromCData(isolate, setter); @@ -126,31 +127,6 @@ bool Accessors::IsJSArrayBufferViewFieldAccessor(Handle map, } -bool SetPropertyOnInstanceIfInherited( - Isolate* isolate, const v8::PropertyCallbackInfo& info, - v8::Local name, Handle value) { - Handle holder = Utils::OpenHandle(*info.Holder()); - Handle receiver = Utils::OpenHandle(*info.This()); - if (*holder == *receiver) return false; - if (receiver->IsJSObject()) { - Handle object = Handle::cast(receiver); - // This behaves sloppy since we lost the actual strict-mode. - // TODO(verwaest): Fix by making ExecutableAccessorInfo behave like data - // properties. - if (object->IsJSGlobalProxy()) { - PrototypeIterator iter(isolate, object); - if (iter.IsAtEnd()) return true; - DCHECK(PrototypeIterator::GetCurrent(iter)->IsJSGlobalObject()); - object = Handle::cast(PrototypeIterator::GetCurrent(iter)); - } - if (!object->map()->is_extensible()) return true; - JSObject::SetOwnPropertyIgnoreAttributes(object, Utils::OpenHandle(*name), - value, NONE).Check(); - } - return true; -} - - // // Accessors::ArgumentsIterator // @@ -174,8 +150,6 @@ void Accessors::ArgumentsIteratorSetter( Handle object = Utils::OpenHandle(*info.This()); Handle value = Utils::OpenHandle(*val); - if (SetPropertyOnInstanceIfInherited(isolate, info, name, value)) return; - LookupIterator it(object, Utils::OpenHandle(*name)); CHECK_EQ(LookupIterator::ACCESSOR, it.state()); DCHECK(it.HolderIsReceiverOrHiddenPrototype()); @@ -234,9 +208,6 @@ void Accessors::ArrayLengthSetter( HandleScope scope(isolate); Handle object = Utils::OpenHandle(*info.This()); Handle value = Utils::OpenHandle(*val); - if (SetPropertyOnInstanceIfInherited(isolate, info, name, value)) { - return; - } value = FlattenNumber(isolate, value); @@ -970,9 +941,6 @@ void Accessors::FunctionPrototypeSetter( i::Isolate* isolate = reinterpret_cast(info.GetIsolate()); HandleScope scope(isolate); Handle value = Utils::OpenHandle(*val); - if (SetPropertyOnInstanceIfInherited(isolate, info, name, value)) { - return; - } Handle object = Handle::cast(Utils::OpenHandle(*info.Holder())); if (SetFunctionPrototype(isolate, object, value).is_null()) { @@ -1061,8 +1029,6 @@ void Accessors::FunctionLengthSetter( HandleScope scope(isolate); Handle value = Utils::OpenHandle(*val); - if (SetPropertyOnInstanceIfInherited(isolate, info, name, value)) return; - Handle object = Handle::cast(Utils::OpenHandle(*info.Holder())); if (SetFunctionLength(isolate, object, value).is_null()) { @@ -1120,8 +1086,6 @@ void Accessors::FunctionNameSetter( HandleScope scope(isolate); Handle value = Utils::OpenHandle(*val); - if (SetPropertyOnInstanceIfInherited(isolate, info, name, value)) return; - Handle object = Handle::cast(Utils::OpenHandle(*info.Holder())); if (SetFunctionName(isolate, object, value).is_null()) { diff --git a/src/lookup.cc b/src/lookup.cc index 809fd0e..85745ce 100644 --- a/src/lookup.cc +++ b/src/lookup.cc @@ -142,7 +142,9 @@ void LookupIterator::PrepareTransitionToDataProperty( Handle value, PropertyAttributes attributes, Object::StoreFromKeyed store_mode) { if (state_ == TRANSITION) return; - DCHECK_NE(LookupIterator::ACCESSOR, state_); + DCHECK(state_ != LookupIterator::ACCESSOR || + (GetAccessors()->IsAccessorInfo() && + AccessorInfo::cast(*GetAccessors())->is_special_data_property())); DCHECK_NE(LookupIterator::INTEGER_INDEXED_EXOTIC, state_); DCHECK(state_ == NOT_FOUND || !HolderIsReceiverOrHiddenPrototype()); // Can only be called when the receiver is a JSObject. JSProxy has to be diff --git a/src/objects-inl.h b/src/objects-inl.h index 6d0d491..026166f 100644 --- a/src/objects-inl.h +++ b/src/objects-inl.h @@ -7045,6 +7045,16 @@ void AccessorInfo::set_all_can_write(bool value) { } +bool AccessorInfo::is_special_data_property() { + return BooleanBit::get(flag(), kSpecialDataProperty); +} + + +void AccessorInfo::set_is_special_data_property(bool value) { + set_flag(BooleanBit::set(flag(), kSpecialDataProperty, value)); +} + + PropertyAttributes AccessorInfo::property_attributes() { return AttributesField::decode(static_cast(flag()->value())); } diff --git a/src/objects.cc b/src/objects.cc index 8d984e3..15398fe 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -3193,14 +3193,21 @@ MaybeHandle Object::SetPropertyInternal(LookupIterator* it, } break; - case LookupIterator::ACCESSOR: + case LookupIterator::ACCESSOR: { if (it->property_details().IsReadOnly()) { return WriteToReadOnlyProperty(it, value, language_mode); } + Handle accessors = it->GetAccessors(); + if (accessors->IsAccessorInfo() && + !it->HolderIsReceiverOrHiddenPrototype() && + AccessorInfo::cast(*accessors)->is_special_data_property()) { + done = true; + break; + } return SetPropertyWithAccessor(it->GetReceiver(), it->name(), value, - it->GetHolder(), - it->GetAccessors(), language_mode); - + it->GetHolder(), accessors, + language_mode); + } case LookupIterator::INTEGER_INDEXED_EXOTIC: done = true; break; diff --git a/src/objects.h b/src/objects.h index 5437530..898f7ac 100644 --- a/src/objects.h +++ b/src/objects.h @@ -10512,6 +10512,9 @@ class AccessorInfo: public Struct { inline bool all_can_write(); inline void set_all_can_write(bool value); + inline bool is_special_data_property(); + inline void set_is_special_data_property(bool value); + inline PropertyAttributes property_attributes(); inline void set_property_attributes(PropertyAttributes attributes); @@ -10544,7 +10547,8 @@ class AccessorInfo: public Struct { // Bit positions in flag. static const int kAllCanReadBit = 0; static const int kAllCanWriteBit = 1; - class AttributesField: public BitField {}; + static const int kSpecialDataProperty = 2; + class AttributesField : public BitField {}; DISALLOW_IMPLICIT_CONSTRUCTORS(AccessorInfo); }; diff --git a/test/mjsunit/array-length.js b/test/mjsunit/array-length.js index 16867db..04b1750 100644 --- a/test/mjsunit/array-length.js +++ b/test/mjsunit/array-length.js @@ -119,3 +119,9 @@ for (var i = 0; i < 7; i++) { t = a.length = 7; assertEquals(7, t); } + +(function () { + "use strict"; + var frozen_object = Object.freeze({__proto__:[]}); + assertThrows(function () { frozen_object.length = 10 }); +})(); diff --git a/test/mjsunit/es6/arguments-iterator.js b/test/mjsunit/es6/arguments-iterator.js index 32d4b11..cf1e1f9 100644 --- a/test/mjsunit/es6/arguments-iterator.js +++ b/test/mjsunit/es6/arguments-iterator.js @@ -219,10 +219,7 @@ function TestArgumentsAsProto() { assertSame([][Symbol.iterator], o[Symbol.iterator]); assertFalse(o.hasOwnProperty(Symbol.iterator)); assertSame([][Symbol.iterator], o[Symbol.iterator]); - // This should throw, but currently it doesn't, because - // ExecutableAccessorInfo callbacks don't see the current strict mode. - // See note in accessors.cc:SetPropertyOnInstanceIfInherited. - o[Symbol.iterator] = 10; + assertThrows(function () { o[Symbol.iterator] = 10 }); assertFalse(o.hasOwnProperty(Symbol.iterator)); assertEquals([][Symbol.iterator], o[Symbol.iterator]); assertSame([][Symbol.iterator], arguments[Symbol.iterator]); -- 2.7.4