[runtime] fix ClusterFuzz regressions (and remaining nits) in CloneObject

Includes fixes for several ClusterFuzz regressions:

1) fix an invalid Handle-cast in ic.cc (chromium:866282)

2) fix for improper accounting of used/unused inobject
fields, found by clusterfuzz (chromium:866357).

3) fix number of control outputs for the JSCloneObject
operator to be used by IfSuccess and IfException nodes (chromium:866727).

4) fix property constness in out-of-object properties of fast-cloned
object to be compatible with DCHECKs in StoreIC (chromium:866861).

Also includes the fixups missing from the initial commit, and
regression tests

BUG=v8:7611, chromium:866282, chromium:866357, chromium:866727, chromium:866861
R=jkummerow@chromium.org, mvstanton@chromium.org
TBR=rmcilroy@chromium.org

Change-Id: I77220308482f16db2893c0dcebec36530d0f5540
Reviewed-on: https://chromium-review.googlesource.com/1146297
Commit-Queue: Caitlin Potter <caitp@igalia.com>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Michael Stanton <mvstanton@chromium.org>
Cr-Commit-Position: refs/heads/master@{#54706}

src/compiler/js-operator.cc

@@ -1210,7 +1210,7 @@ const Operator* JSOperatorBuilder::CloneObject(VectorSlotPair const& feedback,
       IrOpcode::kJSCloneObject,                          // opcode
       Operator::kNoProperties,                           // properties
       "JSCloneObject",                                   // name
-      1, 1, 1, 1, 1, 1,                                  // counts
+      1, 1, 1, 1, 1, 2,                                  // counts
       parameters);                                       // parameter
 }
 

src/ic/ic.cc

@@ -2517,7 +2517,7 @@ static Handle<Map> FastCloneObjectMap(Isolate* isolate,
   Handle<LayoutDescriptor> layout =
       LayoutDescriptor::New(isolate, map, descriptors, size);
   map->InitializeDescriptors(*descriptors, *layout);
-  map->CopyUnusedPropertyFields(*source_map);
+  map->CopyUnusedPropertyFieldsAdjustedForInstanceSize(*source_map);
 
   return map;
 }
