Avoid sharing AccessorPairs during Genesis.

src/bootstrapper.cc

Index: src/bootstrapper.cc
diff --git a/src/bootstrapper.cc b/src/bootstrapper.cc
index ef53df46cd17141937fc916539b5a70cee08d04c..fe00700a82be1025c0c35bd7077270c4ad6d9bec 100644
--- a/src/bootstrapper.cc
+++ b/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 InitArgumentsAndCaller(Handle<Map> map);

[Michael Starzinger, 2012/02/17 13:33:28]

In ECMA-Script speak this method should better be called
PoisonArgumentsAndCaller().

[Sven Panne, 2012/02/20 08:26:03]

Done.

+
   // 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);
@@ -532,9 +532,7 @@ Handle<JSFunction> Genesis::CreateEmptyFunction(Isolate* isolate) {
 
 
 Handle<DescriptorArray> Genesis::ComputeStrictFunctionInstanceDescriptor(
-    PrototypePropertyMode prototypeMode,
-    Handle<AccessorPair> arguments,
-    Handle<AccessorPair> caller) {
+    PrototypePropertyMode prototypeMode) {
   Handle<DescriptorArray> descriptors =
       factory()->NewDescriptorArray(prototypeMode == DONT_ADD_PROTOTYPE
                                     ? 4
@@ -555,12 +553,14 @@ Handle<DescriptorArray> Genesis::ComputeStrictFunctionInstanceDescriptor(
     descriptors->Set(1, &d, witness);
   }
   {  // arguments
+    Handle<AccessorPair> arguments(factory()->NewAccessorPair());

[Michael Starzinger, 2012/02/17 13:33:28]

Could you rewrite this as using the assignment operator so that it is in sync
with the blocks above and below?

[Sven Panne, 2012/02/20 08:26:03]

As discussed offline, I'll do it the other way round. Furthermore, I'll sync
this with the non-strict version of this method.

[Kevin Millikin (Chromium), 2012/02/20 09:30:19]

The existing V8 style is to prefer '=' notation for constructors that are
(essentially) a copy (e.g., the pointer assignment Handle constructors) and
constructor call notation for ones that are doing some sort of allocation or
other work in the constructor itself (e.g., the HandleScope::CreateHandle Handle
constructors).

We've been pretty consistent about that, so you'll have to change a lot of sites
if you think it's worth changing.

     CallbacksDescriptor d(*factory()->arguments_symbol(),
                           *arguments,
                           attributes);
     descriptors->Set(2, &d, witness);
   }
   {  // caller
+    Handle<AccessorPair> caller(factory()->NewAccessorPair());

[Michael Starzinger, 2012/02/17 13:33:28]

Likewise.

[Sven Panne, 2012/02/20 08:26:03]

See above.

     CallbacksDescriptor d(*factory()->caller_symbol(), *caller, attributes);
     descriptors->Set(3, &d, witness);
   }
@@ -603,14 +603,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 +615,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 +631,41 @@ 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);
+      CreateStrictModeFunctionMap(ADD_WRITEABLE_PROTOTYPE, empty);
 
   // Create the ThrowTypeError function instance.
   Handle<JSFunction> throw_function =

[Michael Starzinger, 2012/02/17 13:33:28]

This is now unused, remove it.

[Sven Panne, 2012/02/20 08:26:03]

Done.

       GetThrowTypeErrorFunction();
 
   // Complete the callbacks.
-  arguments->set_getter(*throw_function);
-  arguments->set_setter(*throw_function);
-  caller->set_getter(*throw_function);
-  caller->set_setter(*throw_function);
+  InitArgumentsAndCaller(strict_mode_function_instance_map);
+  InitArgumentsAndCaller(strict_mode_function_without_prototype_map);
+  InitArgumentsAndCaller(strict_mode_function_map);
+  InitArgumentsAndCaller(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::InitArgumentsAndCaller(Handle<Map> map) {
+  SetAccessors(map, factory()->arguments_symbol(), GetThrowTypeErrorFunction());
+  SetAccessors(map, factory()->caller_symbol(), GetThrowTypeErrorFunction());
 }