Bug 197552 - [JSC] Generator CodeBlock generation should be idempotent
Summary: [JSC] Generator CodeBlock generation should be idempotent
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-05-02 23:52 PDT by Yusuke Suzuki
Modified: 2019-05-13 12:24 PDT (History)
7 users (show)

See Also:


Attachments
Patch (97.39 KB, patch)
2019-05-03 01:28 PDT, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews101 for mac-highsierra (2.86 MB, application/zip)
2019-05-03 02:38 PDT, EWS Watchlist
no flags Details
Archive of layout-test-results from ews107 for mac-highsierra-wk2 (2.53 MB, application/zip)
2019-05-03 02:49 PDT, EWS Watchlist
no flags Details
Patch (95.86 KB, patch)
2019-05-03 03:02 PDT, Yusuke Suzuki
keith_miller: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yusuke Suzuki 2019-05-02 23:52:52 PDT
ES6 Generator can save the execution state inside an object.
When the underlying CodeBlock is jettisoned and generated again, if the CodeBlock content is different, resume becomes totally broken!
e.g. regenerating CodeBlock with Debugger.
Comment 1 Yusuke Suzuki 2019-05-02 23:54:40 PDT
<rdar://problem/48538201>
Comment 2 Yusuke Suzuki 2019-05-03 01:28:54 PDT
Created attachment 368915 [details]
Patch
Comment 3 EWS Watchlist 2019-05-03 02:38:44 PDT
Comment on attachment 368915 [details]
Patch

Attachment 368915 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/12082656

Number of test failures exceeded the failure limit.
Comment 4 EWS Watchlist 2019-05-03 02:38:46 PDT
Created attachment 368917 [details]
Archive of layout-test-results from ews101 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-highsierra  Platform: Mac OS X 10.13.6
Comment 5 EWS Watchlist 2019-05-03 02:49:08 PDT
Comment on attachment 368915 [details]
Patch

Attachment 368915 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/12082694

Number of test failures exceeded the failure limit.
Comment 6 EWS Watchlist 2019-05-03 02:49:10 PDT
Created attachment 368918 [details]
Archive of layout-test-results from ews107 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6
Comment 7 Yusuke Suzuki 2019-05-03 03:02:23 PDT
Created attachment 368919 [details]
Patch
Comment 8 Yusuke Suzuki 2019-05-03 11:37:48 PDT
Comment on attachment 368919 [details]
Patch

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

> Source/JavaScriptCore/ChangeLog:28
> +           and pass them as a parameter, OptionSet<CodeGeneratorMode>.

Note that each created Generator holds JSFunction to the generator body. So state & ScriptExecutable is always tightly coupled. Storing CodeGeneratorMode in ScriptExecutable allows us to regenerate the same CodeBlock for that if the generator is created before the debugger is enabled.
Comment 9 Keith Miller 2019-05-03 11:51:29 PDT
Comment on attachment 368919 [details]
Patch

r=me.
Comment 10 Yusuke Suzuki 2019-05-03 11:54:50 PDT
Committed r244915: <https://trac.webkit.org/changeset/244915>
Comment 11 Saam Barati 2019-05-13 12:24:06 PDT
Comment on attachment 368919 [details]
Patch

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

> Source/JavaScriptCore/runtime/ScriptExecutable.cpp:318
> +    // We continue using the same CodeGenerationMode for Generators because live generator objects can
> +    // keep the state which is only valid with the CodeBlock compiled with the same CodeGenerationMode.

So what happens when we enable the debugger? Will we never be able to pause on this generator then? Or will we be able to pause for new invocations?

It seems like this makes it so new invocations of the generator won't have debugging opcodes ever. Can we find a way to not make that the case?