v8-move-the-Stack-object-from-ThreadLocalTop.patch 8.1 KB

123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118119120121122123124125126127128129130131132133134135136137138139140141142143144145146147148149150151152153154155156157158159160161162163164165166167168169170171172173174175176177178179180181182183184185186187188189190191192193194195196197198199200201202203204205
  1. From 7b6fbcd0a6700db498ad55db046ecda92c8ee8c1 Mon Sep 17 00:00:00 2001
  2. From: Nikolaos Papaspyrou <nikolaos@chromium.org>
  3. Date: Sun, 29 Jan 2023 17:18:08 +0100
  4. Subject: [PATCH] Merge: [heap] Move the Stack object from ThreadLocalTop to
  5. Isolate
  6. This is just for nodejs, do not backmerge to 11.0.
  7. (cherry picked from commit 1e4b71d99fea5ea6bb4bf6420585a7819872bb0f)
  8. > Change-Id: I026a35af3bc6999a09b21f277756d4454c086343
  9. > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4152476
  10. > Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
  11. > Reviewed-by: Omer Katz <omerkatz@chromium.org>
  12. > Commit-Queue: Nikolaos Papaspyrou <nikolaos@chromium.org>
  13. > Cr-Commit-Position: refs/heads/main@{#85445}
  14. Stack information is thread-specific and, until now, it was stored in a
  15. field in ThreadLocalTop. This CL moves stack information to the isolate
  16. and makes sure to update the stack start whenever a main thread enters
  17. the isolate. At the same time, the Stack object is refactored and
  18. simplified.
  19. As a side effect, after removing the Stack object, ThreadLocalTop
  20. satisfies the std::standard_layout trait; this fixes some issues
  21. observed with different C++ compilers.
  22. Bug: v8:13630
  23. Bug: v8:13257
  24. Change-Id: I4be1f04fe90699e1a6e456dad3e0dd623851acce
  25. ---
  26. src/execution/isolate.cc | 36 +++++++++++++++----------------
  27. src/execution/isolate.h | 6 ++++++
  28. src/execution/thread-local-top.cc | 2 --
  29. src/execution/thread-local-top.h | 6 +-----
  30. src/heap/heap.cc | 4 +---
  31. 5 files changed, 25 insertions(+), 29 deletions(-)
  32. diff --git a/src/execution/isolate.cc b/src/execution/isolate.cc
  33. index 4edf364e0a..be4fd400d2 100644
  34. --- a/src/execution/isolate.cc
  35. +++ b/src/execution/isolate.cc
  36. @@ -3074,22 +3074,23 @@ void Isolate::AddSharedWasmMemory(Handle<WasmMemoryObject> memory_object) {
  37. void Isolate::RecordStackSwitchForScanning() {
  38. Object current = root(RootIndex::kActiveContinuation);
  39. DCHECK(!current.IsUndefined());
  40. - thread_local_top()->stack_.ClearStackSegments();
  41. - wasm::StackMemory* stack = Managed<wasm::StackMemory>::cast(
  42. - WasmContinuationObject::cast(current).stack())
  43. - .get()
  44. - .get();
  45. + stack().ClearStackSegments();
  46. + wasm::StackMemory* wasm_stack =
  47. + Managed<wasm::StackMemory>::cast(
  48. + WasmContinuationObject::cast(current).stack())
  49. + .get()
  50. + .get();
  51. current = WasmContinuationObject::cast(current).parent();
  52. - thread_local_top()->stack_.SetStackStart(
  53. - reinterpret_cast<void*>(stack->base()));
  54. + heap()->SetStackStart(reinterpret_cast<void*>(wasm_stack->base()));
  55. // We don't need to add all inactive stacks. Only the ones in the active chain
  56. // may contain cpp heap pointers.
  57. while (!current.IsUndefined()) {
  58. auto cont = WasmContinuationObject::cast(current);
  59. - auto* stack = Managed<wasm::StackMemory>::cast(cont.stack()).get().get();
  60. - thread_local_top()->stack_.AddStackSegment(
  61. - reinterpret_cast<const void*>(stack->base()),
  62. - reinterpret_cast<const void*>(stack->jmpbuf()->sp));
  63. + auto* wasm_stack =
  64. + Managed<wasm::StackMemory>::cast(cont.stack()).get().get();
  65. + stack().AddStackSegment(
  66. + reinterpret_cast<const void*>(wasm_stack->base()),
  67. + reinterpret_cast<const void*>(wasm_stack->jmpbuf()->sp));
  68. current = cont.parent();
  69. }
  70. }
  71. @@ -3377,20 +3378,13 @@ void Isolate::Delete(Isolate* isolate) {
  72. Isolate* saved_isolate = isolate->TryGetCurrent();
  73. SetIsolateThreadLocals(isolate, nullptr);
  74. isolate->set_thread_id(ThreadId::Current());
  75. - isolate->thread_local_top()->stack_ =
  76. - saved_isolate ? std::move(saved_isolate->thread_local_top()->stack_)
  77. - : ::heap::base::Stack(base::Stack::GetStackStart());
  78. + isolate->heap()->SetStackStart(base::Stack::GetStackStart());
  79. bool owns_shared_isolate = isolate->owns_shared_isolate_;
  80. Isolate* maybe_shared_isolate = isolate->shared_isolate_;
  81. isolate->Deinit();
  82. - // Restore the saved isolate's stack.
  83. - if (saved_isolate)
  84. - saved_isolate->thread_local_top()->stack_ =
  85. - std::move(isolate->thread_local_top()->stack_);
  86. -
  87. #ifdef DEBUG
  88. non_disposed_isolates_--;
  89. #endif // DEBUG
  90. @@ -4647,6 +4641,10 @@ bool Isolate::Init(SnapshotData* startup_snapshot_data,
  91. void Isolate::Enter() {
  92. Isolate* current_isolate = nullptr;
  93. PerIsolateThreadData* current_data = CurrentPerIsolateThreadData();
  94. +
  95. + // Set the stack start for the main thread that enters the isolate.
  96. + heap()->SetStackStart(base::Stack::GetStackStart());
  97. +
  98. if (current_data != nullptr) {
  99. current_isolate = current_data->isolate_;
  100. DCHECK_NOT_NULL(current_isolate);
  101. diff --git a/src/execution/isolate.h b/src/execution/isolate.h
  102. index a32f999fe5..1cb6e10661 100644
  103. --- a/src/execution/isolate.h
  104. +++ b/src/execution/isolate.h
  105. @@ -32,6 +32,7 @@
  106. #include "src/execution/stack-guard.h"
  107. #include "src/handles/handles.h"
  108. #include "src/handles/traced-handles.h"
  109. +#include "src/heap/base/stack.h"
  110. #include "src/heap/factory.h"
  111. #include "src/heap/heap.h"
  112. #include "src/heap/read-only-heap.h"
  113. @@ -2022,6 +2023,8 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
  114. SimulatorData* simulator_data() { return simulator_data_; }
  115. #endif
  116. + ::heap::base::Stack& stack() { return stack_; }
  117. +
  118. #ifdef V8_ENABLE_WEBASSEMBLY
  119. wasm::StackMemory*& wasm_stacks() { return wasm_stacks_; }
  120. // Update the thread local's Stack object so that it is aware of the new stack
  121. @@ -2520,6 +2523,9 @@ class V8_EXPORT_PRIVATE Isolate final : private HiddenFactory {
  122. // The mutex only guards adding pages, the retrieval is signal safe.
  123. base::Mutex code_pages_mutex_;
  124. + // Stack information for the main thread.
  125. + ::heap::base::Stack stack_;
  126. +
  127. #ifdef V8_ENABLE_WEBASSEMBLY
  128. wasm::StackMemory* wasm_stacks_;
  129. #endif
  130. diff --git a/src/execution/thread-local-top.cc b/src/execution/thread-local-top.cc
  131. index 0d7071ddda..05cc20b8e4 100644
  132. --- a/src/execution/thread-local-top.cc
  133. +++ b/src/execution/thread-local-top.cc
  134. @@ -37,14 +37,12 @@ void ThreadLocalTop::Clear() {
  135. current_embedder_state_ = nullptr;
  136. failed_access_check_callback_ = nullptr;
  137. thread_in_wasm_flag_address_ = kNullAddress;
  138. - stack_ = ::heap::base::Stack();
  139. }
  140. void ThreadLocalTop::Initialize(Isolate* isolate) {
  141. Clear();
  142. isolate_ = isolate;
  143. thread_id_ = ThreadId::Current();
  144. - stack_.SetStackStart(base::Stack::GetStackStart());
  145. #if V8_ENABLE_WEBASSEMBLY
  146. thread_in_wasm_flag_address_ = reinterpret_cast<Address>(
  147. trap_handler::GetThreadInWasmThreadLocalAddress());
  148. diff --git a/src/execution/thread-local-top.h b/src/execution/thread-local-top.h
  149. index 43fec0a7df..989c817f31 100644
  150. --- a/src/execution/thread-local-top.h
  151. +++ b/src/execution/thread-local-top.h
  152. @@ -10,7 +10,6 @@
  153. #include "include/v8-unwinder.h"
  154. #include "src/common/globals.h"
  155. #include "src/execution/thread-id.h"
  156. -#include "src/heap/base/stack.h"
  157. #include "src/objects/contexts.h"
  158. #include "src/utils/utils.h"
  159. @@ -30,7 +29,7 @@ class ThreadLocalTop {
  160. // TODO(all): This is not particularly beautiful. We should probably
  161. // refactor this to really consist of just Addresses and 32-bit
  162. // integer fields.
  163. - static constexpr uint32_t kSizeInBytes = 30 * kSystemPointerSize;
  164. + static constexpr uint32_t kSizeInBytes = 25 * kSystemPointerSize;
  165. // Does early low-level initialization that does not depend on the
  166. // isolate being present.
  167. @@ -147,9 +146,6 @@ class ThreadLocalTop {
  168. // Address of the thread-local "thread in wasm" flag.
  169. Address thread_in_wasm_flag_address_;
  170. -
  171. - // Stack information.
  172. - ::heap::base::Stack stack_;
  173. };
  174. } // namespace internal
  175. diff --git a/src/heap/heap.cc b/src/heap/heap.cc
  176. index 51a90ddcab..b5722ab6ec 100644
  177. --- a/src/heap/heap.cc
  178. +++ b/src/heap/heap.cc
  179. @@ -5851,9 +5851,7 @@ void Heap::SetStackStart(void* stack_start) {
  180. stack().SetStackStart(stack_start);
  181. }
  182. -::heap::base::Stack& Heap::stack() {
  183. - return isolate_->thread_local_top()->stack_;
  184. -}
  185. +::heap::base::Stack& Heap::stack() { return isolate_->stack(); }
  186. void Heap::RegisterExternallyReferencedObject(Address* location) {
  187. Object object = TracedHandles::Mark(location, TracedHandles::MarkMode::kAll);