[objects] Always initialize double fields (with hole NaN).

This removes a special case from JSObject::WriteToField() where we
didn't store anything in case of initializing a double field with
the uninitialized sentinel. Instead we now store the hole NaN pattern
there, as in other places. This makes it possible to do stricter
checking in the TurboFan frontend when it comes to detecting bit
patterns.

Drive-by-fix: Refactor the related code in MigrateFastToFast() to
make it easier to follow the control flow.

Bug: v8:9299
Change-Id: Ic35d05c69fbbb136d422d29ce6abf2b09ebe22a6
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1631606Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#61879}

src/objects/js-objects-inl.h

@@ -387,10 +387,6 @@ void JSObject::WriteToField(int descriptor, PropertyDetails details,
   DisallowHeapAllocation no_gc;
   FieldIndex index = FieldIndex::ForDescriptor(map(), descriptor);
   if (details.representation().IsDouble()) {
-    // Nothing more to be done.
-    if (value.IsUninitialized()) {
-      return;
-    }
     // Manipulating the signaling NaN used for the hole and uninitialized
     // double field sentinel in C++, e.g. with bit_cast or value()/set_value(),
     // will change its value on ia32 (the x87 stack is used to return values
@@ -398,6 +394,8 @@ void JSObject::WriteToField(int descriptor, PropertyDetails details,
     uint64_t bits;
     if (value.IsSmi()) {
       bits = bit_cast<uint64_t>(static_cast<double>(Smi::ToInt(value)));
+    } else if (value.IsUninitialized()) {
+      bits = kHoleNanInt64;
     } else {
       DCHECK(value.IsHeapNumber());
       bits = HeapNumber::cast(value).value_as_bits();

src/objects/js-objects.cc

@@ -2579,6 +2579,7 @@ void JSObject::NotifyMapChange(Handle<Map> old_map, Handle<Map> new_map,
 }
 
 namespace {
+
 // To migrate a fast instance to a fast map:
 // - First check whether the instance needs to be rewritten. If not, simply
 //   change the map.
@@ -2606,31 +2607,28 @@ void MigrateFastToFast(Handle<JSObject> object, Handle<Map> new_map) {
       return;
     }
 
+    // If the map adds a new kDescriptor property, simply set the map.
     PropertyDetails details = new_map->GetLastDescriptorDetails();
-    int target_index = details.field_index() - new_map->GetInObjectProperties();
-    int property_array_length = object->property_array().length();
-    bool have_space = old_map->UnusedPropertyFields() > 0 ||
-                      (details.location() == kField && target_index >= 0 &&
-                       property_array_length > target_index);
-    // Either new_map adds an kDescriptor property, or a kField property for
-    // which there is still space, and which does not require a mutable double
-    // box (an out-of-object double).
-    if (details.location() == kDescriptor ||
-        (have_space && ((FLAG_unbox_double_fields && target_index < 0) ||
-                        !details.representation().IsDouble()))) {
+    if (details.location() == kDescriptor) {
       object->synchronized_set_map(*new_map);
       return;
     }
 
-    // If there is still space in the object, we need to allocate a mutable
-    // double box.
-    if (have_space) {
-      FieldIndex index =
-          FieldIndex::ForDescriptor(*new_map, new_map->LastAdded());
-      DCHECK(details.representation().IsDouble());
-      DCHECK(!new_map->IsUnboxedDoubleField(index));
-      auto value = isolate->factory()->NewMutableHeapNumberWithHoleNaN();
-      object->RawFastPropertyAtPut(index, *value);
+    // Check if we still have space in the {object}, in which case we
+    // can also simply set the map (modulo a special case for mutable
+    // double boxes).
+    FieldIndex index =
+        FieldIndex::ForDescriptor(*new_map, new_map->LastAdded());
+    if (index.is_inobject() ||
+        index.outobject_array_index() < object->property_array().length()) {
+      // We still need to allocate MutableHeapNumbers for double fields
+      // if either double field unboxing is disabled or the double field
+      // is in the PropertyArray backing store (where we don't support
+      // double field unboxing).
+      if (index.is_double() && !new_map->IsUnboxedDoubleField(index)) {
+        auto value = isolate->factory()->NewMutableHeapNumberWithHoleNaN();
+        object->RawFastPropertyAtPut(index, *value);
+      }
       object->synchronized_set_map(*new_map);
       return;
     }
@@ -2651,8 +2649,8 @@ void MigrateFastToFast(Handle<JSObject> object, Handle<Map> new_map) {
     }
     DCHECK_EQ(kField, details.location());
     DCHECK_EQ(kData, details.kind());
-    DCHECK_GE(target_index, 0);  // Must be a backing store index.
-    new_storage->set(target_index, *value);
+    DCHECK(!index.is_inobject());  // Must be a backing store index.
+    new_storage->set(index.outobject_array_index(), *value);
 
     // From here on we cannot fail and we shouldn't GC anymore.
     DisallowHeapAllocation no_allocation;