You can not select more than 25 topics Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
chromium/chromium-111-v8-std-layout2...

199 lines
7.1 KiB

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