Fix corner-case behavior of JSObject::SetPrototype.

src/objects.cc

 // Copyright 2013 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 <sstream>
 
@@ skipping 13921 matching lines @@
   // setPrototypeOf.
   if (from_javascript && object->map()->is_strong()) {
     RETURN_FAILURE(isolate, should_throw,
                    NewTypeError(MessageTemplate::kStrongSetProto, object));
   }
   Heap* heap = isolate->heap();
   // Silently ignore the change if value is not a JSObject or null.
   // SpiderMonkey behaves this way.
   if (!value->IsJSReceiver() && !value->IsNull()) return Just(true);
 
+  bool dictionary_elements_in_chain =
+      object->map()->DictionaryElementsInPrototypeChainOnly();
+
+  bool all_extensible = object->map()->is_extensible();
+  Handle<JSObject> real_receiver = object;
+  if (from_javascript) {
+    // Find the first object in the chain whose prototype object is not
+    // hidden.
+    PrototypeIterator iter(isolate, real_receiver);
+    while (!iter.IsAtEnd(PrototypeIterator::END_AT_NON_HIDDEN)) {
+      real_receiver = PrototypeIterator::GetCurrent<JSObject>(iter);

[Toon Verwaest, 2015/10/22 13:21:07]

This code is a bit overkill, since this is only relevant for JSGlobalObjects
behind JSGlobalProxies, but I guess it's ok for now.
Real receiver is quite a bad name though...

+      iter.Advance();
+      all_extensible = all_extensible && real_receiver->map()->is_extensible();
+    }
+  }
+  Handle<Map> map(real_receiver->map());

[Toon Verwaest, 2015/10/22 13:21:07]

Slight overkill to allocate a handle to just dereference it once below...

[neis, 2015/10/23 14:26:54]

It's used again further down.

+
+  // Nothing to do if prototype is already set.
+  if (map->prototype() == *value) return Just(true);
+
   // From 8.6.2 Object Internal Methods
   // ...
   // In addition, if [[Extensible]] is false the value of the [[Class]] and
   // [[Prototype]] internal properties of the object may not be modified.
   // ...
   // Implementation specific extensions that modify [[Class]], [[Prototype]]
   // or [[Extensible]] must not violate the invariants defined in the preceding
   // paragraph.
-  if (!object->map()->is_extensible()) {
+  if (!all_extensible) {
     RETURN_FAILURE(isolate, should_throw,
                    NewTypeError(MessageTemplate::kNonExtensibleProto, object));
-    // TODO(neis): Don't fail if new and old prototype happen to be the same.
   }
 
-  // Before we can set the prototype we need to be sure
-  // prototype cycles are prevented.
-  // It is sufficient to validate that the receiver is not in the new prototype
-  // chain.
+  // Before we can set the prototype we need to be sure prototype cycles are
+  // prevented.  It is sufficient to validate that the receiver is not in the
+  // new prototype chain.
   for (PrototypeIterator iter(isolate, *value,
                               PrototypeIterator::START_AT_RECEIVER);
        !iter.IsAtEnd(); iter.Advance()) {
     if (iter.GetCurrent<JSReceiver>() == *object) {
       // Cycle detected.
       RETURN_FAILURE(isolate, should_throw,
                      NewTypeError(MessageTemplate::kCyclicProto));
     }
   }
 
-  bool dictionary_elements_in_chain =
-      object->map()->DictionaryElementsInPrototypeChainOnly();
-  Handle<JSObject> real_receiver = object;
-
-  if (from_javascript) {
-    // Find the first object in the chain whose prototype object is not
-    // hidden and set the new prototype on that object.
-    PrototypeIterator iter(isolate, real_receiver);
-    while (!iter.IsAtEnd(PrototypeIterator::END_AT_NON_HIDDEN)) {
-      real_receiver = PrototypeIterator::GetCurrent<JSObject>(iter);
-      iter.Advance();
-      if (!real_receiver->map()->is_extensible()) {
-        RETURN_FAILURE(
-            isolate, should_throw,
-            NewTypeError(MessageTemplate::kNonExtensibleProto, object));
-      }
-    }
-  }
-
   // Set the new prototype of the object.
-  Handle<Map> map(real_receiver->map());
-
-  // Nothing to do if prototype is already set.
-  if (map->prototype() == *value) return Just(true);
 
   isolate->UpdateArrayProtectorOnSetPrototype(real_receiver);
 
   PrototypeOptimizationMode mode =
       from_javascript ? REGULAR_PROTOTYPE : FAST_PROTOTYPE;
   Handle<Map> new_map = Map::TransitionToPrototype(map, value, mode);
   DCHECK(new_map->prototype() == *value);
   JSObject::MigrateToMap(real_receiver, new_map);
 
   if (from_javascript && !dictionary_elements_in_chain &&
@@ skipping 3806 matching lines @@
   if (cell->value() != *new_value) {
     cell->set_value(*new_value);
     Isolate* isolate = cell->GetIsolate();
     cell->dependent_code()->DeoptimizeDependentCodeGroup(
         isolate, DependentCode::kPropertyCellChangedGroup);
   }
 }
 
 }  // namespace internal
 }  // namespace v8