summarylogtreecommitdiffstats
path: root/v8-move-the-Stack-object-from-ThreadLocalTop.patch
blob: a060448342496162ddced4883b7780a7cf9ad723 (plain)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
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);