Fix issues with name and length on poison pill function

src/bootstrapper.cc

Index: src/bootstrapper.cc
diff --git a/src/bootstrapper.cc b/src/bootstrapper.cc
index af75efd543a215b645e29484a0c9c91b2a272e38..73836baf25faa3d767e262b9ac2222e1fd379d56 100644
--- a/src/bootstrapper.cc
+++ b/src/bootstrapper.cc
@@ -296,7 +296,7 @@ class Genesis BASE_EMBEDDED {
   // prototype, maps.
   Handle<Map> sloppy_function_map_writable_prototype_;
   Handle<Map> strict_function_map_writable_prototype_;
-  Handle<JSFunction> strict_poison_function;
+  Handle<JSFunction> strict_poison_function_;
   Handle<JSFunction> generator_poison_function;
 
   BootstrapperActive active_;
@@ -614,20 +614,36 @@ void Genesis::SetStrongFunctionInstanceDescriptor(Handle<Map> map) {
 }
 
 
-// ECMAScript 5th Edition, 13.2.3
+// ECMAScript 6th Edition, 9.2.7.1
 Handle<JSFunction> Genesis::GetStrictPoisonFunction() {
-  if (strict_poison_function.is_null()) {
+  if (strict_poison_function_.is_null()) {
     Handle<String> name = factory()->InternalizeOneByteString(
         STATIC_CHAR_VECTOR("ThrowTypeError"));
     Handle<Code> code(isolate()->builtins()->builtin(
         Builtins::kStrictModePoisonPill));
-    strict_poison_function = factory()->NewFunctionWithoutPrototype(name, code);
-    strict_poison_function->set_map(native_context()->sloppy_function_map());
-    strict_poison_function->shared()->DontAdaptArguments();
+    Handle<JSFunction> function =
+        factory()->NewFunctionWithoutPrototype(name, code);
+    function->set_map(native_context()->sloppy_function_map());
+    function->shared()->DontAdaptArguments();
+
+    // %ThrowTypeError% must not have a name property.
+    JSReceiver::DeleteProperty(function, factory()->name_string()).Assert();

[adamk, 2015/04/07 20:02:58]

So this'll put this into "slow" mode, but we don't really care since no one will
ever look at properties on this object?

[arv (Not doing code reviews), 2015/04/07 20:10:16]

Yes. But that is fine since the would have to extract the accessor and use its
properties. There is no valid reason to do that.

+
+    // length needs to be non configurable
+    LookupIterator it(function, factory()->length_string());
+    CHECK_EQ(LookupIterator::ACCESSOR, it.state());

[adamk, 2015/04/07 20:02:58]

Seems like this could be a DCHECK_EQ just as well.

[arv (Not doing code reviews), 2015/04/07 20:10:16]

It had to be CHECK before when I was using lower level primitives. I'll change
it (if it works).

+    DCHECK(it.HolderIsReceiverOrHiddenPrototype());
+    Handle<Object> value(Smi::FromInt(function->shared()->length()), isolate());
+    JSObject::SetOwnPropertyIgnoreAttributes(

[adamk, 2015/04/07 20:02:57]

So does this end up getting rid of the accessor? But it's OK because it will
never change?

[arv (Not doing code reviews), 2015/04/07 20:10:16]

Yes. But let me expand the test to make sure that writing to it fails.

+        function, factory()->length_string(), value,
+        static_cast<PropertyAttributes>(DONT_ENUM | DONT_DELETE | READ_ONLY))
+        .Assert();
+
+    JSObject::PreventExtensions(function).Assert();
 
-    JSObject::PreventExtensions(strict_poison_function).Assert();
+    strict_poison_function_ = function;
   }
-  return strict_poison_function;
+  return strict_poison_function_;
 }