[inspector] gracefully handle stack overflows in the inspector.

Hopefully we can avoid going through JS at all, so we can avoid this issue.

R=jgruber@chromium.org, kozyatinskiy@chromium.org
BUG=v8:5654

Review-Url: https://codereview.chromium.org/2510093002
Cr-Commit-Position: refs/heads/master@{#41802}

src/d8.cc

@@ -1931,9 +1931,8 @@ class InspectorFrontend final : public v8_inspector::V8Inspector::Channel {
                                     "Maximum call stack size exceeded",
                                     v8::NewStringType::kNormal)
                 .ToLocalChecked();
-        CHECK(exception->Get(context, key)
-                  .ToLocalChecked()
-                  ->StrictEquals(expected));
+        Local<Value> value = exception->Get(context, key).ToLocalChecked();
+        CHECK(value->StrictEquals(expected));
       }
 #endif
     }

src/inspector/java-script-call-frame.cc

@@ -91,7 +91,7 @@ bool JavaScriptCallFrame::isAtReturn() const {
   return result.As<v8::Boolean>()->BooleanValue(context).FromMaybe(false);
 }
 
-v8::Local<v8::Object> JavaScriptCallFrame::details() const {
+v8::MaybeLocal<v8::Object> JavaScriptCallFrame::details() const {
   v8::MicrotasksScope microtasks(m_isolate,
                                  v8::MicrotasksScope::kDoNotRunMicrotasks);
   v8::Local<v8::Context> context =
@@ -101,8 +101,12 @@ v8::Local<v8::Object> JavaScriptCallFrame::details() const {
   v8::Local<v8::Function> func = v8::Local<v8::Function>::Cast(
       callFrame->Get(context, toV8StringInternalized(m_isolate, "details"))
           .ToLocalChecked());
-  return v8::Local<v8::Object>::Cast(
-      func->Call(context, callFrame, 0, nullptr).ToLocalChecked());
+  v8::TryCatch try_catch(m_isolate);
+  v8::Local<v8::Value> details;
+  if (func->Call(context, callFrame, 0, nullptr).ToLocal(&details)) {
+    return v8::Local<v8::Object>::Cast(details);
+  }
+  return v8::MaybeLocal<v8::Object>();
 }
 
 v8::MaybeLocal<v8::Value> JavaScriptCallFrame::evaluate(
@@ -129,6 +133,7 @@ v8::MaybeLocal<v8::Value> JavaScriptCallFrame::restart() {
   v8::Local<v8::Function> restartFunction = v8::Local<v8::Function>::Cast(
       callFrame->Get(context, toV8StringInternalized(m_isolate, "restart"))
           .ToLocalChecked());
+  v8::TryCatch try_catch(m_isolate);
   v8::debug::SetLiveEditEnabled(m_isolate, true);
   v8::MaybeLocal<v8::Value> result = restartFunction->Call(
       m_debuggerContext.Get(m_isolate), callFrame, 0, nullptr);
@@ -154,6 +159,7 @@ v8::MaybeLocal<v8::Value> JavaScriptCallFrame::setVariableValue(
   v8::Local<v8::Value> argv[] = {
       v8::Local<v8::Value>(v8::Integer::New(m_isolate, scopeNumber)),
       variableName, newValue};
+  v8::TryCatch try_catch(m_isolate);
   return setVariableValueFunction->Call(context, callFrame, arraysize(argv),
                                         argv);
 }

src/inspector/java-script-call-frame.h

@@ -55,7 +55,7 @@ class JavaScriptCallFrame {
   int contextId() const;
 
   bool isAtReturn() const;
-  v8::Local<v8::Object> details() const;
+  v8::MaybeLocal<v8::Object> details() const;
 
   v8::MaybeLocal<v8::Value> evaluate(v8::Local<v8::Value> expression);
   v8::MaybeLocal<v8::Value> restart();

src/inspector/v8-debugger-agent-impl.cc

@@ -944,8 +944,9 @@ Response V8DebuggerAgentImpl::currentCallFrames(
     const std::unique_ptr<JavaScriptCallFrame>& currentCallFrame =
         m_pausedCallFrames[frameOrdinal];
 
-    v8::Local<v8::Object> details = currentCallFrame->details();
-    if (details.IsEmpty()) return Response::InternalError();
+    v8::Local<v8::Object> details;
+    if (!currentCallFrame->details().ToLocal(&details))
+      return Response::InternalError();
 
     int contextId = currentCallFrame->contextId();
 

src/inspector/v8-debugger.cc

@@ -50,6 +50,7 @@ v8::MaybeLocal<v8::Value> V8Debugger::callDebuggerMethod(
       debuggerScript
           ->Get(context, toV8StringInternalized(m_isolate, functionName))
           .ToLocalChecked());
+  v8::TryCatch try_catch(m_isolate);
   return function->Call(context, debuggerScript, argc, argv);
 }
 
@@ -409,16 +410,16 @@ JavaScriptCallFrames V8Debugger::currentCallFrames(int limit) {
                 ->Get(debuggerContext(),
                       toV8StringInternalized(m_isolate, "currentCallFrames"))
                 .ToLocalChecked());
-    currentCallFramesV8 =
-        v8::debug::Call(debuggerContext(), currentCallFramesFunction,
-                        v8::Integer::New(m_isolate, limit))
-            .ToLocalChecked();
+    if (!v8::debug::Call(debuggerContext(), currentCallFramesFunction,
+                         v8::Integer::New(m_isolate, limit))
+             .ToLocal(&currentCallFramesV8))
+      return JavaScriptCallFrames();
   } else {
     v8::Local<v8::Value> argv[] = {m_executionState,
                                    v8::Integer::New(m_isolate, limit)};
-    currentCallFramesV8 =
-        callDebuggerMethod("currentCallFrames", arraysize(argv), argv)
-            .ToLocalChecked();
+    if (!callDebuggerMethod("currentCallFrames", arraysize(argv), argv)
+             .ToLocal(&currentCallFramesV8))
+      return JavaScriptCallFrames();
   }
   DCHECK(!currentCallFramesV8.IsEmpty());
   if (!currentCallFramesV8->IsArray()) return JavaScriptCallFrames();
@@ -588,8 +589,10 @@ void V8Debugger::handleV8DebugEvent(
                        isUncaught);
   } else if (event == v8::Break) {
     v8::Local<v8::Value> argv[] = {eventDetails.GetEventData()};
-    v8::Local<v8::Value> hitBreakpoints =
-        callDebuggerMethod("getBreakpointNumbers", 1, argv).ToLocalChecked();
+    v8::Local<v8::Value> hitBreakpoints;
+    if (!callDebuggerMethod("getBreakpointNumbers", 1, argv)
+             .ToLocal(&hitBreakpoints))
+      return;
     DCHECK(hitBreakpoints->IsArray());
     handleProgramBreak(eventContext, eventDetails.GetExecutionState(),
                        v8::Local<v8::Value>(), hitBreakpoints.As<v8::Array>());
@@ -777,9 +780,11 @@ v8::Local<v8::Value> V8Debugger::collectionEntries(
     return v8::Undefined(m_isolate);
   }
   v8::Local<v8::Value> argv[] = {object};
-  v8::Local<v8::Value> entriesValue =
-      callDebuggerMethod("getCollectionEntries", 1, argv).ToLocalChecked();
-  if (!entriesValue->IsArray()) return v8::Undefined(m_isolate);
+  v8::Local<v8::Value> entriesValue;
+  if (!callDebuggerMethod("getCollectionEntries", 1, argv)
+           .ToLocal(&entriesValue) ||
+      !entriesValue->IsArray())
+    return v8::Undefined(m_isolate);
 
   v8::Local<v8::Array> entries = entriesValue.As<v8::Array>();
   v8::Local<v8::Array> copiedArray =
@@ -812,11 +817,11 @@ v8::Local<v8::Value> V8Debugger::generatorObjectLocation(
     return v8::Null(m_isolate);
   }
   v8::Local<v8::Value> argv[] = {object};
-  v8::Local<v8::Value> location =
-      callDebuggerMethod("getGeneratorObjectLocation", 1, argv)
-          .ToLocalChecked();
+  v8::Local<v8::Value> location;
   v8::Local<v8::Value> copied;
-  if (!copyValueFromDebuggerContext(m_isolate, debuggerContext(), context,
+  if (!callDebuggerMethod("getGeneratorObjectLocation", 1, argv)
+           .ToLocal(&location) ||
+      !copyValueFromDebuggerContext(m_isolate, debuggerContext(), context,
                                     location)
            .ToLocal(&copied) ||
       !copied->IsObject())

test/debugger/debugger.status

@@ -27,6 +27,9 @@
 
   # Slow test
   'debug/debug-scopes': [PASS, SLOW],
+
+  # Too slow in debug mode with --stress-opt mode.
+  'regress/regress-2318': [PASS, ['mode == debug or (arch != ia32 and arch != x64)', SKIP]],
 }],  # ALWAYS
 
 ##############################################################################

test/mjsunit/regress/regress-2318.js → test/debugger/regress/regress-2318.js

@@ -25,7 +25,7 @@
 // (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: --expose-debug-as debug --stack-size=150
+// Flags: --stack-size=70
 
 function f() {
   var i = 0;

test/debugger/test-api.js

@@ -819,6 +819,8 @@ class DebugWrapper {
         break;
     }
 
+    if (!params.callFrames[0]) return;
+
     // Skip break events in this file.
     if (params.callFrames[0].location.scriptId == this.thisScriptId) return;
 

test/mjsunit/mjsunit.status

@@ -55,7 +55,6 @@
 
   ##############################################################################
   # Too slow in debug mode with --stress-opt mode.
-  'regress/regress-2318': [PASS, ['mode == debug', SKIP]],
   'regress/regress-create-exception': [PASS, ['mode == debug', SKIP]],
 
   ##############################################################################