Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: cherry-pick 2aac556145af from v8 #23059

Merged
merged 3 commits into from Apr 13, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions patches/v8/.patches
Expand Up @@ -9,4 +9,6 @@ do_not_export_private_v8_symbols_on_windows.patch
include_string_in_v8_h.patch
objects_fix_memory_leak_in_prototypeusers_add.patch
fix_bug_in_receiver_maps_inference.patch
make_createdynamicfunction_throw_if_disallowed.patch
intl_fix_intl_numberformat_constructor.patch
merged_make_createdynamicfunction_switch_context_before_throwing.patch
66 changes: 66 additions & 0 deletions patches/v8/make_createdynamicfunction_throw_if_disallowed.patch
@@ -0,0 +1,66 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Georg Neis <neis@chromium.org>
Date: Mon, 30 Mar 2020 11:55:10 +0200
Subject: Make CreateDynamicFunction throw if disallowed

... instead of returning undefined.

Bug: chromium:1065094
Change-Id: I0b0397a8affd44b58e7f4777f32ba22bbd001ab1
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2124837
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Commit-Queue: Georg Neis <neis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#66902}

diff --git a/src/builtins/builtins-function.cc b/src/builtins/builtins-function.cc
index f75014d034626643ae83dc8f167a473b7291522c..b5c2e8b25f10f2fff9011a0a1bd1aeef23de5cb9 100644
--- a/src/builtins/builtins-function.cc
+++ b/src/builtins/builtins-function.cc
@@ -31,7 +31,7 @@ MaybeHandle<Object> CreateDynamicFunction(Isolate* isolate,

if (!Builtins::AllowDynamicFunction(isolate, target, target_global_proxy)) {
isolate->CountUsage(v8::Isolate::kFunctionConstructorReturnedUndefined);
- return isolate->factory()->undefined_value();
+ THROW_NEW_ERROR(isolate, NewTypeError(MessageTemplate::kNoAccess), Object);
}

// Build the source string.
diff --git a/test/mjsunit/mjsunit.status b/test/mjsunit/mjsunit.status
index 134a49f7480c87acbb08133a2d6a4d028bbdd090..7dcaa412199770d7c4f688d7f8a66d887130bf82 100644
--- a/test/mjsunit/mjsunit.status
+++ b/test/mjsunit/mjsunit.status
@@ -232,6 +232,9 @@
# BUG(v8:6306).
'wasm/huge-memory': [SKIP],

+ # Needs to be adapted after changes to Function constructor. chromium:1065094
+ 'cross-realm-filtering': [SKIP],
+
# Allocates a huge string and then flattens it, very slow in debug mode.
'regress/regress-752764': [PASS, ['mode == debug', SLOW]],

diff --git a/test/mjsunit/regress-1065094.js b/test/mjsunit/regress-1065094.js
new file mode 100644
index 0000000000000000000000000000000000000000..365e20285bb0505dec3f84c4df57db0525e7acc3
--- /dev/null
+++ b/test/mjsunit/regress-1065094.js
@@ -0,0 +1,19 @@
+// Copyright 2020 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-natives-syntax
+
+function f(fnConstructor) {
+ return Object.is(new fnConstructor(), undefined);
+}
+
+const realmIndex = Realm.createAllowCrossRealmAccess();
+const otherFunction = Realm.global(realmIndex).Function;
+Realm.detachGlobal(realmIndex);
+
+%PrepareFunctionForOptimization(f);
+assertFalse(f(Function));
+assertThrows(_ => f(otherFunction));
+%OptimizeFunctionOnNextCall(f);
+assertThrows(_ => f(otherFunction));
@@ -0,0 +1,45 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Georg Neis <neis@chromium.org>
Date: Tue, 31 Mar 2020 18:49:38 +0200
Subject: Merged: Make CreateDynamicFunction switch context before throwing

Revision: 093019ee1ab3a92de915c1f3a1a7a5a8c86e3a5d

BUG=chromium:1065094,v8:10361
NOTRY=true
NOPRESUBMIT=true
NOTREECHECKS=true
R=verwaest@chromium.org

Change-Id: I19528a1bc6058e6596f34f73d13d9c249cc6c3a7
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2130127
Commit-Queue: Georg Neis <neis@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/branch-heads/8.0@{#60}
Cr-Branched-From: 69827db645fcece065bf16a795a4ec8d3a51057f-refs/heads/8.0.426@{#2}
Cr-Branched-From: 2fe1552c5809d0dd92e81d36a5535cbb7c518800-refs/heads/master@{#65318}

diff --git a/src/builtins/builtins-function.cc b/src/builtins/builtins-function.cc
index b5c2e8b25f10f2fff9011a0a1bd1aeef23de5cb9..7275e2fcede16a41cc5998b4957be9b8a8e53b88 100644
--- a/src/builtins/builtins-function.cc
+++ b/src/builtins/builtins-function.cc
@@ -2,6 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

+#include "src/api/api-inl.h"
#include "src/builtins/builtins-utils-inl.h"
#include "src/builtins/builtins.h"
#include "src/codegen/code-factory.h"
@@ -31,6 +32,11 @@ MaybeHandle<Object> CreateDynamicFunction(Isolate* isolate,

if (!Builtins::AllowDynamicFunction(isolate, target, target_global_proxy)) {
isolate->CountUsage(v8::Isolate::kFunctionConstructorReturnedUndefined);
+ // TODO(verwaest): We would like to throw using the calling context instead
+ // of the entered context but we don't currently have access to that.
+ HandleScopeImplementer* impl = isolate->handle_scope_implementer();
+ SaveAndSwitchContext save(
+ isolate, impl->LastEnteredOrMicrotaskContext()->native_context());
THROW_NEW_ERROR(isolate, NewTypeError(MessageTemplate::kNoAccess), Object);
}