Bug 193246 – ASSERT when paused in debugger and console evaluation causes exception

• Component: JavaScriptCore
• Status: NEW

[Joseph Pecoraro]

ASSERT when paused in debugger and console evaluation causes exception

Steps to reproduce:
1. Inspect a web page
2. Pause in debugger
3. Type the following in the inspector console: js> {}.x.x.x
  => By the time you get to "{}.x.|" the inspected page crashes

Assertion:
ASSERTION FAILED: exec == topCallFrame || exec->isGlobalExec()
./runtime/VM.cpp(833) : void JSC::VM::throwException(JSC::ExecState *, JSC::Exception *)
1   0x42f38f589 WTFCrash
2   0x42f39057b WTFCrashWithInfo(int, char const*, char const*, int)
3   0x430ab1bc8 JSC::VM::throwException(JSC::ExecState*, JSC::Exception*)
4   0x430ab1e30 JSC::VM::throwException(JSC::ExecState*, JSC::JSValue)
5   0x430a902d1 JSC::ThrowScope::throwException(JSC::ExecState*, JSC::JSValue)
6   0x42f8e24cd JSC::throwException(JSC::ExecState*, JSC::ThrowScope&, JSC::JSValue)
7   0x43017279d JSC::throwVMError(JSC::ExecState*, JSC::ThrowScope&, JSC::JSValue)
8   0x4307fa115 JSC::DirectEvalExecutable::create(JSC::ExecState*, JSC::SourceCode const&, bool, JSC::DerivedContextType, bool, JSC::EvalContextType, JSC::VariableEnvironment const*)
9   0x42fe96832 JSC::DebuggerCallFrame::evaluateWithScopeExtension(WTF::String const&, JSC::JSObject*, WTF::NakedPtr<JSC::Exception>&)
10  0x43047ccbc Inspector::JavaScriptCallFrame::evaluateWithScopeExtension(WTF::String const&, JSC::JSObject*, WTF::NakedPtr<JSC::Exception>&) const
11  0x43047cbd8 Inspector::JSJavaScriptCallFrame::evaluateWithScopeExtension(JSC::ExecState*)
12  0x43047eac2 Inspector::jsJavaScriptCallFramePrototypeFunctionEvaluateWithScopeExtension(JSC::ExecState*)
13  0x27aa86001177
14  0x42f895029 llint_entry
15  0x42f895029 llint_entry
16  0x42f881c72 vmEntryToJavaScript
17  0x4304e14f7 JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*)
18  0x4304e1b97 JSC::Interpreter::executeCall(JSC::ExecState*, JSC::JSObject*, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&)
19  0x4307ae68c JSC::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&)
20  0x4307ae77a JSC::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&)
21  0x419db2128 WebCore::JSExecState::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&)
22  0x419dff0e9 WebCore::functionCallHandlerFromAnyThread(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&, WTF::NakedPtr<JSC::Exception>&)
23  0x42fc0a9a4 Deprecated::ScriptFunctionCall::call(bool&)
24  0x4303de212 Inspector::InjectedScriptBase::callFunctionWithEvalEnabled(Deprecated::ScriptFunctionCall&, bool&) const
25  0x4303dc7c7 Inspector::InjectedScriptBase::makeCall(Deprecated::ScriptFunctionCall&)
26  0x4303dbc30 Inspector::InjectedScriptBase::makeEvalCall(WTF::String&, Deprecated::ScriptFunctionCall&, WTF::RefPtr<Inspector::Protocol::Runtime::RemoteObject, WTF::DumbPtrTraits<Inspector::Protocol::Runtime::RemoteObject> >&, WTF::Optional<bool>&, WTF::Optional<int>&)
27  0x4303dc507 Inspector::InjectedScript::evaluateOnCallFrame(WTF::String&, JSC::JSValue, WTF::String const&, WTF::String const&, WTF::String const&, bool, bool, bool, bool, WTF::RefPtr<Inspector::Protocol::Runtime::RemoteObject, WTF::DumbPtrTraits<Inspector::Protocol::Runtime::RemoteObject> >&, WTF::Optional<bool>&, WTF::Optional<int>&)
28  0x4304a9582 Inspector::InspectorDebuggerAgent::evaluateOnCallFrame(WTF::String&, WTF::String const&, WTF::String const&, WTF::String const*, bool const*, bool const*, bool const*, bool const*, bool const*, WTF::RefPtr<Inspector::Protocol::Runtime::RemoteObject, WTF::DumbPtrTraits<Inspector::Protocol::Runtime::RemoteObject> >&, WTF::Optional<bool>&, WTF::Optional<int>&)
29  0x4304a972c non-virtual thunk to Inspector::InspectorDebuggerAgent::evaluateOnCallFrame(WTF::String&, WTF::String const&, WTF::String const&, WTF::String const*, bool const*, bool const*, bool const*, bool const*, bool const*, WTF::RefPtr<Inspector::Protocol::Runtime::RemoteObject, WTF::DumbPtrTraits<Inspector::Protocol::Runtime::RemoteObject> >&, WTF::Optional<bool>&, WTF::Optional<int>&)
30  0x43040b9a5 Inspector::DebuggerBackendDispatcher::evaluateOnCallFrame(long, WTF::RefPtr<WTF::JSONImpl::Object, WTF::DumbPtrTraits<WTF::JSONImpl::Object> >&&)
31  0x430407b2b Inspector::DebuggerBackendDispatcher::dispatch(long, WTF::String const&, WTF::Ref<WTF::JSONImpl::Object, WTF::DumbPtrTraits<WTF::JSONImpl::Object> >&&)
...

