Bug 143037 - WebContent Crash when instantiating class with Type Profiling enabled
Summary: WebContent Crash when instantiating class with Type Profiling enabled
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Joseph Pecoraro
URL:
Keywords: InRadar
Depends on:
Blocks: 140491
  Show dependency treegraph
 
Reported: 2015-03-24 21:49 PDT by Joseph Pecoraro
Modified: 2015-03-26 20:00 PDT (History)
8 users (show)

See Also:


Attachments
[PATCH] Proposed Fix (5.65 KB, patch)
2015-03-26 18:10 PDT, Joseph Pecoraro
rniwa: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 2015-03-24 21:49:42 PDT
* SUMMARY
WebContent Crash when instantiating class in Web Inspector console.

* STEPS TO REPRODUCE
1. Inspect about:blank
2. Paste and run in the console:
var baseclass = class A { constructor(){} methodA(a,b){} };
var derivedclass = class B extends baseclass { constructor(){} methodB(a, b){} };
new derivedclass;
  => CRASH

* NOTES
- I was testing at r181930.
- When Web Inspector is evaluating in the console, it wraps this code up in a with block and evals it. Seems like it could be related

* LLDB Backtrace:
(lldb) [0x0000000000000000 - 0x00000000000001ba)
[0x00000000000001ba - 0x0000000000000376)
[0x0000000000000376 - 0x00000000000003a4)
[0x00000000000003a4 - 0x00000000000003ac)
[0x00000000000003ac - 0x0000000000007228)
Process 42045 stopped
* thread #1: tid = 0x14388c, 0x00000001117a14fb JavaScriptCore`llint_entry + 21311, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
    frame #0: 0x00000001117a14fb JavaScriptCore`llint_entry + 21311
