git://git.webkit.org / WebKit-https.git / commitdiff
? search:
summary | shortlog | log | commit | commitdiff | tree
raw | patch | inline | side by side (parent: 0919c8d)
ASSERT when paused in debugger and console evaluation causes exception
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 9 Jan 2019 00:39:45 +0000 (00:39 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 9 Jan 2019 00:39:45 +0000 (00:39 +0000)
https://bugs.webkit.org/show_bug.cgi?id=193246

Patch by Joseph Pecoraro <pecoraro@apple.com> on 2019-01-08
Reviewed by Mark Lam.

Source/JavaScriptCore:

* runtime/VM.cpp:
(JSC::VM::throwException):
Improve assertion to allow for the debugger's evaluate on call frame condition.

* runtime/JSGlobalObject.h:
(JSC::JSGlobalObject::callFrameAtDebuggerEntry const):
(JSC::JSGlobalObject::setCallFrameAtDebuggerEntry):
Debugger call frame only used by assertions.

* debugger/DebuggerCallFrame.cpp:
(JSC::DebuggerCallFrame::evaluateWithScopeExtension):
* debugger/DebuggerEvalEnabler.h:
(JSC::DebuggerEvalEnabler::DebuggerEvalEnabler):
(JSC::DebuggerEvalEnabler::~DebuggerEvalEnabler):
When evaluating on a call frame, set a debug GlobalObject state.

LayoutTests:

* inspector/debugger/evaluateOnCallFrame-CommandLineAPI.html:
Correct a typo.

* inspector/debugger/evaluateOnCallFrame-exception-expected.txt: Added.
* inspector/debugger/evaluateOnCallFrame-exception.html: Added.
New test that would have asserted before.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@239753 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog patch | blob | history
LayoutTests/inspector/debugger/evaluateOnCallFrame-CommandLineAPI.html patch | blob | history
LayoutTests/inspector/debugger/evaluateOnCallFrame-exception-expected.txt [new file with mode: 0644] patch | blob
LayoutTests/inspector/debugger/evaluateOnCallFrame-exception.html [new file with mode: 0644] patch | blob
Source/JavaScriptCore/ChangeLog patch | blob | history
Source/JavaScriptCore/debugger/DebuggerCallFrame.cpp patch | blob | history
Source/JavaScriptCore/debugger/DebuggerEvalEnabler.h patch | blob | history
Source/JavaScriptCore/runtime/JSGlobalObject.h patch | blob | history
Source/JavaScriptCore/runtime/VM.cpp patch | blob | history

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 6131695..3ae2879 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,17 @@
+2019-01-08  Joseph Pecoraro  <pecoraro@apple.com>
+
+        ASSERT when paused in debugger and console evaluation causes exception
+        https://bugs.webkit.org/show_bug.cgi?id=193246
+
+        Reviewed by Mark Lam.
+
+        * inspector/debugger/evaluateOnCallFrame-CommandLineAPI.html:
+        Correct a typo.
+
+        * inspector/debugger/evaluateOnCallFrame-exception-expected.txt: Added.
+        * inspector/debugger/evaluateOnCallFrame-exception.html: Added.
+        New test that would have asserted before.
+
 2019-01-08  Jiewen Tan  <jiewen_tan@apple.com>
 
         [WebAuthN] Support U2F HID Authenticators on macOS
diff --git a/LayoutTests/inspector/debugger/evaluateOnCallFrame-CommandLineAPI.html b/LayoutTests/inspector/debugger/evaluateOnCallFrame-CommandLineAPI.html
index dbc19e9..c80eb01 100644 (file)
--- a/LayoutTests/inspector/debugger/evaluateOnCallFrame-CommandLineAPI.html
+++ b/LayoutTests/inspector/debugger/evaluateOnCallFrame-CommandLineAPI.html
@@ -76,7 +76,7 @@ function test()
 
     suite.addTestCase({
         name: "ValidateCallFrames",
-        description: "Test6 evaluate can access CommandLineAPI methods.",
+        description: "Test evaluate can access CommandLineAPI methods.",
         test(resolve, reject) {
             let targetData = WI.debuggerManager.dataForTarget(WI.debuggerManager.activeCallFrame.target);
             InspectorTest.expectThat(WI.debuggerManager.activeCallFrame.functionName === "bar", "Strict call frame is `bar`.");
diff --git a/LayoutTests/inspector/debugger/evaluateOnCallFrame-exception-expected.txt b/LayoutTests/inspector/debugger/evaluateOnCallFrame-exception-expected.txt
new file mode 100644 (file)
index 0000000..5ac00f0
--- /dev/null
+++ b/LayoutTests/inspector/debugger/evaluateOnCallFrame-exception-expected.txt
@@ -0,0 +1,9 @@
+Tests exceptions with Debugger.evaluateOnCallFrame.
+
+
+== Running test suite: Debugger.evaluateOnCallFrame.Exception
+-- Running test case: Debugger.evaluateOnCallFrame.Exception
+PASS: Should be a runtime error.
+
+-- Running test case: Complete
+
diff --git a/LayoutTests/inspector/debugger/evaluateOnCallFrame-exception.html b/LayoutTests/inspector/debugger/evaluateOnCallFrame-exception.html
new file mode 100644 (file)
index 0000000..71d6ede
--- /dev/null
+++ b/LayoutTests/inspector/debugger/evaluateOnCallFrame-exception.html
@@ -0,0 +1,53 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="../../http/tests/inspector/resources/inspector-test.js"></script>
+<script>
+function triggerPause() {
+    debugger;
+}
+
+function test()
+{
+    const objectGroup = "test";
+    const includeCommandLineAPI = true;
+    const returnByValue = true;
+
+    InspectorTest.debug();
+    let suite = InspectorTest.createAsyncSuite("Debugger.evaluateOnCallFrame.Exception");
+
+    suite.addTestCase({
+        name: "Debugger.evaluateOnCallFrame.Exception",
+        description: "Test exception from evaluateOnCallFrame.",
+        test(resolve, reject) {
+            let callFrame = WI.debuggerManager.activeCallFrame;
+            let targetData = WI.debuggerManager.dataForTarget(callFrame.target);
+            let callFrameId = callFrame.id;
+            let expression = "{}.x";
+            DebuggerAgent.evaluateOnCallFrame.invoke({callFrameId, expression, objectGroup, includeCommandLineAPI, returnByValue}, (error, resultValue, wasThrown) => {
+                InspectorTest.assert(!error, "Should not be a protocol error.");
+                InspectorTest.expectTrue(wasThrown, "Should be a runtime error.");
+                resolve();
+            });
+        }
+    });
+
+    suite.addTestCase({
+        name: "Complete",
+        test(resolve, reject) {
+            WI.debuggerManager.resume();
+            resolve();
+        }
+    })
+
+    InspectorTest.evaluateInPage("triggerPause()");
+    WI.debuggerManager.singleFireEventListener(WI.DebuggerManager.Event.Paused, (event) => {
+        suite.runTestCasesAndFinish();
+    });
+}
+</script>
+</head>
+<body onload="runTest()">
+<p>Tests exceptions with Debugger.evaluateOnCallFrame.</p>
+</body>
+</html>
diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index f7a3ab4..210d0be 100644 (file)
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,26 @@
+2019-01-08  Joseph Pecoraro  <pecoraro@apple.com>
+
+        ASSERT when paused in debugger and console evaluation causes exception
+        https://bugs.webkit.org/show_bug.cgi?id=193246
+
+        Reviewed by Mark Lam.
+
+        * runtime/VM.cpp:
+        (JSC::VM::throwException):
+        Improve assertion to allow for the debugger's evaluate on call frame condition.
+
+        * runtime/JSGlobalObject.h:
+        (JSC::JSGlobalObject::callFrameAtDebuggerEntry const):
+        (JSC::JSGlobalObject::setCallFrameAtDebuggerEntry):
+        Debugger call frame only used by assertions.
+
+        * debugger/DebuggerCallFrame.cpp:
+        (JSC::DebuggerCallFrame::evaluateWithScopeExtension):
+        * debugger/DebuggerEvalEnabler.h:
+        (JSC::DebuggerEvalEnabler::DebuggerEvalEnabler):
+        (JSC::DebuggerEvalEnabler::~DebuggerEvalEnabler):
+        When evaluating on a call frame, set a debug GlobalObject state.
+
 2019-01-08  Keith Miller  <keith_miller@apple.com>
 
         Move JSValueMakeSymbol to private header
diff --git a/Source/JavaScriptCore/debugger/DebuggerCallFrame.cpp b/Source/JavaScriptCore/debugger/DebuggerCallFrame.cpp
index 334d542..630f8f0 100644 (file)
--- a/Source/JavaScriptCore/debugger/DebuggerCallFrame.cpp
+++ b/Source/JavaScriptCore/debugger/DebuggerCallFrame.cpp
@@ -232,7 +232,7 @@ JSValue DebuggerCallFrame::evaluateWithScopeExtension(const String& script, JSOb
     if (!codeBlock)
         return jsUndefined();
     
-    DebuggerEvalEnabler evalEnabler(callFrame);
+    DebuggerEvalEnabler evalEnabler(callFrame, DebuggerEvalEnabler::Mode::EvalOnCallFrameAtDebuggerEntry);
 
     EvalContextType evalContextType;
     
diff --git a/Source/JavaScriptCore/debugger/DebuggerEvalEnabler.h b/Source/JavaScriptCore/debugger/DebuggerEvalEnabler.h
index 0e4c4cf..9b27395 100644 (file)
--- a/Source/JavaScriptCore/debugger/DebuggerEvalEnabler.h
+++ b/Source/JavaScriptCore/debugger/DebuggerEvalEnabler.h
@@ -32,29 +32,50 @@ namespace JSC {
 
 class DebuggerEvalEnabler {
 public:
-    explicit DebuggerEvalEnabler(const ExecState* exec)
+    enum class Mode {
+        EvalOnCurrentCallFrame,
+        EvalOnCallFrameAtDebuggerEntry,
+    };
+
+    DebuggerEvalEnabler(const ExecState* exec, Mode mode = Mode::EvalOnCurrentCallFrame)
         : m_exec(exec)
-        , m_evalWasDisabled(false)
+#if !ASSERT_DISABLED
+        , m_mode(mode)
+#endif
+        
     {
+        UNUSED_PARAM(mode);
         if (exec) {
             JSGlobalObject* globalObject = exec->lexicalGlobalObject();
             m_evalWasDisabled = !globalObject->evalEnabled();
             if (m_evalWasDisabled)
                 globalObject->setEvalEnabled(true, globalObject->evalDisabledErrorMessage());
+#if !ASSERT_DISABLED
+            if (m_mode == Mode::EvalOnCallFrameAtDebuggerEntry)
+                globalObject->setCallFrameAtDebuggerEntry(exec);
+#endif
         }
     }
 
     ~DebuggerEvalEnabler()
     {
-        if (m_evalWasDisabled) {
+        if (m_exec) {
             JSGlobalObject* globalObject = m_exec->lexicalGlobalObject();
-            globalObject->setEvalEnabled(false, globalObject->evalDisabledErrorMessage());
+            if (m_evalWasDisabled)
+                globalObject->setEvalEnabled(false, globalObject->evalDisabledErrorMessage());
+#if !ASSERT_DISABLED
+            if (m_mode == Mode::EvalOnCallFrameAtDebuggerEntry)
+                globalObject->setCallFrameAtDebuggerEntry(nullptr);
+#endif
         }
     }
 
 private:
     const ExecState* m_exec;
-    bool m_evalWasDisabled;
+    bool m_evalWasDisabled { false };
+#if !ASSERT_DISABLED
+    DebuggerEvalEnabler::Mode m_mode;
+#endif
 };
 
 } // namespace JSC
diff --git a/Source/JavaScriptCore/runtime/JSGlobalObject.h b/Source/JavaScriptCore/runtime/JSGlobalObject.h
index 357ca8b..ef597fb 100644 (file)
--- a/Source/JavaScriptCore/runtime/JSGlobalObject.h
+++ b/Source/JavaScriptCore/runtime/JSGlobalObject.h
@@ -490,6 +490,10 @@ public:
     RuntimeFlags m_runtimeFlags;
     ConsoleClient* m_consoleClient { nullptr };
 
+#if !ASSERT_DISABLED
+    const ExecState* m_callFrameAtDebuggerEntry { nullptr };
+#endif
+
     static JS_EXPORT_PRIVATE const GlobalObjectMethodTable s_globalObjectMethodTable;
     const GlobalObjectMethodTable* m_globalObjectMethodTable;
 
@@ -895,6 +899,11 @@ public:
         m_webAssemblyDisabledErrorMessage = errorMessage;
     }
 
+#if !ASSERT_DISABLED
+    const ExecState* callFrameAtDebuggerEntry() const { return m_callFrameAtDebuggerEntry; }
+    void setCallFrameAtDebuggerEntry(const ExecState* callFrame) { m_callFrameAtDebuggerEntry = callFrame; }
+#endif
+
     void resetPrototype(VM&, JSValue prototype);
 
     VM& vm() const { return m_vm; }
diff --git a/Source/JavaScriptCore/runtime/VM.cpp b/Source/JavaScriptCore/runtime/VM.cpp
index 190e571..fbbfe5a 100644 (file)
--- a/Source/JavaScriptCore/runtime/VM.cpp
+++ b/Source/JavaScriptCore/runtime/VM.cpp
@@ -830,7 +830,7 @@ void VM::clearSourceProviderCaches()
 
 void VM::throwException(ExecState* exec, Exception* exception)
 {
-    ASSERT(exec == topCallFrame || exec->isGlobalExec());
+    ASSERT(exec == topCallFrame || exec->isGlobalExec() || exec == exec->lexicalGlobalObject()->callFrameAtDebuggerEntry());
     CallFrame* throwOriginFrame = exec->isGlobalExec() ? exec : topJSCallFrame();
 
     if (Options::breakOnThrow()) {
Mirror of the WebKit open source project from svn.webkit.org.