Avoid sharing AccessorPairs during Genesis.

To test the upcoming changes for map sharing in the presence of accessors, it is
essential that we keep a few global invariants: The map tree should always stay
a tree and AccessorPairs should not be shared between different DescriptorArrays
and/or StringDictionaries. This CL adds a test method for the latter invariant
and makes some changes to the bootstrapping process to avoid such sharing.

Note that we can't enable the new test method permanently yet, because we
currently go back and forth between fast mode and slow mode when adding an
accessor and break this invariant temporarily. This will be handled in a
separate CL.

Review URL: https://chromiumcodereview.appspot.com/9417043

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@10744 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/bootstrapper.cc

@@ -172,6 +172,10 @@ class Genesis BASE_EMBEDDED {
   Handle<JSFunction> GetThrowTypeErrorFunction();
 
   void CreateStrictModeFunctionMaps(Handle<JSFunction> empty);
+
+  // Make the "arguments" and "caller" properties throw a TypeError on access.
+  void PoisonArgumentsAndCaller(Handle<Map> map);
+
   // Creates the global objects using the global and the template passed in
   // through the API.  We call this regardless of whether we are building a
   // context from scratch or using a deserialized one from the partial snapshot
@@ -256,14 +260,10 @@ class Genesis BASE_EMBEDDED {
 
   Handle<Map> CreateStrictModeFunctionMap(
       PrototypePropertyMode prototype_mode,
-      Handle<JSFunction> empty_function,
-      Handle<AccessorPair> arguments_callbacks,
-      Handle<AccessorPair> caller_callbacks);
+      Handle<JSFunction> empty_function);
 
   Handle<DescriptorArray> ComputeStrictFunctionInstanceDescriptor(
-      PrototypePropertyMode propertyMode,
-      Handle<AccessorPair> arguments,
-      Handle<AccessorPair> caller);
+      PrototypePropertyMode propertyMode);
 
   static bool CompileBuiltin(Isolate* isolate, int index);
   static bool CompileExperimentalBuiltin(Isolate* isolate, int index);
