Skip to content

Commit 82bdd58

Browse files
committed
deps: V8: backport a05321ebd98e
Original commit message: [execution] Lookup interceptor for RestrictedGlobalProperty When the script context looks up if a global property is restricted, it should also query the global interceptor. To avoid calling the interceptor for every declaration unconditionally, an interceptor has to be defined with `PropertyHandlerFlags::kHasDontDeleteProperty` to intercept restricted global property queries. Refs: #63715 Refs: #52634 Change-Id: I623ff285c4e8773d8ee7f681cbad68ba24bd3f40 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/7898818 Reviewed-by: Leszek Swirski <leszeks@chromium.org> Reviewed-by: Igor Sheludko <ishell@chromium.org> Commit-Queue: Igor Sheludko <ishell@chromium.org> Cr-Commit-Position: refs/heads/main@{#108280} Refs: v8/v8@a05321e Signed-off-by: Chengzhong Wu <cwu631@bloomberg.net>
1 parent bee4016 commit 82bdd58

10 files changed

Lines changed: 86 additions & 11 deletions

File tree

common.gypi

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@
4141

4242
# Reset this number to 0 on major V8 upgrades.
4343
# Increment by one for each non-official patch applied to deps/v8.
44-
'v8_embedder_string': '-node.24',
44+
'v8_embedder_string': '-node.25',
4545

4646
##### V8 defaults for Node.js #####
4747

