Bug 143105 - REGRESSION (r181993): inspector-protocol/debugger/setBreakpoint-dfg-and-modify-local.html crashes
Summary: REGRESSION (r181993): inspector-protocol/debugger/setBreakpoint-dfg-and-modif...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
URL:
Keywords: InRadar
: 143109 (view as bug list)
Depends on: 143160
Blocks:
  Show dependency treegraph
 
Reported: 2015-03-26 10:14 PDT by Alexey Proskuryakov
Modified: 2015-03-30 17:46 PDT (History)
6 users (show)

See Also:


Attachments
the patch. (8.56 KB, patch)
2015-03-30 15:48 PDT, Mark Lam
fpizlo: review-
Details | Formatted Diff | Diff
patch 2: applied Filip's feedback (8.70 KB, patch)
2015-03-30 17:31 PDT, Mark Lam
fpizlo: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 2015-03-26 10:14:16 PDT
inspector-protocol/debugger/setBreakpoint-dfg-and-modify-local.html crashes every time on Windows and Linux:

https://webkit-test-results.appspot.com/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=inspector-protocol%2Fdebugger%2FsetBreakpoint-dfg-and-modify-local.html

STDERR: ASSERTION FAILED: from.isCell() && from.asCell()->JSCell::inherits(std::remove_pointer<To>::type::info())
STDERR: ../../Source/JavaScriptCore/runtime/JSCell.h(250) : To JSC::jsCast(JSC::JSValue) [with To = JSC::JSScope*]
STDERR: 1   0x7f63e6ff3479 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(WTFCrashWithSecurityImplication+0x1e) [0x7f63e6ff3479]
STDERR: 2   0x7f63e69871ae /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(JSC::JSScope* JSC::jsCast<JSC::JSScope*>(JSC::JSValue)+0x6a) [0x7f63e69871ae]
STDERR: 3   0x7f63e6986e3e /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(JSC::Register::scope() const+0x20) [0x7f63e6986e3e]
STDERR: 4   0x7f63e6986dd7 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(JSC::ExecState::scope(int) const+0x2b) [0x7f63e6986dd7]
STDERR: 5   0x7f63e69865dc /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(JSC::DebuggerCallFrame::scope()+0x112) [0x7f63e69865dc]
STDERR: 6   0x7f63e6c710f5 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(Inspector::ScriptDebugServer::exceptionOrCaughtValue(JSC::ExecState*)+0x97) [0x7f63e6c710f5]
STDERR: 7   0x7f63e6c7003a /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(Inspector::ScriptDebugServer::dispatchDidPause(Inspector::ScriptDebugListener*)+0x134) [0x7f63e6c7003a]
STDERR: 8   0x7f63e6c70b57 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(Inspector::ScriptDebugServer::dispatchFunctionToListeners(WTF::HashSet<Inspector::ScriptDebugListener*, WTF::PtrHash<Inspector::ScriptDebugListener*>, WTF::HashTraits<Inspector::ScriptDebugListener*> > const&, void (Inspector::ScriptDebugServer::*)(Inspector::ScriptDebugListener*))+0x9f) [0x7f63e6c70b57]
STDERR: 9   0x7f63e6c70a86 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(Inspector::ScriptDebugServer::dispatchFunctionToListeners(void (Inspector::ScriptDebugServer::*)(Inspector::ScriptDebugListener*))+0x9a) [0x7f63e6c70a86]
STDERR: 10  0x7f63e6c70e4d /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(Inspector::ScriptDebugServer::handlePause(JSC::JSGlobalObject*, JSC::Debugger::ReasonForPause)+0x45) [0x7f63e6c70e4d]
STDERR: 11  0x7f63e69768a1 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(JSC::Debugger::pauseIfNeeded(JSC::ExecState*)+0x25d) [0x7f63e69768a1]
STDERR: 12  0x7f63e6976622 /home/slave/webkitgtk/gtk-linux-64-debug/build/WebKitBuild/Debug/lib/libjavascriptcoregtk-4.0.so.18(JSC::Debugger::updateCallFrameAndPauseIfNeeded(JSC::ExecState*)+0x36) [0x7f63e6976622]
...
Comment 1 Radar WebKit Bug Importer 2015-03-26 10:14:52 PDT
<rdar://problem/20310775>
Comment 2 Alexey Proskuryakov 2015-03-26 13:44:21 PDT
*** Bug 143109 has been marked as a duplicate of this bug. ***
Comment 3 Mark Lam 2015-03-26 13:52:53 PDT
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.
Comment 4 Mark Lam 2015-03-26 13:57:53 PDT
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)
Comment 5 Filip Pizlo 2015-03-26 13:59:11 PDT
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.
Comment 6 Mark Lam 2015-03-26 14:01:50 PDT
(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
      }
Comment 7 Mark Lam 2015-03-26 14:11:55 PDT
(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
Comment 8 Mark Lam 2015-03-27 12:05:50 PDT
Skipped the crashing test in r182072: <http://trac.webkit.org/r182072>.  Please revert when the bug is fixed.
Comment 9 Mark Lam 2015-03-30 15:48:09 PDT
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 10 Filip Pizlo 2015-03-30 15:57:07 PDT
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.
Comment 11 Mark Lam 2015-03-30 17:31:37 PDT
Created attachment 249784 [details]
patch 2: applied Filip's feedback
Comment 12 Mark Lam 2015-03-30 17:46:21 PDT
Thanks.  Landed in r182167: <http://trac.webkit.org/r182167>.