JavaScriptCore`llint_entry:
->  0x1117a14fb <+21311>: movl   (%rax), %ebx
    0x1117a14fd <+21313>: movl   %ebx, 0x10(%rcx)
    0x1117a1500 <+21316>: addq   $0x18, %rcx
    0x1117a1504 <+21320>: movq   %rcx, 0x10(%rdx)

(lldb) btjs
* thread #1: tid = 0x14388c, 0x00000001117a14fb, queue = 'com.apple.main-thread, stop reason = EXC_BAD_ACCESS (code=1, addre?f0
    frame #0: 0x00000001117a14fb B#EQKQNo [LLInt](<JSValue()>)
    frame #1: 0x000000011c80e2f0 B#EQKQNo [LLInt](<JSValue()>)
    frame #2: 0x00000001117a27e0 <eval>#BboBZw [LLInt](Cell[JSDOMWindowShell ID: 339]: 0x11c0dffb0)
    frame #3: 0x000000011179bf79 JavaScriptCore`vmEntryToJavaScript + 361
    frame #4: 0x000000011160809a JavaScriptCore`JSC::JITCode::execute(this=0x0000000125fecb70, vm=0x000000011c02fcc0, protoCallFrame=0x00007fff58eca448) + 266 at JITCode.cpp:77
    frame #5: 0x00000001115e8761 JavaScriptCore`JSC::Interpreter::execute(this=0x000000011dff3138, eval=0x000000011c377070, callFrame=0x00007fff58ecb820, thisValue=JSValue at 0x00007fff58eca5d0, scope=0x000000011c12f470) + 2577 at Interpreter.cpp:1142
    frame #6: 0x00000001116a850d JavaScriptCore`JSC::globalFuncEval(exec=0x00007fff58ecb820) + 877 at JSGlobalObjectFunctions.cpp:578
    frame #7: 0x0000215428601028 0x1117a265a
    frame #8: 0x00000001117a265a _evaluateOn#BGPlyZ [LLInt](Cell[Object ID: 1172]: 0x11c2cff60, Cell[Function ID: 41]: 0x11c1ae230, Cell[InjectedScriptHost ID: 67]: 0x11c14ee90, \"console\", \"var baseclass = class A { constructor(){} m
    frame #9: 0x00000001117a265a _evaluateAndWrap#AhmPO9 [LLInt](Cell[Object ID: 1172]: 0x11c2cff60, Cell[Function ID: 41]: 0x11c1ae230, Cell[InjectedScriptHost ID: 67]: 0x11c14ee90, \"var baseclass = class A { constructor(){} methodA
    frame #10: 0x00000001117a265a evaluate#BBLmsT [LLInt](Cell[Object ID: 1172]: 0x11c2cff60, \"var baseclass = class A { constructor(){} methodA(a,b){} };\nvar derivedclass = class B extends baseclass { constructor(){} methodB(a, b){} 
    frame #11: 0x000000011179bf79 JavaScriptCore`vmEntryToJavaScript + 361
    frame #12: 0x000000011160809a JavaScriptCore`JSC::JITCode::execute(this=0x0000000123ff2e70, vm=0x000000011c02fcc0, protoCallFrame=0x00007fff58ecbd08) + 266 at JITCode.cpp:77
    frame #13: 0x00000001115ebcbe JavaScriptCore`JSC::Interpreter::executeCall(this=0x000000011dff3138, callFrame=0x000000011c12f4b0, function=0x000000011c3184f0, callType=CallTypeJS, callData=0x00007fff58ecc130, thisValue=JSValue at 0x00007fff58ecbde0, args=0x00007fff58ecc0e8) + 1486 at Interpreter.cpp:919
    frame #14: 0x00000001110c77de JavaScriptCore`JSC::call(exec=0x000000011c12f4b0, functionObject=JSValue at 0x00007fff58ecbec0, callType=CallTypeJS, callData=0x00007fff58ecc130, thisValue=JSValue at 0x00007fff58ecbeb8, args=0x00007fff58ecc0e8) + 190 at CallData.cpp:39
    frame #15: 0x00000001110c7843 JavaScriptCore`JSC::call(exec=0x000000011c12f4b0, functionObject=JSValue at 0x00007fff58ecbf40, callType=CallTypeJS, callData=0x00007fff58ecc130, thisValue=JSValue at 0x00007fff58ecbf38, args=0x00007fff58ecc0e8, exception=0x00007fff58ecc110) + 83 at CallData.cpp:44
    frame #16: 0x0000000113ea7eab WebCore`WebCore::JSMainThreadExecState::call(exec=0x000000011c12f4b0, functionObject=JSValue at 0x00007fff58ecbfc0, callType=CallTypeJS, callData=0x00007fff58ecc130, thisValue=JSValue at 0x00007fff58ecbfb8, args=0x00007fff58ecc0e8, exception=0x00007fff58ecc110) + 107 at JSMainThreadExecState.h:56
    frame #17: 0x000000011415769d WebCore`WebCore::functionCallHandlerFromAnyThread(exec=0x000000011c12f4b0, functionObject=JSValue at 0x00007fff58ecc040, callType=CallTypeJS, callData=0x00007fff58ecc130, thisValue=JSValue at 0x00007fff58ecc038, args=0x00007fff58ecc0e8, exception=0x00007fff58ecc110) + 109 at JSMainThreadExecState.cpp:52
    frame #18: 0x000000011191beb8 JavaScriptCore`Deprecated::ScriptFunctionCall::call(this=0x00007fff58ecc4b8, hadException=0x00007fff58ecc2bf) + 488 at ScriptFunctionCall.cpp:138
    frame #19: 0x000000011154b8d1 JavaScriptCore`Inspector::InjectedScriptBase::callFunctionWithEvalEnabled(this=0x00007fff58ecc640, function=0x00007fff58ecc4b8, hadException=0x00007fff58ecc2bf) const + 193 at InjectedScriptBase.cpp:87
    frame #20: 0x000000011154ba09 JavaScriptCore`Inspector::InjectedScriptBase::makeCall(this=0x00007fff58ecc640, function=0x00007fff58ecc4b8, result=0x00007fff58ecc438) + 137 at InjectedScriptBase.cpp:104
    frame #21: 0x000000011154bb9e JavaScriptCore`Inspector::InjectedScriptBase::makeEvalCall(this=0x00007fff58ecc640, errorString=0x00007fff58ecca88, function=0x00007fff58ecc4b8, objectResult=0x00007fff58ecca78, wasThrown=0x00007fff58ecca70, savedResultIndex=0x00007fff58ecca68) + 78 at InjectedScriptBase.cpp:118
    frame #22: 0x0000000111546fad JavaScriptCore`Inspector::InjectedScript::evaluate(this=0x00007fff58ecc640, errorString=0x00007fff58ecca88, expression=0x00007fff58eccb68, objectGroup=0x00007fff58ecc630, includeCommandLineAPI=true, returnByValue=false, generatePreview=true, saveResult=true, result=0x00007fff58ecca78, wasThrown=0x00007fff58ecca70, savedResultIndex=0x00007fff58ecca68) + 445 at InjectedScript.cpp:68
    frame #23: 0x00000001115d9ddc JavaScriptCore`Inspector::InspectorRuntimeAgent::evaluate(this=0x000000011dfe2420, errorString=0x00007fff58ecca88, expression=0x00007fff58eccb68, objectGroup=0x00007fff58eccb48, includeCommandLineAPI=0x00007fff58eccb36, doNotPauseOnExceptionsAndMuteConsole=0x00007fff58eccb1e, executionContextId=0x0000000000000000, returnByValue=0x00007fff58eccaee, generatePreview=0x00007fff58eccad6, saveResult=0x00007fff58eccabe, result=0x00007fff58ecca78, wasThrown=0x00007fff58ecca70, savedResultIndex=0x00007fff58ecca68) + 636 at InspectorRuntimeAgent.cpp:129
    frame #24: 0x00000001115da02c JavaScriptCore`non-virtual thunk to Inspector::InspectorRuntimeAgent::evaluate(this=0x000000011dfe2430, errorString=0x00007fff58ecca88, expression=0x00007fff58eccb68, objectGroup=0x00007fff58eccb48, includeCommandLineAPI=0x00007fff58eccb36, doNotPauseOnExceptionsAndMuteConsole=0x00007fff58eccb1e, executionContextId=0x0000000000000000, returnByValue=0x00007fff58eccaee, generatePreview=0x00007fff58eccad6, saveResult=0x00007fff58eccabe, result=0x00007fff58ecca78, wasThrown=0x00007fff58ecca70, savedResultIndex=0x00007fff58ecca68) + 252 at InspectorRuntimeAgent.cpp:135
    frame #25: 0x00000001115a7ca2 JavaScriptCore`Inspector::RuntimeBackendDispatcher::evaluate(this=0x000000011de128e8, callId=54, message=0x0000000125fdc910) + 2690 at InspectorBackendDispatchers.cpp:4810
    frame #26: 0x00000001115a6cf3 JavaScriptCore`Inspector::RuntimeBackendDispatcher::dispatch(this=0x000000011de128e8, callId=54, method=0x00007fff58eccd90, message=0x00007fff58eccd88) + 739 at InspectorBackendDispatchers.cpp:4733
    frame #27: 0x000000011155f345 JavaScriptCore`Inspector::BackendDispatcher::dispatch(this=0x000000011de15968, message=0x00007fff58ecd040) + 1509 at InspectorBackendDispatcher.cpp:129
    frame #28: 0x0000000113d66e51 WebCore`WebCore::InspectorController::dispatchMessageFromFrontend(this=0x000000011dfe3000, message=0x00007fff58ecd040) + 81 at InspectorController.cpp:356
    frame #29: 0x000000010e501253 WebKit`WebKit::WebInspector::sendMessageToBackend(this=0x00007fe49700f6a8, message=0x00007fff58ecd040) + 83 at WebInspector.cpp:245
    frame #30: 0x000000010e50c24f WebKit`void IPC::callMemberFunctionImpl<WebKit::WebInspector, void (WebKit::WebInspector::*)(WTF::String const&), std::__1::tuple<WTF::String>, 0ul>(object=0x00007fe49700f6a8, function=0x000000010e501200, args=0x00007fff58ecd040, (null)=index_sequence<0> at 0x00007fff58eccf70)(WTF::String const&), std::__1::tuple<WTF::String>&&, std::index_sequence<0ul>) + 159 at HandleMessage.h:16
    frame #31: 0x000000010e50c1a8 WebKit`void IPC::callMemberFunction<WebKit::WebInspector, void (WebKit::WebInspector::*)(WTF::String const&), std::__1::tuple<WTF::String>, std::make_index_sequence<1ul> >(args=0x00007fff58ecd040, object=0x00007fe49700f6a8, function=0x000000010e501200)(WTF::String const&)) + 88 at HandleMessage.h:22
    frame #32: 0x000000010e50c116 WebKit`void IPC::handleMessage<Messages::WebInspector::SendMessageToBackend, WebKit::WebInspector, void (WebKit::WebInspector::*)(WTF::String const&)>(decoder=0x0000000125f9cc00, object=0x00007fe49700f6a8, function=0x000000010e501200)(WTF::String const&)) + 230 at HandleMessage.h:92
    frame #33: 0x000000010e50b64a WebKit`WebKit::WebInspector::didReceiveMessage(this=0x00007fe49700f6a8, connection=0x000000011d7fb798, decoder=0x0000000125f9cc00) + 1306 at WebInspectorMessageReceiver.cpp:76
    frame #34: 0x000000010e50b6b7 WebKit`non-virtual thunk to WebKit::WebInspector::didReceiveMessage(this=0x00007fe49700f6b8, connection=0x000000011d7fb798, decoder=0x0000000125f9cc00) + 55 at WebInspectorMessageReceiver.cpp:94
    frame #35: 0x000000010dec9873 WebKit`IPC::Connection::dispatchMessage(this=0x000000011d7fb798, decoder=0x0000000125f9cc00) + 51 at Connection.cpp:847
    frame #36: 0x000000010dec1c80 WebKit`IPC::Connection::dispatchMessage(this=0x000000011d7fb798, message=unique_ptr<IPC::MessageDecoder, std::__1::default_delete<IPC::MessageDecoder> > at 0x00007fff58ecd4c8) + 416 at Connection.cpp:870
    frame #37: 0x000000010dec9e6f WebKit`IPC::Connection::dispatchOneMessage(this=0x000000011d7fb798) + 1519 at Connection.cpp:898
    frame #38: 0x000000010decb55d WebKit`IPC::Connection::enqueueIncomingMessage(this=0x00007fe492c120a8)::$_9::operator()() const + 29 at Connection.cpp:841
    frame #39: 0x000000010decb52c WebKit`std::__1::__function::__func<IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::MessageDecoder, std::__1::default_delete<IPC::MessageDecoder> >)::$_9, std::__1::allocator<IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::MessageDecoder, std::__1::default_delete<IPC::MessageDecoder> >)::$_9>, void ()>::operator()() [inlined] decltype(this=0x00007fe492c120a8, __f=0x00007fe492c120a8)::$_9&>(fp)(std::__1::forward<>(fp0))) std::__1::__invoke<IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::MessageDecoder, std::__1::default_delete<IPC::MessageDecoder> >)::$_9&>(IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::MessageDecoder, std::__1::default_delete<IPC::MessageDecoder> >)::$_9&&&) + 60 at __functional_base:413
    frame #40: 0x000000010decb51b WebKit`std::__1::__function::__func<IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::MessageDecoder, std::__1::default_delete<IPC::MessageDecoder> >)::$_9, std::__1::allocator<IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::MessageDecoder, std::__1::default_delete<IPC::MessageDecoder> >)::$_9>, void ()>::operator(this=0x00007fe492c120a0)() + 43 at functional:1370
    frame #41: 0x000000011153434a JavaScriptCore`std::__1::function<void ()>::operator(this=0x00007fff58ecd9c0)() const + 26 at functional:1755
    frame #42: 0x0000000111a2f452 JavaScriptCore`WTF::RunLoop::performWork(this=0x000000011dff9000) + 306 at RunLoop.cpp:104
    frame #43: 0x0000000111a30724 JavaScriptCore`WTF::RunLoop::performWork(context=0x000000011dff9000) + 36 at RunLoopCF.cpp:38
    frame #44: 0x00007fff85b0ba01 CoreFoundation`__CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17
    frame #45: 0x00007fff85afdb8d CoreFoundation`__CFRunLoopDoSources0 + 269
    frame #46: 0x00007fff85afd1bf CoreFoundation`__CFRunLoopRun + 927
    frame #47: 0x00007fff85afcbd8 CoreFoundation`CFRunLoopRunSpecific + 296
    frame #48: 0x00007fff8ada356f HIToolbox`RunCurrentEventLoopInMode + 235
    frame #49: 0x00007fff8ada32ea HIToolbox`ReceiveNextEventCommon + 431
    frame #50: 0x00007fff8ada312b HIToolbox`_BlockUntilNextEventMatchingListInModeWithFilter + 71
    frame #51: 0x00007fff87dd59bb AppKit`_DPSNextEvent + 978
    frame #52: 0x00007fff87dd4f68 AppKit`-[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 346
    frame #53: 0x00007fff87dcabf3 AppKit`-[NSApplication run] + 594
    frame #54: 0x00007fff87d47354 AppKit`NSApplicationMain + 1832
    frame #55: 0x00007fff8fc10958 libxpc.dylib`_xpc_objc_main + 793
    frame #56: 0x00007fff8fc12060 libxpc.dylib`xpc_main + 490
    frame #57: 0x0000000106d31185 com.apple.WebKit.WebContent.Development`main(argc=1, argv=0x00007fff58ecf2e8) + 37 at XPCServiceMain.Development.mm:162
    frame #58: 0x00007fff898065c9 libdyld.dylib`start + 1
    frame #59: 0x00007fff898065c9 libdyld.dylib`start + 1
