Reviewers: danno,

Message:
PTAL.

The meat of the change is adding SetLastAdded(kNoneAdded) to
src/objects.cc:4817. This ensures that, in addition to CopyNormalized,
CopyDropDescriptors is also initialized to kNoneAdded (no descriptors).

Without that line, the added assertion in src/heap.cc:4162 failed. That line is
now a bit superfluous, so I can remove it again if wanted.

Description:
Set LastAdded to kNoneAdded in RawCopy.

This ensures it is properly initialized if no descriptors are later set.


Please review this at https://chromiumcodereview.appspot.com/10833033/

SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge

Affected files:
  M src/heap.cc
  M src/objects-inl.h
  M src/objects.cc


Index: src/heap.cc
diff --git a/src/heap.cc b/src/heap.cc
index b038c77e6c356fab3fdee26507aecc618c384d54..69529ad6de97de7d6d3912603b44c372280a8c59 100644
--- a/src/heap.cc
+++ b/src/heap.cc
@@ -4125,13 +4125,11 @@ MaybeObject* Heap::AllocateGlobalObject(JSFunction* constructor) { int initial_size = map->instance_type() == JS_GLOBAL_OBJECT_TYPE ? 64 : 512;

   // Allocate a dictionary object for backing storage.
-  Object* obj;
-  { MaybeObject* maybe_obj =
-        StringDictionary::Allocate(
-            map->NumberOfDescribedProperties() * 2 + initial_size);
-    if (!maybe_obj->ToObject(&obj)) return maybe_obj;
-  }
-  StringDictionary* dictionary = StringDictionary::cast(obj);
+  StringDictionary* dictionary;
+  MaybeObject* maybe_dictionary =
+      StringDictionary::Allocate(
+          map->NumberOfDescribedProperties() * 2 + initial_size);
+  if (!maybe_dictionary->To(&dictionary)) return maybe_dictionary;

// The global object might be created from an object template with accessors.
   // Fill these accessors into the dictionary.
@@ -4142,29 +4140,26 @@ MaybeObject* Heap::AllocateGlobalObject(JSFunction* constructor) {
     PropertyDetails d =
         PropertyDetails(details.attributes(), CALLBACKS, details.index());
     Object* value = descs->GetCallbacksObject(i);
-    { MaybeObject* maybe_value = AllocateJSGlobalPropertyCell(value);
-      if (!maybe_value->ToObject(&value)) return maybe_value;
-    }
+    MaybeObject* maybe_value = AllocateJSGlobalPropertyCell(value);
+    if (!maybe_value->ToObject(&value)) return maybe_value;

-    Object* result;
- { MaybeObject* maybe_result = dictionary->Add(descs->GetKey(i), value, d);
-      if (!maybe_result->ToObject(&result)) return maybe_result;
-    }
-    dictionary = StringDictionary::cast(result);
+    MaybeObject* maybe_added = dictionary->Add(descs->GetKey(i), value, d);
+    if (!maybe_added->To(&dictionary)) return maybe_added;
   }

   // Allocate the global object and initialize it with the backing store.
-  { MaybeObject* maybe_obj = Allocate(map, OLD_POINTER_SPACE);
-    if (!maybe_obj->ToObject(&obj)) return maybe_obj;
-  }
-  JSObject* global = JSObject::cast(obj);
+  JSObject* global;
+  MaybeObject* maybe_global = Allocate(map, OLD_POINTER_SPACE);
+  if (!maybe_global->To(&global)) return maybe_global;
+
   InitializeJSObjectFromMap(global, dictionary, map);

   // Create a new map for the global object.
   Map* new_map;
-  { MaybeObject* maybe_map = map->CopyDropDescriptors();
-    if (!maybe_map->To(&new_map)) return maybe_map;
-  }
+  MaybeObject* maybe_map = map->CopyDropDescriptors();
+  if (!maybe_map->To(&new_map)) return maybe_map;
+
+  ASSERT(new_map->LastAdded() == Map::kNoneAdded);

   // Set up the global object as a normalized object.
   global->set_map(new_map);
Index: src/objects-inl.h
diff --git a/src/objects-inl.h b/src/objects-inl.h
index 8b22755c8230211278b9874ce97415cdb4fcf9f2..e47b44b3833aa88859c26aa5ed0afaa0e172f8f8 100644
--- a/src/objects-inl.h
+++ b/src/objects-inl.h
@@ -3523,7 +3523,7 @@ void Map::InitializeDescriptors(DescriptorArray* descriptors) {
     }
   }

-  ASSERT(len == 0 ||
+  ASSERT((len == 0 && LastAdded() == kNoneAdded) ||
          len == descriptors->GetDetails(LastAdded()).index());
 }

Index: src/objects.cc
diff --git a/src/objects.cc b/src/objects.cc
index fe94a42c4ad9fcb709c9c2af86b0a15fb34cf32f..57f416514333a65d65752c4ea1a8698fd8da3301 100644
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -4805,16 +4805,16 @@ Object* JSObject::SlowReverseLookup(Object* value) {

 MaybeObject* Map::RawCopy(int instance_size) {
   Map* result;
-  { MaybeObject* maybe_result =
-        GetHeap()->AllocateMap(instance_type(), instance_size);
-    if (!maybe_result->To(&result)) return maybe_result;
-  }
+  MaybeObject* maybe_result =
+      GetHeap()->AllocateMap(instance_type(), instance_size);
+  if (!maybe_result->To(&result)) return maybe_result;

   result->set_prototype(prototype());
   result->set_constructor(constructor());
   result->set_bit_field(bit_field());
   result->set_bit_field2(bit_field2());
   result->set_bit_field3(bit_field3());
+  result->SetLastAdded(kNoneAdded);
   return result;
 }

@@ -4834,12 +4834,11 @@ MaybeObject* Map::CopyNormalized(PropertyNormalizationMode mode,
     result->set_inobject_properties(inobject_properties());
   }

-  result->SetLastAdded(kNoneAdded);
   result->set_code_cache(code_cache());
   result->set_is_shared(sharing == SHARED_NORMALIZED_MAP);

 #ifdef DEBUG
-  if (FLAG_verify_heap && Map::cast(result)->is_shared()) {
+  if (FLAG_verify_heap && result->is_shared()) {
     result->SharedMapVerify();
   }
 #endif
@@ -4938,9 +4937,7 @@ MaybeObject* Map::CopyWithPreallocatedFieldDescriptors() {
       initial_descriptors->Copy(DescriptorArray::MAY_BE_SHARED);
   if (!maybe_descriptors->To(&descriptors)) return maybe_descriptors;

-  int last_added = initial_descriptors->IsEmpty()
-      ? kNoneAdded
-      : initial_map->LastAdded();
+  int last_added = initial_map->LastAdded();

return CopyReplaceDescriptors(descriptors, NULL, last_added, OMIT_TRANSITION);
 }
@@ -4952,7 +4949,7 @@ MaybeObject* Map::Copy(DescriptorArray::SharedMode shared_mode) {
   MaybeObject* maybe_descriptors = source_descriptors->Copy(shared_mode);
   if (!maybe_descriptors->To(&descriptors)) return maybe_descriptors;

- int last_added = source_descriptors->IsEmpty() ? kNoneAdded : LastAdded();
+  int last_added = LastAdded();

return CopyReplaceDescriptors(descriptors, NULL, last_added, OMIT_TRANSITION);
 }


--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to