136192 2014-08-22 21:15:52 -0700 After r172867 another crash in in js/dom/line-column-numbers.html 2014-08-25 12:33:29 -0700 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 msaboff msaboff fpizlo ggaren mark.lam oldest_to_newest 1030885 0 msaboff 2014-08-22 21:15:52 -0700 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) 1030893 1 237026 msaboff 2014-08-22 21:44:16 -0700 Created attachment 237026 Patch 1031169 2 237026 mark.lam 2014-08-25 10:46:24 -0700 Comment on attachment 237026 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? 1031173 3 msaboff 2014-08-25 10:57:43 -0700 (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. 1031174 4 237026 ggaren 2014-08-25 11:04:11 -0700 Comment on attachment 237026 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. 1031176 5 237026 mark.lam 2014-08-25 11:06:33 -0700 Comment on attachment 237026 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()." 1031203 6 msaboff 2014-08-25 12:32:58 -0700 Committed r172932: <http://trac.webkit.org/changeset/172932> 1031204 7 msaboff 2014-08-25 12:33:29 -0700 (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. 237026 2014-08-22 21:44:16 -0700 2014-08-25 11:06:33 -0700 Patch 136192.patch text/plain 1452 msaboff SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTcyODc5KQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE1IEBA CisyMDE0LTA4LTIyICBNaWNoYWVsIFNhYm9mZiAgPG1zYWJvZmZAYXBwbGUuY29tPgorCisgICAg ICAgIEFmdGVyIHIxNzI4NjcgYW5vdGhlciBjcmFzaCBpbiBpbiBqcy9kb20vbGluZS1jb2x1bW4t bnVtYmVycy5odG1sCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNn aT9pZD0xMzYxOTIKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAg ICAgICAqIGppdC9KSVRPcGVyYXRpb25zLmNwcDoKKyAgICAgICAgKEpTQzo6bG9va3VwRXhjZXB0 aW9uSGFuZGxlckZyb21DYWxsZXJGcmFtZSk6IENoYW5nZWQgTmF0aXZlQ2FsbEZyYW1lVHJhY2Vy KCkgdG8KKyAgICAgICAgTmF0aXZlQ2FsbEZyYW1lVHJhY2VyV2l0aFJlc3RvcmUoKSBzbyB0aGF0 IFZNOjp0b3BWTUVudHJ5RnJhbWUgd2lsbCBiZSB1cGRhdGVkCisgICAgICAgIGJlZm9yZSBjYWxs aW5nIGdlbmVyaWNVbndpbmQoKS4KKwogMjAxNC0wOC0yMSAgTWljaGFlbCBTYWJvZmYgIDxtc2Fi b2ZmQGFwcGxlLmNvbT4KIAogICAgICAgICBSRUdSRVNTSU9OKHIxNjMxNzkpOiBTcG9yYWRpYyBj cmFzaCBpbiBqcy9kb20vbGluZS1jb2x1bW4tbnVtYmVycy5odG1sIHRlc3QKSW5kZXg6IFNvdXJj ZS9KYXZhU2NyaXB0Q29yZS9qaXQvSklUT3BlcmF0aW9ucy5jcHAKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL2ppdC9KSVRPcGVyYXRpb25zLmNwcAkocmV2aXNpb24gMTcyODY3 KQorKysgU291cmNlL0phdmFTY3JpcHRDb3JlL2ppdC9KSVRPcGVyYXRpb25zLmNwcAkod29ya2lu ZyBjb3B5KQpAQCAtMTg0NCw3ICsxODQ0LDcgQEAgdm9pZCBKSVRfT1BFUkFUSU9OIGxvb2t1cEV4 Y2VwdGlvbkhhbmRsZQogICAgIENhbGxGcmFtZSogY2FsbGVyRnJhbWUgPSBleGVjLT5jYWxsZXJG cmFtZSh2bUVudHJ5RnJhbWUpOwogICAgIEFTU0VSVChjYWxsZXJGcmFtZSk7CiAKLSAgICBOYXRp dmVDYWxsRnJhbWVUcmFjZXIgdHJhY2VyKHZtLCBjYWxsZXJGcmFtZSk7CisgICAgTmF0aXZlQ2Fs bEZyYW1lVHJhY2VyV2l0aFJlc3RvcmUgdHJhY2VyKHZtLCB2bUVudHJ5RnJhbWUsIGNhbGxlckZy YW1lKTsKIAogICAgIEpTVmFsdWUgZXhjZXB0aW9uVmFsdWUgPSB2bS0+ZXhjZXB0aW9uKCk7CiAg ICAgQVNTRVJUKGV4Y2VwdGlvblZhbHVlKTsK