This is the cause for the crash described in <https://bugs.webkit.org/show_bug.cgi?id=150251> that was temporarily resolved by turning off tail calls. Here is the relevant part of the backtrace: * thread #1: tid = 0xb381ca, 0x000000011687b4be, queue = 'com.apple.main-thread, stop reason = EXC_BAD_ACCESS (code=EXC_I386) frame #0: 0x000000011687b4be JavaScriptCore`JSC::JITCode::execute(this=<unavailable>, vm=0xffff000000000000, protoCallFrame=0x00007fff50a830b8) + 158 at JITCode.cpp:81 frame #1: 0x0000000116857dcf JavaScriptCore`JSC::Interpreter::executeCall(this=<unavailable>, callFrame=0x00007fff50a83220, function=0x000000011d836d70, callType=<unavailable>, callData=0x00007fff50a83180, thisValue=JSValue @ 0x00007fff50a83098, args=<unavailable>) + 447 at Interpreter.cpp:1024 frame #2: 0x00000001164f99ce JavaScriptCore`JSC::call(exec=<unavailable>, functionObject=<unavailable>, callType=<unavailable>, callData=<unavailable>, thisValue=<unavailable>, args=<unavailable>) + 62 at CallData.cpp:39 frame #3: 0x00000001168c409a JavaScriptCore`JSC::boundFunctionCall(exec=0x00007fff50a83220) + 618 at JSBoundFunction.cpp:54 frame #4: 0x00003cff43e01028 0x3cff43e5c5a6 frame #5: 0x00003cff43e5c5a6 shouldComponentUpdate#CiYug9 [Baseline](Cell[Object ID: 19284]: 0x1351bf340, Cell[Object ID: 17094]: 0x11ecfa9e0, Cell[Object ID: 19279]: 0x125946080, Cell[Object ID: 9671]: 0x13bd3c140) frame #6: 0x00003cff446f7c67 performUpdateIfNecessary#ADcVpe [DFG](Cell[Object ID: 19284]: 0x1351bf340, Cell[Object ID: 15608]: 0x11e863370) frame #7: 0x00003cff44219470 receiveComponent#D0omON [DFG](Cell[Object ID: 19284]: 0x1351bf340, Cell[Object ID: 15394]: 0x12e9ce200, Cell[Object ID: 15608]: 0x11e863370) frame #8: 0x00003cff4428f565 _updateChildren#D8I5sZ [DFG](Cell[Object ID: 15798]: 0x11dcf7400, Cell[Array ID: 184]: 0x12285a320, Cell[Object ID: 15608]: 0x11e863370) frame #9: 0x00003cff44294caf updateChildren#AAAS3a [DFG](Cell[Object ID: 15798]: 0x11dcf7400, Cell[Array ID: 184]: 0x12285a320, Cell[Object ID: 15608]: 0x11e863370) ... The damage actually occurs before we get here. The problem is that vm has a totally bad value (0xffff000000000000). This happens to be the tagTypeNumber constant used by JSVALUE64 builds. The top 4 frames of the stack show that we are calling a bound function. The called function and its callee are: hasFalkorPropChanged#CPIwPX: function(e) { "use strict"; if (this.customModelChangeDetector) return this.customModelChangeDetector.didChange(); var t = e.getBoundValue(), r = t.__generation, n = t.__key; return this.state.prevGeneration !== r || this.state.prevKey !== n } didChange#Agw6xZ: function() { "use strict"; var e = this.model.getValueSync("length"); return e !== this.prevModelLength ? !0 : this.isTransientRow() && this.generateUniqueKey() !== this.prevUniqueKey ? !0 : !1 } generateUniqueKey#As9POD: function() { "use strict"; for (var e, t = this.model.getValueSync("length"), n = "", i = 0; t > i; i++) e = this.model.getValueSync([i, "summary"]), e && e.id && (n += e.id.toString().substr(-4)); return n } During DFG compilation, didChange() and generateUniqueKey() get inlined into hasFalkorPropChanged(), with didChange() in tail position and therefore made a tail call. We OSR exit from generateUniqueKey() due to a structure change. It appears that the OSR exit handler does not properly restore the callee saves registers for didChange(), which should be the callee saves for hasFalkorPropChanged().
Created attachment 263499 [details] Reduced test case
Created attachment 263533 [details] Patch
<rdar://problem/22951399>
Comment on attachment 263533 [details] Patch r=me
Comment on attachment 263533 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=263533&action=review > Source/JavaScriptCore/dfg/DFGOSRExitCompilerCommon.cpp:218 > + // Restore the inline call frame's callee saves registers callee save registers > Source/JavaScriptCore/dfg/DFGOSRExitCompilerCommon.cpp:229 > + for (unsigned i = 0; i < registerCount; i++) { > + RegisterAtOffset entry = calleeSaves->at(i); > + if (dontSaveRegisters.get(entry.reg())) > + continue; > + > + GPRReg registerToWrite; This function is very similar to emitSaveCalleeSavesFor, with the only difference being the inline offset adjustment and the !trueCaller special case. You should either factor this into a helper function that lives next to emitSaveCalleeSavesFor or parameterize emitSaveCalleeSavesFor to handle both cases. > Source/JavaScriptCore/dfg/DFGOSRExitCompilerCommon.cpp:234 > + // If we need to restore a "tag" register for a tail callee that will be the top frame, > + // get the value saved for the outermost frame instead of the actual register which > + // has a tag constant. This comment says what you did but not why. Why did you do this?
(In reply to comment #5) > Comment on attachment 263533 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=263533&action=review > > > Source/JavaScriptCore/dfg/DFGOSRExitCompilerCommon.cpp:218 > > + // Restore the inline call frame's callee saves registers > > callee save registers Fixed. > > Source/JavaScriptCore/dfg/DFGOSRExitCompilerCommon.cpp:229 > > + for (unsigned i = 0; i < registerCount; i++) { > > + RegisterAtOffset entry = calleeSaves->at(i); > > + if (dontSaveRegisters.get(entry.reg())) > > + continue; > > + > > + GPRReg registerToWrite; > > This function is very similar to emitSaveCalleeSavesFor, with the only > difference being the inline offset adjustment and the !trueCaller special > case. > > You should either factor this into a helper function that lives next to > emitSaveCalleeSavesFor or parameterize emitSaveCalleeSavesFor to handle both > cases. I refactored and moved it to be next to emitSaveCalleeSavesFor(). > > Source/JavaScriptCore/dfg/DFGOSRExitCompilerCommon.cpp:234 > > + // If we need to restore a "tag" register for a tail callee that will be the top frame, > > + // get the value saved for the outermost frame instead of the actual register which > > + // has a tag constant. > > This comment says what you did but not why. Why did you do this? Changed the comment to: // If this inlined frame is a tail call that will return back to the original caller, we need to // copy the prior contents of the tag registers already saved for the outer frame to this frame. With the refactoring, I now have in AssemblyHelpers.h: enum RestoreTagRegisterMode { UseExistingTagRegisterContents, CopySavedTagRegistersFromBaseFrame }; void emitSaveOrCopyCalleeSavesFor(CodeBlock* codeBlock, VirtualRegister offsetVirtualRegister, RestoreTagRegisterMode tagRegisterMode, GPRReg temp) { ASSERT(codeBlock); RegisterAtOffsetList* calleeSaves = codeBlock->calleeSaveRegisters(); RegisterSet dontSaveRegisters = RegisterSet(RegisterSet::stackRegisters(), RegisterSet::allFPRs()); unsigned registerCount = calleeSaves->size(); for (unsigned i = 0; i < registerCount; i++) { RegisterAtOffset entry = calleeSaves->at(i); if (dontSaveRegisters.get(entry.reg())) continue; GPRReg registerToWrite; #if USE(JSVALUE32_64) UNUSED_PARAM(tagRegisterMode); UNUSED_PARAM(temp); #else if (tagRegisterMode == CopySavedTagRegistersFromBaseFrame && (entry.reg() == GPRInfo::tagTypeNumberRegister || entry.reg() == GPRInfo::tagMaskRegister)) { registerToWrite = temp; loadPtr(AssemblyHelpers::Address(GPRInfo::callFrameRegister, entry.offset()), registerToWrite); } else #endif registerToWrite = entry.reg().gpr(); storePtr(registerToWrite, Address(framePointerRegister, offsetVirtualRegister.offsetInBytes() + entry.offset())); } } and a call site of: // Restore the inline call frame's callee save registers. // If this inlined frame is a tail call that will return back to the original caller, we need to // copy the prior contents of the tag registers already saved for the outer frame to this frame. jit.emitSaveOrCopyCalleeSavesFor( baselineCodeBlock, static_cast<VirtualRegister>(inlineCallFrame->stackOffset), trueCaller ? AssemblyHelpers::UseExistingTagRegisterContents : AssemblyHelpers::CopySavedTagRegistersFromBaseFrame, GPRInfo::regT2);
Created attachment 263602 [details] Patch for Landing
Committed r191360: <http://trac.webkit.org/changeset/191360>