Bug 158806 – Failing baseline JIT compilation of a code block and then trying to compile it from OSR from DFG/FTL will corrupt the CodeBlock

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Filip Pizlo]

As the title says:

1) Inline a LLInt code block into something else in DFG or FTL.
2) Attempt to compile that LLInt code block and fail.  Failed JIT compilations leave cruft in the CodeBlock that we never clean up.
3) OSR exit from the DFG or FTL code block and force compilation of the LLInt code block.  Now we corrupt the CodeBlock.

[Filip Pizlo]

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

[Alexey Proskuryakov]

Migrating radar number from duplicate.

<rdar://problem/26905445>

[Filip Pizlo]

Created attachment 281914 [details]
the patch

[Saam Barati]

Comment on attachment 281914 [details]
the patch

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

> Source/JavaScriptCore/bytecode/CodeBlock.h:269
> +    // We call this when we want to reattempt compiling something with the baseline JIT. Ideally
> +    // the baseline JIT would not add data to CodeBlock, but instead it would put its data into
> +    // a newly created JITCode, which could be thrown away if we bail on JIT compilation. Then we
> +    // would be able to get rid of this silly function.

Can we open a bug for this and link a FIXME here?
This seems like a good idea.

[Filip Pizlo]

(In reply to comment #4)
> Comment on attachment 281914 [details]
> the patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=281914&action=review
> 
> > Source/JavaScriptCore/bytecode/CodeBlock.h:269
> > +    // We call this when we want to reattempt compiling something with the baseline JIT. Ideally
> > +    // the baseline JIT would not add data to CodeBlock, but instead it would put its data into
> > +    // a newly created JITCode, which could be thrown away if we bail on JIT compilation. Then we
> > +    // would be able to get rid of this silly function.
> 
> Can we open a bug for this and link a FIXME here?
> This seems like a good idea.

https://bugs.webkit.org/show_bug.cgi?id=159061

[Build Bot]

Comment on attachment 281914 [details]
the patch

Attachment 281914 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/1556243

New failing tests:
imported/w3c/web-platform-tests/dom/ranges/Range-extractContents.html

[Build Bot]

Created attachment 281919 [details]
Archive of layout-test-results from ews105 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 281914 [details]
the patch

Attachment 281914 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/1556244

New failing tests:
imported/w3c/web-platform-tests/dom/ranges/Range-extractContents.html

[Build Bot]

Created attachment 281920 [details]
Archive of layout-test-results from ews121 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews121  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.4

[Filip Pizlo]

Yup, this patch definitely introduces a regression.  Investigating.

[Filip Pizlo]

Created attachment 281922 [details]
patch for landing?

I think that I was calling resetJITData() too early, and so I might call it interleaved with the concurrent baseline JIT.  I need to call it later, when we know that the concurrent baseline JIT thread is not working on this code block.  That's an easy change.

[Filip Pizlo]

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