Hydrogen object literals: always initialize in-object properties

This fixes a bug where new-space GC could be triggered by non-folded allocations for some of the in-object properties, while the object was only partially initialized.

BUG=chromium:500497
LOG=y
R=ishell@chromium.org

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

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

diff --git a/src/hydrogen.cc b/src/hydrogen.cc
index ad9655b..2a0011f 100644 (file)
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -9667,15 +9667,7 @@ void HOptimizedGraphBuilder::VisitCallNew(CallNew* expr) {
           HObjectAccess::ForMapAndOffset(initial_map,
                                          JSObject::kElementsOffset),
           empty_fixed_array);
-      if (initial_map->inobject_properties() != 0) {
-        HConstant* undefined = graph()->GetConstantUndefined();
-        for (int i = 0; i < initial_map->inobject_properties(); i++) {
-          int property_offset = initial_map->GetInObjectPropertyOffset(i);
-          Add<HStoreNamedField>(receiver,
-              HObjectAccess::ForMapAndOffset(initial_map, property_offset),
-              undefined);
-        }
-      }
+      BuildInitializeInobjectProperties(receiver, initial_map);
     }
 
     // Replace the constructor function with a newly allocated receiver using
@@ -9718,6 +9710,20 @@ void HOptimizedGraphBuilder::VisitCallNew(CallNew* expr) {
 }
 
 
+void HOptimizedGraphBuilder::BuildInitializeInobjectProperties(
+    HValue* receiver, Handle<Map> initial_map) {
+  if (initial_map->inobject_properties() != 0) {
+    HConstant* undefined = graph()->GetConstantUndefined();
+    for (int i = 0; i < initial_map->inobject_properties(); i++) {
+      int property_offset = initial_map->GetInObjectPropertyOffset(i);
+      Add<HStoreNamedField>(receiver, HObjectAccess::ForMapAndOffset(
+                                          initial_map, property_offset),
+                            undefined);
+    }
+  }
+}
+
+
 HValue* HGraphBuilder::BuildAllocateEmptyArrayBuffer(HValue* byte_length) {
   HAllocate* result =
       BuildAllocate(Add<HConstant>(JSArrayBuffer::kSizeWithInternalFields),
@@ -11302,13 +11308,13 @@ HInstruction* HOptimizedGraphBuilder::BuildFastLiteral(
     Handle<JSObject> boilerplate_object,
     AllocationSiteUsageContext* site_context) {
   NoObservableSideEffectsScope no_effects(this);
-  InstanceType instance_type = boilerplate_object->map()->instance_type();
+  Handle<Map> initial_map(boilerplate_object->map());
+  InstanceType instance_type = initial_map->instance_type();
   DCHECK(instance_type == JS_ARRAY_TYPE || instance_type == JS_OBJECT_TYPE);
 
   HType type = instance_type == JS_ARRAY_TYPE
       ? HType::JSArray() : HType::JSObject();
-  HValue* object_size_constant = Add<HConstant>(
-      boilerplate_object->map()->instance_size());
+  HValue* object_size_constant = Add<HConstant>(initial_map->instance_size());
 
   PretenureFlag pretenure_flag = NOT_TENURED;
   Handle<AllocationSite> current_site(*site_context->current(), isolate());
@@ -11333,6 +11339,11 @@ HInstruction* HOptimizedGraphBuilder::BuildFastLiteral(
 
   BuildEmitObjectHeader(boilerplate_object, object);
 
+  // Similarly to the elements pointer, there is no guarantee that all
+  // property allocations can get folded, so pre-initialize all in-object
+  // properties to a safe value.
+  BuildInitializeInobjectProperties(object, initial_map);
+
   Handle<FixedArrayBase> elements(boilerplate_object->elements());
   int elements_size = (elements->length() > 0 &&
       elements->map() != isolate()->heap()->fixed_cow_array_map()) ?
@@ -11371,8 +11382,8 @@ HInstruction* HOptimizedGraphBuilder::BuildFastLiteral(
   }
 
   // Copy in-object properties.
-  if (boilerplate_object->map()->NumberOfFields() != 0 ||
-      boilerplate_object->map()->unused_property_fields() > 0) {
+  if (initial_map->NumberOfFields() != 0 ||
+      initial_map->unused_property_fields() > 0) {
     BuildEmitInObjectProperties(boilerplate_object, object, site_context,
                                 pretenure_flag);
   }

diff --git a/src/hydrogen.h b/src/hydrogen.h
index ad98f7c..48e333f 100644 (file)
--- a/src/hydrogen.h
+++ b/src/hydrogen.h
@@ -2505,6 +2505,9 @@ class HOptimizedGraphBuilder : public HGraphBuilder, public AstVisitor {
   void BuildInlinedCallArray(Expression* expression, int argument_count,
                              Handle<AllocationSite> site);
 
+  void BuildInitializeInobjectProperties(HValue* receiver,
+                                         Handle<Map> initial_map);
+
   class PropertyAccessInfo {
    public:
     PropertyAccessInfo(HOptimizedGraphBuilder* builder,

diff --git a/src/runtime/runtime-test.cc b/src/runtime/runtime-test.cc
index 59b695c..1325eeb 100644 (file)
--- a/src/runtime/runtime-test.cc
+++ b/src/runtime/runtime-test.cc
@@ -180,7 +180,7 @@ RUNTIME_FUNCTION(Runtime_GetOptimizationStatus) {
       base::OS::Sleep(base::TimeDelta::FromMilliseconds(50));
     }
   }
-  if (FLAG_always_opt) {
+  if (FLAG_always_opt || FLAG_prepare_always_opt) {
     // With --always-opt, optimization status expectations might not
     // match up, so just return a sentinel.
     return Smi::FromInt(3);  // 3 == "always".

diff --git a/test/mjsunit/regress/regress-crbug-500497.js b/test/mjsunit/regress/regress-crbug-500497.js
new file mode 100644 (file)
index 0000000..9117440
--- /dev/null
+++ b/test/mjsunit/regress/regress-crbug-500497.js
@@ -0,0 +1,33 @@
+// Copyright 2015 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.
+
+// New space must be at max capacity to trigger pretenuring decision.
+// Flags: --allow-natives-syntax --verify-heap --max-semi-space-size=1
+
+var global = [];  // Used to keep some objects alive.
+
+function Ctor() {
+  var result = {a: {}, b: {}, c: {}, d: {}, e: {}, f: {}, g: {}};
+  return result;
+}
+
+for (var i = 0; i < 120; i++) {
+  // Make the "a" property long-lived, while everything else is short-lived.
+  global.push(Ctor().a);
+  (function FillNewSpace() { new Array(10000); })();
+}
+
+// The bad situation is only triggered if Ctor wasn't optimized too early.
+assertUnoptimized(Ctor);
+// Optimized code for Ctor will pretenure the "a" property, so it will have
+// three allocations:
+// #1 Allocate the "result" object in new-space.
+// #2 Allocate the object stored in the "a" property in old-space.
+// #3 Allocate the objects for the "b" through "g" properties in new-space.
+%OptimizeFunctionOnNextCall(Ctor);
+for (var i = 0; i < 10000; i++) {
+  // At least one of these calls will run out of new space. The bug is
+  // triggered when it is allocation #3 that triggers GC.
+  Ctor();
+}