github/electron
1
1
Fork 0
mirror of https://github.com/electron/electron.git synced 2026-04-10 03:01:51 -04:00
Code Issues Packages Projects Releases Wiki Activity
Files
21258845f05d71059b910cdb42c16bd951e80950
electron/patches/v8/m90-lts_squashed_multiple_commits.patch
Pedro Pontes 308470c98d fix: cherry-pick 9a9c936879 from v8 (#29779) 
* cherry-pick 9a9c936879 from v8

* chore: update patches

Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
2021-06-21 15:53:37 +09:00

300 lines
13 KiB
Diff
Raw Blame History

From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: "ishell@chromium.org" <ishell@chromium.org>
Date: Tue, 8 Jun 2021 17:33:32 +0200
Subject: Squashed multiple commits.
Merged: [runtime] Fix handling of interceptors
Revision: f9857fdf74
Merged: [runtime] Fix handling of interceptors, pt.2
Revision: 1f5113816c
BUG=chromium:1216437
NOTRY=true
NOPRESUBMIT=true
NOTREECHECKS=true
(cherry picked from commit 1936d568193b37d50d99218724ebbb76785a30d2)
Change-Id: Ief3da51866c8d0b5e85c76fad00b25ac2379f615
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2947407
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Cr-Original-Commit-Position: refs/branch-heads/9.1@{#71}
Cr-Original-Branched-From: 0e4ac64a8cf298b14034a22f9fe7b085d2cb238d-refs/heads/9.1.269@{#1}
Cr-Original-Branched-From: f565e72d5ba88daae35a59d0f978643e2343e912-refs/heads/master@{#73847}
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2948661
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Reviewed-by: Artem Sumaneev <asumaneev@google.com>
Commit-Queue: Victor-Gabriel Savu <vsavu@google.com>
Cr-Commit-Position: refs/branch-heads/9.0@{#64}
Cr-Branched-From: bd0108b4c88e0d6f2350cb79b5f363fbd02f3eb7-refs/heads/9.0.257@{#1}
Cr-Branched-From: 349bcc6a075411f1a7ce2d866c3dfeefc2efa39d-refs/heads/master@{#73001}
diff --git a/src/objects/objects.cc b/src/objects/objects.cc
index 5e17fa85fca29e709204950382c610f40d616a64..c5450d50527bb8db1ae67b93aafc4b455b10053b 100644
--- a/src/objects/objects.cc
+++ b/src/objects/objects.cc
@@ -2519,9 +2519,21 @@ Maybe<bool> Object::SetPropertyInternal(LookupIterator* it,
if ((maybe_attributes.FromJust() & READ_ONLY) != 0) {
return WriteToReadOnlyProperty(it, value, should_throw);
}
- if (maybe_attributes.FromJust() == ABSENT) break;
- *found = false;
- return Nothing<bool>();
+ // At this point we might have called interceptor's query or getter
+ // callback. Assuming that the callbacks have side effects, we use
+ // Object::SetSuperProperty() which works properly regardless on
+ // whether the property was present on the receiver or not when
+ // storing to the receiver.
+ if (maybe_attributes.FromJust() == ABSENT) {
+ // Proceed lookup from the next state.
+ it->Next();
+ } else {
+ // Finish lookup in order to make Object::SetSuperProperty() store
+ // property to the receiver.
+ it->NotFound();
+ }
+ return Object::SetSuperProperty(it, value, store_origin,
+ should_throw);
}
break;
}
@@ -2596,6 +2608,8 @@ Maybe<bool> Object::SetProperty(LookupIterator* it, Handle<Object> value,
if (found) return result;
}
+ // TODO(ishell): refactor this: both SetProperty and and SetSuperProperty have
+ // this piece of code.
// If the receiver is the JSGlobalObject, the store was contextual. In case
// the property did not exist yet on the global object itself, we have to
// throw a reference error in strict mode. In sloppy mode, we continue.
@@ -2632,6 +2646,8 @@ Maybe<bool> Object::SetSuperProperty(LookupIterator* it, Handle<Object> value,
}
Handle<JSReceiver> receiver = Handle<JSReceiver>::cast(it->GetReceiver());
+ // Note, the callers rely on the fact that this code is redoing the full own
+ // lookup from scratch.
LookupIterator::Configuration c = LookupIterator::OWN;
LookupIterator own_lookup =
it->IsElement() ? LookupIterator(isolate, receiver, it->index(), c)
@@ -2694,6 +2710,25 @@ Maybe<bool> Object::SetSuperProperty(LookupIterator* it, Handle<Object> value,
}
}
+ // TODO(ishell): refactor this: both SetProperty and and SetSuperProperty have
+ // this piece of code.
+ // If the receiver is the JSGlobalObject, the store was contextual. In case
+ // the property did not exist yet on the global object itself, we have to
+ // throw a reference error in strict mode. In sloppy mode, we continue.
+ if (receiver->IsJSGlobalObject() &&
+ (GetShouldThrow(isolate, should_throw) == ShouldThrow::kThrowOnError)) {
+ if (own_lookup.state() == LookupIterator::TRANSITION) {
+ // The property cell that we have created is garbage because we are going
+ // to throw now instead of putting it into the global dictionary. However,
+ // the cell might already have been stored into the feedback vector, so
+ // we must invalidate it nevertheless.
+ own_lookup.transition_cell()->ClearAndInvalidate(ReadOnlyRoots(isolate));
+ }
+ isolate->Throw(*isolate->factory()->NewReferenceError(
+ MessageTemplate::kNotDefined, own_lookup.GetName()));
+ return Nothing<bool>();
+ }
+
return AddDataProperty(&own_lookup, value, NONE, should_throw, store_origin);
}
diff --git a/test/cctest/test-api-interceptors.cc b/test/cctest/test-api-interceptors.cc
index 236053eb45c02201d3425ea2fd98cf3913084bfe..3d3e970fa52b633ffc9e734077e7971ae37306bc 100644
--- a/test/cctest/test-api-interceptors.cc
+++ b/test/cctest/test-api-interceptors.cc
@@ -875,9 +875,11 @@ THREADED_TEST(InterceptorHasOwnPropertyCausingGC) {
CHECK(!value->BooleanValue(isolate));
}
-static void CheckInterceptorIC(v8::GenericNamedPropertyGetterCallback getter,
- v8::GenericNamedPropertyQueryCallback query,
- const char* source, int expected) {
+namespace {
+
+void CheckInterceptorIC(v8::GenericNamedPropertyGetterCallback getter,
+ v8::GenericNamedPropertyQueryCallback query,
+ const char* source, int expected) {
v8::Isolate* isolate = CcTest::isolate();
v8::HandleScope scope(isolate);
v8::Local<v8::ObjectTemplate> templ = ObjectTemplate::New(isolate);
@@ -892,14 +894,13 @@ static void CheckInterceptorIC(v8::GenericNamedPropertyGetterCallback getter,
CHECK_EQ(expected, value->Int32Value(context.local()).FromJust());
}
-static void CheckInterceptorLoadIC(
- v8::GenericNamedPropertyGetterCallback getter, const char* source,
- int expected) {
+void CheckInterceptorLoadIC(v8::GenericNamedPropertyGetterCallback getter,
+ const char* source, int expected) {
CheckInterceptorIC(getter, nullptr, source, expected);
}
-static void InterceptorLoadICGetter(
- Local<Name> name, const v8::PropertyCallbackInfo<v8::Value>& info) {
+void InterceptorLoadICGetter(Local<Name> name,
+ const v8::PropertyCallbackInfo<v8::Value>& info) {
ApiTestFuzzer::Fuzz();
v8::Isolate* isolate = CcTest::isolate();
CHECK_EQ(isolate, info.GetIsolate());
@@ -909,6 +910,7 @@ static void InterceptorLoadICGetter(
info.GetReturnValue().Set(v8::Integer::New(isolate, 42));
}
+} // namespace
// This test should hit the load IC for the interceptor case.
THREADED_TEST(InterceptorLoadIC) {
@@ -925,9 +927,23 @@ THREADED_TEST(InterceptorLoadIC) {
// configurations of interceptor and explicit fields works fine
// (those cases are special cased to get better performance).
-static void InterceptorLoadXICGetter(
+namespace {
+
+void InterceptorLoadXICGetter(Local<Name> name,
+ const v8::PropertyCallbackInfo<v8::Value>& info) {
+ ApiTestFuzzer::Fuzz();
+ info.GetReturnValue().Set(
+ v8_str("x")
+ ->Equals(info.GetIsolate()->GetCurrentContext(), name)
+ .FromJust()
+ ? v8::Local<v8::Value>(v8::Integer::New(info.GetIsolate(), 42))
+ : v8::Local<v8::Value>());
+}
+
+void InterceptorLoadXICGetterWithSideEffects(
Local<Name> name, const v8::PropertyCallbackInfo<v8::Value>& info) {
ApiTestFuzzer::Fuzz();
+ CompileRun("interceptor_getter_side_effect()");
info.GetReturnValue().Set(
v8_str("x")
->Equals(info.GetIsolate()->GetCurrentContext(), name)
@@ -936,6 +952,7 @@ static void InterceptorLoadXICGetter(
: v8::Local<v8::Value>());
}
+} // namespace
THREADED_TEST(InterceptorLoadICWithFieldOnHolder) {
CheckInterceptorLoadIC(InterceptorLoadXICGetter,
@@ -1460,6 +1477,18 @@ void HasICQueryToggle(TKey name,
isolate, toggle ? v8::internal::ABSENT : v8::internal::NONE));
}
+template <typename TKey, v8::internal::PropertyAttributes attribute>
+void HasICQuerySideEffect(TKey name,
+ const v8::PropertyCallbackInfo<v8::Integer>& info) {
+ ApiTestFuzzer::Fuzz();
+ v8::Isolate* isolate = CcTest::isolate();
+ CHECK_EQ(isolate, info.GetIsolate());
+ CompileRun("interceptor_query_side_effect()");
+ if (attribute != v8::internal::ABSENT) {
+ info.GetReturnValue().Set(v8::Integer::New(isolate, attribute));
+ }
+}
+
int named_query_counter = 0;
void NamedQueryCallback(Local<Name> name,
const v8::PropertyCallbackInfo<v8::Integer>& info) {
@@ -1525,6 +1554,42 @@ THREADED_TEST(InterceptorHasICQueryToggle) {
500);
}
+THREADED_TEST(InterceptorStoreICWithSideEffectfulCallbacks) {
+ CheckInterceptorIC(EmptyInterceptorGetter,
+ HasICQuerySideEffect<Local<Name>, v8::internal::ABSENT>,
+ "let r;"
+ "let inside_side_effect = false;"
+ "let interceptor_query_side_effect = function() {"
+ " if (!inside_side_effect) {"
+ " inside_side_effect = true;"
+ " r.x = 153;"
+ " inside_side_effect = false;"
+ " }"
+ "};"
+ "for (var i = 0; i < 20; i++) {"
+ " r = { __proto__: o };"
+ " r.x = i;"
+ "}",
+ 19);
+
+ CheckInterceptorIC(InterceptorLoadXICGetterWithSideEffects,
+ nullptr, // query callback is not provided
+ "let r;"
+ "let inside_side_effect = false;"
+ "let interceptor_getter_side_effect = function() {"
+ " if (!inside_side_effect) {"
+ " inside_side_effect = true;"
+ " r.y = 153;"
+ " inside_side_effect = false;"
+ " }"
+ "};"
+ "for (var i = 0; i < 20; i++) {"
+ " r = { __proto__: o };"
+ " r.y = i;"
+ "}",
+ 19);
+}
+
static void InterceptorStoreICSetter(
Local<Name> key, Local<Value> value,
const v8::PropertyCallbackInfo<v8::Value>& info) {
@@ -1574,6 +1639,52 @@ THREADED_TEST(InterceptorStoreICWithNoSetter) {
CHECK_EQ(239 + 42, value->Int32Value(context.local()).FromJust());
}
+THREADED_TEST(EmptyInterceptorDoesNotShadowReadOnlyProperty) {
+ // Interceptor should not shadow readonly property 'x' on the prototype, and
+ // attempt to store to 'x' must throw.
+ CheckInterceptorIC(EmptyInterceptorGetter,
+ HasICQuery<Local<Name>, v8::internal::ABSENT>,
+ "'use strict';"
+ "let p = {};"
+ "Object.defineProperty(p, 'x', "
+ " {value: 153, writable: false});"
+ "o.__proto__ = p;"
+ "let result = 0;"
+ "let r;"
+ "for (var i = 0; i < 20; i++) {"
+ " r = { __proto__: o };"
+ " try {"
+ " r.x = i;"
+ " } catch (e) {"
+ " result++;"
+ " }"
+ "}"
+ "result",
+ 20);
+}
+
+THREADED_TEST(InterceptorShadowsReadOnlyProperty) {
+ // Interceptor claims that it has a writable property 'x', so the existence
+ // of the readonly property 'x' on the prototype should not cause exceptions.
+ CheckInterceptorIC(InterceptorLoadXICGetter,
+ nullptr, // query callback
+ "'use strict';"
+ "let p = {};"
+ "Object.defineProperty(p, 'x', "
+ " {value: 153, writable: false});"
+ "o.__proto__ = p;"
+ "let result = 0;"
+ "let r;"
+ "for (var i = 0; i < 20; i++) {"
+ " r = { __proto__: o };"
+ " try {"
+ " r.x = i;"
+ " result++;"
+ " } catch (e) {}"
+ "}"
+ "result",
+ 20);
+}
THREADED_TEST(EmptyInterceptorDoesNotShadowAccessors) {
v8::HandleScope scope(CcTest::isolate());
Reference in New Issue View Git Blame Copy Permalink