commit 3c78dc4e938194be8af407f93c2e40108fc7cb0e Author: Erik Corry Date: Tue Sep 17 23:20:38 2024 +0200 Use Handles more, Tagged less, in V8 patch diff --git a/patches/v8/0019-Implement-cross-request-context-promise-resolve-hand.patch b/patches/v8/0019-Implement-cross-request-context-promise-resolve-hand.patch index 28397f91..7d4ef4dc 100644 --- a/patches/v8/0019-Implement-cross-request-context-promise-resolve-hand.patch +++ b/patches/v8/0019-Implement-cross-request-context-promise-resolve-hand.patch @@ -6,17 +6,19 @@ Subject: [PATCH] Implement cross-request context promise resolve handling --- include/v8-callbacks.h | 21 +++++++++++ include/v8-isolate.h | 2 ++ - src/api/api.cc | 5 +++ + src/api/api.cc | 7 +++- src/builtins/promise-abstract-operations.tq | 6 +++- src/builtins/promise-resolve.tq | 4 ++- - src/execution/isolate-inl.h | 9 +++++ - src/execution/isolate.cc | 22 ++++++++++++ - src/execution/isolate.h | 14 +++++++- + src/execution/isolate-inl.h | 21 +++++++---- + src/execution/isolate.cc | 24 +++++++++++-- + src/execution/isolate.h | 19 +++++++--- + src/heap/factory.cc | 11 +++--- src/objects/js-promise.h | 5 +++ src/objects/objects.cc | 39 +++++++++++++++++++++ - src/runtime/runtime-promise.cc | 18 ++++++++++ + src/roots/roots.h | 3 +- + src/runtime/runtime-promise.cc | 26 +++++++++++--- src/runtime/runtime.h | 3 +- - 12 files changed, 144 insertions(+), 4 deletions(-) + 14 files changed, 164 insertions(+), 27 deletions(-) diff --git a/include/v8-callbacks.h b/include/v8-callbacks.h index 25f0c8189e5..7fcb1602df3 100644 @@ -64,10 +66,10 @@ index 8112231319b..9fe57f7bde4 100644 Isolate() = delete; ~Isolate() = delete; diff --git a/src/api/api.cc b/src/api/api.cc -index d3fc6d77ea3..3255fd77e25 100644 +index d3fc6d77ea3..248a78ad24c 100644 --- a/src/api/api.cc +++ b/src/api/api.cc -@@ -12246,6 +12246,11 @@ void Isolate::SetPromiseCrossContextCallback(PromiseCrossContextCallback callbac +@@ -12246,12 +12246,17 @@ void Isolate::SetPromiseCrossContextCallback(PromiseCrossContextCallback callbac isolate->set_promise_cross_context_callback(callback); } @@ -79,6 +81,13 @@ index d3fc6d77ea3..3255fd77e25 100644 Isolate::PromiseContextScope::PromiseContextScope(Isolate* isolate, v8::Local tag) : isolate_(reinterpret_cast(isolate)) { DCHECK(!isolate_->has_promise_context_tag()); + DCHECK(!tag.IsEmpty()); + i::Handle handle = Utils::OpenHandle(*tag); +- isolate_->set_promise_context_tag(*handle); ++ isolate_->set_promise_context_tag(handle); + } + + Isolate::PromiseContextScope::~PromiseContextScope() { diff --git a/src/builtins/promise-abstract-operations.tq b/src/builtins/promise-abstract-operations.tq index fdec3d268e4..456d051d3c0 100644 --- a/src/builtins/promise-abstract-operations.tq @@ -119,10 +128,36 @@ index 202180adbba..c93ac5905d7 100644 deferred { return runtime::ResolvePromise(promise, resolution); diff --git a/src/execution/isolate-inl.h b/src/execution/isolate-inl.h -index 9e0969460fd..1084adfa7f5 100644 +index 9e0969460fd..aa2b1cea6b7 100644 --- a/src/execution/isolate-inl.h +++ b/src/execution/isolate-inl.h -@@ -154,6 +154,15 @@ void Isolate::set_promise_cross_context_callback(PromiseCrossContextCallback cal +@@ -134,26 +134,35 @@ bool Isolate::is_execution_terminating() { + i::ReadOnlyRoots(this).termination_exception(); + } + +-Tagged Isolate::promise_context_tag() { +- return promise_context_tag_; ++Handle Isolate::promise_context_tag() { ++ return root_handle(RootIndex::kPromiseContextTag); + } + + bool Isolate::has_promise_context_tag() { +- return promise_context_tag_ != ReadOnlyRoots(this).the_hole_value(); ++ return heap()->promise_context_tag() != ReadOnlyRoots(this).the_hole_value(); + } + + void Isolate::clear_promise_context_tag() { +- set_promise_context_tag(ReadOnlyRoots(this).the_hole_value()); ++ heap()->set_promise_context_tag(ReadOnlyRoots(this).the_hole_value()); + } + +-void Isolate::set_promise_context_tag(Tagged tag) { +- promise_context_tag_ = tag; ++void Isolate::set_promise_context_tag(Handle tag) { ++ heap()->set_promise_context_tag(*tag); + } + + void Isolate::set_promise_cross_context_callback(PromiseCrossContextCallback callback) { promise_cross_context_callback_ = callback; } @@ -142,6 +177,15 @@ diff --git a/src/execution/isolate.cc b/src/execution/isolate.cc index 94194d59bae..cfba2a44966 100644 --- a/src/execution/isolate.cc +++ b/src/execution/isolate.cc +@@ -593,8 +593,6 @@ void Isolate::Iterate(RootVisitor* v, ThreadLocalTop* thread) { + FullObjectSlot(&thread->pending_message_)); + v->VisitRootPointer(Root::kStackRoots, nullptr, + FullObjectSlot(&thread->context_)); +- v->VisitRootPointer(Root::kStackRoots, nullptr, +- FullObjectSlot(&promise_context_tag_)); + + for (v8::TryCatch* block = thread->try_catch_handler_; block != nullptr; + block = block->next_) { @@ -7459,5 +7459,27 @@ MaybeHandle Isolate::RunPromiseCrossContextCallback(Handle promise_context_tag(); ++ inline Handle promise_context_tag(); + inline bool has_promise_context_tag(); inline void clear_promise_context_tag(); - inline void set_promise_context_tag(Tagged tag); +- inline void set_promise_context_tag(Tagged tag); ++ inline void set_promise_context_tag(Handle tag); inline void set_promise_cross_context_callback(PromiseCrossContextCallback callback); + inline void set_promise_cross_context_resolve_callback( + PromiseCrossContextResolveCallback callback); @@ -200,10 +251,11 @@ index 59926e68e97..2bd24b914b0 100644 #ifdef V8_ENABLE_WASM_SIMD256_REVEC void set_wasm_revec_verifier_for_test( -@@ -2764,8 +2774,10 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory { +@@ -2769,9 +2779,10 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory { + int current_thread_counter_ = 0; #endif - Tagged promise_context_tag_; +- Tagged promise_context_tag_; - PromiseCrossContextCallback promise_cross_context_callback_; + PromiseCrossContextCallback promise_cross_context_callback_ = nullptr; + PromiseCrossContextResolveCallback promise_cross_context_resolve_callback_ = nullptr; @@ -212,6 +264,34 @@ index 59926e68e97..2bd24b914b0 100644 class PromiseCrossContextCallbackScope; +diff --git a/src/heap/factory.cc b/src/heap/factory.cc +index c0c46447c2f..47ef01bae12 100644 +--- a/src/heap/factory.cc ++++ b/src/heap/factory.cc +@@ -4413,18 +4413,17 @@ Handle Factory::NewJSPromiseWithoutHook() { + Handle promise = + Cast(NewJSObject(isolate()->promise_function())); + DisallowGarbageCollection no_gc; +- Tagged raw = *promise; +- raw->set_reactions_or_result(Smi::zero(), SKIP_WRITE_BARRIER); ++ promise->set_reactions_or_result(Smi::zero(), SKIP_WRITE_BARRIER); + if (!isolate()->has_promise_context_tag()) { +- raw->set_context_tag(Smi::zero(), SKIP_WRITE_BARRIER); ++ promise->set_context_tag(Smi::zero(), SKIP_WRITE_BARRIER); + } else { +- raw->set_context_tag(isolate()->promise_context_tag()); ++ promise->set_context_tag(*isolate()->promise_context_tag()); + } + +- raw->set_flags(0); ++ promise->set_flags(0); + // TODO(v8) remove once embedder data slots are always zero-initialized. + InitEmbedderFields(*promise, Smi::zero()); +- DCHECK_EQ(raw->GetEmbedderFieldCount(), v8::Promise::kEmbedderFieldCount); ++ DCHECK_EQ(promise->GetEmbedderFieldCount(), v8::Promise::kEmbedderFieldCount); + return promise; + } + diff --git a/src/objects/js-promise.h b/src/objects/js-promise.h index e4c1e42c63e..87bebb70d0d 100644 --- a/src/objects/js-promise.h @@ -229,22 +309,22 @@ index e4c1e42c63e..87bebb70d0d 100644 // ES section #sec-triggerpromisereactions static Handle TriggerPromiseReactions(Isolate* isolate, diff --git a/src/objects/objects.cc b/src/objects/objects.cc -index 76e284e19a2..32ccc84d8f8 100644 +index 76e284e19a2..da98c30fb86 100644 --- a/src/objects/objects.cc +++ b/src/objects/objects.cc @@ -4878,6 +4878,21 @@ Handle JSPromise::Fulfill(DirectHandle promise, // 6. Set promise.[[PromiseState]] to "fulfilled". promise->set_status(Promise::kFulfilled); -+ Tagged obj = promise->context_tag(); ++ Handle obj(promise->context_tag(), isolate); + bool needs_promise_context_switch = -+ !(obj == Smi::zero() || -+ obj == isolate->promise_context_tag() || ++ !(*obj == Smi::zero() || ++ obj.is_identical_to(isolate->promise_context_tag()) || + !isolate->has_promise_context_resolve_callback()); + if (needs_promise_context_switch) { + isolate->RunPromiseCrossContextResolveCallback( + reinterpret_cast(isolate), -+ Handle(Cast(obj), isolate), ++ Cast(obj), + reactions, + value, + PromiseReaction::kFulfill); @@ -258,15 +338,15 @@ index 76e284e19a2..32ccc84d8f8 100644 isolate->ReportPromiseReject(promise, reason, kPromiseRejectWithNoHandler); } -+ Tagged obj = promise->context_tag(); ++ Handle obj(promise->context_tag(), isolate); + bool needs_promise_context_switch = -+ !(obj == Smi::zero() || -+ obj == isolate->promise_context_tag() || ++ !(*obj == Smi::zero() || ++ obj.is_identical_to(isolate->promise_context_tag()) || + !isolate->has_promise_context_resolve_callback()); + if (needs_promise_context_switch) { + isolate->RunPromiseCrossContextResolveCallback( + reinterpret_cast(isolate), -+ Handle(Cast(obj), isolate), ++ Cast(obj), + reactions, + reason, + PromiseReaction::kReject); @@ -292,10 +372,46 @@ index 76e284e19a2..32ccc84d8f8 100644 Handle JSPromise::TriggerPromiseReactions( Isolate* isolate, DirectHandle reactions, DirectHandle argument, PromiseReaction::Type type) { +diff --git a/src/roots/roots.h b/src/roots/roots.h +index 288dee65e04..51af59f2efa 100644 +--- a/src/roots/roots.h ++++ b/src/roots/roots.h +@@ -405,7 +405,8 @@ class RootVisitor; + V(FunctionTemplateInfo, error_stack_getter_fun_template, \ + ErrorStackGetterSharedFun) \ + V(FunctionTemplateInfo, error_stack_setter_fun_template, \ +- ErrorStackSetterSharedFun) ++ ErrorStackSetterSharedFun) \ ++ V(Object, promise_context_tag, PromiseContextTag) + + // Entries in this list are limited to Smis and are not visited during GC. + #define SMI_ROOT_LIST(V) \ diff --git a/src/runtime/runtime-promise.cc b/src/runtime/runtime-promise.cc -index 4fbdf90319c..bb4e9c7fe41 100644 +index 4fbdf90319c..2d065d55d86 100644 --- a/src/runtime/runtime-promise.cc +++ b/src/runtime/runtime-promise.cc +@@ -222,8 +222,8 @@ RUNTIME_FUNCTION(Runtime_PromiseContextInit) { + if (!isolate->has_promise_context_tag()) { + args.at(0)->set_context_tag(Smi::zero()); + } else { +- CHECK(!IsUndefined(isolate->promise_context_tag())); +- args.at(0)->set_context_tag(isolate->promise_context_tag()); ++ CHECK(!IsUndefined(*isolate->promise_context_tag())); ++ args.at(0)->set_context_tag(*isolate->promise_context_tag()); + } + return ReadOnlyRoots(isolate).undefined_value(); + } +@@ -237,8 +237,8 @@ RUNTIME_FUNCTION(Runtime_PromiseContextCheck) { + // If promise.context_tag() is strict equal to isolate.promise_context_tag(), + // or if the promise being checked does not have a context tag, we'll just return + // promise directly. +- Tagged obj = promise->context_tag(); +- if (obj == Smi::zero() || obj == isolate->promise_context_tag()) { ++ Handle obj(promise->context_tag(), isolate); ++ if (*obj == Smi::zero() || obj.is_identical_to(isolate->promise_context_tag())) { + return *promise; + } + @@ -251,5 +251,23 @@ RUNTIME_FUNCTION(Runtime_PromiseContextCheck) { return *result; } @@ -308,9 +424,9 @@ index 4fbdf90319c..bb4e9c7fe41 100644 + // or if the promise being checked does not have a context tag, or if the + // resolve callback has not been set, we'll just return false here to indicate + // that the default handling should be used. -+ Tagged obj = promise->context_tag(); -+ if (obj == Smi::zero() || -+ obj == isolate->promise_context_tag() || ++ Handle obj(promise->context_tag(), isolate); ++ if (*obj == Smi::zero() || ++ obj.is_identical_to(isolate->promise_context_tag()) || + !isolate->has_promise_context_resolve_callback()) { + return isolate->heap()->ToBoolean(false); + }