Fix Object.preventExtensions, .seal, .freeze on typed arrays

BUG=v8:4460
LOG=n

Review URL: https://codereview.chromium.org/1419823008

Cr-Commit-Position: refs/heads/master@{#31556}

src/messages.h

@@ -95,8 +95,8 @@ class CallSite {
   T(CalledOnNullOrUndefined, "% called on null or undefined")                  \
   T(CannotConvertToPrimitive, "Cannot convert object to primitive value")      \
   T(CannotPreventExt, "Cannot prevent extensions")                             \
-  T(CannotPreventExtExternalArray,                                             \
-    "Cannot prevent extension of an object with external array elements")      \
+  T(CannotFreezeArrayBufferView,                                               \
+    "Cannot freeze array buffer views with elements")                          \
   T(CircularStructure, "Converting circular structure to JSON")                \
   T(ConstAssign, "Assignment to constant variable.")                           \
   T(ConstructorNonCallable,                                                    \

src/objects.cc

@@ -6794,20 +6794,16 @@ Maybe<bool> JSObject::PreventExtensions(Handle<JSObject> object,
                              should_throw);
   }
 
-  // It's not possible to seal objects with external array elements
-  if (object->HasFixedTypedArrayElements()) {
-    isolate->Throw(*isolate->factory()->NewTypeError(
-        MessageTemplate::kCannotPreventExtExternalArray));
-    return Nothing<bool>();
+  if (!object->HasFixedTypedArrayElements()) {
+    // If there are fast elements we normalize.
+    Handle<SeededNumberDictionary> dictionary = NormalizeElements(object);
+    DCHECK(object->HasDictionaryElements() ||
+           object->HasSlowArgumentsElements());
+
+    // Make sure that we never go back to fast case.
+    object->RequireSlowElements(*dictionary);
   }
 
-  // If there are fast elements we normalize.
-  Handle<SeededNumberDictionary> dictionary = NormalizeElements(object);
-  DCHECK(object->HasDictionaryElements() || object->HasSlowArgumentsElements());
-
-  // Make sure that we never go back to fast case.
-  object->RequireSlowElements(*dictionary);
-
   // Do a map transition, other objects with this map may still
   // be extensible.
   // TODO(adamk): Extend the NormalizedMapCache to handle non-extensible maps.
@@ -6895,15 +6891,9 @@ Maybe<bool> JSObject::PreventExtensionsWithTransition(
         PrototypeIterator::GetCurrent<JSObject>(iter), should_throw);
   }
 
-  // It's not possible to seal or freeze objects with external array elements
-  if (object->HasFixedTypedArrayElements()) {
-    isolate->Throw(*isolate->factory()->NewTypeError(
-        MessageTemplate::kCannotPreventExtExternalArray));
-    return Nothing<bool>();
-  }
-
   Handle<SeededNumberDictionary> new_element_dictionary;
-  if (!object->HasDictionaryElements()) {
+  if (!object->HasFixedTypedArrayElements() &&
+      !object->HasDictionaryElements()) {
     int length =
         object->IsJSArray()
             ? Smi::cast(Handle<JSArray>::cast(object)->length())->value()
@@ -6929,7 +6919,8 @@ Maybe<bool> JSObject::PreventExtensionsWithTransition(
       TransitionArray::SearchSpecial(*old_map, *transition_marker);
   if (transition != NULL) {
     Handle<Map> transition_map(transition, isolate);
-    DCHECK(transition_map->has_dictionary_elements());
+    DCHECK(transition_map->has_dictionary_elements() ||
+           transition_map->has_fixed_typed_array_elements());
     DCHECK(!transition_map->is_extensible());
     JSObject::MigrateToMap(object, transition_map);
   } else if (TransitionArray::CanHaveMoreTransitions(old_map)) {
@@ -6948,7 +6939,9 @@ Maybe<bool> JSObject::PreventExtensionsWithTransition(
     Handle<Map> new_map =
         Map::Copy(handle(object->map()), "SlowCopyForPreventExtensions");
     new_map->set_is_extensible(false);
-    new_map->set_elements_kind(DICTIONARY_ELEMENTS);
+    if (!new_element_dictionary.is_null()) {
+      new_map->set_elements_kind(DICTIONARY_ELEMENTS);
+    }
     JSObject::MigrateToMap(object, new_map);
 
     if (attrs != NONE) {
@@ -6960,6 +6953,18 @@ Maybe<bool> JSObject::PreventExtensionsWithTransition(
     }
   }
 
+  // Both seal and preventExtensions always go through without modifications to
+  // typed array elements. Freeze works only if there are no actual elements.
+  if (object->HasFixedTypedArrayElements()) {
+    if (attrs == FROZEN &&
+        JSArrayBufferView::cast(*object)->byte_length()->Number() > 0) {
+      isolate->Throw(*isolate->factory()->NewTypeError(
+          MessageTemplate::kCannotFreezeArrayBufferView));
+      return Nothing<bool>();
+    }
+    return Just(true);
+  }
+
   DCHECK(object->map()->has_dictionary_elements());
   if (!new_element_dictionary.is_null()) {
     object->set_elements(*new_element_dictionary);
@@ -8520,7 +8525,9 @@ Handle<Map> Map::CopyForPreventExtensions(Handle<Map> map,
       map, new_desc, new_layout_descriptor, INSERT_TRANSITION,
       transition_marker, reason, SPECIAL_TRANSITION);
   new_map->set_is_extensible(false);
-  new_map->set_elements_kind(DICTIONARY_ELEMENTS);
+  if (!IsFixedTypedArrayElementsKind(map->elements_kind())) {
+    new_map->set_elements_kind(DICTIONARY_ELEMENTS);
+  }
   return new_map;
 }
 

test/mjsunit/compiler/regress-447567.js

@@ -2,9 +2,9 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-assertThrows(function() {
-  [0].every(function(){ Object.seal((new Int8Array())); });
-})
+assertThrows(function () {
+  Object.freeze(new Int8Array(1))
+});
 
 assertThrows(function() {
   "use strict";

test/mjsunit/messages.js

@@ -62,10 +62,10 @@ test(function() {
   Array.prototype.shift.call(null);
 }, "Array.prototype.shift called on null or undefined", TypeError);
 
-// kCannotPreventExtExternalArray
+// kCannotFreezeArrayBufferView
 test(function() {
-  Object.preventExtensions(new Uint16Array(1));
-}, "Cannot prevent extension of an object with external array elements", TypeError);
+  Object.freeze(new Uint16Array(1));
+}, "Cannot freeze array buffer views with elements", TypeError);
 
 // kConstAssign
 test(function() {

test/mjsunit/regress/regress-95920.js

@@ -28,31 +28,23 @@
 // Tests that objects with external arrays cannot be sealed or have their
 // properties redefined.
 
-(function() {
-  assertThrows(function() {
-    [0].every(function(){ Object.seal((new Int8Array(42))); });
-    assertUnreable();
-    }, TypeError)
-})();
+Object.preventExtensions(new Int8Array(42));
+Object.seal(new Int8Array(42));
 
-(function() {
-  assertThrows(function() {
-    [0].every(function(){ Object.freeze((new Int8Array(42))); });
-    assertUnreable();
-    }, TypeError)
-})();
+// No elements, so should succeed.
+Object.freeze(new Int8Array(0));
 
-(function() {
-  assertThrows(function() {
-    [0].every(function(){ Object.preventExtensions((new Int8Array(42))); });
-    assertUnreable();
-    }, TypeError)
-})();
+var o = new Int8Array(42);
+assertThrows(function() {
+  Object.freeze(o);
+  assertUnreable();
+  }, TypeError);
 
-(function() {
-  assertThrows(function() {
-      Object.defineProperty(new Int8Array(42), "1",
-                            { writable: false, value: "1" });
-      assertUnreable();
-    })
-})();
+// Freeze should still have managed to preventExtensions o.
+assertFalse(Object.isExtensible(o));
+
+assertThrows(function() {
+    Object.defineProperty(new Int8Array(42), "1",
+                          { writable: false, value: "1" });
+    assertUnreable();
+  });