[runtime] PreventExtensionsWithTransition: before adding the new

src/objects.cc

 // Copyright 2015 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/objects.h"
 
 #include <cmath>
 #include <iomanip>
 #include <memory>
 #include <sstream>
@@ skipping 7403 matching lines @@
 }
 
 
 Maybe<bool> JSReceiver::SetIntegrityLevel(Handle<JSReceiver> receiver,
                                           IntegrityLevel level,
                                           ShouldThrow should_throw) {
   DCHECK(level == SEALED || level == FROZEN);
 
   if (receiver->IsJSObject()) {
     Handle<JSObject> object = Handle<JSObject>::cast(receiver);
+
+    // prevent memory leaks by not adding unnecessary transitions
+    Maybe<bool> test = JSObject::TestIntegrityLevel(object, level);
+    MAYBE_RETURN(test, Nothing<bool>());
+    if (test.FromJust()) return Just(true);
+
     if (!object->HasSloppyArgumentsElements()) {  // Fast path.
       if (level == SEALED) {
         return JSObject::PreventExtensionsWithTransition<SEALED>(object,
                                                                  should_throw);
       } else {
         return JSObject::PreventExtensionsWithTransition<FROZEN>(object,
                                                                  should_throw);
       }
     }
   }
@@ skipping 36 matching lines @@
               ? no_conf
               : no_conf_no_write;
       MAYBE_RETURN(
           DefineOwnProperty(isolate, receiver, key, &desc, THROW_ON_ERROR),
           Nothing<bool>());
     }
   }
   return Just(true);
 }
 
