| Summary: | REGRESSION (r181993): inspector-protocol/debugger/setBreakpoint-dfg-and-modify-local.html crashes | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Alexey Proskuryakov <ap> | ||||||
| Component: | JavaScriptCore | Assignee: | Mark Lam <mark.lam> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | bfulgham, fpizlo, ggaren, joepeck, mark.lam, webkit-bug-importer | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Bug Depends on: | 143160 | ||||||||
| Bug Blocks: | |||||||||
| Attachments: |
|
||||||||
|
Description
Alexey Proskuryakov
2015-03-26 10:14:16 PDT
*** 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>. |