diff --git a/include/v8.h b/include/v8.h index a541cbccc0..e3e83f95a5 100644 --- a/include/v8.h +++ b/include/v8.h @@ -129,7 +129,6 @@ class PropertyCallbackArguments; class FunctionCallbackArguments; class GlobalHandles; class ScopedExternalStringLock; -class ThreadLocalTop; namespace wasm { class NativeModule; @@ -7665,9 +7664,6 @@ class V8_EXPORT Isolate { private: internal::Isolate* const isolate_; internal::MicrotaskQueue* const microtask_queue_; - internal::Address previous_stack_height_; - - friend class internal::ThreadLocalTop; }; /** diff --git a/src/api/api.cc b/src/api/api.cc index 014e7e8cc4..6d8b5d7727 100644 --- a/src/api/api.cc +++ b/src/api/api.cc @@ -262,7 +262,7 @@ void CheckMicrotasksScopesConsistency(i::MicrotaskQueue* microtask_queue) { template class CallDepthScope { public: - CallDepthScope(i::Isolate* isolate, Local context) + explicit CallDepthScope(i::Isolate* isolate, Local context) : isolate_(isolate), context_(context), escaped_(false), @@ -273,7 +273,7 @@ class CallDepthScope { ? i::InterruptsScope::kRunInterrupts : i::InterruptsScope::kPostponeInterrupts) : i::InterruptsScope::kNoop) { - isolate_->thread_local_top()->IncrementCallDepth(this); + isolate_->handle_scope_implementer()->IncrementCallDepth(); isolate_->set_next_v8_call_is_safe_for_termination(false); if (!context.IsEmpty()) { i::Handle env = Utils::OpenHandle(*context); @@ -297,7 +297,7 @@ class CallDepthScope { i::Handle env = Utils::OpenHandle(*context_); microtask_queue = env->native_context().microtask_queue(); } - if (!escaped_) isolate_->thread_local_top()->DecrementCallDepth(this); + if (!escaped_) isolate_->handle_scope_implementer()->DecrementCallDepth(); if (do_callback) isolate_->FireCallCompletedCallback(microtask_queue); // TODO(jochen): This should be #ifdef DEBUG #ifdef V8_CHECK_MICROTASKS_SCOPES_CONSISTENCY @@ -309,10 +309,11 @@ class CallDepthScope { void Escape() { DCHECK(!escaped_); escaped_ = true; - auto thread_local_top = isolate_->thread_local_top(); - thread_local_top->DecrementCallDepth(this); - bool clear_exception = thread_local_top->CallDepthIsZero() && - thread_local_top->try_catch_handler_ == nullptr; + auto handle_scope_implementer = isolate_->handle_scope_implementer(); + handle_scope_implementer->DecrementCallDepth(); + bool clear_exception = + handle_scope_implementer->CallDepthIsZero() && + isolate_->thread_local_top()->try_catch_handler_ == nullptr; isolate_->OptionalRescheduleException(clear_exception); } @@ -323,12 +324,6 @@ class CallDepthScope { bool do_callback_; bool safe_for_termination_; i::InterruptsScope interrupts_scope_; - i::Address previous_stack_height_; - - friend class i::ThreadLocalTop; - - DISALLOW_NEW_AND_DELETE() - DISALLOW_COPY_AND_ASSIGN(CallDepthScope); }; } // namespace @@ -8073,13 +8068,13 @@ Isolate::SuppressMicrotaskExecutionScope::SuppressMicrotaskExecutionScope( Isolate* isolate) : isolate_(reinterpret_cast(isolate)), microtask_queue_(isolate_->default_microtask_queue()) { - isolate_->thread_local_top()->IncrementCallDepth(this); + isolate_->handle_scope_implementer()->IncrementCallDepth(); microtask_queue_->IncrementMicrotasksSuppressions(); } Isolate::SuppressMicrotaskExecutionScope::~SuppressMicrotaskExecutionScope() { microtask_queue_->DecrementMicrotasksSuppressions(); - isolate_->thread_local_top()->DecrementCallDepth(this); + isolate_->handle_scope_implementer()->DecrementCallDepth(); } Isolate::SafeForTerminationScope::SafeForTerminationScope(v8::Isolate* isolate) diff --git a/src/api/api.h b/src/api/api.h index 21bbb3a101..e88087e793 100644 --- a/src/api/api.h +++ b/src/api/api.h @@ -357,6 +357,7 @@ class HandleScopeImplementer { explicit HandleScopeImplementer(Isolate* isolate) : isolate_(isolate), spare_(nullptr), + call_depth_(0), last_handle_before_deferred_block_(nullptr) {} ~HandleScopeImplementer() { DeleteArray(spare_); } @@ -375,6 +376,11 @@ class HandleScopeImplementer { inline internal::Address* GetSpareOrNewBlock(); inline void DeleteExtensions(internal::Address* prev_limit); + // Call depth represents nested v8 api calls. + inline void IncrementCallDepth() { call_depth_++; } + inline void DecrementCallDepth() { call_depth_--; } + inline bool CallDepthIsZero() { return call_depth_ == 0; } + inline void EnterContext(Context context); inline void LeaveContext(); inline bool LastEnteredContextWas(Context context); @@ -411,6 +417,7 @@ class HandleScopeImplementer { saved_contexts_.detach(); spare_ = nullptr; last_handle_before_deferred_block_ = nullptr; + call_depth_ = 0; } void Free() { @@ -427,7 +434,7 @@ class HandleScopeImplementer { DeleteArray(spare_); spare_ = nullptr; } - DCHECK(isolate_->thread_local_top()->CallDepthIsZero()); + DCHECK_EQ(call_depth_, 0); } void BeginDeferredScope(); @@ -447,6 +454,8 @@ class HandleScopeImplementer { // Used as a stack to keep track of saved contexts. DetachableVector saved_contexts_; Address* spare_; + int call_depth_; + Address* last_handle_before_deferred_block_; // This is only used for threading support. HandleScopeData handle_scope_data_; diff --git a/src/debug/debug.h b/src/debug/debug.h index eef89f9372..684397400a 100644 --- a/src/debug/debug.h +++ b/src/debug/debug.h @@ -375,8 +375,6 @@ class V8_EXPORT_PRIVATE Debug { return thread_local_.break_on_next_function_call_; } - inline bool break_disabled() const { return break_disabled_; } - DebugFeatureTracker* feature_tracker() { return &feature_tracker_; } // For functions in which we cannot set a break point, use a canonical @@ -401,6 +399,7 @@ class V8_EXPORT_PRIVATE Debug { return is_suppressed_ || !is_active_ || isolate_->debug_execution_mode() == DebugInfo::kSideEffects; } + inline bool break_disabled() const { return break_disabled_; } void clear_suspended_generator() { thread_local_.suspended_generator_ = Smi::kZero; diff --git a/src/execution/isolate.cc b/src/execution/isolate.cc index 48bceb4300..bb622ca7e9 100644 --- a/src/execution/isolate.cc +++ b/src/execution/isolate.cc @@ -2023,7 +2023,7 @@ void Isolate::CancelScheduledExceptionFromTryCatch(v8::TryCatch* handler) { DCHECK_EQ(scheduled_exception(), ReadOnlyRoots(heap()).termination_exception()); // Clear termination once we returned from all V8 frames. - if (thread_local_top()->CallDepthIsZero()) { + if (handle_scope_implementer()->CallDepthIsZero()) { thread_local_top()->external_caught_exception_ = false; clear_scheduled_exception(); } @@ -4250,7 +4250,7 @@ void Isolate::RemoveCallCompletedCallback(CallCompletedCallback callback) { } void Isolate::FireCallCompletedCallback(MicrotaskQueue* microtask_queue) { - if (!thread_local_top()->CallDepthIsZero()) return; + if (!handle_scope_implementer()->CallDepthIsZero()) return; bool run_microtasks = microtask_queue && microtask_queue->size() && diff --git a/src/execution/thread-local-top.cc b/src/execution/thread-local-top.cc index cb69fb56ef..569333f276 100644 --- a/src/execution/thread-local-top.cc +++ b/src/execution/thread-local-top.cc @@ -26,15 +26,5 @@ void ThreadLocalTop::Free() { while (promise_on_stack_) isolate_->PopPromise(); } -#if defined(USE_SIMULATOR) -void ThreadLocalTop::StoreCurrentStackPosition() { - last_api_entry_ = simulator_->get_sp(); -} -#elif defined(V8_USE_ADDRESS_SANITIZER) -void ThreadLocalTop::StoreCurrentStackPosition() { - last_api_entry_ = reinterpret_cast
(GetCurrentStackPosition()); -} -#endif - } // namespace internal } // namespace v8 diff --git a/src/execution/thread-local-top.h b/src/execution/thread-local-top.h index 57166299c5..625fcc41dd 100644 --- a/src/execution/thread-local-top.h +++ b/src/execution/thread-local-top.h @@ -8,7 +8,6 @@ #include "src/common/globals.h" #include "src/execution/thread-id.h" #include "src/objects/contexts.h" -#include "src/utils/utils.h" namespace v8 { @@ -26,7 +25,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 = 24 * kSystemPointerSize; + static constexpr uint32_t kSizeInBytes = 23 * kSystemPointerSize; // Does early low-level initialization that does not depend on the // isolate being present. @@ -57,31 +56,6 @@ class ThreadLocalTop { v8::TryCatch::JSStackComparableAddress(try_catch_handler_)); } - // Call depth represents nested v8 api calls. Instead of storing the nesting - // level as an integer, we store the stack height of the last API entry. This - // additional information is used when we decide whether to trigger a debug - // break at a function entry. - template - void IncrementCallDepth(Scope* stack_allocated_scope) { - stack_allocated_scope->previous_stack_height_ = last_api_entry_; -#if defined(USE_SIMULATOR) || defined(V8_USE_ADDRESS_SANITIZER) - StoreCurrentStackPosition(); -#else - last_api_entry_ = reinterpret_cast(stack_allocated_scope); -#endif - } - -#if defined(USE_SIMULATOR) || defined(V8_USE_ADDRESS_SANITIZER) - void StoreCurrentStackPosition(); -#endif - - template - void DecrementCallDepth(Scope* stack_allocated_scope) { - last_api_entry_ = stack_allocated_scope->previous_stack_height_; - } - - bool CallDepthIsZero() const { return last_api_entry_ == kNullAddress; } - void Free(); Isolate* isolate_ = nullptr; @@ -103,8 +77,6 @@ class ThreadLocalTop { Address pending_handler_fp_ = kNullAddress; Address pending_handler_sp_ = kNullAddress; - Address last_api_entry_ = kNullAddress; - // Communication channel between Isolate::Throw and message consumers. Object pending_message_obj_; bool rethrowing_message_ = false; diff --git a/src/runtime/runtime-debug.cc b/src/runtime/runtime-debug.cc index 2d99bb9f03..94320740af 100644 --- a/src/runtime/runtime-debug.cc +++ b/src/runtime/runtime-debug.cc @@ -115,17 +115,10 @@ RUNTIME_FUNCTION(Runtime_DebugBreakAtEntry) { DCHECK(function->shared().HasDebugInfo()); DCHECK(function->shared().GetDebugInfo().BreakAtEntry()); - // Get the top-most JavaScript frame. This is the debug target function. + // Get the top-most JavaScript frame. JavaScriptFrameIterator it(isolate); DCHECK_EQ(*function, it.frame()->function()); - // Check whether the next JS frame is closer than the last API entry. - // if yes, then the call to the debug target came from JavaScript. Otherwise, - // the call to the debug target came from API. Do not break for the latter. - it.Advance(); - if (!it.done() && - it.frame()->fp() < isolate->thread_local_top()->last_api_entry_) { - isolate->debug()->Break(it.frame(), function); - } + isolate->debug()->Break(it.frame(), function); return ReadOnlyRoots(isolate).undefined_value(); } diff --git a/test/cctest/test-debug.cc b/test/cctest/test-debug.cc index d0a0fca858..16a73e6c6b 100644 --- a/test/cctest/test-debug.cc +++ b/test/cctest/test-debug.cc @@ -1097,15 +1097,14 @@ TEST(BreakPointApiFunction) { ExpectInt32("f()", 2); CHECK_EQ(2, break_point_hit_count); - // Direct call through API does not trigger breakpoint. function->Call(env.local(), v8::Undefined(env->GetIsolate()), 0, nullptr) .ToLocalChecked(); - CHECK_EQ(2, break_point_hit_count); + CHECK_EQ(3, break_point_hit_count); // Run without breakpoints. ClearBreakPoint(bp); ExpectInt32("f()", 2); - CHECK_EQ(2, break_point_hit_count); + CHECK_EQ(3, break_point_hit_count); v8::debug::SetDebugDelegate(env->GetIsolate(), nullptr); CheckDebuggerUnloaded(); @@ -1138,14 +1137,13 @@ TEST(BreakPointApiConstructor) { CompileRun("new f()"); CHECK_EQ(2, break_point_hit_count); - // Direct call through API does not trigger breakpoint. function->NewInstance(env.local()).ToLocalChecked(); - CHECK_EQ(2, break_point_hit_count); + CHECK_EQ(3, break_point_hit_count); // Run without breakpoints. ClearBreakPoint(bp); CompileRun("new f()"); - CHECK_EQ(2, break_point_hit_count); + CHECK_EQ(3, break_point_hit_count); v8::debug::SetDebugDelegate(env->GetIsolate(), nullptr); CheckDebuggerUnloaded(); @@ -1190,16 +1188,15 @@ TEST(BreakPointApiGetter) { // Run with breakpoint. bp = SetBreakPoint(function, 0); CompileRun("get_wrapper(o, 'f')"); - CHECK_EQ(0, break_point_hit_count); + CHECK_EQ(1, break_point_hit_count); CompileRun("o.f"); - CHECK_EQ(1, break_point_hit_count); + CHECK_EQ(2, break_point_hit_count); // Run without breakpoints. ClearBreakPoint(bp); CompileRun("get_wrapper(o, 'f', 2)"); - CompileRun("o.f"); - CHECK_EQ(1, break_point_hit_count); + CHECK_EQ(2, break_point_hit_count); v8::debug::SetDebugDelegate(env->GetIsolate(), nullptr); CheckDebuggerUnloaded(); @@ -1248,12 +1245,12 @@ TEST(BreakPointApiSetter) { CHECK_EQ(1, break_point_hit_count); CompileRun("set_wrapper(o, 'f', 2)"); - CHECK_EQ(1, break_point_hit_count); + CHECK_EQ(2, break_point_hit_count); // Run without breakpoints. ClearBreakPoint(bp); CompileRun("o.f = 3"); - CHECK_EQ(1, break_point_hit_count); + CHECK_EQ(2, break_point_hit_count); // === Test API builtin as setter, with condition === break_point_hit_count = 0; @@ -1264,16 +1261,15 @@ TEST(BreakPointApiSetter) { CHECK_EQ(0, break_point_hit_count); CompileRun("set_wrapper(o, 'f', 3)"); - CHECK_EQ(0, break_point_hit_count); + CHECK_EQ(1, break_point_hit_count); CompileRun("o.f = 3"); - CHECK_EQ(1, break_point_hit_count); + CHECK_EQ(2, break_point_hit_count); // Run without breakpoints. ClearBreakPoint(bp); CompileRun("set_wrapper(o, 'f', 2)"); - CompileRun("o.f = 3"); - CHECK_EQ(1, break_point_hit_count); + CHECK_EQ(2, break_point_hit_count); v8::debug::SetDebugDelegate(env->GetIsolate(), nullptr); CheckDebuggerUnloaded();