deps/v8/include/v8-template.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -683,6 +683,13 @@ enum class PropertyHandlerFlags {
683683
*/
684684
kHasNoSideEffect = 1 << 2,
685685

686+
/**
687+
* The interceptor may return non-configurable (PropertyAttribute::DontDelete)
688+
* properties. When set on a global object's interceptor, it will be consulted
689+
* during HasRestrictedGlobalProperty checks for lexical declarations.
690+
*/
691+
kHasDontDeleteProperty = 1 << 3,
692+
686693
/**
687694
* This flag is used to distinguish which callbacks were provided -
688695
* GenericNamedPropertyXXXCallback (old signature) or

deps/v8/src/api/api.cc

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1613,6 +1613,8 @@ i::DirectHandle<i::InterceptorInfo> CreateInterceptorInfo(
16131613
!(flags & PropertyHandlerFlags::kOnlyInterceptStrings));
16141614
obj->set_non_masking(flags & PropertyHandlerFlags::kNonMasking);
16151615
obj->set_has_no_side_effect(flags & PropertyHandlerFlags::kHasNoSideEffect);
1616+
obj->set_has_dont_delete_property(
1617+
flags & PropertyHandlerFlags::kHasDontDeleteProperty);
16161618

16171619
if (data.IsEmpty()) {
16181620
data = v8::Undefined(reinterpret_cast<v8::Isolate*>(i_isolate));

deps/v8/src/execution/execution.cc

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -230,18 +230,14 @@ MaybeDirectHandle<Context> NewScriptContext(
230230
}
231231

232232
if (IsLexicalVariableMode(mode)) {
233-
LookupIterator lookup_it(isolate, global_object, name, global_object,
234-
LookupIterator::OWN_SKIP_INTERCEPTOR);
235-
Maybe<PropertyAttributes> maybe =
236-
JSReceiver::GetPropertyAttributes(&lookup_it);
237-
// Can't fail since the we looking up own properties on the global object
238-
// skipping interceptors.
239-
CHECK(!maybe.IsNothing());
240-
if ((maybe.FromJust() & DONT_DELETE) != 0) {
241-
// ES#sec-globaldeclarationinstantiation 5.a:
233+
Maybe<bool> has_restricted = JSGlobalObject::HasRestrictedGlobalProperty(
234+
isolate, global_object, name);
235+
if (has_restricted.IsNothing()) return MaybeDirectHandle<Context>();
236+
if (has_restricted.FromJust()) {
237+
// https://tc39.es/ecma262/#sec-globaldeclarationinstantiation 3.a:
242238
// If envRec.HasVarDeclaration(name) is true, throw a SyntaxError
243239
// exception.
244-
// ES#sec-globaldeclarationinstantiation 5.d:
240+
// https://tc39.es/ecma262/#sec-globaldeclarationinstantiation 3.d:
245241
// If hasRestrictedGlobal is true, throw a SyntaxError exception.
246242
MessageLocation location(script, 0, 1);
247243
isolate->ThrowAt(isolate->factory()->NewSyntaxError(

deps/v8/src/objects/api-callbacks-inl.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,8 @@ BOOL_ACCESSORS(InterceptorInfo, flags, has_no_side_effect,
203203
// TODO(ishell): remove once all the Api changes are done.
204204
BOOL_ACCESSORS(InterceptorInfo, flags, has_new_callbacks_signature,
205205
HasNewCallbacksSignatureBit::kShift)
206+
BOOL_ACCESSORS(InterceptorInfo, flags, has_dont_delete_property,
207+
HasDontDeletePropertyBit::kShift)
206208

207209
void InterceptorInfo::RemoveCallbackRedirectionForSerialization(
208210
IsolateForSandbox isolate) {

deps/v8/src/objects/api-callbacks.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,7 @@ class InterceptorInfo
182182
// TODO(ishell): remove support for old signatures once they go through
183183
// Api deprecation process.
184184
DECL_BOOLEAN_ACCESSORS(has_new_callbacks_signature)
185+
DECL_BOOLEAN_ACCESSORS(has_dont_delete_property)
185186

186187
DEFINE_TORQUE_GENERATED_INTERCEPTOR_INFO_FLAGS()
187188

deps/v8/src/objects/api-callbacks.tq

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ bitfield struct InterceptorInfoFlags extends uint31 {
88
named: bool: 1 bit;
99
has_no_side_effect: bool: 1 bit;
1010
has_new_callbacks_signature: bool: 1 bit;
11+
has_dont_delete_property: bool: 1 bit;
1112
}
1213

1314
extern class InterceptorInfo extends HeapObject {

deps/v8/src/objects/js-objects.cc

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5785,6 +5785,24 @@ void JSGlobalObject::InvalidatePropertyCell(DirectHandle<JSGlobalObject> global,
57855785
value);
57865786
}
57875787

5788+
// static
5789+
Maybe<bool> JSGlobalObject::HasRestrictedGlobalProperty(
5790+
Isolate* isolate, DirectHandle<JSGlobalObject> global,
5791+
DirectHandle<Name> name) {
5792+
LookupIterator::Configuration config = LookupIterator::OWN_SKIP_INTERCEPTOR;
5793+
if (global->HasNamedInterceptor() &&
5794+
global->GetNamedInterceptor()->has_dont_delete_property()) {
5795+
config = LookupIterator::OWN;
5796+
}
5797+
LookupIterator it(isolate, global, name, global, config);
5798+
Maybe<PropertyAttributes> maybe = JSReceiver::GetPropertyAttributes(&it);
5799+
if (maybe.IsNothing()) return Nothing<bool>();
5800+
// Global var and function bindings (except those that are introduced by
5801+
// non-strict direct eval) are non-configurable and are therefore restricted
5802+
// global properties.
5803+
return Just((maybe.FromJust() & DONT_DELETE) != 0);
5804+
}
5805+
57885806
// static
57895807
MaybeDirectHandle<JSDate> JSDate::New(Isolate* isolate,
57905808
DirectHandle<JSFunction> constructor,

deps/v8/src/objects/js-objects.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1218,6 +1218,11 @@ class JSGlobalObject
12181218
static void InvalidatePropertyCell(DirectHandle<JSGlobalObject> object,
12191219
DirectHandle<Name> name);
12201220

1221+
// https://tc39.es/ecma262/#sec-hasrestrictedglobalproperty
1222+
static Maybe<bool> HasRestrictedGlobalProperty(
1223+
Isolate* isolate, DirectHandle<JSGlobalObject> global,
1224+
DirectHandle<Name> name);
1225+
12211226
inline bool IsDetached();
12221227
inline Tagged<NativeContext> native_context();
12231228

deps/v8/test/cctest/test-api-interceptors.cc

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -834,6 +834,49 @@ THREADED_TEST(InterceptorFunctionRedeclareWithQueryCallback) {
834834
v8::Script::Compile(ctx, code).ToLocalChecked()->Run(ctx).ToLocalChecked();
835835
}
836836

837+
// A lexical declaration must throw when the interceptor reports the property
838+
// as non-configurable (DontDelete), per HasRestrictedGlobalProperty checks in
839+
// https://tc39.es/ecma262/#sec-globaldeclarationinstantiation.
840+
THREADED_TEST(LexicalDeclThrowsForRestrictedGlobalViaInterceptor) {
841+
v8::HandleScope scope(CcTest::isolate());
842+
LocalContext env;
843+
v8::Local<v8::FunctionTemplate> templ =
844+
v8::FunctionTemplate::New(CcTest::isolate());
845+
846+
v8::Local<ObjectTemplate> object_template = templ->InstanceTemplate();
847+
object_template->SetHandler(v8::NamedPropertyHandlerConfiguration(
848+
nullptr, nullptr, QueryCallbackSetDontDelete, nullptr, nullptr,
849+
v8::Local<v8::Value>(),
850+
v8::PropertyHandlerFlags::kHasDontDeleteProperty));
851+
v8::Local<v8::Context> ctx =
852+
v8::Context::New(CcTest::isolate(), nullptr, object_template);
853+
854+
v8::TryCatch try_catch(CcTest::isolate());
855+
v8::Local<v8::String> code = v8_str("let x;");
856+
CHECK(v8::Script::Compile(ctx, code).ToLocalChecked()->Run(ctx).IsEmpty());
857+
CHECK(try_catch.HasCaught());
858+
}
859+
860+
// Without kHasDontDeleteProperty, the interceptor is not consulted for
861+
// HasRestrictedGlobalProperty and lexical declarations succeed.
862+
THREADED_TEST(LexicalDeclSucceedsWithoutRestrictedGlobalFlag) {
863+
v8::HandleScope scope(CcTest::isolate());
864+
LocalContext env;
865+
v8::Local<v8::FunctionTemplate> templ =
866+
v8::FunctionTemplate::New(CcTest::isolate());
867+
868+
v8::Local<ObjectTemplate> object_template = templ->InstanceTemplate();
869+
object_template->SetHandler(v8::NamedPropertyHandlerConfiguration(
870+
nullptr, nullptr, QueryCallbackSetDontDelete));
871+
v8::Local<v8::Context> ctx =
872+
v8::Context::New(CcTest::isolate(), nullptr, object_template);
873+
874+
v8::TryCatch try_catch(CcTest::isolate());
875+
v8::Local<v8::String> code = v8_str("let x;");
876+
CHECK(!v8::Script::Compile(ctx, code).ToLocalChecked()->Run(ctx).IsEmpty());
877+
CHECK(!try_catch.HasCaught());
878+
}
879+
837880
// Regression test for chromium bug 656648.
838881
// Do not crash on non-masking, intercepting setter callbacks.
839882
THREADED_TEST(NonMaskingInterceptor) {

0 commit comments

Comments
 (0)