Poison .arguments and .caller for generator functions

src/bootstrapper.cc

Index: src/bootstrapper.cc
diff --git a/src/bootstrapper.cc b/src/bootstrapper.cc
index 4bb0f4d0475ea21f6fb4a9cb0b611811800e068b..4b770897d05989cb409aba3a7d0bd0763ed2c55a 100644
--- a/src/bootstrapper.cc
+++ b/src/bootstrapper.cc
@@ -168,6 +168,8 @@ class Genesis BASE_EMBEDDED {
   Handle<JSFunction> CreateEmptyFunction(Isolate* isolate);
   // Creates the ThrowTypeError function. ECMA 5th Ed. 13.2.3
   Handle<JSFunction> GetThrowTypeErrorFunction();
+  // Poison for sloppy generator function arguments/callee.
+  Handle<JSFunction> GetGeneratorFunctionPoison();
 
   void CreateStrictModeFunctionMaps(Handle<JSFunction> empty);
 
@@ -303,6 +305,7 @@ class Genesis BASE_EMBEDDED {
   Handle<Map> sloppy_function_map_writable_prototype_;
   Handle<Map> strict_function_map_writable_prototype_;
   Handle<JSFunction> throw_type_error_function;

[rossberg, 2014/05/07 09:19:54]

Nit: can we somehow rename this to make it more consistent with the new one?

+  Handle<JSFunction> generator_function_poison;
 
   BootstrapperActive active_;
   friend class Bootstrapper;
@@ -585,6 +588,22 @@ Handle<JSFunction> Genesis::GetThrowTypeErrorFunction() {
 }
 
 
+Handle<JSFunction> Genesis::GetGeneratorFunctionPoison() {
+  if (generator_function_poison.is_null()) {
+    Handle<String> name = factory()->InternalizeOneByteString(
+        STATIC_ASCII_VECTOR("ThrowTypeError"));
+    Handle<Code> code(isolate()->builtins()->builtin(
+        Builtins::kGeneratorPoisonPill));
+    generator_function_poison = factory()->NewFunction(name, code);
+    generator_function_poison->set_map(native_context()->sloppy_function_map());
+    generator_function_poison->shared()->DontAdaptArguments();
+
+    JSObject::PreventExtensions(generator_function_poison).Assert();
+  }
+  return generator_function_poison;
+}
+
+
 Handle<Map> Genesis::CreateStrictFunctionMap(
     PrototypePropertyMode prototype_mode,
     Handle<JSFunction> empty_function) {
@@ -633,6 +652,17 @@ static void SetAccessors(Handle<Map> map,
 }
 
 
+static void ReplaceAccessors(Handle<Map> map,
+                             Handle<String> name,
+                             PropertyAttributes attributes,
+                             Handle<AccessorPair> accessor_pair) {
+  DescriptorArray* descriptors = map->instance_descriptors();
+  int idx = descriptors->SearchWithCache(*name, *map);
+  CallbacksDescriptor descriptor(name, accessor_pair, attributes);
+  descriptors->Replace(idx, &descriptor);
+}
+
+
 void Genesis::PoisonArgumentsAndCaller(Handle<Map> map) {
   SetAccessors(map, factory()->arguments_string(), GetThrowTypeErrorFunction());
   SetAccessors(map, factory()->caller_string(), GetThrowTypeErrorFunction());
@@ -1366,16 +1396,40 @@ void Genesis::InitializeExperimentalGlobal() {
 
     // Create maps for generator functions and their prototypes.  Store those
     // maps in the native context.
-    Handle<Map> function_map(native_context()->sloppy_function_map());
-    Handle<Map> generator_function_map = Map::Copy(function_map);
+    Handle<Map> sloppy_function_map(native_context()->sloppy_function_map());
+    Handle<Map> generator_function_map = Map::Copy(sloppy_function_map);
     generator_function_map->set_prototype(*generator_function_prototype);
     native_context()->set_sloppy_generator_function_map(
         *generator_function_map);
 
-    Handle<Map> strict_mode_function_map(
-        native_context()->strict_function_map());
+    // The "arguments" and "caller" instance properties aren't specified, so
+    // technically we could leave them out.  They make even less sense for
+    // generators than for functions.  Still, the same argument that it makes
+    // sense to keep them around but poisoned in strict mode applies to
+    // generators as well.  With poisoned accessors, naive callers can still
+    // iterate over the properties without accessing them.
+    //
+    // We can't use PoisonArgumentsAndCaller because that mutates accessor pairs
+    // in place, and the initial state of the generator function map shares the
+    // accessor pair with sloppy functions.  Also the error message should be
+    // different.  Also unhappily, we can't use the API accessors to implement
+    // poisoning, because API accessors present themselves as data properties,
+    // not accessor properties, and so getOwnPropertyDescriptor raises an
+    // exception as it tries to get the values.  Sadness.
+    Handle<AccessorPair> poison_pair(factory()->NewAccessorPair());
+    PropertyAttributes rw_attribs =
+        static_cast<PropertyAttributes>(DONT_ENUM | DONT_DELETE);
+    poison_pair->set_getter(*GetGeneratorFunctionPoison());
+    poison_pair->set_setter(*GetGeneratorFunctionPoison());
+    ReplaceAccessors(generator_function_map, factory()->arguments_string(),
+        rw_attribs, poison_pair);
+    ReplaceAccessors(generator_function_map, factory()->caller_string(),
+        rw_attribs, poison_pair);
+
+    Handle<Map> strict_function_map(native_context()->strict_function_map());
     Handle<Map> strict_mode_generator_function_map =

[rossberg, 2014/05/07 09:19:54]

Nit: for consistency, drop _mode_ here, too.

-        Map::Copy(strict_mode_function_map);
+        Map::Copy(strict_function_map);
+    // "arguments" and "caller" already poisoned.
     strict_mode_generator_function_map->set_prototype(
         *generator_function_prototype);
     native_context()->set_strict_generator_function_map(