Bug 123472 – Add InvalidationPoints to the DFG and use them for all watchpoints

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Filip Pizlo]

This allows CodeBlock::jettison() to have stronger semantics: it now means that the receiving code block will not continue execution past the current bytecode instruction(s).

Example #1: CodeBlock A is not currently on the stack.  A->jettison() will ensure that A is never executed again.

Example #2: CodeBlock A is on the stack in a call instruction.  A->jettison() will allow the call instruction to complete in A, but before the next bytecode instruction executes, A will exit to baseline code.  Also A will never be executed again.

[Filip Pizlo]

Created attachment 215430 [details]
the patch

[Mark Hahnenberg]

Comment on attachment 215430 [details]
the patch

r=me

[Filip Pizlo]

Landed in http://trac.webkit.org/changeset/158304

[Csaba Osztrogonác]

(In reply to comment #3)
> Landed in http://trac.webkit.org/changeset/158304

FYI: It broke the Apple Windows build.

[Filip Pizlo]

(In reply to comment #4)
> (In reply to comment #3)
> > Landed in http://trac.webkit.org/changeset/158304
> 
> FYI: It broke the Apple Windows build.

I'm looking!

[Filip Pizlo]

(In reply to comment #5)
> (In reply to comment #4)
> > (In reply to comment #3)
> > > Landed in http://trac.webkit.org/changeset/158304
> > 
> > FYI: It broke the Apple Windows build.
> 
> I'm looking!

Fix in r158307.

[Filip Pizlo]

(In reply to comment #6)
> (In reply to comment #5)
> > (In reply to comment #4)
> > > (In reply to comment #3)
> > > > Landed in http://trac.webkit.org/changeset/158304
> > > 
> > > FYI: It broke the Apple Windows build.
> > 
> > I'm looking!
> 
> Fix in r158307.

And another fix in r158308.

[Ryosuke Niwa]

Looks like debug builds are still broken:
http://build.webkit.org/builders/Apple%20MountainLion%20Debug%20%28Build%29/builds/17934/steps/compile-webkit/logs/stdio

/Volumes/Data/slave/mountainlion-debug/build/WebKitBuild/Debug/JavaScriptCore.framework/PrivateHeaders/AbstractMacroAssembler.h:805:31: error: no member named 'getDifferenceBetweenLabels' in 'JSC::AssemblerLabel'
        return AssemblerType::getDifferenceBetweenLabels(from.m_label, to.m_label);
               ~~~~~~~~~~~~~~~^
fatal error: too many errors emitted, stopping now [-ferror-limit=]
20 errors generated.

[Filip Pizlo]

(In reply to comment #8)
> Looks like debug builds are still broken:
> http://build.webkit.org/builders/Apple%20MountainLion%20Debug%20%28Build%29/builds/17934/steps/compile-webkit/logs/stdio
> 
> /Volumes/Data/slave/mountainlion-debug/build/WebKitBuild/Debug/JavaScriptCore.framework/PrivateHeaders/AbstractMacroAssembler.h:805:31: error: no member named 'getDifferenceBetweenLabels' in 'JSC::AssemblerLabel'
>         return AssemblerType::getDifferenceBetweenLabels(from.m_label, to.m_label);
>                ~~~~~~~~~~~~~~~^
> fatal error: too many errors emitted, stopping now [-ferror-limit=]
> 20 errors generated.

Fix landed in r158313.

[Joseph Pecoraro]

I'm seeing ASSERTs opening the inspector in Debug builds on ToT (r158318). Might be related to this change (I haven't verified yet though):

ASSERTION FAILED: jitType() == JITCode::None
/Volumes/Data/Code/safari/OpenSource/Source/JavaScriptCore/bytecode/CodeBlock.cpp(2481) : JSC::CodeBlock *JSC::CodeBlock::baselineVersion()
1   0x102641170 WTFCrash
2   0x102044d11 JSC::CodeBlock::baselineVersion()
3   0x1020fe6c0 JSC::ProfiledCodeBlockJettisoningWatchpoint::fireInternal()
4   0x1025ffcc6 JSC::Watchpoint::fire()
5   0x1025ff72f JSC::WatchpointSet::fireAllWatchpoints()
6   0x1025ff820 JSC::WatchpointSet::notifyWriteSlow()
7   0x1022c997c JSC::WatchpointSet::notifyWrite()
8   0x102345737 JSC::InlineWatchpointSet::notifyWrite()
9   0x1025c3bac JSC::Structure::notifyTransitionFromThisStructure() const
10  0x1025bfef9 JSC::Structure::Structure(JSC::VM&, JSC::Structure const*)
11  0x1025bfb95 JSC::Structure::Structure(JSC::VM&, JSC::Structure const*)
12  0x1025c49d6 JSC::Structure::create(JSC::VM&, JSC::Structure const*)
13  0x1025c0d53 JSC::Structure::addPropertyTransition(JSC::VM&, JSC::Structure*, JSC::PropertyName, unsigned int, JSC::JSCell*, int&, JSC::PutPropertySlot::Context)
14  0x101fca3e6 bool JSC::JSObject::putDirectInternal<(JSC::JSObject::PutMode)1>(JSC::VM&, JSC::PropertyName, JSC::JSValue, unsigned int, JSC::PutPropertySlot&, JSC::JSCell*)
15  0x101fc809b JSC::JSObject::putDirect(JSC::VM&, JSC::PropertyName, JSC::JSValue, unsigned int)
16  0x10237d5d5 JSC::JSObject::putDirectNativeFunction(JSC::VM&, JSC::JSGlobalObject*, JSC::PropertyName const&, unsigned int, long long (*)(JSC::ExecState*), JSC::Intrinsic, unsigned int)
17  0x102421cd8 JSC::setUpStaticFunctionSlot(JSC::ExecState*, JSC::HashEntry const*, JSC::JSObject*, JSC::PropertyName, JSC::PropertySlot&)
18  0x1023567b4 bool JSC::getStaticFunctionSlot<JSC::JSSegmentedVariableObject>(JSC::ExecState*, JSC::HashTable const&, JSC::JSObject*, JSC::PropertyName, JSC::PropertySlot&)
19  0x102346904 JSC::JSGlobalObject::getOwnPropertySlot(JSC::JSObject*, JSC::ExecState*, JSC::PropertyName, JSC::PropertySlot&)
20  0x1023a0ef6 JSC::abstractAccess(JSC::ExecState*, JSC::JSScope*, JSC::Identifier const&, JSC::GetOrPut, unsigned long, bool&, JSC::ResolveOp&)
21  0x1023a0991 JSC::JSScope::abstractResolve(JSC::ExecState*, JSC::JSScope*, JSC::Identifier const&, JSC::GetOrPut, JSC::ResolveType)
22  0x102041fb7 JSC::CodeBlock::CodeBlock(JSC::ScriptExecutable*, JSC::UnlinkedCodeBlock*, JSC::JSScope*, WTF::PassRefPtr<JSC::SourceProvider>, unsigned int, unsigned int)
23  0x10229907b JSC::FunctionCodeBlock::FunctionCodeBlock(JSC::FunctionExecutable*, JSC::UnlinkedFunctionCodeBlock*, JSC::JSScope*, WTF::PassRefPtr<JSC::SourceProvider>, unsigned int, unsigned int)
24  0x102296961 JSC::FunctionCodeBlock::FunctionCodeBlock(JSC::FunctionExecutable*, JSC::UnlinkedFunctionCodeBlock*, JSC::JSScope*, WTF::PassRefPtr<JSC::SourceProvider>, unsigned int, unsigned int)
25  0x1022940fd JSC::ScriptExecutable::newCodeBlockFor(JSC::CodeSpecializationKind, JSC::JSScope*, JSC::JSObject*&)
26  0x1022945d2 JSC::ScriptExecutable::prepareForExecutionImpl(JSC::ExecState*, JSC::JSScope*, JSC::CodeSpecializationKind)
27  0x101fe4ced JSC::ScriptExecutable::prepareForExecution(JSC::ExecState*, JSC::JSScope*, JSC::CodeSpecializationKind)
28  0x102420d1a JSC::LLInt::setUpCall(JSC::ExecState*, JSC::Instruction*, JSC::CodeSpecializationKind, JSC::JSValue, JSC::LLIntCallLinkInfo*)
29  0x102420bcc JSC::LLInt::genericCall(JSC::ExecState*, JSC::Instruction*, JSC::CodeSpecializationKind)
30  0x10241d02c llint_slow_path_call
31  0x10242644d llint_op_call

[Ryosuke Niwa]

It looks like multiple 4~5+ layout tests started hitting assertions after this patch.

Filed https://bugs.webkit.org/show_bug.cgi?id=123551 to track these assertion failures.