[Joseph Pecoraro]

This is the inspector evaluating JavaScript when paused, so there are a few things.

    1. The page is paused in JavaScript (nested event loop)
    2. The inspector then evaluates script in its InjectedScript (InjectedScript::evaluateOnCallFrame)
    3. The evaluate on call frame performs an eval on the call frame (DebuggerCallFrame::evaluateWithScopeExtension)

So the "topCallFrame" may be a little tricky here.

[Joseph Pecoraro]

Worked through this with Mark. The Debugger is evaluating on a specific call frame (the selected one in Web Inspector) so the target call frame will not be the topCallFrame. We will enhance the assertion for the inspector's case.

[Joseph Pecoraro]

Created attachment 358634 [details]
[PATCH] Proposed Fix

[Mark Lam]

Comment on attachment 358634 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=358634&action=review

r=me with some suggestions.

> Source/JavaScriptCore/debugger/DebuggerEvalEnabler.h:37
> +        EvalOnCurrentFrame,
> +        EvalOnDebuggerCallFrame,

Now that I have more time to think about it, how about naming these as:
    EvalOnCurrentFrame => EvalOnCurrentCallFrame
    EvalOnDebuggerCallFrame => EvalOnCallFrameAtDebuggerEntry

I think these names would be clearer.  What do you think?

> Source/JavaScriptCore/runtime/JSGlobalObject.h:494
> +    const ExecState* m_evalDebuggerCallFrame { nullptr };

I suggest naming this as "m_callFrameAtDebuggerEntry" instead.

> Source/JavaScriptCore/runtime/JSGlobalObject.h:904
> +    const ExecState* evalDebuggerCallFrame() const { return m_evalDebuggerCallFrame; }
> +    void setEvalDebuggerCallFrame(const ExecState* callFrame) { m_evalDebuggerCallFrame = callFrame; }

I suggest renaming these as callFrameAtDebuggerEntry() and setCallFrameAtDebuggerEntry() instead.

[Joseph Pecoraro]

I like those suggestions.

[Joseph Pecoraro]

Created attachment 358636 [details]
[PATCH] For Landing

[WebKit Commit Bot]

Comment on attachment 358636 [details]
[PATCH] For Landing

Clearing flags on attachment: 358636

Committed r239753: <https://trac.webkit.org/changeset/239753>