Bug 119919 – Concurrent JIT crashes in various fast/js/dfg-* tests while the main thread is setting innerHTML

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Filip Pizlo]

In one of the crashes that I saw, the concurrent JIT crashed in QueryableExitSite::hasExitSite(), somewhere in the HashTable's iterator creation.  Super weird.

[Filip Pizlo]

I'm seeing almost identical crashes every ~30 or so runs of v8-v6/v8-deltablue.js in the jsc shell.

[Filip Pizlo]

Found it!!

This is because lazy write barrier adding stores pointers directly into CodeBlock::m_constantRegisters, even while it appends things to that vector, leading to resizes, leading to the pointers being borked.

This appears to be more likely to cause crashes with concurrent JIT because there is more malloc churn while the DFG corrupts memory; in sequential JIT you're less likely to crash because all of the data structures probably get allocated in one go, then they slightly corrupt each other, and then they're all freed.

The fix is to have DesiredWriteBarriers have some way of referencing a write barrier without necessarily using a direct pointer.  Probably, DesiredWriteBarrier should have a Vector*+index mode.

[Mark Hahnenberg]

Created attachment 209014 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 209014 [details]
Patch

Clearing flags on attachment: 209014

Committed r154245: <http://trac.webkit.org/changeset/154245>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Filip Pizlo]

I'm still seeing these crashes, for example:

http://build.webkit.org/results/Apple%20Lion%20Debug%20WK1%20(Tests)/r154283%20(10216)/fast/js/dfg-intrinsic-unused-this-method-check-crash-log.txt

Again, the concurrent JIT thread has crashed in a way that strongly implies malloc corruption.

[Mark Hahnenberg]

Ugh, this is just embarrassing. Working on a fix.

[Mark Hahnenberg]

Created attachment 209215 [details]
Patch

[Mark Hahnenberg]

Comment on attachment 209215 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=209215&action=review

typos galore

> Source/JavaScriptCore/ChangeLog:8
> +        More fixes for WriteBarrier deferral during concurrent JIT-ing. This patch makes the use of DesiredWriteBarriers class and the 
> +        initializeLazyWriteBarrierFor* wrapper functions. 

...makes the use of the DesiredWriteBarriers class and the initializeLazyWriteBarrierFor* wrapper functions more sane.

> Source/JavaScriptCore/ChangeLog:10
> +        Refactored DesiredWriteBarrier to require an owner, a type, a CodeBlock, and an index.. The type indicates how to use CodeBlock

s/../.

s/use CodeBlock/use the CodeBlock

[Geoffrey Garen]

Comment on attachment 209215 [details]
Patch

r=me

[Filip Pizlo]

Comment on attachment 209215 [details]
Patch

r=me too.

[Mark Hahnenberg]

Comment on attachment 209215 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=209215&action=review

Noticed an issue.

> Source/JavaScriptCore/dfg/DFGDesiredWriteBarriers.cpp:64
> +        InlineCallFrame& inlineCallFrame = m_codeBlock->inlineCallFrames()[m_index];
> +        if (!!inlineCallFrame.callee)
> +            break;

This check is redundant because we never would have added this DesiredWriteBarrier if the callee was null. Also, the check itself is the opposite of what it is meant to check :-( I'm going to replace it with an ASSERT (with the correct condition, of course).

[Mark Hahnenberg]

Committed r154351: <http://trac.webkit.org/changeset/154351>