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
Summary: Failing baseline JIT compilation of a code block and then trying to compile i...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Filip Pizlo
URL:
Keywords: InRadar
: 158959 (view as bug list)
Depends on:
Blocks:
 
Reported: 2016-06-15 12:41 PDT by Filip Pizlo
Modified: 2016-06-23 13:56 PDT (History)
8 users (show)

See Also:


Attachments
the patch (16.53 KB, patch)
2016-06-23 10:46 PDT, Filip Pizlo
sbarati: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews105 for mac-yosemite-wk2 (921.14 KB, application/zip)
2016-06-23 11:37 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews121 for ios-simulator-wk2 (611.19 KB, application/zip)
2016-06-23 11:42 PDT, Build Bot
no flags Details
patch for landing? (16.83 KB, patch)
2016-06-23 12:03 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Filip Pizlo 2016-06-15 12:41:03 PDT
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.
Comment 1 Filip Pizlo 2016-06-23 10:08:14 PDT
*** Bug 158959 has been marked as a duplicate of this bug. ***
Comment 2 Alexey Proskuryakov 2016-06-23 10:37:53 PDT
Migrating radar number from duplicate.

<rdar://problem/26905445>
Comment 3 Filip Pizlo 2016-06-23 10:46:23 PDT
Created attachment 281914 [details]
the patch
Comment 4 Saam Barati 2016-06-23 10:54:48 PDT
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.
Comment 5 Filip Pizlo 2016-06-23 11:00:44 PDT
(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
Comment 6 Build Bot 2016-06-23 11:37:55 PDT
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
Comment 7 Build Bot 2016-06-23 11:37:58 PDT
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
Comment 8 Build Bot 2016-06-23 11:42:41 PDT
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
Comment 9 Build Bot 2016-06-23 11:42:44 PDT
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
Comment 10 Filip Pizlo 2016-06-23 12:00:27 PDT
Yup, this patch definitely introduces a regression.  Investigating.
Comment 11 Filip Pizlo 2016-06-23 12:03:04 PDT
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.
Comment 12 Filip Pizlo 2016-06-23 13:56:17 PDT
Landed in http://trac.webkit.org/changeset/202397