Comment 1 Joseph Pecoraro 2015-03-24 21:53:02 PDT
> var baseclass = class A { constructor(){} methodA(a,b){} };
> var derivedclass = class B extends baseclass { constructor(){} methodB(a,b){} };
> new derivedclass;

As written I would expect a TDZ exception since derivedclass does not have a call to super() in its constructor.
Comment 2 Joseph Pecoraro 2015-03-26 17:09:08 PDT
Thanks to Mark Lam's help, we deduced this crash only happens when the type profiler is enabled.

Reduction using `jsc`:

shell> cd Build/Debug
shell> JSC_enableTypeProfiler=1 DYLD_FRAMEWORK_PATH=$PWD ./jsc 
jsc> var base = class A { constructor() {} }; var derived = class B extends base { constructor() { super(); } }; new derived;
Segmentation fault: 11
Comment 3 Joseph Pecoraro 2015-03-26 17:31:39 PDT
<rdar://problem/20279177>
Comment 4 Joseph Pecoraro 2015-03-26 17:34:01 PDT
Ryosuke pointed to an emitMove(&m_thisRegister, addConstantEmptyValue()) we do for derived constructors that should probably not be profiled. Making this particular move not profile fixes the issue.

So we're going to suggest adding a new method emitMoveEmptyValue(). Also looking into adding an ASSERT to catch this earlier, and see if there is a way to write tests with the type profiler enabled.
Comment 5 Saam Barati 2015-03-26 17:57:00 PDT
(In reply to comment #4)
> Ryosuke pointed to an emitMove(&m_thisRegister, addConstantEmptyValue()) we
> do for derived constructors that should probably not be profiled. Making
> this particular move not profile fixes the issue.
> 
> So we're going to suggest adding a new method emitMoveEmptyValue(). Also
> looking into adding an ASSERT to catch this earlier, and see if there is a
> way to write tests with the type profiler enabled.

This bug: https://bugs.webkit.org/show_bug.cgi?id=136359
should fix this problem.
Comment 6 Joseph Pecoraro 2015-03-26 18:10:57 PDT
Created attachment 249547 [details]
[PATCH] Proposed Fix
Comment 7 Joseph Pecoraro 2015-03-26 18:16:34 PDT
Comment on attachment 249547 [details]
[PATCH] Proposed Fix

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

> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:998
> +    m_staticPropertyAnalyzer.mov(dst->index(), emptyValue->index());

I wasn't sure if we could drop this as well:

  "Used for flow-insensitive static analysis of the number of properties assigned to an object"

I don't think we can ever have properties of an object that are uninitialized. My understanding is TDZ only affects lexically scoped variables and `this`, neither of which are properties.
Comment 8 Ryosuke Niwa 2015-03-26 18:42:44 PDT
Comment on attachment 249547 [details]
[PATCH] Proposed Fix

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

>> Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp:998
>> +    m_staticPropertyAnalyzer.mov(dst->index(), emptyValue->index());
> 
> I wasn't sure if we could drop this as well:
> 
>   "Used for flow-insensitive static analysis of the number of properties assigned to an object"
> 
> I don't think we can ever have properties of an object that are uninitialized. My understanding is TDZ only affects lexically scoped variables and `this`, neither of which are properties.

Yeah, just delete this line.
Comment 9 Joseph Pecoraro 2015-03-26 19:57:12 PDT
http://trac.webkit.org/changeset/182050
Comment 10 Joseph Pecoraro 2015-03-26 20:00:19 PDT
> This bug: https://bugs.webkit.org/show_bug.cgi?id=136359
> should fix this problem.

Oh, I missed this comment earlier! Yes, that sounds right.