Reviewers: Jakob,

Description:
Merged r11219, r11220, r11236, r11238 into 3.8 branch.

Fix stack overflow crashes when entering the debugger.

BUG=chromium:119429
[email protected]


Please review this at https://chromiumcodereview.appspot.com/10052008/

SVN Base: https://v8.googlecode.com/svn/branches/3.8

Affected files:
  M src/bootstrapper.cc
  M src/debug.cc
  M src/execution.cc
  M src/flag-definitions.h
  M src/version.cc
  A test/mjsunit/regress/regress-119429.js


Index: src/bootstrapper.cc
diff --git a/src/bootstrapper.cc b/src/bootstrapper.cc
index cedb0efd00127bf54461832b2b706de4db675d0c..1c2d754239554292a6f17f27ab7a4f85842f472b 100644
--- a/src/bootstrapper.cc
+++ b/src/bootstrapper.cc
@@ -1317,6 +1317,12 @@ bool Genesis::CompileNative(Vector<const char> name, Handle<String> source) {
 #ifdef ENABLE_DEBUGGER_SUPPORT
   isolate->debugger()->set_compiling_natives(true);
 #endif
+ // During genesis, the boilerplate for stack overflow won't work until the
+  // environment has been at least partially initialized. Add a stack check
+  // before entering JS code to catch overflow early.
+  StackLimitCheck check(Isolate::Current());
+  if (check.HasOverflowed()) return false;
+
   bool result = CompileScriptCached(name,
                                     source,
                                     NULL,
Index: src/debug.cc
diff --git a/src/debug.cc b/src/debug.cc
index 6d90a6904c94fb29899cea2bbceda2cde6597217..337608fce081513cbcf53701f163b449519d9e3b 100644
--- a/src/debug.cc
+++ b/src/debug.cc
@@ -1,4 +1,4 @@
-// Copyright 2011 the V8 project authors. All rights reserved.
+// Copyright 2012 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
@@ -772,15 +772,22 @@ bool Debug::CompileDebuggerScript(int index) {
   Handle<JSFunction> function =
       factory->NewFunctionFromSharedFunctionInfo(function_info, context);

-  Execution::TryCall(function, Handle<Object>(context->global()),
-                     0, NULL, &caught_exception);
+  Handle<Object> exception =
+      Execution::TryCall(function, Handle<Object>(context->global()),
+                         0, NULL, &caught_exception);

   // Check for caught exceptions.
   if (caught_exception) {
+    ASSERT(!isolate->has_pending_exception());
+    MessageLocation computed_location;
+    isolate->ComputeLocation(&computed_location);
     Handle<Object> message = MessageHandler::MakeMessageObject(
-        "error_loading_debugger", NULL, Vector<Handle<Object> >::empty(),
-        Handle<String>(), Handle<JSArray>());
+        "error_loading_debugger", &computed_location,
+ Vector<Handle<Object> >::empty(), Handle<String>(), Handle<JSArray>());
+    ASSERT(!isolate->has_pending_exception());
+    isolate->set_pending_exception(*exception);
     MessageHandler::ReportMessage(Isolate::Current(), NULL, message);
+    isolate->clear_pending_exception();
     return false;
   }

@@ -818,6 +825,9 @@ bool Debug::Load() {
           v8::Handle<ObjectTemplate>(),
           NULL);

+  // Fail if no context could be created.
+  if (context.is_null()) return false;
+
   // Use the debugger context.
   SaveContext save(isolate_);
   isolate_->set_context(*context);
@@ -3214,7 +3224,7 @@ EnterDebugger::~EnterDebugger() {
   debug->SetBreak(break_frame_id_, break_id_);

   // Check for leaving the debugger.
-  if (prev_ == NULL) {
+  if (!load_failed_ && prev_ == NULL) {
// Clear mirror cache when leaving the debugger. Skip this if there is a
     // pending exception as clearing the mirror cache calls back into
     // JavaScript. This can happen if the v8::Debug::Call is used in which
Index: src/execution.cc
diff --git a/src/execution.cc b/src/execution.cc
index 71e8ea34a1eb72b2050086889d812afa666c68cd..68d0314b7d0f7525c7c51456a513539848321dec 100644
--- a/src/execution.cc
+++ b/src/execution.cc
@@ -820,6 +820,11 @@ Object* Execution::DebugBreakHelper() {
     return isolate->heap()->undefined_value();
   }

+  StackLimitCheck check(isolate);
+  if (check.HasOverflowed()) {
+    return isolate->heap()->undefined_value();
+  }
+
   {
     JavaScriptFrameIterator it(isolate);
     ASSERT(!it.done());
@@ -856,6 +861,11 @@ void Execution::ProcessDebugMessages(bool debug_command_only) {
   // Clear the debug command request flag.
   isolate->stack_guard()->Continue(DEBUGCOMMAND);

+  StackLimitCheck check(isolate);
+  if (check.HasOverflowed()) {
+    return;
+  }
+
   HandleScope scope(isolate);
   // Enter the debugger. Just continue if we fail to enter the debugger.
   EnterDebugger debugger;
Index: src/flag-definitions.h
diff --git a/src/flag-definitions.h b/src/flag-definitions.h
index 3c3baa0f96ca3a209a13a87ee8fad317464e677b..e74d01ec2d7d1caaae2ad5bf9e499dbd2ddbfcbd 100644
--- a/src/flag-definitions.h
+++ b/src/flag-definitions.h
@@ -249,7 +249,9 @@ DEFINE_bool(debugger_auto_break, true,
 DEFINE_bool(enable_liveedit, true, "enable liveedit experimental feature")

 // execution.cc
-DEFINE_int(stack_size, kPointerSize * 128,
+// Slightly less than 1MB on 64-bit, since Windows' default stack size for
+// the main execution thread is 1MB for both 32 and 64-bit.
+DEFINE_int(stack_size, kPointerSize * 123,
"default size of stack region v8 is allowed to use (in KkBytes)")

 // frames.cc
Index: src/version.cc
diff --git a/src/version.cc b/src/version.cc
index 13faf1608a384f8996f1238df4cfa97dc412e812..2b0059ea5aa4c8bdb16eb6aa06ded256e290c464 100644
--- a/src/version.cc
+++ b/src/version.cc
@@ -35,7 +35,7 @@
 #define MAJOR_VERSION     3
 #define MINOR_VERSION     8
 #define BUILD_NUMBER      9
-#define PATCH_LEVEL       16
+#define PATCH_LEVEL       17
 // Use 1 for candidates and 0 otherwise.
 // (Boolean macro values are not supported by all preprocessors.)
 #define IS_CANDIDATE_VERSION 0
Index: test/mjsunit/regress/regress-119429.js
diff --git a/test/mjsunit/regress/regress-119429.js b/test/mjsunit/regress/regress-119429.js
new file mode 100644
index 0000000000000000000000000000000000000000..28637966b7b401eef2fa46b14ac415f99adeb32b
--- /dev/null
+++ b/test/mjsunit/regress/regress-119429.js
@@ -0,0 +1,37 @@
+// Copyright 2012 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+//       notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+//       copyright notice, this list of conditions and the following
+//       disclaimer in the documentation and/or other materials provided
+//       with the distribution.
+//     * Neither the name of Google Inc. nor the names of its
+//       contributors may be used to endorse or promote products derived
+//       from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// Flags: --allow-natives-syntax
+
+var d = 0;
+function recurse() {
+  if (++d == 25515) { // A magic number just below stack overflow  on ia32
+    %DebugBreak();
+  }
+  recurse();
+}
+assertThrows(function() { recurse();} );


--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to