[runtime] PreventExtensionsWithTransition: before adding the new

src/objects.cc

Index: src/objects.cc
diff --git a/src/objects.cc b/src/objects.cc
index 89d86b0fb25628119faa2a3a1434ac4fb0660b9f..3a7af494d71129b351f2da560df512cf649a360a 100644
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -7422,6 +7422,12 @@ Maybe<bool> JSReceiver::SetIntegrityLevel(Handle<JSReceiver> receiver,
 
   if (receiver->IsJSObject()) {
     Handle<JSObject> object = Handle<JSObject>::cast(receiver);
+
+    // prevent memory leaks by adding unnecessary transitions

[Toon Verwaest, 2017/06/13 09:57:05]

Prevent ... *not* adding ... transitions. :)

[kris.selden, 2017/06/16 22:32:12]

Done.

+    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,
@@ -7478,9 +7484,12 @@ Maybe<bool> JSReceiver::SetIntegrityLevel(Handle<JSReceiver> receiver,
   return Just(true);
 }
 
-
 Maybe<bool> JSReceiver::TestIntegrityLevel(Handle<JSReceiver> object,
                                            IntegrityLevel level) {
+  if (object->IsJSObject()) {
+    return JSObject::TestIntegrityLevel(Handle<JSObject>::cast(object), level);
+  }
+
   DCHECK(level == SEALED || level == FROZEN);
   Isolate* isolate = object->GetIsolate();
 
@@ -7510,6 +7519,60 @@ Maybe<bool> JSReceiver::TestIntegrityLevel(Handle<JSReceiver> object,
   return Just(true);
 }
 
+Maybe<bool> JSObject::TestIntegrityLevel(Handle<JSObject> object,
+                                         IntegrityLevel level) {
+  DCHECK(level == SEALED || level == FROZEN);
+
+  if (JSObject::IsExtensible(object)) return Just(false);
+
+  if (object->HasFastProperties() &&
+      IsFastObjectElementsKind(object->GetElementsKind())) {
+    ElementsAccessor* accessor = object->GetElementsAccessor();

[Toon Verwaest, 2017/06/13 09:57:06]

If we only support fast elements, finding any element makes it false.

[Toon Verwaest, 2017/06/16 10:03:02]

Come to think of it; if we ever followed the fast transition, we always end up
with slow elements. See CopyForPreventExtensions; it always sets the elements
kind to something slow (except typed arrays).

[kris.selden, 2017/06/16 22:32:12]

I think what I did addresses this but I need your review.

+    JSObject* rawobj = *object;
+    uint32_t number_of_elements = accessor->NumberOfElements(rawobj);
+    for (uint32_t i = 0; i < number_of_elements; i++) {
+      PropertyDetails details = accessor->GetDetails(rawobj, i);
+      if (details.IsConfigurable()) return Just(false);
+      if (level == FROZEN && details.kind() == kData && !details.IsReadOnly()) {
+        return Just(false);
+      }
+    }
+    Map* map = object->map();
+    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 Just(false);
+      if (level == FROZEN && details.kind() == kData && !details.IsReadOnly()) {
+        return Just(false);
+      }
+    }
+    return Just(true);
+  }
+
+  Handle<JSReceiver> receiver = Handle<JSReceiver>::cast(object);

[Toon Verwaest, 2017/06/13 09:57:05]

We definitely want to share this slow path with the code for JSReceiver above.

[kris.selden, 2017/06/16 22:32:12]

Done.

+  Isolate* isolate = object->GetIsolate();
+  Handle<FixedArray> keys;
+  ASSIGN_RETURN_ON_EXCEPTION_VALUE(
+      isolate, keys, JSObject::OwnPropertyKeys(receiver), 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, receiver, 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);
+}
 
 Maybe<bool> JSReceiver::PreventExtensions(Handle<JSReceiver> object,
                                           ShouldThrow should_throw) {