Make Simulator respect C stack limits as well.
authormstarzinger <mstarzinger@chromium.org>
Mon, 24 Aug 2015 15:55:30 +0000 (08:55 -0700)
committerCommit bot <commit-bot@chromium.org>
Mon, 24 Aug 2015 15:55:40 +0000 (15:55 +0000)
The simulator uses a separate JS stack, exhaustion of the C stack
however is not caught by JS limit checks. This change now lowers the
limit of the JS stack accordingly on function calls.

R=mvstanton@chromium.org
BUG=chromium:522380
TEST=mjsunit/regress/regress-crbug-522380
LOG=n

Review URL: https://codereview.chromium.org/1314623002

Cr-Commit-Position: refs/heads/master@{#30334}

src/arm/simulator-arm.cc
src/arm/simulator-arm.h
src/arm64/simulator-arm64.cc
src/arm64/simulator-arm64.h
src/execution.cc
src/execution.h
src/mips/simulator-mips.cc
src/mips/simulator-mips.h
src/mips64/simulator-mips64.cc
src/mips64/simulator-mips64.h
test/mjsunit/regress/regress-crbug-522380.js [new file with mode: 0644]

index 7fc8adbb02fab4389d8af6652f17077289d76316..5da6204050b926f056d06b8dd576a3ff9514eec3 100644 (file)
@@ -1227,9 +1227,15 @@ void Simulator::WriteDW(int32_t addr, int32_t value1, int32_t value2) {
 
 
 // Returns the limit of the stack area to enable checking for stack overflows.
-uintptr_t Simulator::StackLimit() const {
-  // Leave a safety margin of 1024 bytes to prevent overrunning the stack when
-  // pushing values.
+uintptr_t Simulator::StackLimit(uintptr_t c_limit) const {
+  // The simulator uses a separate JS stack. If we have exhausted the C stack,
+  // we also drop down the JS limit to reflect the exhaustion on the JS stack.
+  if (GetCurrentStackPosition() < c_limit) {
+    return reinterpret_cast<uintptr_t>(get_sp());
+  }
+
+  // Otherwise the limit is the JS stack. Leave a safety margin of 1024 bytes
+  // to prevent overrunning the stack when pushing values.
   return reinterpret_cast<uintptr_t>(stack_) + 1024;
 }
 
@@ -4009,6 +4015,9 @@ void Simulator::Execute() {
 
 
 void Simulator::CallInternal(byte* entry) {
+  // Adjust JS-based stack limit to C-based stack limit.
+  isolate_->stack_guard()->AdjustStackLimitForSimulator();
+
   // Prepare to execute the code at entry
   set_register(pc, reinterpret_cast<int32_t>(entry));
   // Put down marker for end of simulation. The simulator will stop simulation
index eea43efc53a97a30cf44e8d690be34c5a28b740d..a972a77d411c47480bd572bd662b625eb66f73c2 100644 (file)
@@ -181,12 +181,12 @@ class Simulator {
   void set_pc(int32_t value);
   int32_t get_pc() const;
 
-  Address get_sp() {
+  Address get_sp() const {
     return reinterpret_cast<Address>(static_cast<intptr_t>(get_register(sp)));
   }
 
   // Accessor to the internal simulator stack area.
-  uintptr_t StackLimit() const;
+  uintptr_t StackLimit(uintptr_t c_limit) const;
 
   // Executes ARM instructions until the PC reaches end_sim_pc.
   void Execute();
@@ -439,15 +439,14 @@ class Simulator {
 
 
 // The simulator has its own stack. Thus it has a different stack limit from
-// the C-based native code.  Setting the c_limit to indicate a very small
-// stack cause stack overflow errors, since the simulator ignores the input.
-// This is unlikely to be an issue in practice, though it might cause testing
-// trouble down the line.
+// the C-based native code.  The JS-based limit normally points near the end of
+// the simulator stack.  When the C-based limit is exhausted we reflect that by
+// lowering the JS-based limit as well, to make stack checks trigger.
 class SimulatorStack : public v8::internal::AllStatic {
  public:
   static inline uintptr_t JsLimitFromCLimit(v8::internal::Isolate* isolate,
                                             uintptr_t c_limit) {
-    return Simulator::current(isolate)->StackLimit();
+    return Simulator::current(isolate)->StackLimit(c_limit);
   }
 
   static inline uintptr_t RegisterCTryCatch(uintptr_t try_catch_address) {
index fae5ee518b569c50439f326d4e74a8e8e76173b2..83fd164bb6084d8187f9d057b5f75c5009965d5b 100644 (file)
@@ -222,6 +222,9 @@ int64_t Simulator::CallRegExp(byte* entry,
 
 
 void Simulator::CheckPCSComplianceAndRun() {
+  // Adjust JS-based stack limit to C-based stack limit.
+  isolate_->stack_guard()->AdjustStackLimitForSimulator();
+
 #ifdef DEBUG
   CHECK_EQ(kNumberOfCalleeSavedRegisters, kCalleeSaved.Count());
   CHECK_EQ(kNumberOfCalleeSavedFPRegisters, kCalleeSavedFP.Count());
@@ -332,9 +335,15 @@ uintptr_t Simulator::PopAddress() {
 
 
 // Returns the limit of the stack area to enable checking for stack overflows.
-uintptr_t Simulator::StackLimit() const {
-  // Leave a safety margin of 1024 bytes to prevent overrunning the stack when
-  // pushing values.
+uintptr_t Simulator::StackLimit(uintptr_t c_limit) const {
+  // The simulator uses a separate JS stack. If we have exhausted the C stack,
+  // we also drop down the JS limit to reflect the exhaustion on the JS stack.
+  if (GetCurrentStackPosition() < c_limit) {
+    return reinterpret_cast<uintptr_t>(get_sp());
+  }
+
+  // Otherwise the limit is the JS stack. Leave a safety margin of 1024 bytes
+  // to prevent overrunning the stack when pushing values.
   return stack_limit_ + 1024;
 }
 
index 44769283d26220c9f94810cd16e5897dd4f0e53c..6ff0013ebd954cef18ba161352c50e7aa54c8de1 100644 (file)
@@ -266,7 +266,7 @@ class Simulator : public DecoderVisitor {
   uintptr_t PopAddress();
 
   // Accessor to the internal simulator stack area.
-  uintptr_t StackLimit() const;
+  uintptr_t StackLimit(uintptr_t c_limit) const;
 
   void ResetState();
 
@@ -401,7 +401,7 @@ class Simulator : public DecoderVisitor {
   }
   Instruction* lr() { return reg<Instruction*>(kLinkRegCode); }
 
-  Address get_sp() { return reg<Address>(31, Reg31IsStackPointer); }
+  Address get_sp() const { return reg<Address>(31, Reg31IsStackPointer); }
 
   template<typename T>
   T fpreg(unsigned code) const {
@@ -882,13 +882,14 @@ class Simulator : public DecoderVisitor {
 
 
 // The simulator has its own stack. Thus it has a different stack limit from
-// the C-based native code.
-// See also 'class SimulatorStack' in arm/simulator-arm.h.
+// the C-based native code.  The JS-based limit normally points near the end of
+// the simulator stack.  When the C-based limit is exhausted we reflect that by
+// lowering the JS-based limit as well, to make stack checks trigger.
 class SimulatorStack : public v8::internal::AllStatic {
  public:
   static uintptr_t JsLimitFromCLimit(v8::internal::Isolate* isolate,
                                             uintptr_t c_limit) {
-    return Simulator::current(isolate)->StackLimit();
+    return Simulator::current(isolate)->StackLimit(c_limit);
   }
 
   static uintptr_t RegisterCTryCatch(uintptr_t try_catch_address) {
index 173de0e33feb0e9044a142c5bbb3ae89fefbb93d..4ab2403e56f0a0cc9d9345e51e0a233a260ff21c 100644 (file)
@@ -341,14 +341,6 @@ MaybeHandle<Object> Execution::TryGetConstructorDelegate(
 }
 
 
-void StackGuard::EnableInterrupts() {
-  ExecutionAccess access(isolate_);
-  if (has_pending_interrupts(access)) {
-    set_interrupt_limits(access);
-  }
-}
-
-
 void StackGuard::SetStackLimit(uintptr_t limit) {
   ExecutionAccess access(isolate_);
   // If the current limits are special (e.g. due to a pending interrupt) then
@@ -365,6 +357,27 @@ void StackGuard::SetStackLimit(uintptr_t limit) {
 }
 
 
+void StackGuard::AdjustStackLimitForSimulator() {
+  ExecutionAccess access(isolate_);
+  uintptr_t climit = thread_local_.real_climit_;
+  // If the current limits are special (e.g. due to a pending interrupt) then
+  // leave them alone.
+  uintptr_t jslimit = SimulatorStack::JsLimitFromCLimit(isolate_, climit);
+  if (thread_local_.jslimit() == thread_local_.real_jslimit_) {
+    thread_local_.set_jslimit(jslimit);
+    isolate_->heap()->SetStackLimits();
+  }
+}
+
+
+void StackGuard::EnableInterrupts() {
+  ExecutionAccess access(isolate_);
+  if (has_pending_interrupts(access)) {
+    set_interrupt_limits(access);
+  }
+}
+
+
 void StackGuard::DisableInterrupts() {
   ExecutionAccess access(isolate_);
   reset_limits(access);
index 2b17c0a66112dd866c512363870172bed975efe0..2c07a64aaecc5409064ba1bc5eaf2f2edcf73880 100644 (file)
@@ -136,6 +136,11 @@ class StackGuard final {
   // is assumed to grow downwards.
   void SetStackLimit(uintptr_t limit);
 
+  // The simulator uses a separate JS stack. Limits on the JS stack might have
+  // to be adjusted in order to reflect overflows of the C stack, because we
+  // cannot rely on the interleaving of frames on the simulator.
+  void AdjustStackLimitForSimulator();
+
   // Threading support.
   char* ArchiveStackGuard(char* to);
   char* RestoreStackGuard(char* from);
index ce839f23c9923f080031ce9ea931c6611f3994f4..6dea3f09a3969f339af0733c75a5642e5bb4bf3c 100644 (file)
@@ -1807,9 +1807,15 @@ void Simulator::WriteB(int32_t addr, int8_t value) {
 
 
 // Returns the limit of the stack area to enable checking for stack overflows.
-uintptr_t Simulator::StackLimit() const {
-  // Leave a safety margin of 1024 bytes to prevent overrunning the stack when
-  // pushing values.
+uintptr_t Simulator::StackLimit(uintptr_t c_limit) const {
+  // The simulator uses a separate JS stack. If we have exhausted the C stack,
+  // we also drop down the JS limit to reflect the exhaustion on the JS stack.
+  if (GetCurrentStackPosition() < c_limit) {
+    return reinterpret_cast<uintptr_t>(get_sp());
+  }
+
+  // Otherwise the limit is the JS stack. Leave a safety margin of 1024 bytes
+  // to prevent overrunning the stack when pushing values.
   return reinterpret_cast<uintptr_t>(stack_) + 1024;
 }
 
@@ -4464,6 +4470,9 @@ void Simulator::Execute() {
 
 
 void Simulator::CallInternal(byte* entry) {
+  // Adjust JS-based stack limit to C-based stack limit.
+  isolate_->stack_guard()->AdjustStackLimitForSimulator();
+
   // Prepare to execute the code at entry.
   set_register(pc, reinterpret_cast<int32_t>(entry));
   // Put down marker for end of simulation. The simulator will stop simulation
index 1459fbada3226e3dc68f024ca57088cafe455414..6de5163dda6d07050a62649946525bb281033ad5 100644 (file)
@@ -193,12 +193,12 @@ class Simulator {
   void set_pc(int32_t value);
   int32_t get_pc() const;
 
-  Address get_sp() {
+  Address get_sp() const {
     return reinterpret_cast<Address>(static_cast<intptr_t>(get_register(sp)));
   }
 
   // Accessor to the internal simulator stack area.
-  uintptr_t StackLimit() const;
+  uintptr_t StackLimit(uintptr_t c_limit) const;
 
   // Executes MIPS instructions until the PC reaches end_sim_pc.
   void Execute();
@@ -466,15 +466,14 @@ class Simulator {
 
 
 // The simulator has its own stack. Thus it has a different stack limit from
-// the C-based native code.  Setting the c_limit to indicate a very small
-// stack cause stack overflow errors, since the simulator ignores the input.
-// This is unlikely to be an issue in practice, though it might cause testing
-// trouble down the line.
+// the C-based native code.  The JS-based limit normally points near the end of
+// the simulator stack.  When the C-based limit is exhausted we reflect that by
+// lowering the JS-based limit as well, to make stack checks trigger.
 class SimulatorStack : public v8::internal::AllStatic {
  public:
   static inline uintptr_t JsLimitFromCLimit(Isolate* isolate,
                                             uintptr_t c_limit) {
-    return Simulator::current(isolate)->StackLimit();
+    return Simulator::current(isolate)->StackLimit(c_limit);
   }
 
   static inline uintptr_t RegisterCTryCatch(uintptr_t try_catch_address) {
index 577af17dc6e7ca74014e1c73291527811a144cb0..9a0d8fdce8bd359b8701ac56c45396e100eecdc0 100644 (file)
@@ -1800,9 +1800,15 @@ void Simulator::WriteB(int64_t addr, int8_t value) {
 
 
 // Returns the limit of the stack area to enable checking for stack overflows.
-uintptr_t Simulator::StackLimit() const {
-  // Leave a safety margin of 1024 bytes to prevent overrunning the stack when
-  // pushing values.
+uintptr_t Simulator::StackLimit(uintptr_t c_limit) const {
+  // The simulator uses a separate JS stack. If we have exhausted the C stack,
+  // we also drop down the JS limit to reflect the exhaustion on the JS stack.
+  if (GetCurrentStackPosition() < c_limit) {
+    return reinterpret_cast<uintptr_t>(get_sp());
+  }
+
+  // Otherwise the limit is the JS stack. Leave a safety margin of 1024 bytes
+  // to prevent overrunning the stack when pushing values.
   return reinterpret_cast<uintptr_t>(stack_) + 1024;
 }
 
@@ -4620,6 +4626,9 @@ void Simulator::Execute() {
 
 
 void Simulator::CallInternal(byte* entry) {
+  // Adjust JS-based stack limit to C-based stack limit.
+  isolate_->stack_guard()->AdjustStackLimitForSimulator();
+
   // Prepare to execute the code at entry.
   set_register(pc, reinterpret_cast<int64_t>(entry));
   // Put down marker for end of simulation. The simulator will stop simulation
index 5ac178df64e293f437b77ca18f6a5f63db8103bb..dea9e30adfca7d41c0679858038b8ad394274705 100644 (file)
@@ -223,12 +223,12 @@ class Simulator {
   void set_pc(int64_t value);
   int64_t get_pc() const;
 
-  Address get_sp() {
+  Address get_sp() const {
     return reinterpret_cast<Address>(static_cast<intptr_t>(get_register(sp)));
   }
 
   // Accessor to the internal simulator stack area.
-  uintptr_t StackLimit() const;
+  uintptr_t StackLimit(uintptr_t c_limit) const;
 
   // Executes MIPS instructions until the PC reaches end_sim_pc.
   void Execute();
@@ -509,15 +509,14 @@ class Simulator {
 
 
 // The simulator has its own stack. Thus it has a different stack limit from
-// the C-based native code.  Setting the c_limit to indicate a very small
-// stack cause stack overflow errors, since the simulator ignores the input.
-// This is unlikely to be an issue in practice, though it might cause testing
-// trouble down the line.
+// the C-based native code.  The JS-based limit normally points near the end of
+// the simulator stack.  When the C-based limit is exhausted we reflect that by
+// lowering the JS-based limit as well, to make stack checks trigger.
 class SimulatorStack : public v8::internal::AllStatic {
  public:
   static inline uintptr_t JsLimitFromCLimit(Isolate* isolate,
                                             uintptr_t c_limit) {
-    return Simulator::current(isolate)->StackLimit();
+    return Simulator::current(isolate)->StackLimit(c_limit);
   }
 
   static inline uintptr_t RegisterCTryCatch(uintptr_t try_catch_address) {
diff --git a/test/mjsunit/regress/regress-crbug-522380.js b/test/mjsunit/regress/regress-crbug-522380.js
new file mode 100644 (file)
index 0000000..eba07f7
--- /dev/null
@@ -0,0 +1,7 @@
+// Copyright 2015 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.
+
+var global = this;
+global.__defineSetter__('x', function(v) { x = v; });
+assertThrows("global.x = 0", RangeError);