From 368a20bcd75b5444aa50e387ccdbf3842ae38844 Mon Sep 17 00:00:00 2001 From: Igor Sheludko Date: Mon, 7 Jun 2021 23:09:54 +0200 Subject: [PATCH] [runtime] Refactor interceptor handling ... and add regression test for contextual stores to JSGlobalObject with interceptor in the prototype chain. Bug: chromium:1216437 Change-Id: Ibd344288c6327b35f3276f59517995d591acb967 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2944895 Commit-Queue: Igor Sheludko Reviewed-by: Toon Verwaest Cr-Commit-Position: refs/heads/master@{#75038} --- src/objects/objects.cc | 65 +++++++++++++--------------- test/cctest/test-api-interceptors.cc | 31 +++++++++++++ 2 files changed, 61 insertions(+), 35 deletions(-) diff --git a/src/objects/objects.cc b/src/objects/objects.cc index 787c5debca..7be97650ab 100644 --- a/src/objects/objects.cc +++ b/src/objects/objects.cc @@ -2645,6 +2645,34 @@ Maybe Object::SetPropertyInternal(LookupIterator* it, return Nothing(); } +namespace { + +// 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. +// Returns false if the exception was thrown, otherwise true. +bool CheckContextualStoreToJSGlobalObject(LookupIterator* it, + Maybe should_throw) { + Isolate* isolate = it->isolate(); + + if (it->GetReceiver()->IsJSGlobalObject(isolate) && + (GetShouldThrow(isolate, should_throw) == ShouldThrow::kThrowOnError)) { + if (it->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. + it->transition_cell()->ClearAndInvalidate(ReadOnlyRoots(isolate)); + } + isolate->Throw(*isolate->factory()->NewReferenceError( + MessageTemplate::kNotDefined, it->GetName())); + return false; + } + return true; +} + +} // namespace + Maybe Object::SetProperty(LookupIterator* it, Handle value, StoreOrigin store_origin, Maybe should_throw) { @@ -2655,26 +2683,9 @@ Maybe Object::SetProperty(LookupIterator* it, Handle 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. - if (it->GetReceiver()->IsJSGlobalObject() && - (GetShouldThrow(it->isolate(), should_throw) == - ShouldThrow::kThrowOnError)) { - if (it->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. - it->transition_cell()->ClearAndInvalidate(ReadOnlyRoots(it->isolate())); - } - it->isolate()->Throw(*it->isolate()->factory()->NewReferenceError( - MessageTemplate::kNotDefined, it->GetName())); + if (!CheckContextualStoreToJSGlobalObject(it, should_throw)) { return Nothing(); } - return AddDataProperty(it, value, NONE, should_throw, store_origin); } @@ -2764,25 +2775,9 @@ Maybe Object::SetSuperProperty(LookupIterator* it, Handle 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())); + if (!CheckContextualStoreToJSGlobalObject(&own_lookup, should_throw)) { return Nothing(); } - 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 768d7f34fc..815c538d22 100644 --- a/test/cctest/test-api-interceptors.cc +++ b/test/cctest/test-api-interceptors.cc @@ -1698,6 +1698,37 @@ THREADED_TEST(EmptyInterceptorDoesNotShadowAccessors) { ExpectInt32("child.accessor_age", 10); } +THREADED_TEST(EmptyInterceptorVsStoreGlobalICs) { + // In sloppy mode storing to global must succeed. + CheckInterceptorIC(EmptyInterceptorGetter, + HasICQuery, v8::internal::ABSENT>, + "globalThis.__proto__ = o;" + "let result = 0;" + "for (var i = 0; i < 20; i++) {" + " try {" + " x = i;" + " result++;" + " } catch (e) {}" + "}" + "result + x", + 20 + 19); + + // In strict mode storing to global must throw. + CheckInterceptorIC(EmptyInterceptorGetter, + HasICQuery, v8::internal::ABSENT>, + "'use strict';" + "globalThis.__proto__ = o;" + "let result = 0;" + "for (var i = 0; i < 20; i++) {" + " try {" + " x = i;" + " } catch (e) {" + " result++;" + " }" + "}" + "result + (typeof(x) === 'undefined' ? 100 : 0)", + 120); +} THREADED_TEST(LegacyInterceptorDoesNotSeeSymbols) { LocalContext env;