From 0a7584af2ca88b1560cf2dafd0a009b390084f2f Mon Sep 17 00:00:00 2001 From: whessev8 Date: Thu, 28 Aug 2008 10:23:27 +0000 Subject: [PATCH] Always check the prototype chain for a setter, when setting a property that does not exist locally. Previously, map transitions broke this check. git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@25 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/globals.h | 14 ++++--- src/mirror-delay.js | 17 ++++---- src/objects.cc | 110 ++++++++++++++++++++++++++-------------------------- src/property.h | 5 ++- 4 files changed, 78 insertions(+), 68 deletions(-) diff --git a/src/globals.h b/src/globals.h index 4432123..7bf5802 100644 --- a/src/globals.h +++ b/src/globals.h @@ -297,14 +297,18 @@ enum InlineCacheState { // Type of properties. +// Order of properties is significant. +// Must fit in the BitField PropertyDetails::TypeField. +// A copy of this is in mirror-delay.js. enum PropertyType { NORMAL = 0, // only in slow mode - MAP_TRANSITION = 1, // only in fast mode + FIELD = 1, // only in fast mode CONSTANT_FUNCTION = 2, // only in fast mode - FIELD = 3, // only in fast mode - CALLBACKS = 4, - CONSTANT_TRANSITION = 5, // only in fast mode - INTERCEPTOR = 6, + CALLBACKS = 3, + INTERCEPTOR = 4, // only in lookup results, not in descriptors. + FIRST_PHANTOM_PROPERTY_TYPE = 5, // All properties before this are real. + MAP_TRANSITION = 5, // only in fast mode + CONSTANT_TRANSITION = 6, // only in fast mode NULL_DESCRIPTOR = 7 // only in fast mode }; diff --git a/src/mirror-delay.js b/src/mirror-delay.js index f9347bc..c1ffed1 100644 --- a/src/mirror-delay.js +++ b/src/mirror-delay.js @@ -94,14 +94,17 @@ PropertyKind.Named = 1; PropertyKind.Indexed = 2; -// Different types of properties. NOTE value 1 is missing as it is used -// internally for map transition. +// A copy of the PropertyType enum from global.h PropertyType = {}; -PropertyType.Normal = 0; -PropertyType.ConstantFunction = 2; -PropertyType.Field = 3; -PropertyType.Callbacks = 4; -PropertyType.Interceptor = 5; +PropertyType.Normal = 0; +PropertyType.Field = 1; +PropertyType.ConstantFunction = 2; +PropertyType.Callbacks = 3; +PropertyType.Interceptor = 4; +PropertyType.MapTransition = 5; +PropertyType.ConstantTransition = 6; +PropertyType.NullDescriptor = 7; + // Different attributes for a property. diff --git a/src/objects.cc b/src/objects.cc index 29d3144..87e07d9 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -1507,64 +1507,66 @@ Object* JSObject::SetProperty(LookupResult* result, && !Top::MayNamedAccess(this, name, v8::ACCESS_SET)) { return SetPropertyWithFailedAccessCheck(result, name, value); } - - if (result->IsValid()) { - if (!result->IsLoaded()) { - return SetLazyProperty(result, name, value, attributes); + if (result->IsNotFound() || !result->IsProperty()) { + // We could not find a local property so let's check whether there is an + // accessor that wants to handle the property. + LookupResult accessor_result; + LookupCallbackSetterInPrototypes(name, &accessor_result); + if (accessor_result.IsValid()) { + return SetPropertyWithCallback(accessor_result.GetCallbackObject(), + name, + value, + accessor_result.holder()); } - if (result->IsReadOnly() && !result->IsTransitionType()) return value; - switch (result->type()) { - case NORMAL: - property_dictionary()->ValueAtPut(result->GetDictionaryEntry(), value); - return value; - case FIELD: - properties()->set(result->GetFieldIndex(), value); - return value; - case MAP_TRANSITION: - if (attributes == result->GetAttributes()) { - // Only use map transition if attributes matches. - return AddFastPropertyUsingMap(result->GetTransitionMap(), - name, - value); - } else { - return AddFastProperty(name, value, attributes); - } - case CONSTANT_FUNCTION: - if (value == result->GetConstantFunction()) return value; - // Only replace the function if necessary. - return ReplaceConstantFunctionProperty(name, value); - case CALLBACKS: - return SetPropertyWithCallback(result->GetCallbackObject(), + } + if (result->IsNotFound()) { + return AddProperty(name, value, attributes); + } + if (!result->IsLoaded()) { + return SetLazyProperty(result, name, value, attributes); + } + if (result->IsReadOnly() && result->IsProperty()) return value; + // This is a real property that is not read-only, or it is a + // transition or null descriptor and there are no setters in the prototypes. + switch (result->type()) { + case NORMAL: + property_dictionary()->ValueAtPut(result->GetDictionaryEntry(), value); + return value; + case FIELD: + properties()->set(result->GetFieldIndex(), value); + return value; + case MAP_TRANSITION: + if (attributes == result->GetAttributes()) { + // Only use map transition if the attributes match. + return AddFastPropertyUsingMap(result->GetTransitionMap(), name, - value, - result->holder()); - case INTERCEPTOR: - return SetPropertyWithInterceptor(name, value, attributes); - case CONSTANT_TRANSITION: - // Replace with a MAP_TRANSITION to a new map with a FIELD, even - // if the value is a function. - // AddProperty has been extended to do this, in this case. + value); + } else { return AddFastProperty(name, value, attributes); - case NULL_DESCRIPTOR: - UNREACHABLE(); - default: - UNREACHABLE(); - } - } - - // We could not find a local property so let's check whether there is an - // accessor that wants to handle the property. - LookupResult accessor_result; - LookupCallbackSetterInPrototypes(name, &accessor_result); - if (accessor_result.IsValid()) { - return SetPropertyWithCallback(accessor_result.GetCallbackObject(), - name, - value, - accessor_result.holder()); + } + case CONSTANT_FUNCTION: + if (value == result->GetConstantFunction()) return value; + // Only replace the function if necessary. + return ReplaceConstantFunctionProperty(name, value); + case CALLBACKS: + return SetPropertyWithCallback(result->GetCallbackObject(), + name, + value, + result->holder()); + case INTERCEPTOR: + return SetPropertyWithInterceptor(name, value, attributes); + case CONSTANT_TRANSITION: + // Replace with a MAP_TRANSITION to a new map with a FIELD, even + // if the value is a function. + // AddProperty has been extended to do this, in this case. + return AddFastProperty(name, value, attributes); + case NULL_DESCRIPTOR: + UNREACHABLE(); + default: + UNREACHABLE(); } - - // The property was not found - return AddProperty(name, value, attributes); + UNREACHABLE(); + return value; } diff --git a/src/property.h b/src/property.h index 02e255c..deaedd1 100644 --- a/src/property.h +++ b/src/property.h @@ -231,11 +231,12 @@ class LookupResult BASE_EMBEDDED { bool IsDontEnum() { return details_.IsDontEnum(); } bool IsValid() { return lookup_type_ != NOT_FOUND; } + bool IsNotFound() { return lookup_type_ == NOT_FOUND; } // Tells whether the result is a property. - // Excluding transitions. + // Excluding transitions and the null descriptor. bool IsProperty() { - return IsValid() && !IsTransitionType(); + return IsValid() && type() < FIRST_PHANTOM_PROPERTY_TYPE; } bool IsCacheable() { return cacheable_; } -- 2.7.4