From 11368af66d54ce89eefea4445c868014a337b46a Mon Sep 17 00:00:00 2001 From: "yangguo@chromium.org" Date: Tue, 17 Jun 2014 13:54:49 +0000 Subject: [PATCH] Interrupts must not mask stack overflow. R=jarin@chromium.org BUG=385002 LOG=N Review URL: https://codereview.chromium.org/339883002 git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@21874 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/arm/full-codegen-arm.cc | 43 ++++++++------------ src/arm/regexp-macro-assembler-arm.cc | 3 +- src/arm64/full-codegen-arm64.cc | 40 +++++++----------- src/arm64/regexp-macro-assembler-arm64.cc | 3 +- src/execution.cc | 7 ---- src/execution.h | 7 +++- src/ia32/full-codegen-ia32.cc | 39 ++++++++---------- src/ia32/regexp-macro-assembler-ia32.cc | 3 +- src/isolate.cc | 12 ++++++ src/isolate.h | 11 +++-- src/runtime.cc | 6 ++- src/x64/full-codegen-x64.cc | 35 ++++++---------- src/x64/regexp-macro-assembler-x64.cc | 3 +- test/mjsunit/regress/regress-crbug-385002.js | 15 +++++++ 14 files changed, 111 insertions(+), 116 deletions(-) create mode 100644 test/mjsunit/regress/regress-crbug-385002.js diff --git a/src/arm/full-codegen-arm.cc b/src/arm/full-codegen-arm.cc index 241991cd4b..8b079eddd9 100644 --- a/src/arm/full-codegen-arm.cc +++ b/src/arm/full-codegen-arm.cc @@ -88,31 +88,6 @@ class JumpPatchSite BASE_EMBEDDED { }; -static void EmitStackCheck(MacroAssembler* masm_, - Register stack_limit_scratch, - int pointers = 0, - Register scratch = sp) { - Isolate* isolate = masm_->isolate(); - Label ok; - ASSERT(scratch.is(sp) == (pointers == 0)); - Heap::RootListIndex index; - if (pointers != 0) { - __ sub(scratch, sp, Operand(pointers * kPointerSize)); - index = Heap::kRealStackLimitRootIndex; - } else { - index = Heap::kStackLimitRootIndex; - } - __ LoadRoot(stack_limit_scratch, index); - __ cmp(scratch, Operand(stack_limit_scratch)); - __ b(hs, &ok); - Handle stack_check = isolate->builtins()->StackCheck(); - PredictableCodeSizeScope predictable(masm_, - masm_->CallSize(stack_check, RelocInfo::CODE_TARGET)); - __ Call(stack_check, RelocInfo::CODE_TARGET); - __ bind(&ok); -} - - // Generate code for a JS function. On entry to the function the receiver // and arguments have been pushed on the stack left to right. The actual // argument count matches the formal parameter count expected by the @@ -180,7 +155,13 @@ void FullCodeGenerator::Generate() { ASSERT(!info->function()->is_generator() || locals_count == 0); if (locals_count > 0) { if (locals_count >= 128) { - EmitStackCheck(masm_, r2, locals_count, r9); + Label ok; + __ sub(r9, sp, Operand(locals_count * kPointerSize)); + __ LoadRoot(r2, Heap::kRealStackLimitRootIndex); + __ cmp(r9, Operand(r2)); + __ b(hs, &ok); + __ InvokeBuiltin(Builtins::STACK_OVERFLOW, CALL_FUNCTION); + __ bind(&ok); } __ LoadRoot(r9, Heap::kUndefinedValueRootIndex); int kMaxPushes = FLAG_optimize_for_size ? 4 : 32; @@ -321,7 +302,15 @@ void FullCodeGenerator::Generate() { { Comment cmnt(masm_, "[ Stack check"); PrepareForBailoutForId(BailoutId::Declarations(), NO_REGISTERS); - EmitStackCheck(masm_, ip); + Label ok; + __ LoadRoot(ip, Heap::kStackLimitRootIndex); + __ cmp(sp, Operand(ip)); + __ b(hs, &ok); + Handle stack_check = isolate()->builtins()->StackCheck(); + PredictableCodeSizeScope predictable(masm_, + masm_->CallSize(stack_check, RelocInfo::CODE_TARGET)); + __ Call(stack_check, RelocInfo::CODE_TARGET); + __ bind(&ok); } { Comment cmnt(masm_, "[ Body"); diff --git a/src/arm/regexp-macro-assembler-arm.cc b/src/arm/regexp-macro-assembler-arm.cc index 7fa3ce7b0c..e494305ac0 100644 --- a/src/arm/regexp-macro-assembler-arm.cc +++ b/src/arm/regexp-macro-assembler-arm.cc @@ -1044,7 +1044,8 @@ int RegExpMacroAssemblerARM::CheckStackGuardState(Address* return_address, Code* re_code, Address re_frame) { Isolate* isolate = frame_entry(re_frame, kIsolate); - if (isolate->stack_guard()->IsStackOverflow()) { + StackLimitCheck check(isolate); + if (check.JsHasOverflowed()) { isolate->StackOverflow(); return EXCEPTION; } diff --git a/src/arm64/full-codegen-arm64.cc b/src/arm64/full-codegen-arm64.cc index b74a0583fd..4a44e35b05 100644 --- a/src/arm64/full-codegen-arm64.cc +++ b/src/arm64/full-codegen-arm64.cc @@ -87,29 +87,6 @@ class JumpPatchSite BASE_EMBEDDED { }; -static void EmitStackCheck(MacroAssembler* masm_, - int pointers = 0, - Register scratch = jssp) { - Isolate* isolate = masm_->isolate(); - Label ok; - ASSERT(jssp.Is(__ StackPointer())); - ASSERT(scratch.Is(jssp) == (pointers == 0)); - Heap::RootListIndex index; - if (pointers != 0) { - __ Sub(scratch, jssp, pointers * kPointerSize); - index = Heap::kRealStackLimitRootIndex; - } else { - index = Heap::kStackLimitRootIndex; - } - __ CompareRoot(scratch, index); - __ B(hs, &ok); - PredictableCodeSizeScope predictable(masm_, - Assembler::kCallSizeWithRelocation); - __ Call(isolate->builtins()->StackCheck(), RelocInfo::CODE_TARGET); - __ Bind(&ok); -} - - // Generate code for a JS function. On entry to the function the receiver // and arguments have been pushed on the stack left to right. The actual // argument count matches the formal parameter count expected by the @@ -181,7 +158,13 @@ void FullCodeGenerator::Generate() { if (locals_count > 0) { if (locals_count >= 128) { - EmitStackCheck(masm_, locals_count, x10); + Label ok; + ASSERT(jssp.Is(__ StackPointer())); + __ Sub(x10, jssp, locals_count * kPointerSize); + __ CompareRoot(x10, Heap::kRealStackLimitRootIndex); + __ B(hs, &ok); + __ InvokeBuiltin(Builtins::STACK_OVERFLOW, CALL_FUNCTION); + __ Bind(&ok); } __ LoadRoot(x10, Heap::kUndefinedValueRootIndex); if (FLAG_optimize_for_size) { @@ -319,7 +302,14 @@ void FullCodeGenerator::Generate() { { Comment cmnt(masm_, "[ Stack check"); PrepareForBailoutForId(BailoutId::Declarations(), NO_REGISTERS); - EmitStackCheck(masm_); + Label ok; + ASSERT(jssp.Is(__ StackPointer())); + __ CompareRoot(jssp, Heap::kStackLimitRootIndex); + __ B(hs, &ok); + PredictableCodeSizeScope predictable(masm_, + Assembler::kCallSizeWithRelocation); + __ Call(isolate()->builtins()->StackCheck(), RelocInfo::CODE_TARGET); + __ Bind(&ok); } { Comment cmnt(masm_, "[ Body"); diff --git a/src/arm64/regexp-macro-assembler-arm64.cc b/src/arm64/regexp-macro-assembler-arm64.cc index f2363525c5..a772ef2640 100644 --- a/src/arm64/regexp-macro-assembler-arm64.cc +++ b/src/arm64/regexp-macro-assembler-arm64.cc @@ -1289,7 +1289,8 @@ int RegExpMacroAssemblerARM64::CheckStackGuardState(Address* return_address, const byte** input_start, const byte** input_end) { Isolate* isolate = frame_entry(re_frame, kIsolate); - if (isolate->stack_guard()->IsStackOverflow()) { + StackLimitCheck check(isolate); + if (check.JsHasOverflowed()) { isolate->StackOverflow(); return EXCEPTION; } diff --git a/src/execution.cc b/src/execution.cc index fa35689f45..2766e76b8c 100644 --- a/src/execution.cc +++ b/src/execution.cc @@ -307,13 +307,6 @@ MaybeHandle Execution::TryGetConstructorDelegate( } -bool StackGuard::IsStackOverflow() { - ExecutionAccess access(isolate_); - return (thread_local_.jslimit_ != kInterruptLimit && - thread_local_.climit_ != kInterruptLimit); -} - - void StackGuard::EnableInterrupts() { ExecutionAccess access(isolate_); if (has_pending_interrupts(access)) { diff --git a/src/execution.h b/src/execution.h index 16bc6257e2..74d0feb75c 100644 --- a/src/execution.h +++ b/src/execution.h @@ -145,8 +145,6 @@ class StackGuard V8_FINAL { // it has been set up. void ClearThread(const ExecutionAccess& lock); - bool IsStackOverflow(); - #define INTERRUPT_LIST(V) \ V(DEBUGBREAK, DebugBreak) \ V(DEBUGCOMMAND, DebugCommand) \ @@ -266,6 +264,11 @@ enum InterruptFlag { int interrupt_flags_; }; + class StackPointer { + public: + inline uintptr_t address() { return reinterpret_cast(this); } + }; + // TODO(isolates): Technically this could be calculated directly from a // pointer to StackGuard. Isolate* isolate_; diff --git a/src/ia32/full-codegen-ia32.cc b/src/ia32/full-codegen-ia32.cc index c9e280ddf7..0ea77f0914 100644 --- a/src/ia32/full-codegen-ia32.cc +++ b/src/ia32/full-codegen-ia32.cc @@ -78,27 +78,6 @@ class JumpPatchSite BASE_EMBEDDED { }; -static void EmitStackCheck(MacroAssembler* masm_, - int pointers = 0, - Register scratch = esp) { - Label ok; - Isolate* isolate = masm_->isolate(); - ASSERT(scratch.is(esp) == (pointers == 0)); - ExternalReference stack_limit; - if (pointers != 0) { - __ mov(scratch, esp); - __ sub(scratch, Immediate(pointers * kPointerSize)); - stack_limit = ExternalReference::address_of_real_stack_limit(isolate); - } else { - stack_limit = ExternalReference::address_of_stack_limit(isolate); - } - __ cmp(scratch, Operand::StaticVariable(stack_limit)); - __ j(above_equal, &ok, Label::kNear); - __ call(isolate->builtins()->StackCheck(), RelocInfo::CODE_TARGET); - __ bind(&ok); -} - - // Generate code for a JS function. On entry to the function the receiver // and arguments have been pushed on the stack left to right, with the // return address on top of them. The actual argument count matches the @@ -168,7 +147,15 @@ void FullCodeGenerator::Generate() { __ push(Immediate(isolate()->factory()->undefined_value())); } else if (locals_count > 1) { if (locals_count >= 128) { - EmitStackCheck(masm_, locals_count, ecx); + Label ok; + __ mov(ecx, esp); + __ sub(ecx, Immediate(locals_count * kPointerSize)); + ExternalReference stack_limit = + ExternalReference::address_of_real_stack_limit(isolate()); + __ cmp(ecx, Operand::StaticVariable(stack_limit)); + __ j(above_equal, &ok, Label::kNear); + __ InvokeBuiltin(Builtins::STACK_OVERFLOW, CALL_FUNCTION); + __ bind(&ok); } __ mov(eax, Immediate(isolate()->factory()->undefined_value())); const int kMaxPushes = 32; @@ -309,7 +296,13 @@ void FullCodeGenerator::Generate() { { Comment cmnt(masm_, "[ Stack check"); PrepareForBailoutForId(BailoutId::Declarations(), NO_REGISTERS); - EmitStackCheck(masm_); + Label ok; + ExternalReference stack_limit + = ExternalReference::address_of_stack_limit(isolate()); + __ cmp(esp, Operand::StaticVariable(stack_limit)); + __ j(above_equal, &ok, Label::kNear); + __ call(isolate()->builtins()->StackCheck(), RelocInfo::CODE_TARGET); + __ bind(&ok); } { Comment cmnt(masm_, "[ Body"); diff --git a/src/ia32/regexp-macro-assembler-ia32.cc b/src/ia32/regexp-macro-assembler-ia32.cc index 155526f3a0..1945bd6303 100644 --- a/src/ia32/regexp-macro-assembler-ia32.cc +++ b/src/ia32/regexp-macro-assembler-ia32.cc @@ -1076,7 +1076,8 @@ int RegExpMacroAssemblerIA32::CheckStackGuardState(Address* return_address, Code* re_code, Address re_frame) { Isolate* isolate = frame_entry(re_frame, kIsolate); - if (isolate->stack_guard()->IsStackOverflow()) { + StackLimitCheck check(isolate); + if (check.JsHasOverflowed()) { isolate->StackOverflow(); return EXCEPTION; } diff --git a/src/isolate.cc b/src/isolate.cc index 5fd30eb12a..9ec3c9b289 100644 --- a/src/isolate.cc +++ b/src/isolate.cc @@ -2344,4 +2344,16 @@ void Isolate::RunMicrotasks() { } +bool StackLimitCheck::JsHasOverflowed() const { + StackGuard* stack_guard = isolate_->stack_guard(); +#ifdef USE_SIMULATOR + // The simulator uses a separate JS stack. + Address jssp_address = Simulator::current(isolate_)->get_sp(); + uintptr_t jssp = reinterpret_cast(jssp_address); + if (jssp < stack_guard->real_jslimit()) return true; +#endif // USE_SIMULATOR + return reinterpret_cast(this) < stack_guard->real_climit(); +} + + } } // namespace v8::internal diff --git a/src/isolate.h b/src/isolate.h index 439f2ce194..7de73034cd 100644 --- a/src/isolate.h +++ b/src/isolate.h @@ -1390,15 +1390,20 @@ class ExecutionAccess BASE_EMBEDDED { }; -// Support for checking for stack-overflows in C++ code. +// Support for checking for stack-overflows. class StackLimitCheck BASE_EMBEDDED { public: explicit StackLimitCheck(Isolate* isolate) : isolate_(isolate) { } - bool HasOverflowed() const { + // Use this to check for stack-overflows in C++ code. + inline bool HasOverflowed() const { StackGuard* stack_guard = isolate_->stack_guard(); - return (reinterpret_cast(this) < stack_guard->real_climit()); + return reinterpret_cast(this) < stack_guard->real_climit(); } + + // Use this to check for stack-overflow when entering runtime from JS code. + bool JsHasOverflowed() const; + private: Isolate* isolate_; }; diff --git a/src/runtime.cc b/src/runtime.cc index 927386289a..bc2e4458ab 100644 --- a/src/runtime.cc +++ b/src/runtime.cc @@ -9524,7 +9524,8 @@ RUNTIME_FUNCTION(RuntimeHidden_StackGuard) { ASSERT(args.length() == 0); // First check if this is a real stack overflow. - if (isolate->stack_guard()->IsStackOverflow()) { + StackLimitCheck check(isolate); + if (check.JsHasOverflowed()) { return isolate->StackOverflow(); } @@ -9538,7 +9539,8 @@ RUNTIME_FUNCTION(RuntimeHidden_TryInstallOptimizedCode) { CONVERT_ARG_HANDLE_CHECKED(JSFunction, function, 0); // First check if this is a real stack overflow. - if (isolate->stack_guard()->IsStackOverflow()) { + StackLimitCheck check(isolate); + if (check.JsHasOverflowed()) { SealHandleScope shs(isolate); return isolate->StackOverflow(); } diff --git a/src/x64/full-codegen-x64.cc b/src/x64/full-codegen-x64.cc index 1816abe294..fa1eee6e51 100644 --- a/src/x64/full-codegen-x64.cc +++ b/src/x64/full-codegen-x64.cc @@ -78,27 +78,6 @@ class JumpPatchSite BASE_EMBEDDED { }; -static void EmitStackCheck(MacroAssembler* masm_, - int pointers = 0, - Register scratch = rsp) { - Isolate* isolate = masm_->isolate(); - Label ok; - ASSERT(scratch.is(rsp) == (pointers == 0)); - Heap::RootListIndex index; - if (pointers != 0) { - __ movp(scratch, rsp); - __ subp(scratch, Immediate(pointers * kPointerSize)); - index = Heap::kRealStackLimitRootIndex; - } else { - index = Heap::kStackLimitRootIndex; - } - __ CompareRoot(scratch, index); - __ j(above_equal, &ok, Label::kNear); - __ call(isolate->builtins()->StackCheck(), RelocInfo::CODE_TARGET); - __ bind(&ok); -} - - // Generate code for a JS function. On entry to the function the receiver // and arguments have been pushed on the stack left to right, with the // return address on top of them. The actual argument count matches the @@ -168,7 +147,13 @@ void FullCodeGenerator::Generate() { __ PushRoot(Heap::kUndefinedValueRootIndex); } else if (locals_count > 1) { if (locals_count >= 128) { - EmitStackCheck(masm_, locals_count, rcx); + Label ok; + __ movp(rcx, rsp); + __ subp(rcx, Immediate(locals_count * kPointerSize)); + __ CompareRoot(rcx, Heap::kRealStackLimitRootIndex); + __ j(above_equal, &ok, Label::kNear); + __ InvokeBuiltin(Builtins::STACK_OVERFLOW, CALL_FUNCTION); + __ bind(&ok); } __ LoadRoot(rdx, Heap::kUndefinedValueRootIndex); const int kMaxPushes = 32; @@ -309,7 +294,11 @@ void FullCodeGenerator::Generate() { { Comment cmnt(masm_, "[ Stack check"); PrepareForBailoutForId(BailoutId::Declarations(), NO_REGISTERS); - EmitStackCheck(masm_); + Label ok; + __ CompareRoot(rsp, Heap::kStackLimitRootIndex); + __ j(above_equal, &ok, Label::kNear); + __ call(isolate()->builtins()->StackCheck(), RelocInfo::CODE_TARGET); + __ bind(&ok); } { Comment cmnt(masm_, "[ Body"); diff --git a/src/x64/regexp-macro-assembler-x64.cc b/src/x64/regexp-macro-assembler-x64.cc index b24d842805..a8c1cb47a7 100644 --- a/src/x64/regexp-macro-assembler-x64.cc +++ b/src/x64/regexp-macro-assembler-x64.cc @@ -1183,7 +1183,8 @@ int RegExpMacroAssemblerX64::CheckStackGuardState(Address* return_address, Code* re_code, Address re_frame) { Isolate* isolate = frame_entry(re_frame, kIsolate); - if (isolate->stack_guard()->IsStackOverflow()) { + StackLimitCheck check(isolate); + if (check.JsHasOverflowed()) { isolate->StackOverflow(); return EXCEPTION; } diff --git a/test/mjsunit/regress/regress-crbug-385002.js b/test/mjsunit/regress/regress-crbug-385002.js new file mode 100644 index 0000000000..34713e27d4 --- /dev/null +++ b/test/mjsunit/regress/regress-crbug-385002.js @@ -0,0 +1,15 @@ +// Copyright 2014 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Flags: --stack-size=200 --allow-natives-syntax + +%Break(); // Schedule an interrupt that does not go away. + +function f() { f(); } +assertThrows(f, RangeError); + +var locals = ""; +for (var i = 0; i < 1024; i++) locals += "var v" + i + ";"; +eval("function g() {" + locals + "f();}"); +assertThrows("g()", RangeError);