-
-Maybe<bool> JSReceiver::TestIntegrityLevel(Handle<JSReceiver> object,
-                                           IntegrityLevel level) {
-  DCHECK(level == SEALED || level == FROZEN);
+namespace {
+Maybe<bool> GenericTestDescriptorsIntegrity(Handle<JSReceiver> object,
+                                            PropertyAttributes level) {
   Isolate* isolate = object->GetIsolate();
 
-  Maybe<bool> extensible = JSReceiver::IsExtensible(object);
-  MAYBE_RETURN(extensible, Nothing<bool>());
-  if (extensible.FromJust()) return Just(false);
-
   Handle<FixedArray> keys;
   ASSIGN_RETURN_ON_EXCEPTION_VALUE(
       isolate, keys, JSReceiver::OwnPropertyKeys(object), Nothing<bool>());
 
   for (int i = 0; i < keys->length(); ++i) {
     Handle<Object> key(keys->get(i), isolate);
     PropertyDescriptor current_desc;
     Maybe<bool> owned = JSReceiver::GetOwnPropertyDescriptor(
         isolate, object, key, &current_desc);
     MAYBE_RETURN(owned, Nothing<bool>());
     if (owned.FromJust()) {
       if (current_desc.configurable()) return Just(false);
       if (level == FROZEN &&
           PropertyDescriptor::IsDataDescriptor(&current_desc) &&
           current_desc.writable()) {
         return Just(false);
       }
     }
   }
   return Just(true);
 }
 
+template <typename Dictionary>
+bool TestDictionaryPropertyIntegrity(Dictionary dict, Isolate* isolate,
+                                     PropertyAttributes level) {
+  DisallowHeapAllocation no_gc;
+  uint32_t capacity = dict->Capacity();
+  FOR_WITH_HANDLE_SCOPE(isolate, uint32_t, j = 0, j, j < capacity, j++, {

[Toon Verwaest, 2017/06/20 08:43:23]

Since you don't create handles here you don't need a FOR_WITH_HANDLE_SCOPE. Just
use a regular for instead.

[kris.selden, 2017/06/20 19:16:21]

sorry, that was a bit of copy & paste from some other 'Fast' method.

+    Object* k = dict->KeyAt(j);
+    if (!dict->IsKey(isolate, k)) continue;

[Toon Verwaest, 2017/06/20 08:43:23]

You also need to check whether the key is "deleted". In the case of global
properties we actually create "deleted" entries so we can cache property lookups
that find properties behind the global object.

Below I describe that we probably shouldn't support the global object on this
fast path, but even so I think we should add the IsDeleted check here to avoid
forgetting to add it later if we ever use this for other purposes.

+    PropertyDetails details = dict->DetailsAt(j);
+    if (details.IsConfigurable()) return false;
+    if (level == FROZEN && details.kind() == kData && !details.IsReadOnly()) {
+      return false;
+    }
+  });
+  return true;
+}
+
+bool TestElementsIntegrityLevel(JSObject* object, PropertyAttributes level) {
+  ElementsKind kind = object->GetElementsKind();
+
+  DCHECK(!IsSloppyArgumentsElementsKind(kind));
+
+  if (IsDictionaryElementsKind(kind)) {
+    return TestDictionaryPropertyIntegrity(
+        SeededNumberDictionary::cast(object->elements()), object->GetIsolate(),
+        level);
+  }
+
+  ElementsAccessor* accessor = ElementsAccessor::ForKind(kind);
+  // Only DICTIONARY_ELEMENTS and SLOW_SLOPPY_ARGUMENTS_ELEMENTS have
+  // PropertyAttributes so just test if empty
+  return accessor->NumberOfElements(object) == 0;
+}
+
+bool TestFastPropertiesIntegrityLevel(Map* map, PropertyAttributes level) {
+  DescriptorArray* descriptors = map->instance_descriptors();
+  int number_of_own_descriptors = map->NumberOfOwnDescriptors();
+  for (int i = 0; i < number_of_own_descriptors; i++) {
+    PropertyDetails details = descriptors->GetDetails(i);
+    if (details.IsConfigurable()) return false;
+    if (level == FROZEN && details.kind() == kData && !details.IsReadOnly()) {
+      return false;
+    }
+  }
+  return true;
+}
+
+bool TestPropertiesIntegrityLevel(JSObject* object, PropertyAttributes level) {
+  if (object->HasFastProperties()) {
+    return TestFastPropertiesIntegrityLevel(object->map(), level);
+  }
+
+  if (object->IsJSGlobalObject()) {
+    return TestDictionaryPropertyIntegrity(object->global_dictionary(),
+                                           object->GetIsolate(), level);
+  }
+
+  return TestDictionaryPropertyIntegrity(object->property_dictionary(),
+                                         object->GetIsolate(), level);
+}
+
+}  // namespace
+
+Maybe<bool> JSReceiver::TestIntegrityLevel(Handle<JSReceiver> object,
+                                           IntegrityLevel level) {
+  if (object->IsJSObject()) {

[Toon Verwaest, 2017/06/20 08:43:23]

object->instance_type() > LAST_CUSTOM_ELEMENTS_RECEIVER

This makes sure that objects with indexed and named interceptors are dealt with
generically (e.g., localStorage); and we only take the fast path in case we only
need to deal with regular-type properties are you're doing below.

+    return JSObject::TestIntegrityLevel(Handle<JSObject>::cast(object), level);
+  }
+
+  DCHECK(level == SEALED || level == FROZEN);
+
+  Maybe<bool> extensible = JSReceiver::IsExtensible(object);
+  MAYBE_RETURN(extensible, Nothing<bool>());
+  if (extensible.FromJust()) return Just(false);
+
+  return GenericTestDescriptorsIntegrity(object, level);
+}
+
+Maybe<bool> JSObject::TestIntegrityLevel(Handle<JSObject> object,
+                                         IntegrityLevel level) {
+  DCHECK(level == SEALED || level == FROZEN);
+
+  Isolate* isolate = object->GetIsolate();
+  if (object->IsAccessCheckNeeded() &&

[Toon Verwaest, 2017/06/20 08:43:23]

If you only support > LAST_CUSTOM_ELEMENTS_RECEIVER as described above, this
shouldn't be necessary.

I presume the performance of Object.freeze(window) isn't of vital importance ...
:)

+      !isolate->MayAccess(handle(isolate->context()), object)) {
+    return Just(false);
+  }
+
+  if (object->IsJSGlobalProxy()) {

[Toon Verwaest, 2017/06/20 08:43:23]

And in that case you can also drop this.

+    PrototypeIterator iter(isolate, object);
+    if (iter.IsAtEnd()) return Just(true);
+    DCHECK(PrototypeIterator::GetCurrent(iter)->IsJSGlobalObject());
+    return TestIntegrityLevel(PrototypeIterator::GetCurrent<JSObject>(iter),
+                              level);
+  }
+
+  if (object->map()->is_extensible()) return Just(false);
+
+  if (object->HasSloppyArgumentsElements()) {
+    return GenericTestDescriptorsIntegrity(Handle<JSReceiver>::cast(object),
+                                           level);
+  }
+
+  if (!TestElementsIntegrityLevel(*object, level)) {

[Toon Verwaest, 2017/06/20 08:43:23]

return Just(!object->map()->is_extensible() && 
            TestElementsIntegrityLevel(*object, level) &&
            TestPropertiesIntegrityLevel(*object, level));

+    return Just(false);
+  }
+
+  if (!TestPropertiesIntegrityLevel(*object, level)) {
+    return Just(false);
+  }
+
+  return Just(true);
+}
 
 Maybe<bool> JSReceiver::PreventExtensions(Handle<JSReceiver> object,
                                           ShouldThrow should_throw) {
   if (object->IsJSProxy()) {
     return JSProxy::PreventExtensions(Handle<JSProxy>::cast(object),
                                       should_throw);
   }
   DCHECK(object->IsJSObject());
   return JSObject::PreventExtensions(Handle<JSObject>::cast(object),
                                      should_throw);
@@ skipping 12651 matching lines @@
       // not
       // depend on this.
       return DICTIONARY_ELEMENTS;
     }
     DCHECK_LE(kind, LAST_ELEMENTS_KIND);
     return kind;
   }
 }
 }  // namespace internal
 }  // namespace v8