199 lines
7.1 KiB
Diff
199 lines
7.1 KiB
Diff
From 916641c08de44e0a915df223d55781c6c912717c Mon Sep 17 00:00:00 2001
|
|
From: Nikolaos Papaspyrou <nikolaos@chromium.org>
|
|
Date: Thu, 19 Jan 2023 13:43:10 +0100
|
|
Subject: [PATCH] [heap] Move the Stack object from ThreadLocalTop to Isolate
|
|
|
|
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: I026a35af3bc6999a09b21f277756d4454c086343
|
|
|
|
(stripped tests)
|
|
---
|
|
|
|
diff --git a/src/execution/isolate.cc b/src/execution/isolate.cc
|
|
index 31536a1..22b8492 100644
|
|
--- a/v8/src/execution/isolate.cc
|
|
+++ b/v8/src/execution/isolate.cc
|
|
@@ -3069,21 +3069,23 @@
|
|
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();
|
|
- heap()->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();
|
|
}
|
|
}
|
|
@@ -3371,23 +3373,13 @@
|
|
Isolate* saved_isolate = isolate->TryGetCurrent();
|
|
SetIsolateThreadLocals(isolate, nullptr);
|
|
isolate->set_thread_id(ThreadId::Current());
|
|
- if (saved_isolate) {
|
|
- isolate->thread_local_top()->stack_ =
|
|
- std::move(saved_isolate->thread_local_top()->stack_);
|
|
- } else {
|
|
- isolate->heap()->SetStackStart(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
|
|
@@ -4652,6 +4644,10 @@
|
|
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 afc3d0b..415e476 100644
|
|
--- a/v8/src/execution/isolate.h
|
|
+++ b/v8/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"
|
|
@@ -2028,6 +2029,8 @@
|
|
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
|
|
@@ -2526,6 +2529,9 @@
|
|
// 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 c115ae0..05cc20b 100644
|
|
--- a/v8/src/execution/thread-local-top.cc
|
|
+++ b/v8/src/execution/thread-local-top.cc
|
|
@@ -37,7 +37,6 @@
|
|
current_embedder_state_ = nullptr;
|
|
failed_access_check_callback_ = nullptr;
|
|
thread_in_wasm_flag_address_ = kNullAddress;
|
|
- stack_ = ::heap::base::Stack();
|
|
}
|
|
|
|
void ThreadLocalTop::Initialize(Isolate* isolate) {
|
|
@@ -45,12 +44,8 @@
|
|
isolate_ = isolate;
|
|
thread_id_ = ThreadId::Current();
|
|
#if V8_ENABLE_WEBASSEMBLY
|
|
- stack_.SetStackStart(base::Stack::GetStackStart(),
|
|
- v8_flags.experimental_wasm_stack_switching);
|
|
thread_in_wasm_flag_address_ = reinterpret_cast<Address>(
|
|
trap_handler::GetThreadInWasmThreadLocalAddress());
|
|
-#else
|
|
- stack_.SetStackStart(base::Stack::GetStackStart(), false);
|
|
#endif // V8_ENABLE_WEBASSEMBLY
|
|
#ifdef USE_SIMULATOR
|
|
simulator_ = Simulator::current(isolate);
|
|
diff --git a/src/execution/thread-local-top.h b/src/execution/thread-local-top.h
|
|
index 43fec0a..989c817 100644
|
|
--- a/v8/src/execution/thread-local-top.h
|
|
+++ b/v8/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 @@
|
|
// 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 @@
|
|
|
|
// 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 f4b7da0..6efd486 100644
|
|
--- a/v8/src/heap/heap.cc
|
|
+++ b/v8/src/heap/heap.cc
|
|
@@ -5830,9 +5830,7 @@
|
|
#endif // V8_ENABLE_WEBASSEMBLY
|
|
}
|
|
|
|
-::heap::base::Stack& Heap::stack() {
|
|
- return isolate_->thread_local_top()->stack_;
|
|
-}
|
|
+::heap::base::Stack& Heap::stack() { return isolate_->stack(); }
|
|
|
|
void Heap::StartTearDown() {
|
|
// Finish any ongoing sweeping to avoid stray background tasks still accessing
|