123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118119120121122123124125126127128129130131132133134135136137138139140141142143144145146147148149150151152153154155156157158159160161162163164165166167168169170171172173174175176177178179180181182183184185186187188189190191192193194195196197198199200201202203204205 |
- From 7b6fbcd0a6700db498ad55db046ecda92c8ee8c1 Mon Sep 17 00:00:00 2001
- From: Nikolaos Papaspyrou <nikolaos@chromium.org>
- Date: Sun, 29 Jan 2023 17:18:08 +0100
- Subject: [PATCH] Merge: [heap] Move the Stack object from ThreadLocalTop to
- Isolate
- This is just for nodejs, do not backmerge to 11.0.
- (cherry picked from commit 1e4b71d99fea5ea6bb4bf6420585a7819872bb0f)
- > Change-Id: I026a35af3bc6999a09b21f277756d4454c086343
- > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4152476
- > Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
- > Reviewed-by: Omer Katz <omerkatz@chromium.org>
- > Commit-Queue: Nikolaos Papaspyrou <nikolaos@chromium.org>
- > Cr-Commit-Position: refs/heads/main@{#85445}
- Stack information is thread-specific and, until now, it was stored in a
- field in ThreadLocalTop. This CL moves stack information to the isolate
- and makes sure to update the stack start whenever a main thread enters
- the isolate. At the same time, the Stack object is refactored and
- simplified.
- As a side effect, after removing the Stack object, ThreadLocalTop
- satisfies the std::standard_layout trait; this fixes some issues
- observed with different C++ compilers.
- Bug: v8:13630
- Bug: v8:13257
- Change-Id: I4be1f04fe90699e1a6e456dad3e0dd623851acce
- ---
- src/execution/isolate.cc | 36 +++++++++++++++----------------
- src/execution/isolate.h | 6 ++++++
- src/execution/thread-local-top.cc | 2 --
- src/execution/thread-local-top.h | 6 +-----
- src/heap/heap.cc | 4 +---
- 5 files changed, 25 insertions(+), 29 deletions(-)
- diff --git a/src/execution/isolate.cc b/src/execution/isolate.cc
- index 4edf364e0a..be4fd400d2 100644
- --- a/src/execution/isolate.cc
- +++ b/src/execution/isolate.cc
- @@ -3074,22 +3074,23 @@ void Isolate::AddSharedWasmMemory(Handle<WasmMemoryObject> memory_object) {
- void Isolate::RecordStackSwitchForScanning() {
- Object current = root(RootIndex::kActiveContinuation);
- DCHECK(!current.IsUndefined());
- - thread_local_top()->stack_.ClearStackSegments();
- - wasm::StackMemory* stack = Managed<wasm::StackMemory>::cast(
- - WasmContinuationObject::cast(current).stack())
- - .get()
- - .get();
- + stack().ClearStackSegments();
- + wasm::StackMemory* wasm_stack =
- + Managed<wasm::StackMemory>::cast(
- + WasmContinuationObject::cast(current).stack())
- + .get()
- + .get();
- current = WasmContinuationObject::cast(current).parent();
- - thread_local_top()->stack_.SetStackStart(
- - reinterpret_cast<void*>(stack->base()));
- + heap()->SetStackStart(reinterpret_cast<void*>(wasm_stack->base()));
- // We don't need to add all inactive stacks. Only the ones in the active chain
- // may contain cpp heap pointers.
- while (!current.IsUndefined()) {
- auto cont = WasmContinuationObject::cast(current);
- - auto* stack = Managed<wasm::StackMemory>::cast(cont.stack()).get().get();
- - thread_local_top()->stack_.AddStackSegment(
- - reinterpret_cast<const void*>(stack->base()),
- - reinterpret_cast<const void*>(stack->jmpbuf()->sp));
- + auto* wasm_stack =
- + Managed<wasm::StackMemory>::cast(cont.stack()).get().get();
- + stack().AddStackSegment(
- + reinterpret_cast<const void*>(wasm_stack->base()),
- + reinterpret_cast<const void*>(wasm_stack->jmpbuf()->sp));
- current = cont.parent();
- }
- }
- @@ -3377,20 +3378,13 @@ void Isolate::Delete(Isolate* isolate) {
- Isolate* saved_isolate = isolate->TryGetCurrent();
- SetIsolateThreadLocals(isolate, nullptr);
- isolate->set_thread_id(ThreadId::Current());
- - isolate->thread_local_top()->stack_ =
- - saved_isolate ? std::move(saved_isolate->thread_local_top()->stack_)
- - : ::heap::base::Stack(base::Stack::GetStackStart());
- + isolate->heap()->SetStackStart(base::Stack::GetStackStart());
-
- bool owns_shared_isolate = isolate->owns_shared_isolate_;
- Isolate* maybe_shared_isolate = isolate->shared_isolate_;
-
- isolate->Deinit();
-
- - // Restore the saved isolate's stack.
- - if (saved_isolate)
- - saved_isolate->thread_local_top()->stack_ =
- - std::move(isolate->thread_local_top()->stack_);
- -
- #ifdef DEBUG
- non_disposed_isolates_--;
- #endif // DEBUG
- @@ -4647,6 +4641,10 @@ bool Isolate::Init(SnapshotData* startup_snapshot_data,
- void Isolate::Enter() {
- Isolate* current_isolate = nullptr;
- PerIsolateThreadData* current_data = CurrentPerIsolateThreadData();
- +
- + // Set the stack start for the main thread that enters the isolate.
- + heap()->SetStackStart(base::Stack::GetStackStart());
- +
- if (current_data != nullptr) {
- current_isolate = current_data->isolate_;
- DCHECK_NOT_NULL(current_isolate);
- diff --git a/src/execution/isolate.h b/src/execution/isolate.h
- index a32f999fe5..1cb6e10661 100644
- --- a/src/execution/isolate.h
- +++ b/src/execution/isolate.h
- @@ -32,6 +32,7 @@
- #include "src/execution/stack-guard.h"
- #include "src/handles/handles.h"
- #include "src/handles/traced-handles.h"
- +#include "src/heap/base/stack.h"
- #include "src/heap/factory.h"
- #include "src/heap/heap.h"
- #include "src/heap/read-only-heap.h"
- @@ -2022,6 +2023,8 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
- SimulatorData* simulator_data() { return simulator_data_; }
- #endif
-
- + ::heap::base::Stack& stack() { return stack_; }
- +
- #ifdef V8_ENABLE_WEBASSEMBLY
- wasm::StackMemory*& wasm_stacks() { return wasm_stacks_; }
- // Update the thread local's Stack object so that it is aware of the new stack
- @@ -2520,6 +2523,9 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
- // The mutex only guards adding pages, the retrieval is signal safe.
- base::Mutex code_pages_mutex_;
-
- + // Stack information for the main thread.
- + ::heap::base::Stack stack_;
- +
- #ifdef V8_ENABLE_WEBASSEMBLY
- wasm::StackMemory* wasm_stacks_;
- #endif
- diff --git a/src/execution/thread-local-top.cc b/src/execution/thread-local-top.cc
- index 0d7071ddda..05cc20b8e4 100644
- --- a/src/execution/thread-local-top.cc
- +++ b/src/execution/thread-local-top.cc
- @@ -37,14 +37,12 @@ void ThreadLocalTop::Clear() {
- current_embedder_state_ = nullptr;
- failed_access_check_callback_ = nullptr;
- thread_in_wasm_flag_address_ = kNullAddress;
- - stack_ = ::heap::base::Stack();
- }
-
- void ThreadLocalTop::Initialize(Isolate* isolate) {
- Clear();
- isolate_ = isolate;
- thread_id_ = ThreadId::Current();
- - stack_.SetStackStart(base::Stack::GetStackStart());
- #if V8_ENABLE_WEBASSEMBLY
- thread_in_wasm_flag_address_ = reinterpret_cast<Address>(
- trap_handler::GetThreadInWasmThreadLocalAddress());
- diff --git a/src/execution/thread-local-top.h b/src/execution/thread-local-top.h
- index 43fec0a7df..989c817f31 100644
- --- a/src/execution/thread-local-top.h
- +++ b/src/execution/thread-local-top.h
- @@ -10,7 +10,6 @@
- #include "include/v8-unwinder.h"
- #include "src/common/globals.h"
- #include "src/execution/thread-id.h"
- -#include "src/heap/base/stack.h"
- #include "src/objects/contexts.h"
- #include "src/utils/utils.h"
-
- @@ -30,7 +29,7 @@ class ThreadLocalTop {
- // TODO(all): This is not particularly beautiful. We should probably
- // refactor this to really consist of just Addresses and 32-bit
- // integer fields.
- - static constexpr uint32_t kSizeInBytes = 30 * kSystemPointerSize;
- + static constexpr uint32_t kSizeInBytes = 25 * kSystemPointerSize;
-
- // Does early low-level initialization that does not depend on the
- // isolate being present.
- @@ -147,9 +146,6 @@ class ThreadLocalTop {
-
- // Address of the thread-local "thread in wasm" flag.
- Address thread_in_wasm_flag_address_;
- -
- - // Stack information.
- - ::heap::base::Stack stack_;
- };
-
- } // namespace internal
- diff --git a/src/heap/heap.cc b/src/heap/heap.cc
- index 51a90ddcab..b5722ab6ec 100644
- --- a/src/heap/heap.cc
- +++ b/src/heap/heap.cc
- @@ -5851,9 +5851,7 @@ void Heap::SetStackStart(void* stack_start) {
- stack().SetStackStart(stack_start);
- }
-
- -::heap::base::Stack& Heap::stack() {
- - return isolate_->thread_local_top()->stack_;
- -}
- +::heap::base::Stack& Heap::stack() { return isolate_->stack(); }
-
- void Heap::RegisterExternallyReferencedObject(Address* location) {
- Object object = TracedHandles::Mark(location, TracedHandles::MarkMode::kAll);
|