From 41d6cae46daa882df71acb6b982903a6c9810222 Mon Sep 17 00:00:00 2001 From: "sgjesse@chromium.org" Date: Thu, 5 Nov 2009 13:59:40 +0000 Subject: [PATCH] Fix issue 493: Infinite loop when debug break is set when entering function.apply. In the generated code for function.apply there was a loop checking the stack limit for interruption. This loop would call into the runtime system to handle interuption and keep running until there was no interruption. However if the interuption was debug break the runtime system would never clear the interruption as debug break is prevented in builtins are prevented and the assumption here was that returning with the debug break flag set would move execution forward. Renamed initial_jslimit and initial_climit to real_jslimit and real_climit. Renamed a few external references related to the stack limit as well. Exposed the real stack limit to generated code to make the stack check when entering function.apply use the real stack limit and not the stack limit which is changed to signal interruption. Added the real stack limit to the roots array. BUG=http://code.google.com/p/v8/issues/detail?id=493 TEST=cctest/test-debug/DebugBreakFunctionApply Review URL: http://codereview.chromium.org/345048 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@3229 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm/builtins-arm.cc | 38 +++++------------- src/arm/regexp-macro-assembler-arm.cc | 12 +++--- src/assembler.cc | 7 +++- src/assembler.h | 5 ++- src/execution.cc | 27 ++++++------- src/execution.h | 41 +++++++++++++------ src/heap.cc | 12 ++++-- src/heap.h | 9 +++-- src/ia32/builtins-ia32.cc | 39 +++++------------- src/ia32/codegen-ia32.cc | 6 +-- src/ia32/fast-codegen-ia32.cc | 6 +-- src/ia32/regexp-macro-assembler-ia32.cc | 12 +++--- src/serialize.cc | 38 ++++++++++-------- src/v8.cc | 2 +- src/x64/builtins-x64.cc | 41 +++++-------------- src/x64/regexp-macro-assembler-x64.cc | 12 +++--- test/cctest/test-debug.cc | 53 +++++++++++++++++++++++++ test/cctest/test-serialize.cc | 22 ++++++---- 18 files changed, 208 insertions(+), 174 deletions(-) diff --git a/src/arm/builtins-arm.cc b/src/arm/builtins-arm.cc index 6db554a77c..5689ddf01e 100644 --- a/src/arm/builtins-arm.cc +++ b/src/arm/builtins-arm.cc @@ -1029,44 +1029,24 @@ void Builtins::Generate_FunctionApply(MacroAssembler* masm) { __ push(r0); __ InvokeBuiltin(Builtins::APPLY_PREPARE, CALL_JS); - Label no_preemption, retry_preemption; - __ bind(&retry_preemption); - ExternalReference stack_guard_limit_address = - ExternalReference::address_of_stack_guard_limit(); - __ mov(r2, Operand(stack_guard_limit_address)); - __ ldr(r2, MemOperand(r2)); - __ cmp(sp, r2); - __ b(hi, &no_preemption); - - // We have encountered a preemption or stack overflow already before we push - // the array contents. Save r0 which is the Smi-tagged length of the array. - __ push(r0); - - // Runtime routines expect at least one argument, so give it a Smi. - __ mov(r0, Operand(Smi::FromInt(0))); - __ push(r0); - __ CallRuntime(Runtime::kStackGuard, 1); - - // Since we returned, it wasn't a stack overflow. Restore r0 and try again. - __ pop(r0); - __ b(&retry_preemption); - - __ bind(&no_preemption); - - // Eagerly check for stack-overflow before starting to push the arguments. - // r0: number of arguments. - // r2: stack limit. + // Check the stack for overflow. We are not trying need to catch + // interruptions (e.g. debug break and preemption) here, so the "real stack + // limit" is checked. Label okay; + __ LoadRoot(r2, Heap::kRealStackLimitRootIndex); + // Make r2 the space we have left. The stack might already be overflowed + // here which will cause r2 to become negative. __ sub(r2, sp, r2); - + // Check if the arguments will overflow the stack. __ cmp(r2, Operand(r0, LSL, kPointerSizeLog2 - kSmiTagSize)); - __ b(hi, &okay); + __ b(gt, &okay); // Signed comparison. // Out of stack space. __ ldr(r1, MemOperand(fp, kFunctionOffset)); __ push(r1); __ push(r0); __ InvokeBuiltin(Builtins::APPLY_OVERFLOW, CALL_JS); + // End of stack check. // Push current limit and index. __ bind(&okay); diff --git a/src/arm/regexp-macro-assembler-arm.cc b/src/arm/regexp-macro-assembler-arm.cc index bd50428d8b..24b6a9c81a 100644 --- a/src/arm/regexp-macro-assembler-arm.cc +++ b/src/arm/regexp-macro-assembler-arm.cc @@ -588,9 +588,9 @@ Handle RegExpMacroAssemblerARM::GetCode(Handle source) { Label stack_limit_hit; Label stack_ok; - ExternalReference stack_guard_limit = - ExternalReference::address_of_stack_guard_limit(); - __ mov(r0, Operand(stack_guard_limit)); + ExternalReference stack_limit = + ExternalReference::address_of_stack_limit(); + __ mov(r0, Operand(stack_limit)); __ ldr(r0, MemOperand(r0)); __ sub(r0, sp, r0, SetCC); // Handle it if the stack pointer is already below the stack limit. @@ -1090,9 +1090,9 @@ void RegExpMacroAssemblerARM::Pop(Register target) { void RegExpMacroAssemblerARM::CheckPreemption() { // Check for preemption. - ExternalReference stack_guard_limit = - ExternalReference::address_of_stack_guard_limit(); - __ mov(r0, Operand(stack_guard_limit)); + ExternalReference stack_limit = + ExternalReference::address_of_stack_limit(); + __ mov(r0, Operand(stack_limit)); __ ldr(r0, MemOperand(r0)); __ cmp(sp, r0); SafeCall(&check_preempt_label_, ls); diff --git a/src/assembler.cc b/src/assembler.cc index 34346a9105..1620fef00c 100644 --- a/src/assembler.cc +++ b/src/assembler.cc @@ -583,11 +583,16 @@ ExternalReference ExternalReference::roots_address() { } -ExternalReference ExternalReference::address_of_stack_guard_limit() { +ExternalReference ExternalReference::address_of_stack_limit() { return ExternalReference(StackGuard::address_of_jslimit()); } +ExternalReference ExternalReference::address_of_real_stack_limit() { + return ExternalReference(StackGuard::address_of_real_jslimit()); +} + + ExternalReference ExternalReference::address_of_regexp_stack_limit() { return ExternalReference(RegExpStack::limit_address()); } diff --git a/src/assembler.h b/src/assembler.h index 311dadd53c..434e3bccde 100644 --- a/src/assembler.h +++ b/src/assembler.h @@ -408,7 +408,10 @@ class ExternalReference BASE_EMBEDDED { static ExternalReference roots_address(); // Static variable StackGuard::address_of_jslimit() - static ExternalReference address_of_stack_guard_limit(); + static ExternalReference address_of_stack_limit(); + + // Static variable StackGuard::address_of_real_jslimit() + static ExternalReference address_of_real_stack_limit(); // Static variable RegExpStack::limit_address() static ExternalReference address_of_regexp_stack_limit(); diff --git a/src/execution.cc b/src/execution.cc index 7b4ff8b175..2f646a5638 100644 --- a/src/execution.cc +++ b/src/execution.cc @@ -227,15 +227,14 @@ void StackGuard::SetStackLimit(uintptr_t limit) { // If the current limits are special (eg due to a pending interrupt) then // leave them alone. uintptr_t jslimit = SimulatorStack::JsLimitFromCLimit(limit); - if (thread_local_.jslimit_ == thread_local_.initial_jslimit_) { + if (thread_local_.jslimit_ == thread_local_.real_jslimit_) { thread_local_.jslimit_ = jslimit; - Heap::SetStackLimit(jslimit); } - if (thread_local_.climit_ == thread_local_.initial_climit_) { + if (thread_local_.climit_ == thread_local_.real_climit_) { thread_local_.climit_ = limit; } - thread_local_.initial_climit_ = limit; - thread_local_.initial_jslimit_ = jslimit; + thread_local_.real_climit_ = limit; + thread_local_.real_jslimit_ = jslimit; } @@ -344,7 +343,7 @@ char* StackGuard::ArchiveStackGuard(char* to) { char* StackGuard::RestoreStackGuard(char* from) { ExecutionAccess access; memcpy(reinterpret_cast(&thread_local_), from, sizeof(ThreadLocal)); - Heap::SetStackLimit(thread_local_.jslimit_); + Heap::SetStackLimits(); return from + sizeof(ThreadLocal); } @@ -356,33 +355,33 @@ static internal::Thread::LocalStorageKey stack_limit_key = void StackGuard::FreeThreadResources() { Thread::SetThreadLocal( stack_limit_key, - reinterpret_cast(thread_local_.initial_climit_)); + reinterpret_cast(thread_local_.real_climit_)); } void StackGuard::ThreadLocal::Clear() { - initial_jslimit_ = kIllegalLimit; + real_jslimit_ = kIllegalLimit; jslimit_ = kIllegalLimit; - initial_climit_ = kIllegalLimit; + real_climit_ = kIllegalLimit; climit_ = kIllegalLimit; nesting_ = 0; postpone_interrupts_nesting_ = 0; interrupt_flags_ = 0; - Heap::SetStackLimit(kIllegalLimit); + Heap::SetStackLimits(); } void StackGuard::ThreadLocal::Initialize() { - if (initial_climit_ == kIllegalLimit) { + if (real_climit_ == kIllegalLimit) { // Takes the address of the limit variable in order to find out where // the top of stack is right now. uintptr_t limit = reinterpret_cast(&limit) - kLimitSize; ASSERT(reinterpret_cast(&limit) > kLimitSize); - initial_jslimit_ = SimulatorStack::JsLimitFromCLimit(limit); + real_jslimit_ = SimulatorStack::JsLimitFromCLimit(limit); jslimit_ = SimulatorStack::JsLimitFromCLimit(limit); - initial_climit_ = limit; + real_climit_ = limit; climit_ = limit; - Heap::SetStackLimit(SimulatorStack::JsLimitFromCLimit(limit)); + Heap::SetStackLimits(); } nesting_ = 0; postpone_interrupts_nesting_ = 0; diff --git a/src/execution.h b/src/execution.h index ac00aa46fd..52198c420d 100644 --- a/src/execution.h +++ b/src/execution.h @@ -150,10 +150,6 @@ class StackGuard : public AllStatic { // is assumed to grow downwards. static void SetStackLimit(uintptr_t limit); - static Address address_of_jslimit() { - return reinterpret_cast
(&thread_local_.jslimit_); - } - // Threading support. static char* ArchiveStackGuard(char* to); static char* RestoreStackGuard(char* from); @@ -181,16 +177,24 @@ class StackGuard : public AllStatic { #endif static void Continue(InterruptFlag after_what); - // This provides an asynchronous read of the stack limit for the current + // This provides an asynchronous read of the stack limits for the current // thread. There are no locks protecting this, but it is assumed that you // have the global V8 lock if you are using multiple V8 threads. static uintptr_t climit() { return thread_local_.climit_; } - static uintptr_t jslimit() { return thread_local_.jslimit_; } + static uintptr_t real_jslimit() { + return thread_local_.real_jslimit_; + } + static Address address_of_jslimit() { + return reinterpret_cast
(&thread_local_.jslimit_); + } + static Address address_of_real_jslimit() { + return reinterpret_cast
(&thread_local_.real_jslimit_); + } private: // You should hold the ExecutionAccess lock when calling this method. @@ -198,17 +202,17 @@ class StackGuard : public AllStatic { // You should hold the ExecutionAccess lock when calling this method. static void set_limits(uintptr_t value, const ExecutionAccess& lock) { - Heap::SetStackLimit(value); thread_local_.jslimit_ = value; thread_local_.climit_ = value; + Heap::SetStackLimits(); } - // Reset limits to initial values. For example after handling interrupt. + // Reset limits to actual values. For example after handling interrupt. // You should hold the ExecutionAccess lock when calling this method. static void reset_limits(const ExecutionAccess& lock) { - thread_local_.jslimit_ = thread_local_.initial_jslimit_; - Heap::SetStackLimit(thread_local_.jslimit_); - thread_local_.climit_ = thread_local_.initial_climit_; + thread_local_.jslimit_ = thread_local_.real_jslimit_; + thread_local_.climit_ = thread_local_.real_climit_; + Heap::SetStackLimits(); } // Enable or disable interrupts. @@ -232,10 +236,21 @@ class StackGuard : public AllStatic { // Clear. void Initialize(); void Clear(); - uintptr_t initial_jslimit_; + + // The stack limit is split into a JavaScript and a C++ stack limit. These + // two are the same except when running on a simulator where the C++ and + // JavaScript stacks are separate. Each of the two stack limits have two + // values. The one eith the real_ prefix is the actual stack limit + // set for the VM. The one without the real_ prefix has the same value as + // the actual stack limit except when there is an interruption (e.g. debug + // break or preemption) in which case it is lowered to make stack checks + // fail. Both the generated code and the runtime system check against the + // one without the real_ prefix. + uintptr_t real_jslimit_; // Actual JavaScript stack limit set for the VM. uintptr_t jslimit_; - uintptr_t initial_climit_; + uintptr_t real_climit_; // Actual C++ stack limit set for the VM. uintptr_t climit_; + int nesting_; int postpone_interrupts_nesting_; int interrupt_flags_; diff --git a/src/heap.cc b/src/heap.cc index ae18fbe6b7..23e4b3b8fc 100644 --- a/src/heap.cc +++ b/src/heap.cc @@ -3455,14 +3455,18 @@ bool Heap::Setup(bool create_heap_objects) { } -void Heap::SetStackLimit(intptr_t limit) { +void Heap::SetStackLimits() { // On 64 bit machines, pointers are generally out of range of Smis. We write // something that looks like an out of range Smi to the GC. - // Set up the special root array entry containing the stack guard. - // This is actually an address, but the tag makes the GC ignore it. + // Set up the special root array entries containing the stack limits. + // These are actually addresses, but the tag makes the GC ignore it. roots_[kStackLimitRootIndex] = - reinterpret_cast((limit & ~kSmiTagMask) | kSmiTag); + reinterpret_cast( + (StackGuard::jslimit() & ~kSmiTagMask) | kSmiTag); + roots_[kRealStackLimitRootIndex] = + reinterpret_cast( + (StackGuard::real_jslimit() & ~kSmiTagMask) | kSmiTag); } diff --git a/src/heap.h b/src/heap.h index 285260594f..6639e641d9 100644 --- a/src/heap.h +++ b/src/heap.h @@ -148,6 +148,7 @@ namespace internal { V(FixedArray, single_character_string_cache, SingleCharacterStringCache) \ V(FixedArray, natives_source_cache, NativesSourceCache) \ V(Object, last_script_id, LastScriptId) \ + V(Smi, real_stack_limit, RealStackLimit) \ #if V8_TARGET_ARCH_ARM && V8_NATIVE_REGEXP #define STRONG_ROOT_LIST(V) \ @@ -250,10 +251,10 @@ class Heap : public AllStatic { // Destroys all memory allocated by the heap. static void TearDown(); - // Sets the stack limit in the roots_ array. Some architectures generate code - // that looks here, because it is faster than loading from the static jslimit_ - // variable. - static void SetStackLimit(intptr_t limit); + // Set the stack limit in the roots_ array. Some architectures generate + // code that looks here, because it is faster than loading from the static + // jslimit_/real_jslimit_ variable in the StackGuard. + static void SetStackLimits(); // Returns whether Setup has been called. static bool HasBeenSetup(); diff --git a/src/ia32/builtins-ia32.cc b/src/ia32/builtins-ia32.cc index 963b0e3ac8..ee4d1149a9 100644 --- a/src/ia32/builtins-ia32.cc +++ b/src/ia32/builtins-ia32.cc @@ -522,43 +522,26 @@ void Builtins::Generate_FunctionApply(MacroAssembler* masm) { __ push(Operand(ebp, 2 * kPointerSize)); // push arguments __ InvokeBuiltin(Builtins::APPLY_PREPARE, CALL_FUNCTION); - // Check the stack for overflow or a break request. - // We need to catch preemptions right here, otherwise an unlucky preemption - // could show up as a failed apply. - ExternalReference stack_guard_limit = - ExternalReference::address_of_stack_guard_limit(); - Label retry_preemption; - Label no_preemption; - __ bind(&retry_preemption); - __ mov(edi, Operand::StaticVariable(stack_guard_limit)); - __ cmp(esp, Operand(edi)); - __ j(above, &no_preemption, taken); - - // Preemption! - // Because builtins always remove the receiver from the stack, we - // have to fake one to avoid underflowing the stack. - __ push(eax); - __ push(Immediate(Smi::FromInt(0))); - - // Do call to runtime routine. - __ CallRuntime(Runtime::kStackGuard, 1); - __ pop(eax); - __ jmp(&retry_preemption); - - __ bind(&no_preemption); - + // Check the stack for overflow. We are not trying need to catch + // interruptions (e.g. debug break and preemption) here, so the "real stack + // limit" is checked. Label okay; - // Make ecx the space we have left. + ExternalReference real_stack_limit = + ExternalReference::address_of_real_stack_limit(); + __ mov(edi, Operand::StaticVariable(real_stack_limit)); + // Make ecx the space we have left. The stack might already be overflowed + // here which will cause ecx to become negative. __ mov(ecx, Operand(esp)); __ sub(ecx, Operand(edi)); // Make edx the space we need for the array when it is unrolled onto the // stack. __ mov(edx, Operand(eax)); __ shl(edx, kPointerSizeLog2 - kSmiTagSize); + // Check if the arguments will overflow the stack. __ cmp(ecx, Operand(edx)); - __ j(greater, &okay, taken); + __ j(greater, &okay, taken); // Signed comparison. - // Too bad: Out of stack space. + // Out of stack space. __ push(Operand(ebp, 4 * kPointerSize)); // push this __ push(eax); __ InvokeBuiltin(Builtins::APPLY_OVERFLOW, CALL_FUNCTION); diff --git a/src/ia32/codegen-ia32.cc b/src/ia32/codegen-ia32.cc index cc05932a15..d49718bfc6 100644 --- a/src/ia32/codegen-ia32.cc +++ b/src/ia32/codegen-ia32.cc @@ -2181,9 +2181,9 @@ void DeferredStackCheck::Generate() { void CodeGenerator::CheckStack() { DeferredStackCheck* deferred = new DeferredStackCheck; - ExternalReference stack_guard_limit = - ExternalReference::address_of_stack_guard_limit(); - __ cmp(esp, Operand::StaticVariable(stack_guard_limit)); + ExternalReference stack_limit = + ExternalReference::address_of_stack_limit(); + __ cmp(esp, Operand::StaticVariable(stack_limit)); deferred->Branch(below); deferred->BindExit(); } diff --git a/src/ia32/fast-codegen-ia32.cc b/src/ia32/fast-codegen-ia32.cc index 57b5646ba9..638f90e497 100644 --- a/src/ia32/fast-codegen-ia32.cc +++ b/src/ia32/fast-codegen-ia32.cc @@ -91,9 +91,9 @@ void FastCodeGenerator::Generate(FunctionLiteral* fun) { { Comment cmnt(masm_, "[ Stack check"); Label ok; - ExternalReference stack_guard_limit = - ExternalReference::address_of_stack_guard_limit(); - __ cmp(esp, Operand::StaticVariable(stack_guard_limit)); + ExternalReference stack_limit = + ExternalReference::address_of_stack_limit(); + __ cmp(esp, Operand::StaticVariable(stack_limit)); __ j(above_equal, &ok, taken); StackCheckStub stub; __ CallStub(&stub); diff --git a/src/ia32/regexp-macro-assembler-ia32.cc b/src/ia32/regexp-macro-assembler-ia32.cc index 76d36a939c..2e13d8aeed 100644 --- a/src/ia32/regexp-macro-assembler-ia32.cc +++ b/src/ia32/regexp-macro-assembler-ia32.cc @@ -598,10 +598,10 @@ Handle RegExpMacroAssemblerIA32::GetCode(Handle source) { Label stack_limit_hit; Label stack_ok; - ExternalReference stack_guard_limit = - ExternalReference::address_of_stack_guard_limit(); + ExternalReference stack_limit = + ExternalReference::address_of_stack_limit(); __ mov(ecx, esp); - __ sub(ecx, Operand::StaticVariable(stack_guard_limit)); + __ sub(ecx, Operand::StaticVariable(stack_limit)); // Handle it if the stack pointer is already below the stack limit. __ j(below_equal, &stack_limit_hit, not_taken); // Check if there is room for the variable number of registers above @@ -1081,9 +1081,9 @@ void RegExpMacroAssemblerIA32::Pop(Register target) { void RegExpMacroAssemblerIA32::CheckPreemption() { // Check for preemption. Label no_preempt; - ExternalReference stack_guard_limit = - ExternalReference::address_of_stack_guard_limit(); - __ cmp(esp, Operand::StaticVariable(stack_guard_limit)); + ExternalReference stack_limit = + ExternalReference::address_of_stack_limit(); + __ cmp(esp, Operand::StaticVariable(stack_limit)); __ j(above, &no_preempt, taken); SafeCall(&check_preempt_label_); diff --git a/src/serialize.cc b/src/serialize.cc index 3c333d0688..17ed7b4a12 100644 --- a/src/serialize.cc +++ b/src/serialize.cc @@ -688,76 +688,80 @@ void ExternalReferenceTable::PopulateTable() { UNCLASSIFIED, 3, "Heap::roots_address()"); - Add(ExternalReference::address_of_stack_guard_limit().address(), + Add(ExternalReference::address_of_stack_limit().address(), UNCLASSIFIED, 4, "StackGuard::address_of_jslimit()"); - Add(ExternalReference::address_of_regexp_stack_limit().address(), + Add(ExternalReference::address_of_real_stack_limit().address(), UNCLASSIFIED, 5, + "StackGuard::address_of_real_jslimit()"); + Add(ExternalReference::address_of_regexp_stack_limit().address(), + UNCLASSIFIED, + 6, "RegExpStack::limit_address()"); Add(ExternalReference::new_space_start().address(), UNCLASSIFIED, - 6, + 7, "Heap::NewSpaceStart()"); Add(ExternalReference::heap_always_allocate_scope_depth().address(), UNCLASSIFIED, - 7, + 8, "Heap::always_allocate_scope_depth()"); Add(ExternalReference::new_space_allocation_limit_address().address(), UNCLASSIFIED, - 8, + 9, "Heap::NewSpaceAllocationLimitAddress()"); Add(ExternalReference::new_space_allocation_top_address().address(), UNCLASSIFIED, - 9, + 10, "Heap::NewSpaceAllocationTopAddress()"); #ifdef ENABLE_DEBUGGER_SUPPORT Add(ExternalReference::debug_break().address(), UNCLASSIFIED, - 10, + 11, "Debug::Break()"); Add(ExternalReference::debug_step_in_fp_address().address(), UNCLASSIFIED, - 11, + 12, "Debug::step_in_fp_addr()"); #endif Add(ExternalReference::double_fp_operation(Token::ADD).address(), UNCLASSIFIED, - 12, + 13, "add_two_doubles"); Add(ExternalReference::double_fp_operation(Token::SUB).address(), UNCLASSIFIED, - 13, + 14, "sub_two_doubles"); Add(ExternalReference::double_fp_operation(Token::MUL).address(), UNCLASSIFIED, - 14, + 15, "mul_two_doubles"); Add(ExternalReference::double_fp_operation(Token::DIV).address(), UNCLASSIFIED, - 15, + 16, "div_two_doubles"); Add(ExternalReference::double_fp_operation(Token::MOD).address(), UNCLASSIFIED, - 16, + 17, "mod_two_doubles"); Add(ExternalReference::compare_doubles().address(), UNCLASSIFIED, - 17, + 18, "compare_doubles"); #ifdef V8_NATIVE_REGEXP Add(ExternalReference::re_case_insensitive_compare_uc16().address(), UNCLASSIFIED, - 18, + 19, "NativeRegExpMacroAssembler::CaseInsensitiveCompareUC16()"); Add(ExternalReference::re_check_stack_guard_state().address(), UNCLASSIFIED, - 19, + 20, "RegExpMacroAssembler*::CheckStackGuardState()"); Add(ExternalReference::re_grow_stack().address(), UNCLASSIFIED, - 20, + 21, "NativeRegExpMacroAssembler::GrowStack()"); #endif } diff --git a/src/v8.cc b/src/v8.cc index 1646ab7cad..c216e37566 100644 --- a/src/v8.cc +++ b/src/v8.cc @@ -105,7 +105,7 @@ bool V8::Initialize(GenericDeserializer *des) { // Deserializing may put strange things in the root array's copy of the // stack guard. - Heap::SetStackLimit(StackGuard::jslimit()); + Heap::SetStackLimits(); // Setup the CPU support. Must be done after heap setup and after // any deserialization because we have to have the initial heap diff --git a/src/x64/builtins-x64.cc b/src/x64/builtins-x64.cc index 8590365a17..f444d2cf85 100644 --- a/src/x64/builtins-x64.cc +++ b/src/x64/builtins-x64.cc @@ -320,42 +320,23 @@ void Builtins::Generate_FunctionApply(MacroAssembler* masm) { __ push(Operand(rbp, kArgumentsOffset)); __ InvokeBuiltin(Builtins::APPLY_PREPARE, CALL_FUNCTION); - // Check the stack for overflow or a break request. - // We need to catch preemptions right here, otherwise an unlucky preemption - // could show up as a failed apply. - Label retry_preemption; - Label no_preemption; - __ bind(&retry_preemption); - ExternalReference stack_guard_limit = - ExternalReference::address_of_stack_guard_limit(); - __ movq(kScratchRegister, stack_guard_limit); - __ movq(rcx, rsp); - __ subq(rcx, Operand(kScratchRegister, 0)); - // rcx contains the difference between the stack limit and the stack top. - // We use it below to check that there is enough room for the arguments. - __ j(above, &no_preemption); - - // Preemption! - // Because runtime functions always remove the receiver from the stack, we - // have to fake one to avoid underflowing the stack. - __ push(rax); - __ Push(Smi::FromInt(0)); - - // Do call to runtime routine. - __ CallRuntime(Runtime::kStackGuard, 1); - __ pop(rax); - __ jmp(&retry_preemption); - - __ bind(&no_preemption); - + // Check the stack for overflow. We are not trying need to catch + // interruptions (e.g. debug break and preemption) here, so the "real stack + // limit" is checked. Label okay; + __ LoadRoot(kScratchRegister, Heap::kRealStackLimitRootIndex); + __ movq(rcx, rsp); + // Make rcx the space we have left. The stack might already be overflowed + // here which will cause rcx to become negative. + __ subq(rcx, kScratchRegister); // Make rdx the space we need for the array when it is unrolled onto the // stack. __ PositiveSmiTimesPowerOfTwoToInteger64(rdx, rax, kPointerSizeLog2); + // Check if the arguments will overflow the stack. __ cmpq(rcx, rdx); - __ j(greater, &okay); + __ j(greater, &okay); // Signed comparison. - // Too bad: Out of stack space. + // Out of stack space. __ push(Operand(rbp, kFunctionOffset)); __ push(rax); __ InvokeBuiltin(Builtins::APPLY_OVERFLOW, CALL_FUNCTION); diff --git a/src/x64/regexp-macro-assembler-x64.cc b/src/x64/regexp-macro-assembler-x64.cc index 88636f843e..43cf9b9b4a 100644 --- a/src/x64/regexp-macro-assembler-x64.cc +++ b/src/x64/regexp-macro-assembler-x64.cc @@ -643,10 +643,10 @@ Handle RegExpMacroAssemblerX64::GetCode(Handle source) { Label stack_limit_hit; Label stack_ok; - ExternalReference stack_guard_limit = - ExternalReference::address_of_stack_guard_limit(); + ExternalReference stack_limit = + ExternalReference::address_of_stack_limit(); __ movq(rcx, rsp); - __ movq(kScratchRegister, stack_guard_limit); + __ movq(kScratchRegister, stack_limit); __ subq(rcx, Operand(kScratchRegister, 0)); // Handle it if the stack pointer is already below the stack limit. __ j(below_equal, &stack_limit_hit); @@ -1196,9 +1196,9 @@ void RegExpMacroAssemblerX64::Drop() { void RegExpMacroAssemblerX64::CheckPreemption() { // Check for preemption. Label no_preempt; - ExternalReference stack_guard_limit = - ExternalReference::address_of_stack_guard_limit(); - __ load_rax(stack_guard_limit); + ExternalReference stack_limit = + ExternalReference::address_of_stack_limit(); + __ load_rax(stack_limit); __ cmpq(rsp, rax); __ j(above, &no_preempt); diff --git a/test/cctest/test-debug.cc b/test/cctest/test-debug.cc index 656a456faa..7b2b91dfac 100644 --- a/test/cctest/test-debug.cc +++ b/test/cctest/test-debug.cc @@ -866,6 +866,26 @@ static void DebugEventBreak(v8::DebugEvent event, } +// Debug event handler which re-issues a debug break until a limit has been +// reached. +int max_break_point_hit_count = 0; +static void DebugEventBreakMax(v8::DebugEvent event, + v8::Handle exec_state, + v8::Handle event_data, + v8::Handle data) { + // When hitting a debug event listener there must be a break set. + CHECK_NE(v8::internal::Debug::break_id(), 0); + + if (event == v8::Break && break_point_hit_count < max_break_point_hit_count) { + // Count the number of breaks. + break_point_hit_count++; + + // Set the break flag again to come back here as soon as possible. + v8::Debug::DebugBreak(); + } +} + + // --- M e s s a g e C a l l b a c k @@ -5438,3 +5458,36 @@ TEST(GetMirror) { v8::Handle result = run_test->Call(env->Global(), 1, &obj); CHECK(result->IsTrue()); } + + +// Test that the debug break flag works with function.apply. +TEST(DebugBreakFunctionApply) { + v8::HandleScope scope; + DebugLocalContext env; + + // Create a function for testing breaking in apply. + v8::Local foo = CompileFunction( + &env, + "function baz(x) { }" + "function bar(x) { baz(); }" + "function foo(){ bar.apply(this, [1]); }", + "foo"); + + // Register a debug event listener which steps and counts. + v8::Debug::SetDebugEventListener(DebugEventBreakMax); + + // Set the debug break flag before calling the code using function.apply. + v8::Debug::DebugBreak(); + + // Limit the number of debug breaks. This is a regression test for issue 493 + // where this test would enter an infinite loop. + break_point_hit_count = 0; + max_break_point_hit_count = 10000; // 10000 => infinite loop. + foo->Call(env->Global(), 0, NULL); + + // When keeping the debug break several break will happen. + CHECK_EQ(3, break_point_hit_count); + + v8::Debug::SetDebugEventListener(NULL); + CheckDebuggerUnloaded(); +} diff --git a/test/cctest/test-serialize.cc b/test/cctest/test-serialize.cc index 2a58ac8747..4bf29ff14f 100644 --- a/test/cctest/test-serialize.cc +++ b/test/cctest/test-serialize.cc @@ -123,13 +123,17 @@ TEST(ExternalReferenceEncoder) { ExternalReference::the_hole_value_location(); CHECK_EQ(make_code(UNCLASSIFIED, 2), encoder.Encode(the_hole_value_location.address())); - ExternalReference stack_guard_limit_address = - ExternalReference::address_of_stack_guard_limit(); + ExternalReference stack_limit_address = + ExternalReference::address_of_stack_limit(); CHECK_EQ(make_code(UNCLASSIFIED, 4), - encoder.Encode(stack_guard_limit_address.address())); - CHECK_EQ(make_code(UNCLASSIFIED, 10), + encoder.Encode(stack_limit_address.address())); + ExternalReference real_stack_limit_address = + ExternalReference::address_of_real_stack_limit(); + CHECK_EQ(make_code(UNCLASSIFIED, 5), + encoder.Encode(real_stack_limit_address.address())); + CHECK_EQ(make_code(UNCLASSIFIED, 11), encoder.Encode(ExternalReference::debug_break().address())); - CHECK_EQ(make_code(UNCLASSIFIED, 6), + CHECK_EQ(make_code(UNCLASSIFIED, 7), encoder.Encode(ExternalReference::new_space_start().address())); CHECK_EQ(make_code(UNCLASSIFIED, 3), encoder.Encode(ExternalReference::roots_address().address())); @@ -158,12 +162,14 @@ TEST(ExternalReferenceDecoder) { decoder.Decode(make_code(UNCLASSIFIED, 1))); CHECK_EQ(ExternalReference::the_hole_value_location().address(), decoder.Decode(make_code(UNCLASSIFIED, 2))); - CHECK_EQ(ExternalReference::address_of_stack_guard_limit().address(), + CHECK_EQ(ExternalReference::address_of_stack_limit().address(), decoder.Decode(make_code(UNCLASSIFIED, 4))); + CHECK_EQ(ExternalReference::address_of_real_stack_limit().address(), + decoder.Decode(make_code(UNCLASSIFIED, 5))); CHECK_EQ(ExternalReference::debug_break().address(), - decoder.Decode(make_code(UNCLASSIFIED, 10))); + decoder.Decode(make_code(UNCLASSIFIED, 11))); CHECK_EQ(ExternalReference::new_space_start().address(), - decoder.Decode(make_code(UNCLASSIFIED, 6))); + decoder.Decode(make_code(UNCLASSIFIED, 7))); }