Bug 143105

<rdar://problem/20310775>

*** Bug 143109 has been marked as a duplicate of this bug. ***

This bug is not unique to Windows and Linux. On Mac, the inspector-protocol/debugger tests were just excluded by TestExpectations. I verified that inspector-protocol/debugger/setBreakpoint-dfg-and-modify-local.html runs to completion with a debug build of r181992. With r182004 (which has all of Fil's fixes after 181993), the crash reproduces.

frame #1: 0x00000001004baaef JavaScriptCore`JSC::JSScope* JSC::jsCast<JSC::JSScope*>(from=JSValue at 0x00007fff5fbf9e88) + 111 at JSCell.h:250 247 template<typename To> 248 inline To jsCast(JSValue from) 249 { -> 250 ASSERT_WITH_SECURITY_IMPLICATION(from.isCell() && from.asCell()->JSCell::inherits(std::remove_pointer<To>::type::info())); 251 return static_cast<To>(from.asCell()); 252 } 253 (lldb) up frame #2: 0x00000001004baa72 JavaScriptCore`JSC::Register::scope(this=0x00007fff5fbfa468) const + 34 at JSScope.h:239 236 237 inline JSScope* Register::scope() const 238 { -> 239 return jsCast<JSScope*>(jsValue()); 240 } 241 242 inline JSGlobalObject* ExecState::lexicalGlobalObject() const (lldb) p jsValue() (JSC::JSValue) $0 = { u = { asInt64 = 10 ptr = 0x000000000000000a asBits = (payload = 10, tag = 0) } } (lldb) up frame #3: 0x00000001004b9f6d JavaScriptCore`JSC::ExecState::scope(this=0x00007fff5fbfa470, scopeRegisterOffset=-1) const + 45 at CallFrame.h:50 47 CodeBlock* codeBlock() const { return this[JSStack::CodeBlock].Register::codeBlock(); } 48 JSScope* scope(int scopeRegisterOffset) const 49 { -> 50 ASSERT(this[scopeRegisterOffset].Register::scope()); 51 return this[scopeRegisterOffset].Register::scope(); 52 } 53 (lldb) p this->vm().topCallFrame (JSC::ExecState *) $2 = 0x00007fff5fbfa420 (lldb) p this (JSC::ExecState *) $3 = 0x00007fff5fbfa470 (lldb) p debugPrintStack(this->vm().topCallFrame) frame 0x7fff5fbfa420 { name 'breakpointBasic' sourceURL 'file:///Volumes/Data/ws2/OpenSource/LayoutTests/inspector-protocol/debugger/resources/breakpoint.js' isInlinedFrame 0 callee 0x11a169770 returnPC 0x34b3272039e7 callerFrame 0x7fff5fbfa470 rawLocationBits 13 0xd codeBlock 0x11b6a7720 bytecodeOffset 13 0xd / 54 line 3 column 5 jitType 2 <InterpreterThunk> isOptimizingJIT 0 hasCodeOrigins 0 } frame 0x7fff5fbfa470 { <============================================ the CallFrame whose Scope we're not happy about. name 'notInlineable2' sourceURL 'file:///Volumes/Data/ws2/OpenSource/LayoutTests/inspector-protocol/debugger/resources/breakpoint.js' isInlinedFrame 0 callee 0x11a1694f0 returnPC 0x100aeb906 callerFrame 0x7fff5fbfa4d0 rawLocationBits 111 0x6f codeBlock 0x11b7844c0 bytecodeOffset 111 0x6f / 143 line 64 column 24 jitType 3 <BaselineJIT> isOptimizingJIT 0 hasCodeOrigins 0 } frame 0x7fff5fbfa4d0 { name 'callNotInlineable2' sourceURL 'file:///Volumes/Data/ws2/OpenSource/LayoutTests/inspector-protocol/debugger/resources/breakpoint.js' isInlinedFrame 0 callee 0x11a169470 returnPC 0x100aeb906 callerFrame 0x7fff5fbfa520 rawLocationBits 42 0x2a codeBlock 0x11b6a7000 bytecodeOffset 42 0x2a / 187 line 79 column 32 jitType 2 <InterpreterThunk> isOptimizingJIT 0 hasCodeOrigins 0 } frame 0x7fff5fbfa520 { name 'eval code' sourceURL '' isInlinedFrame 0 callee 0x11a0ff150 returnPC 0x100ae51c9 callerFrame 0x7fff5fbfbaf0 rawLocationBits 32 0x20 codeBlock 0x11b577be0 bytecodeOffset 32 0x20 / 48 line 1 column 19 jitType 2 <InterpreterThunk> isOptimizingJIT 0 hasCodeOrigins 0 } frame 0x7fff5fbfbaf0 { name 'eval' sourceURL '[native code]' isInlinedFrame 0 callee 0x11a16e230 returnPC 0x100aeb893 callerFrame 0x7fff5fbfbb60 rawLocationBits 1 0x1 codeBlock 0x0 } frame 0x7fff5fbfbb60 { name '_evaluateOn' sourceURL '' isInlinedFrame 0 callee 0x11a24eeb0 returnPC 0x100aeb893 callerFrame 0x7fff5fbfbc30 rawLocationBits 1173 0x495 codeBlock 0x11b790be0 bytecodeOffset 1173 0x495 / 1630 line 108 column 29 jitType 2 <InterpreterThunk> isOptimizingJIT 0 hasCodeOrigins 0 } frame 0x7fff5fbfbc30 { name '_evaluateAndWrap' sourceURL '' isInlinedFrame 0 callee 0x11a24ef30 returnPC 0x100aeb893 callerFrame 0x7fff5fbfbcd0 rawLocationBits 199 0xc7 codeBlock 0x11b790980 bytecodeOffset 199 0xc7 / 421 line 86 column 105 jitType 2 <InterpreterThunk> isOptimizingJIT 0 hasCodeOrigins 0 } frame 0x7fff5fbfbcd0 { name 'evaluate' sourceURL '' isInlinedFrame 0 callee 0x11a24eff0 returnPC 0x100ae51c9 callerFrame 0x0 rawLocationBits 169 0xa9 codeBlock 0x11b790720 bytecodeOffset 169 0xa9 / 185 line 76 column 30 jitType 2 <InterpreterThunk> isOptimizingJIT 0 hasCodeOrigins 0 } (lldb)

If we are in DFG or FTL code then there is no notion of a scope register. If all is well, the DFG/FTL CodeBlock should report that its scope register is invalid. The debugger should then avoid reading the scope register.

(In reply to comment #5) > If we are in DFG or FTL code then there is no notion of a scope register. > If all is well, the DFG/FTL CodeBlock should report that its scope register > is invalid. The debugger should then avoid reading the scope register. The offending call frame is a Baseline JIT frame (jitType 3): frame 0x7fff5fbfa470 { name 'notInlineable2' sourceURL 'file:///Volumes/Data/ws2/OpenSource/LayoutTests/inspector-protocol/debugger/resources/breakpoint.js' isInlinedFrame 0 callee 0x11a1694f0 returnPC 0x100aeb906 callerFrame 0x7fff5fbfa4d0 rawLocationBits 111 0x6f codeBlock 0x11b7844c0 bytecodeOffset 111 0x6f / 143 line 64 column 24 jitType 3 <BaselineJIT> isOptimizingJIT 0 hasCodeOrigins 0 }

(In reply to comment #6) > The offending call frame is a Baseline JIT frame (jitType 3): > > frame 0x7fff5fbfa470 { > name 'notInlineable2' According to compilation logging, I see: Optimized #ETji8F:[0x11b741720->0x11b7bfbe0->0x11a1ad370, NoneFunctionCall, 200] using DFGMode with DFG into 1409 bytes in 5.993043 ms. Optimized notInlineable2#B9qPQQ:[0x11b5f04c0->0x11b7884c0->0x11a1aec70, NoneFunctionCall, 143] using DFGMode with DFG into 2086 bytes in 3.700699 ms. // Note: notInlineable2 was DFG compiled. Optimized dispatchMessageAsync#C3jYlh:[0x11b5e5260->0x11b7bf980->0x11a1adf70, NoneFunctionCall, 204] using DFGMode with DFG into 2044 bytes in 5.551525 ms. Speculation failure in dispatchMessageAsync#C3jYlh:[0x11b5e5260->0x11b7bf980->0x11a1adf70, DFGFunctionCall, 204] @ exit #4 (bc#93, BadConstantCache) with executeCounter = 0.000000/1706.000000, -1000, reoptimizationRetryCounter = 0, optimizationDelayCounter = 5, osrExitCounter = 0 ... // And a whole bunch more of these, and then ... Speculation failure in notInlineable2#B9qPQQ:[0x11b5f04c0->0x11b7884c0->0x11a1aec70, DFGFunctionCall, 143] @ exit #5 (bc#111, InadequateCoverage) with executeCounter = 0.000000/1564.000000, -1000, reoptimizationRetryCounter = 0, optimizationDelayCounter = 5, osrExitCounter = 0 GPRs at time of exit: rax:0x7 rdx:0x11a027301 rcx:0x1 rbx:0x11a5df8f0 rdi:0x11a57e070 rsi:0x10107af20 r8:0x2a01 r9:0x15 r10:0x17 r12:0x0 r13:0x7fff5fbfc7a0 FPRs at time of exit: xmm0:cdcdcdcdcdcdcdcd:-6277438562204192487878988888393020692503707483087375482269988814848.000000 xmm1:4000000000000000:2.000000 xmm2:4330000000000000:4503599627370496.000000 xmm3:c3dffffffffff41a:-9223372036851656704.000000 xmm4:6c6f437472617473:21049630149871791053826342618222142983692456644967968893798936375648742956543650966195804022640459996779983966877891001984633477511305401509268213325978467394641525279739396411731393716219460619266686303157939601408.000000 xmm5:7473222c22223a22:8767393038784822975565048324879067089945013814682974788281033316551431634195104624424045912715265664370247167655522244915117819586740099857108239245644458920684638480833845936489766873021065194489714291357449872173134346573967951577187762141067353260032.000000

Skipped the crashing test in r182072: <http://trac.webkit.org/r182072>. Please revert when the bug is fixed.

Created attachment 249778 [details] the patch. I tested this patch by turning on Options::forceDebuggerBytecodeGeneration and running the JSC stress tests with the patch. Unfortunately, this uncovered may pre-existing regressions not due to this patch (see webkit.org/b/143160). So, instead of expecting no failures, I made sure that there are no new failures between the results of running with and without this patch, both with Options::forceDebuggerBytecodeGeneration enabled.

Comment on attachment 249778 [details] the patch. View in context: https://bugs.webkit.org/attachment.cgi?id=249778&action=review You need to fix the flushing in setLocal(). > Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:402 > + if (m_hasDebuggerEnabled && operand == m_codeBlock->scopeRegister()) > + flush(operand); This is wrong. You have to flush before setting, not after. Please look at how the argument flush happens, above. You should do the flush at the same point where argument flushes happen. A Flush is a liveness marker. You flush at the point where a variable gets killed. Since the scope is "always" live, it gets killed when you set a new value. Therefore, just before doing a SetLocal with a new value, you must do a Flush on the old value. Also, I believe that you need to conditionalize this on setMode != ImmediateNakedSet. So, inside that if block, just after where we dofindArgumentPositionForLocal(), you should do this Flush. > Source/JavaScriptCore/dfg/DFGStackLayoutPhase.cpp:178 > + if (LIKELY(!m_graph.hasDebuggerEnabled())) Probably no need to use LIKELY in the compiler, because the costs of these operations are so small relative to the real heavy phases. But I don't object to this and you don't have to remove it.

Created attachment 249784 [details] patch 2: applied Filip's feedback

Thanks. Landed in r182167: <http://trac.webkit.org/r182167>.