@@ -384,44 +384,40 @@ static Handle<JSFunction> InstallFunction(Handle<JSObject> target,
 
 Handle<DescriptorArray> Genesis::ComputeFunctionInstanceDescriptor(
     PrototypePropertyMode prototypeMode) {
-  Handle<DescriptorArray> descriptors =
-      factory()->NewDescriptorArray(prototypeMode == DONT_ADD_PROTOTYPE
-                                    ? 4
-                                    : 5);
-  PropertyAttributes attributes =
-      static_cast<PropertyAttributes>(DONT_ENUM | DONT_DELETE | READ_ONLY);
+  int size = (prototypeMode == DONT_ADD_PROTOTYPE) ? 4 : 5;
+  Handle<DescriptorArray> descriptors(factory()->NewDescriptorArray(size));
+  PropertyAttributes attribs = static_cast<PropertyAttributes>(
+      DONT_ENUM | DONT_DELETE | READ_ONLY);
 
   DescriptorArray::WhitenessWitness witness(*descriptors);
 
   {  // Add length.
-    Handle<Foreign> foreign = factory()->NewForeign(&Accessors::FunctionLength);
-    CallbacksDescriptor d(*factory()->length_symbol(), *foreign, attributes);
+    Handle<Foreign> f(factory()->NewForeign(&Accessors::FunctionLength));
+    CallbacksDescriptor d(*factory()->length_symbol(), *f, attribs);
     descriptors->Set(0, &d, witness);
   }
   {  // Add name.
-    Handle<Foreign> foreign = factory()->NewForeign(&Accessors::FunctionName);
-    CallbacksDescriptor d(*factory()->name_symbol(), *foreign, attributes);
+    Handle<Foreign> f(factory()->NewForeign(&Accessors::FunctionName));
+    CallbacksDescriptor d(*factory()->name_symbol(), *f, attribs);
     descriptors->Set(1, &d, witness);
   }
   {  // Add arguments.
-    Handle<Foreign> foreign =
-        factory()->NewForeign(&Accessors::FunctionArguments);
-    CallbacksDescriptor d(*factory()->arguments_symbol(), *foreign, attributes);
+    Handle<Foreign> f(factory()->NewForeign(&Accessors::FunctionArguments));
+    CallbacksDescriptor d(*factory()->arguments_symbol(), *f, attribs);
     descriptors->Set(2, &d, witness);
   }
   {  // Add caller.
-    Handle<Foreign> foreign = factory()->NewForeign(&Accessors::FunctionCaller);
-    CallbacksDescriptor d(*factory()->caller_symbol(), *foreign, attributes);
+    Handle<Foreign> f(factory()->NewForeign(&Accessors::FunctionCaller));
+    CallbacksDescriptor d(*factory()->caller_symbol(), *f, attribs);
     descriptors->Set(3, &d, witness);
   }
   if (prototypeMode != DONT_ADD_PROTOTYPE) {
     // Add prototype.
     if (prototypeMode == ADD_WRITEABLE_PROTOTYPE) {
-      attributes = static_cast<PropertyAttributes>(attributes & ~READ_ONLY);
+      attribs = static_cast<PropertyAttributes>(attribs & ~READ_ONLY);
     }
-    Handle<Foreign> foreign =
-        factory()->NewForeign(&Accessors::FunctionPrototype);
-    CallbacksDescriptor d(*factory()->prototype_symbol(), *foreign, attributes);
+    Handle<Foreign> f(factory()->NewForeign(&Accessors::FunctionPrototype));
+    CallbacksDescriptor d(*factory()->prototype_symbol(), *f, attribs);
     descriptors->Set(4, &d, witness);
   }
   descriptors->Sort(witness);
@@ -532,47 +528,42 @@ Handle<JSFunction> Genesis::CreateEmptyFunction(Isolate* isolate) {
 
 
 Handle<DescriptorArray> Genesis::ComputeStrictFunctionInstanceDescriptor(
-    PrototypePropertyMode prototypeMode,
-    Handle<AccessorPair> arguments,
-    Handle<AccessorPair> caller) {
-  Handle<DescriptorArray> descriptors =
-      factory()->NewDescriptorArray(prototypeMode == DONT_ADD_PROTOTYPE
-                                    ? 4
-                                    : 5);
-  PropertyAttributes attributes = static_cast<PropertyAttributes>(
+    PrototypePropertyMode prototypeMode) {
+  int size = (prototypeMode == DONT_ADD_PROTOTYPE) ? 4 : 5;
+  Handle<DescriptorArray> descriptors(factory()->NewDescriptorArray(size));
+  PropertyAttributes attribs = static_cast<PropertyAttributes>(
       DONT_ENUM | DONT_DELETE);
 
   DescriptorArray::WhitenessWitness witness(*descriptors);
 
-  {  // length
-    Handle<Foreign> foreign = factory()->NewForeign(&Accessors::FunctionLength);
-    CallbacksDescriptor d(*factory()->length_symbol(), *foreign, attributes);
+  {  // Add length.
+    Handle<Foreign> f(factory()->NewForeign(&Accessors::FunctionLength));
+    CallbacksDescriptor d(*factory()->length_symbol(), *f, attribs);
     descriptors->Set(0, &d, witness);
   }
-  {  // name
-    Handle<Foreign> foreign = factory()->NewForeign(&Accessors::FunctionName);
-    CallbacksDescriptor d(*factory()->name_symbol(), *foreign, attributes);
+  {  // Add name.
+    Handle<Foreign> f(factory()->NewForeign(&Accessors::FunctionName));
+    CallbacksDescriptor d(*factory()->name_symbol(), *f, attribs);
     descriptors->Set(1, &d, witness);
   }
-  {  // arguments
-    CallbacksDescriptor d(*factory()->arguments_symbol(),
-                          *arguments,
-                          attributes);
+  {  // Add arguments.
+    Handle<AccessorPair> arguments(factory()->NewAccessorPair());
+    CallbacksDescriptor d(*factory()->arguments_symbol(), *arguments, attribs);
     descriptors->Set(2, &d, witness);
   }
-  {  // caller
-    CallbacksDescriptor d(*factory()->caller_symbol(), *caller, attributes);
+  {  // Add caller.
+    Handle<AccessorPair> caller(factory()->NewAccessorPair());
+    CallbacksDescriptor d(*factory()->caller_symbol(), *caller, attribs);
     descriptors->Set(3, &d, witness);
   }
 
-  // prototype
   if (prototypeMode != DONT_ADD_PROTOTYPE) {
+    // Add prototype.
     if (prototypeMode != ADD_WRITEABLE_PROTOTYPE) {
-      attributes = static_cast<PropertyAttributes>(attributes | READ_ONLY);
+      attribs = static_cast<PropertyAttributes>(attribs | READ_ONLY);
     }
-    Handle<Foreign> foreign =
-        factory()->NewForeign(&Accessors::FunctionPrototype);
-    CallbacksDescriptor d(*factory()->prototype_symbol(), *foreign, attributes);
+    Handle<Foreign> f(factory()->NewForeign(&Accessors::FunctionPrototype));
+    CallbacksDescriptor d(*factory()->prototype_symbol(), *f, attribs);
     descriptors->Set(4, &d, witness);
   }
 
@@ -603,14 +594,10 @@ Handle<JSFunction> Genesis::GetThrowTypeErrorFunction() {
 
 Handle<Map> Genesis::CreateStrictModeFunctionMap(
     PrototypePropertyMode prototype_mode,
-    Handle<JSFunction> empty_function,
-    Handle<AccessorPair> arguments_callbacks,
-    Handle<AccessorPair> caller_callbacks) {
+    Handle<JSFunction> empty_function) {
   Handle<Map> map = factory()->NewMap(JS_FUNCTION_TYPE, JSFunction::kSize);
   Handle<DescriptorArray> descriptors =
-      ComputeStrictFunctionInstanceDescriptor(prototype_mode,
-                                              arguments_callbacks,
-                                              caller_callbacks);
+      ComputeStrictFunctionInstanceDescriptor(prototype_mode);
   map->set_instance_descriptors(*descriptors);
   map->set_function_with_prototype(prototype_mode != DONT_ADD_PROTOTYPE);
   map->set_prototype(*empty_function);
@@ -619,23 +606,15 @@ Handle<Map> Genesis::CreateStrictModeFunctionMap(
 
 
 void Genesis::CreateStrictModeFunctionMaps(Handle<JSFunction> empty) {
-  // Create the callbacks arrays for ThrowTypeError functions.
-  // The get/set callacks are filled in after the maps are created below.
-  Factory* factory = empty->GetIsolate()->factory();
-  Handle<AccessorPair> arguments(factory->NewAccessorPair());
-  Handle<AccessorPair> caller(factory->NewAccessorPair());
-
   // Allocate map for the strict mode function instances.
   Handle<Map> strict_mode_function_instance_map =
-      CreateStrictModeFunctionMap(
-          ADD_WRITEABLE_PROTOTYPE, empty, arguments, caller);
+      CreateStrictModeFunctionMap(ADD_WRITEABLE_PROTOTYPE, empty);
   global_context()->set_strict_mode_function_instance_map(
       *strict_mode_function_instance_map);
 
   // Allocate map for the prototype-less strict mode instances.
   Handle<Map> strict_mode_function_without_prototype_map =
-      CreateStrictModeFunctionMap(
-          DONT_ADD_PROTOTYPE, empty, arguments, caller);
+      CreateStrictModeFunctionMap(DONT_ADD_PROTOTYPE, empty);
   global_context()->set_strict_mode_function_without_prototype_map(
       *strict_mode_function_without_prototype_map);
 
@@ -643,26 +622,38 @@ void Genesis::CreateStrictModeFunctionMaps(Handle<JSFunction> empty) {
   // only for processing of builtins.
   // Later the map is replaced with writable prototype map, allocated below.
   Handle<Map> strict_mode_function_map =
-      CreateStrictModeFunctionMap(
-          ADD_READONLY_PROTOTYPE, empty, arguments, caller);
+      CreateStrictModeFunctionMap(ADD_READONLY_PROTOTYPE, empty);
   global_context()->set_strict_mode_function_map(
       *strict_mode_function_map);
 
   // The final map for the strict mode functions. Writeable prototype.
   // This map is installed in MakeFunctionInstancePrototypeWritable.
   strict_mode_function_instance_map_writable_prototype_ =
-      CreateStrictModeFunctionMap(
-          ADD_WRITEABLE_PROTOTYPE, empty, arguments, caller);
-
-  // Create the ThrowTypeError function instance.
-  Handle<JSFunction> throw_function =
-      GetThrowTypeErrorFunction();
+      CreateStrictModeFunctionMap(ADD_WRITEABLE_PROTOTYPE, empty);
 
   // Complete the callbacks.
-  arguments->set_getter(*throw_function);
-  arguments->set_setter(*throw_function);
-  caller->set_getter(*throw_function);
-  caller->set_setter(*throw_function);
+  PoisonArgumentsAndCaller(strict_mode_function_instance_map);
+  PoisonArgumentsAndCaller(strict_mode_function_without_prototype_map);
+  PoisonArgumentsAndCaller(strict_mode_function_map);
+  PoisonArgumentsAndCaller(
+      strict_mode_function_instance_map_writable_prototype_);
+}
+
+
+static void SetAccessors(Handle<Map> map,
+                         Handle<String> name,
+                         Handle<JSFunction> func) {
+  DescriptorArray* descs = map->instance_descriptors();
+  int number = descs->Search(*name);
+  AccessorPair* accessors = AccessorPair::cast(descs->GetValue(number));
+  accessors->set_getter(*func);
+  accessors->set_setter(*func);
+}
+
+
+void Genesis::PoisonArgumentsAndCaller(Handle<Map> map) {
+  SetAccessors(map, factory()->arguments_symbol(), GetThrowTypeErrorFunction());
+  SetAccessors(map, factory()->caller_symbol(), GetThrowTypeErrorFunction());
 }
 
 

src/heap.cc

@@ -5062,8 +5062,39 @@ void Heap::Verify() {
   cell_space_->Verify(&no_dirty_regions_visitor);
 
   lo_space_->Verify();
+
+  // TODO(svenpanne) We should enable this when our fast->slow->fast-mode dance
+  // for setting accessor properties is fixed.
+  // VerifyNoAccessorPairSharing();
 }
 
+
+void Heap::VerifyNoAccessorPairSharing() {
+  // Verification is done in 2 phases: First we mark all AccessorPairs, checking
+  // that we mark only unmarked pairs, then we clear all marks, restoring the
+  // initial state. We use the Smi tag of the AccessorPair's getter as the
+  // marking bit, because we can never see a Smi as the getter.
+  for (int phase = 0; phase < 2; phase++) {
+    HeapObjectIterator iter(map_space());
+    for (HeapObject* obj = iter.Next(); obj != NULL; obj = iter.Next()) {
+      if (obj->IsMap()) {
+        DescriptorArray* descs = Map::cast(obj)->instance_descriptors();
+        for (int i = 0; i < descs->number_of_descriptors(); i++) {
+          if (descs->GetType(i) == CALLBACKS &&
+              descs->GetValue(i)->IsAccessorPair()) {
+            AccessorPair* accessors = AccessorPair::cast(descs->GetValue(i));
+            uintptr_t before = reinterpret_cast<intptr_t>(accessors->getter());
+            uintptr_t after = (phase == 0) ?
+                ((before & ~kSmiTagMask) | kSmiTag) :
+                ((before & ~kHeapObjectTag) | kHeapObjectTag);
+            CHECK(before != after);
+            accessors->set_getter(reinterpret_cast<Object*>(after));
+          }
+        }
+      }
+    }
+  }
+}
 #endif  // DEBUG
 
 

src/heap.h

@@ -1219,6 +1219,10 @@ class Heap {
   // Verify the heap is in its normal state before or after a GC.
   void Verify();
 
+  // Verify that AccessorPairs are not shared, i.e. make sure that they have
+  // exactly one pointer to them.
+  void VerifyNoAccessorPairSharing();
+
   void OldPointerSpaceCheckStoreBuffer();
   void MapSpaceCheckStoreBuffer();
   void LargeObjectSpaceCheckStoreBuffer();