Revert of Don't compile functions in a context the caller doesn't have access...

Revert of Don't compile functions in a context the caller doesn't have access to (patchset #9 id:160001 of https://codereview.chromium.org/2034083002/ )

Reason for revert:
Causes crashes on Canary

Original issue's description:
> Don't compile functions in a context the caller doesn't have access to
>
> Instead just return undefined
>
> A side effect of this is that it's no longer possible to compile
> functions in a detached context.
>
> BUG=chromium:541703
> R=verwaest@chromium.org,bmeurer@chromium.org
> CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_chromium_rel_ng
>
> Committed: https://crrev.com/992e34c21635b179a993b82ac1d81753e7a6a57a
> Cr-Commit-Position: refs/heads/master@{#37657}

TBR=bmeurer@chromium.org,verwaest@chromium.org,jochen@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=chromium:541703, chromium:628053

Review-Url: https://codereview.chromium.org/2148163002
Cr-Commit-Position: refs/heads/master@{#37736}

include/v8.h

@@ -5732,7 +5732,6 @@ class V8_EXPORT Isolate {
     kDecimalWithLeadingZeroInStrictMode = 32,
     kLegacyDateParser = 33,
     kDefineGetterOrSetterWouldThrow = 34,
-    kFunctionConstructorReturnedUndefined = 35,
 
     // If you add new values here, you'll also need to update Chromium's:
     // UseCounter.h, V8PerIsolateData.cpp, histograms.xml

src/builtins.cc

@@ -4723,23 +4723,13 @@ void Builtins::Generate_DatePrototypeGetUTCSeconds(MacroAssembler* masm) {
 namespace {
 
 // ES6 section 19.2.1.1.1 CreateDynamicFunction
-MaybeHandle<Object> CreateDynamicFunction(Isolate* isolate,
-                                          BuiltinArguments args,
-                                          const char* token) {
+MaybeHandle<JSFunction> CreateDynamicFunction(Isolate* isolate,
+                                              BuiltinArguments args,
+                                              const char* token) {
   // Compute number of arguments, ignoring the receiver.
   DCHECK_LE(1, args.length());
   int const argc = args.length() - 1;
 
-  Handle<JSFunction> target = args.target<JSFunction>();
-  Handle<JSObject> target_global_proxy(target->global_proxy(), isolate);
-
-  HandleScopeImplementer* impl = isolate->handle_scope_implementer();
-  if (!FLAG_allow_unsafe_function_constructor &&
-      !isolate->MayAccess(impl->LastEnteredContext(), target_global_proxy)) {
-    isolate->CountUsage(v8::Isolate::kFunctionConstructorReturnedUndefined);
-    return isolate->factory()->undefined_value();
-  }
-
   // Build the source string.
   Handle<String> source;
   {
@@ -4754,7 +4744,7 @@ MaybeHandle<Object> CreateDynamicFunction(Isolate* isolate,
         Handle<String> param;
         ASSIGN_RETURN_ON_EXCEPTION(
             isolate, param, Object::ToString(isolate, args.at<Object>(i)),
-            Object);
+            JSFunction);
         param = String::Flatten(param);
         builder.AppendString(param);
         // If the formal parameters string include ) - an illegal
@@ -4779,35 +4769,37 @@ MaybeHandle<Object> CreateDynamicFunction(Isolate* isolate,
       Handle<String> body;
       ASSIGN_RETURN_ON_EXCEPTION(
           isolate, body, Object::ToString(isolate, args.at<Object>(argc)),
-          Object);
+          JSFunction);
       builder.AppendString(body);
     }
     builder.AppendCString("\n})");
-    ASSIGN_RETURN_ON_EXCEPTION(isolate, source, builder.Finish(), Object);
+    ASSIGN_RETURN_ON_EXCEPTION(isolate, source, builder.Finish(), JSFunction);
 
     // The SyntaxError must be thrown after all the (observable) ToString
     // conversions are done.
     if (parenthesis_in_arg_string) {
       THROW_NEW_ERROR(isolate,
                       NewSyntaxError(MessageTemplate::kParenthesisInArgString),
-                      Object);
+                      JSFunction);
     }
   }
 
   // Compile the string in the constructor and not a helper so that errors to
   // come from here.
+  Handle<JSFunction> target = args.target<JSFunction>();
+  Handle<JSObject> target_global_proxy(target->global_proxy(), isolate);
   Handle<JSFunction> function;
   {
     ASSIGN_RETURN_ON_EXCEPTION(
         isolate, function,
         CompileString(handle(target->native_context(), isolate), source,
                       ONLY_SINGLE_FUNCTION_LITERAL),
-        Object);
+        JSFunction);
     Handle<Object> result;
     ASSIGN_RETURN_ON_EXCEPTION(
         isolate, result,
         Execution::Call(isolate, function, target_global_proxy, 0, nullptr),
-        Object);
+        JSFunction);
     function = Handle<JSFunction>::cast(result);
     function->shared()->set_name_should_print_as_anonymous(true);
   }
@@ -4826,7 +4818,7 @@ MaybeHandle<Object> CreateDynamicFunction(Isolate* isolate,
     Handle<Map> initial_map;
     ASSIGN_RETURN_ON_EXCEPTION(
         isolate, initial_map,
-        JSFunction::GetDerivedMap(isolate, target, new_target), Object);
+        JSFunction::GetDerivedMap(isolate, target, new_target), JSFunction);
 
     Handle<SharedFunctionInfo> shared_info(function->shared(), isolate);
     Handle<Map> map = Map::AsLanguageMode(
@@ -4845,7 +4837,7 @@ MaybeHandle<Object> CreateDynamicFunction(Isolate* isolate,
 // ES6 section 19.2.1.1 Function ( p1, p2, ... , pn, body )
 BUILTIN(FunctionConstructor) {
   HandleScope scope(isolate);
-  Handle<Object> result;
+  Handle<JSFunction> result;
   ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
       isolate, result, CreateDynamicFunction(isolate, args, "function"));
   return *result;
@@ -4978,15 +4970,12 @@ BUILTIN(GeneratorFunctionConstructor) {
 
 BUILTIN(AsyncFunctionConstructor) {
   HandleScope scope(isolate);
-  Handle<Object> maybe_func;
+  Handle<JSFunction> func;
   ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
-      isolate, maybe_func,
-      CreateDynamicFunction(isolate, args, "async function"));
-  if (!maybe_func->IsJSFunction()) return *maybe_func;
+      isolate, func, CreateDynamicFunction(isolate, args, "async function"));
 
   // Do not lazily compute eval position for AsyncFunction, as they may not be
   // determined after the function is resumed.
-  Handle<JSFunction> func = Handle<JSFunction>::cast(maybe_func);
   Handle<Script> script = handle(Script::cast(func->shared()->script()));
   int position = script->GetEvalPosition();
   USE(position);

src/flag-definitions.h

@@ -587,10 +587,6 @@ DEFINE_BOOL(builtins_in_stack_traces, false,
             "show built-in functions in stack traces")
 DEFINE_BOOL(disable_native_files, false, "disable builtin natives files")
 
-// builtins.cc
-DEFINE_BOOL(allow_unsafe_function_constructor, false,
-            "allow invoking the function constructor without security checks")
-
 // builtins-ia32.cc
 DEFINE_BOOL(inline_new, true, "use fast inline allocation")
 

test/cctest/test-api.cc

@@ -10131,12 +10131,6 @@ static bool AccessAlwaysBlocked(Local<v8::Context> accessing_context,
   return false;
 }
 
-static bool AccessAlwaysAllowed(Local<v8::Context> accessing_context,
-                                Local<v8::Object> global,
-                                Local<v8::Value> data) {
-  i::PrintF("Access allowed.\n");
-  return true;
-}
 
 THREADED_TEST(AccessControlGetOwnPropertyNames) {
   v8::Isolate* isolate = CcTest::isolate();
@@ -25349,73 +25343,3 @@ THREADED_TEST(ImmutableProto) {
             ->Equals(context.local(), original_proto)
             .FromJust());
 }
-
-Local<v8::Context> call_eval_context;
-Local<v8::Function> call_eval_bound_function;
-
-static void CallEval(const v8::FunctionCallbackInfo<v8::Value>& args) {
-  v8::Context::Scope scope(call_eval_context);
-  args.GetReturnValue().Set(
-      call_eval_bound_function
-          ->Call(call_eval_context, call_eval_context->Global(), 0, NULL)
-          .ToLocalChecked());
-}
-
-TEST(CrossActivationEval) {
-  LocalContext env;
-  v8::Isolate* isolate = env->GetIsolate();
-  v8::HandleScope scope(isolate);
-  {
-    call_eval_context = v8::Context::New(isolate);
-    v8::Context::Scope scope(call_eval_context);
-    call_eval_bound_function =
-        Local<Function>::Cast(CompileRun("eval.bind(this, '1')"));
-  }
-  env->Global()
-      ->Set(env.local(), v8_str("CallEval"),
-            v8::FunctionTemplate::New(isolate, CallEval)
-                ->GetFunction(env.local())
-                .ToLocalChecked())
-      .FromJust();
-  Local<Value> result = CompileRun("CallEval();");
-  CHECK(result->IsInt32());
-  CHECK_EQ(1, result->Int32Value(env.local()).FromJust());
-}
-
-TEST(EvalInAccessCheckedContext) {
-  v8::Isolate* isolate = CcTest::isolate();
-  v8::HandleScope scope(isolate);
-
-  v8::Local<v8::ObjectTemplate> obj_template = v8::ObjectTemplate::New(isolate);
-
-  obj_template->SetAccessCheckCallback(AccessAlwaysAllowed);
-
-  v8::Local<Context> context0 = Context::New(isolate, NULL, obj_template);
-  v8::Local<Context> context1 = Context::New(isolate, NULL, obj_template);
-
-  Local<Value> foo = v8_str("foo");
-  Local<Value> bar = v8_str("bar");
-
-  // Set to different domains.
-  context0->SetSecurityToken(foo);
-  context1->SetSecurityToken(bar);
-
-  // Set up function in context0 that uses eval from context0.
-  context0->Enter();
-  v8::Local<v8::Value> fun = CompileRun(
-      "var x = 42;"
-      "(function() {"
-      "  var e = eval;"
-      "  return function(s) { return e(s); }"
-      "})()");
-  context0->Exit();
-
-  // Put the function into context1 and call it. Since the access check
-  // callback always returns true, the call succeeds even though the tokens
-  // are different.
-  context1->Enter();
-  context1->Global()->Set(context1, v8_str("fun"), fun).FromJust();
-  v8::Local<v8::Value> x_value = CompileRun("fun('x')");
-  CHECK_EQ(42, x_value->Int32Value(context1).FromJust());
-  context1->Exit();
-}

test/mjsunit/cross-realm-filtering.js

@@ -2,8 +2,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-// Flags: --restricted-realms
-
 var realms = [Realm.current(), Realm.create()];
 
 // Check stack trace filtering across security contexts.
@@ -90,77 +88,3 @@ o = Realm.eval(realmIndex, "new f()");
 proto = Object.getPrototypeOf(o);
 assertFalse(proto === Object.prototype);
 assertTrue(proto === otherObject.prototype);
-
-// Check function constructor.
-var ctor_script = "Function";
-var ctor_a_script =
-    "(function() { return Function.apply(this, ['return 1;']); })";
-var ctor_b_script = "Function.bind(this, 'return 1;')";
-var ctor_c_script =
-    "(function() { return Function.call(this, 'return 1;'); })";
-Realm.shared = {
-  ctor_0 : Realm.eval(realms[0], ctor_script),
-  ctor_1 : Realm.eval(realms[1], ctor_script),
-  ctor_a_0 : Realm.eval(realms[0], ctor_a_script),
-  ctor_a_1 : Realm.eval(realms[1], ctor_a_script),
-  ctor_b_0 : Realm.eval(realms[0], ctor_b_script),
-  ctor_b_1 : Realm.eval(realms[1], ctor_b_script),
-  ctor_c_0 : Realm.eval(realms[0], ctor_c_script),
-  ctor_c_1 : Realm.eval(realms[1], ctor_c_script),
-}
-var script_0 = "                                                               \
-  var ctor_0 = Realm.shared.ctor_0;                                            \
-  Realm.shared.direct_0 = ctor_0('return 1');                                  \
-  Realm.shared.indirect_0 = (function() { return ctor_0('return 1;'); })();    \
-  Realm.shared.apply_0 = ctor_0.apply(this, ['return 1']);                     \
-  Realm.shared.bind_0 = ctor_0.bind(this, 'return 1')();                       \
-  Realm.shared.call_0 = ctor_0.call(this, 'return 1');                         \
-  Realm.shared.proxy_0 = new Proxy(ctor_0, {})('return 1');                    \
-  Realm.shared.reflect_0 = Reflect.apply(ctor_0, this, ['return 1']);          \
-  Realm.shared.a_0 = Realm.shared.ctor_a_0();                                  \
-  Realm.shared.b_0 = Realm.shared.ctor_b_0();                                  \
-  Realm.shared.c_0 = Realm.shared.ctor_c_0();                                  \
-";
-script = script_0 + script_0.replace(/_0/g, "_1");
-Realm.eval(realms[0], script);
-assertSame(1, Realm.shared.direct_0());
-assertSame(1, Realm.shared.indirect_0());
-assertSame(1, Realm.shared.apply_0());
-assertSame(1, Realm.shared.bind_0());
-assertSame(1, Realm.shared.call_0());
-assertSame(1, Realm.shared.proxy_0());
-assertSame(1, Realm.shared.reflect_0());
-assertSame(1, Realm.shared.a_0());
-assertSame(1, Realm.shared.b_0());
-assertSame(1, Realm.shared.c_0());
-assertSame(undefined, Realm.shared.direct_1);
-assertSame(undefined, Realm.shared.indirect_1);
-assertSame(undefined, Realm.shared.apply_1);
-assertSame(undefined, Realm.shared.bind_1);
-assertSame(undefined, Realm.shared.call_1);
-assertSame(undefined, Realm.shared.proxy_1);
-assertSame(undefined, Realm.shared.reflect_1);
-assertSame(undefined, Realm.shared.a_1);
-assertSame(undefined, Realm.shared.b_1);
-assertSame(undefined, Realm.shared.c_1);
-Realm.eval(realms[1], script);
-assertSame(undefined, Realm.shared.direct_0);
-assertSame(undefined, Realm.shared.indirect_0);
-assertSame(undefined, Realm.shared.apply_0);
-assertSame(undefined, Realm.shared.bind_0);
-assertSame(undefined, Realm.shared.call_0);
-assertSame(undefined, Realm.shared.proxy_0);
-assertSame(undefined, Realm.shared.reflect_0);
-assertSame(undefined, Realm.shared.a_0);
-assertSame(undefined, Realm.shared.b_0);
-assertSame(undefined, Realm.shared.c_0);
-assertSame(1, Realm.shared.direct_1());
-assertSame(1, Realm.shared.indirect_1());
-assertSame(1, Realm.shared.apply_1());
-assertSame(1, Realm.shared.bind_1());
-assertSame(1, Realm.shared.call_1());
-assertSame(1, Realm.shared.proxy_1());
-assertSame(1, Realm.shared.reflect_1());
-assertSame(1, Realm.shared.a_1());
-assertSame(1, Realm.shared.b_1());
-assertSame(1, Realm.shared.c_1());

test/mjsunit/es6/reflect-construct.js

 // Copyright 2014 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
-//
-// Flags: --allow-unsafe-function-constructor
 
 
 (function testReflectConstructArity() {