After r172867: <http://trac.webkit.org/changeset/172867>, there is a new crash in js/dom/line-column-numbers.html: Process: com.apple.WebKit.WebContent.Development [38894] Path: /Volumes/VOLUME/*/WebKit.framework/Versions/A/XPCServices/com.apple.WebKit.WebContent.Development.xpc/Contents/MacOS/com.apple.WebKit.WebContent.Development Identifier: com.apple.WebKit.WebContent.Development Version: 538+ (538.45+) Code Type: X86-64 (Native) Parent Process: ??? [1] Responsible: com.apple.WebKit.WebContent.Development [38894] User ID: 501 Date/Time: 2014-08-22 16:50:17.556 -0700 OS Version: Mac OS X 10.9.4 (13E28) Report Version: 11 Anonymous UUID: 615A0368-B225-16FD-FF14-A202D44A3EC4 Crashed Thread: 0 Dispatch queue: com.apple.main-thread Exception Type: EXC_BAD_ACCESS (SIGSEGV) Exception Codes: EXC_I386_GPFLT Application Specific Information: CRASHING TEST:js/dom/line-column-numbers.html Thread 0 Crashed:: Dispatch queue: com.apple.main-thread 0 com.apple.JavaScriptCore 0x0000000112cfbea7 JSC::CodeBlock::handlerForBytecodeOffset(unsigned int) + 39 (Vector.h:610) 1 com.apple.JavaScriptCore 0x0000000112f01f5f JSC::UnwindFunctor::operator()(JSC::StackVisitor&) + 111 (Interpreter.cpp:665) 2 com.apple.JavaScriptCore 0x0000000112efef5b JSC::Interpreter::unwind(void*&, JSC::ExecState*&, JSC::JSValue&) + 555 (StackVisitor.h:129) 3 com.apple.JavaScriptCore 0x0000000112f1940b JSC::genericUnwind(JSC::VM*, JSC::ExecState*, JSC::JSValue) + 91 (JITExceptions.cpp:67) 4 com.apple.JavaScriptCore 0x0000000112f38e4c lookupExceptionHandlerFromCallerFrame + 60 (JITOperations.cpp:1854) 5 ??? 0x000046399ca02d83 0 + 77213254823299 6 com.apple.JavaScriptCore 0x00000001130015e9 vmEntryToJavaScript + 326 7 com.apple.JavaScriptCore 0x0000000112f17b73 JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*) + 35 (VM.h:373) 8 com.apple.JavaScriptCore 0x0000000112f01096 JSC::Interpreter::executeCall(JSC::ExecState*, JSC::JSObject*, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) + 438 (Interpreter.cpp:989) 9 com.apple.JavaScriptCore 0x0000000112ceb9be JSC::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) + 62 (CallData.cpp:39) 10 com.apple.JavaScriptCore 0x0000000112f90295 JSC::JSObject::defaultValue(JSC::JSObject const*, JSC::ExecState*, JSC::PreferredPrimitiveType) + 1189 (Register.h:116) 11 ??? 0x000046399ca02c8c 0 + 77213254823052 12 com.apple.JavaScriptCore 0x00000001130015e9 vmEntryToJavaScript + 326 13 com.apple.JavaScriptCore 0x0000000112f17b73 JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*) + 35 (VM.h:373) 14 com.apple.JavaScriptCore 0x0000000112f01096 JSC::Interpreter::executeCall(JSC::ExecState*, JSC::JSObject*, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) + 438 (Interpreter.cpp:989) 15 com.apple.JavaScriptCore 0x0000000112ceb9be JSC::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) + 62 (CallData.cpp:39) 16 com.apple.JavaScriptCore 0x0000000112f90295 JSC::JSObject::defaultValue(JSC::JSObject const*, JSC::ExecState*, JSC::PreferredPrimitiveType) + 1189 (Register.h:116) 17 ??? 0x000046399ca02c8c 0 + 77213254823052 18 com.apple.JavaScriptCore 0x00000001130015e9 vmEntryToJavaScript + 326 19 com.apple.JavaScriptCore 0x0000000112f17b73 JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*) + 35 (VM.h:373) 20 com.apple.JavaScriptCore 0x0000000112f01096 JSC::Interpreter::executeCall(JSC::ExecState*, JSC::JSObject*, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) + 438 (Interpreter.cpp:989) 21 com.apple.JavaScriptCore 0x0000000112ceb9be JSC::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) + 62 (CallData.cpp:39)
Created attachment 237026 [details] Patch
Comment on attachment 237026 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=237026&action=review > Source/JavaScriptCore/jit/JITOperations.cpp:1847 > - NativeCallFrameTracer tracer(vm, callerFrame); > + NativeCallFrameTracerWithRestore tracer(vm, vmEntryFrame, callerFrame); Why the need for the restoration of vm->topVMEntryFrame and vm->topCallFrame?
(In reply to comment #2) > (From update of attachment 237026 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=237026&action=review > > > Source/JavaScriptCore/jit/JITOperations.cpp:1847 > > - NativeCallFrameTracer tracer(vm, callerFrame); > > + NativeCallFrameTracerWithRestore tracer(vm, vmEntryFrame, callerFrame); > > Why the need for the restoration of vm->topVMEntryFrame and vm->topCallFrame? We need to use the caller's CallFrame and VMEntryFrame when calling genericUnwind(). NativeCallFrameTracerWithRestore() does that for us. In general, NativeCallFrameTracerWithRestore(), restores the values because we may do more processing that requires the current callFrame and vmEntryFrame before we get to the catch handler where we change these to the catch values. In this particular case, that restoration isn't currently needed, but we add complexity and possible future confusion if we create another NativeCallFrameTracerXXX() version that doesn't restore the values.
Comment on attachment 237026 [details] Patch r=me The sentinel value option is looking better every day. VM::topVMEntryFrame is turning out to be very subtle, and easy to use wrong.
Comment on attachment 237026 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=237026&action=review >>> Source/JavaScriptCore/jit/JITOperations.cpp:1847 >>> + NativeCallFrameTracerWithRestore tracer(vm, vmEntryFrame, callerFrame); >> >> Why the need for the restoration of vm->topVMEntryFrame and vm->topCallFrame? > > We need to use the caller's CallFrame and VMEntryFrame when calling genericUnwind(). NativeCallFrameTracerWithRestore() does that for us. > > In general, NativeCallFrameTracerWithRestore(), restores the values because we may do more processing that requires the current callFrame and vmEntryFrame before we get to the catch handler where we change these to the catch values. In this particular case, that restoration isn't currently needed, but we add complexity and possible future confusion if we create another NativeCallFrameTracerXXX() version that doesn't restore the values. Would you mind adding this comment to the ChangeLog? I think it’s more informative than the existing one that says "NativeCallFrameTracerWithRestore() so that VM::topVMEntryFrame will be updated before calling genericUnwind()."
Committed r172932: <http://trac.webkit.org/changeset/172932>
(In reply to comment #5) > (From update of attachment 237026 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=237026&action=review > > >>> Source/JavaScriptCore/jit/JITOperations.cpp:1847 > >>> + NativeCallFrameTracerWithRestore tracer(vm, vmEntryFrame, callerFrame); > >> > >> Why the need for the restoration of vm->topVMEntryFrame and vm->topCallFrame? > > > > We need to use the caller's CallFrame and VMEntryFrame when calling genericUnwind(). NativeCallFrameTracerWithRestore() does that for us. > > > > In general, NativeCallFrameTracerWithRestore(), restores the values because we may do more processing that requires the current callFrame and vmEntryFrame before we get to the catch handler where we change these to the catch values. In this particular case, that restoration isn't currently needed, but we add complexity and possible future confusion if we create another NativeCallFrameTracerXXX() version that doesn't restore the values. > > Would you mind adding this comment to the ChangeLog? I think it’s more informative than the existing one that says "NativeCallFrameTracerWithRestore() so that VM::topVMEntryFrame will be updated before calling genericUnwind()." Added to ChangeLog.