Bug 201331 - [JSC] DFG ByteCodeParser should not copy JIT-related part of SimpleJumpTable
Summary: [JSC] DFG ByteCodeParser should not copy JIT-related part of SimpleJumpTable
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-08-29 22:49 PDT by Yusuke Suzuki
Modified: 2019-08-30 03:00 PDT (History)
7 users (show)

See Also:


Attachments
Patch (6.10 KB, patch)
2019-08-29 22:54 PDT, Yusuke Suzuki
mark.lam: 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-08-29 22:49:23 PDT
...
Comment 1 Yusuke Suzuki 2019-08-29 22:50:03 PDT
<rdar://problem/53977605>
Comment 2 Yusuke Suzuki 2019-08-29 22:54:34 PDT
Created attachment 377679 [details]
Patch
Comment 3 Mark Lam 2019-08-30 01:47:13 PDT
Comment on attachment 377679 [details]
Patch

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

r=me

> Source/JavaScriptCore/ChangeLog:9
> +        For example, ctiOffsets can be grown by Baseline JIT compiler. There is race condition as follows.

Interesting.  I've seen races between the mutator and the DFG before, but this is the first time I've heard of a race between the baselineJIT and the DFG.

> Source/JavaScriptCore/bytecode/JumpTable.h:80
>          // FIXME: The two Vectors can be combind into one Vector<OffsetLocation>

Not your typo but might as well fix: /combind/combined/

> Source/JavaScriptCore/bytecode/JumpTable.h:122
> +#if ENABLE(DFG_JIT)
>          void clear()

This looks legit to me, but I hope you've tested it with a test build with ENABLE_DFG_JIT set to false.
Comment 4 Yusuke Suzuki 2019-08-30 02:54:15 PDT
Comment on attachment 377679 [details]
Patch

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

>> Source/JavaScriptCore/bytecode/JumpTable.h:80
>>          // FIXME: The two Vectors can be combind into one Vector<OffsetLocation>
> 
> Not your typo but might as well fix: /combind/combined/

Fixed.

>> Source/JavaScriptCore/bytecode/JumpTable.h:122
>>          void clear()
> 
> This looks legit to me, but I hope you've tested it with a test build with ENABLE_DFG_JIT set to false.

I checked this function is only called from DFGJITCompiler.cpp's finalizing timing, and make it `ENABLE(DFG_JIT)`.
Comment 5 Yusuke Suzuki 2019-08-30 03:00:35 PDT
Committed r249319: <https://trac.webkit.org/changeset/249319>