@@ -2571,7 +2571,7 @@ RUNTIME_FUNCTION(Runtime_CloneObjectIC_Miss) {
 RUNTIME_FUNCTION(Runtime_CloneObjectIC_Slow) {
   HandleScope scope(isolate);
   DCHECK_EQ(2, args.length());
-  Handle<JSReceiver> source = args.at<JSReceiver>(0);
+  Handle<HeapObject> source = args.at<HeapObject>(0);
   int flags = args.smi_at(1);
   RETURN_RESULT_OR_FAILURE(isolate,
                            CloneObjectSlowPath(isolate, source, flags));

src/interpreter/bytecode-generator.cc

@@ -2116,9 +2116,9 @@ void BytecodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
 
   // Create literal object.
   int property_index = 0;
-  bool is_spread =
+  bool clone_object_spread =
       expr->properties()->first()->kind() == ObjectLiteral::Property::SPREAD;
-  if (is_spread) {
+  if (clone_object_spread) {
     // Avoid the slow path for spreads in the following common cases:
     //   1) `let obj = { ...source }`
     //   2) `let obj = { ...source, override: 1 }`
@@ -2137,9 +2137,6 @@ void BytecodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
     builder()->CloneObject(from_value, flags, clone_index);
     builder()->StoreAccumulatorInRegister(literal);
     property_index++;
-
-    // FIXME: incorporate compile-time constants following the initial spread
-    // into the CloneObject opcode, to be included in the final value.
   } else {
     size_t entry;
     // If constant properties is an empty fixed array, use a cached empty fixed
@@ -2162,7 +2159,7 @@ void BytecodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
   for (; property_index < expr->properties()->length(); property_index++) {
     ObjectLiteral::Property* property = expr->properties()->at(property_index);
     if (property->is_computed_name()) break;
-    if (!is_spread && property->IsCompileTimeValue()) continue;
+    if (!clone_object_spread && property->IsCompileTimeValue()) continue;
 
     RegisterAllocationScope inner_register_scope(this);
     Literal* key = property->key()->AsLiteral();
@@ -2171,7 +2168,7 @@ void BytecodeGenerator::VisitObjectLiteral(ObjectLiteral* expr) {
         UNREACHABLE();
       case ObjectLiteral::Property::CONSTANT:
       case ObjectLiteral::Property::MATERIALIZED_LITERAL:
-        DCHECK(is_spread || !property->value()->IsCompileTimeValue());
+        DCHECK(clone_object_spread || !property->value()->IsCompileTimeValue());
         V8_FALLTHROUGH;
       case ObjectLiteral::Property::COMPUTED: {
         // It is safe to use [[Put]] here because the boilerplate already

src/objects.cc

@@ -10224,9 +10224,9 @@ Handle<DescriptorArray> DescriptorArray::CopyForFastObjectClone(
 
     // Ensure the ObjectClone property details are NONE, and that all source
     // details did not contain DONT_ENUM.
-    PropertyDetails new_details(
-        kData, NONE, details.location(), kDefaultFieldConstness,
-        details.representation(), details.field_index());
+    PropertyDetails new_details(kData, NONE, details.location(),
+                                details.constness(), details.representation(),
+                                details.field_index());
     descriptors->Set(i, key, src->GetValue(i), new_details);
   }
 

src/objects/map-inl.h

@@ -360,6 +360,17 @@ void Map::CopyUnusedPropertyFields(Map* map) {
   DCHECK_EQ(UnusedPropertyFields(), map->UnusedPropertyFields());
 }
 
+void Map::CopyUnusedPropertyFieldsAdjustedForInstanceSize(Map* map) {
+  int value = map->used_or_unused_instance_size_in_words();
+  if (value >= JSValue::kFieldsAdded) {
+    // Unused in-object fields. Adjust the offset from the object’s start
+    // so it matches the distance to the object’s end.
+    value += instance_size_in_words() - map->instance_size_in_words();
+  }
+  set_used_or_unused_instance_size_in_words(value);
+  DCHECK_EQ(UnusedPropertyFields(), map->UnusedPropertyFields());
+}
+
 void Map::AccountAddedPropertyField() {
   // Update used instance size and unused property fields number.
   STATIC_ASSERT(JSObject::kFieldsAdded == JSObject::kHeaderSize / kPointerSize);

src/objects/map.h

@@ -219,6 +219,7 @@ class Map : public HeapObject {
   // Updates the counters tracking unused fields in the property array.
   inline void SetOutOfObjectUnusedPropertyFields(int unused_property_fields);
   inline void CopyUnusedPropertyFields(Map* map);
+  inline void CopyUnusedPropertyFieldsAdjustedForInstanceSize(Map* map);
   inline void AccountAddedPropertyField();
   inline void AccountAddedOutOfObjectPropertyField(
       int unused_in_property_array);

test/mjsunit/es9/regress/regress-866282.js

@@ -0,0 +1,17 @@
+// Copyright 2018 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.
+
+// Runtime_ObjectCloneIC_Slow() source argument must be a HeapObject handle,
+// because undefined/null are allowed.
+function spread(o) { return { ...o }; }
+
+// Transition to MEGAMORPHIC
+assertEquals({}, spread(new function C1() {}));
+assertEquals({}, spread(new function C2() {}));
+assertEquals({}, spread(new function C3() {}));
+assertEquals({}, spread(new function C4() {}));
+assertEquals({}, spread(new function C5() {}));
+
+// Trigger Runtime_ObjectCloneIC_Slow() with a non-JSReceiver.
+assertEquals({}, spread(undefined));

test/mjsunit/es9/regress/regress-866357.js

@@ -0,0 +1,17 @@
+// Copyright 2018 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.
+
+// Flags: --allow-natives-syntax
+
+// Check that we do appropriate used/unused field accounting
+var p = Promise.resolve();
+var then = p.then = () => {};
+
+function spread() { return { ...p }; }
+
+assertEquals({ then }, spread());
+assertEquals({ then }, spread());
+assertEquals({ then }, spread());
+%OptimizeFunctionOnNextCall(spread);
+assertEquals({ then }, spread());

test/mjsunit/es9/regress/regress-866727.js

@@ -0,0 +1,19 @@
+// Copyright 2018 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.
+
+// Flags: --allow-natives-syntax
+
+// Check that IfException/IfSuccess rewiring works in JSInliner
+function test() {
+  var spread = function(value) { return { ...value }; }
+  try {
+    assertEquals({}, spread());
+  } catch (e) {}
+}
+
+test();
+test();
+test();
+%OptimizeFunctionOnNextCall(test);
+test();

test/mjsunit/es9/regress/regress-866861.js

@@ -0,0 +1,11 @@
+// Copyright 2018 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.
+
+// Check that property constness for out-of-object fields is valid
+var o = {};
+var toString = o.toString = function() {};
+try {
+assertEquals({ toString }, o = { ...o });
+} catch (e) {}
+o.toString = [];