RESOLVED FIXED 143037
WebContent Crash when instantiating class with Type Profiling enabled
https://bugs.webkit.org/show_bug.cgi?id=143037
Summary WebContent Crash when instantiating class with Type Profiling enabled
Joseph Pecoraro
Reported 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
Attachments
[PATCH] Proposed Fix (5.65 KB, patch)
2015-03-26 18:10 PDT, Joseph Pecoraro
rniwa: review+
Joseph Pecoraro
Comment 1 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.
Joseph Pecoraro
Comment 2 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
Joseph Pecoraro
Comment 3 2015-03-26 17:31:39 PDT
Joseph Pecoraro
Comment 4 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.
Saam Barati
Comment 5 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.
Joseph Pecoraro
Comment 6 2015-03-26 18:10:57 PDT
Created attachment 249547 [details] [PATCH] Proposed Fix
Joseph Pecoraro
Comment 7 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.
Ryosuke Niwa
Comment 8 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.
Joseph Pecoraro
Comment 9 2015-03-26 19:57:12 PDT
Joseph Pecoraro
Comment 10 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.
Note You need to log in before you can comment on or make changes to this bug.