Bug 203177 - [JSC] DFG::CommonData modification by DFG reallyAdd should be guarded by CodeBlock's lock
Summary: [JSC] DFG::CommonData modification by DFG reallyAdd should be guarded by Code...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-10-18 17:35 PDT by Yusuke Suzuki
Modified: 2019-10-18 23:31 PDT (History)
7 users (show)

See Also:

Attachments
Patch (11.58 KB, patch)
2019-10-18 17:37 PDT, Yusuke Suzuki 		no flags Details | Formatted Diff | Diff
Patch (11.39 KB, patch)
2019-10-18 17:41 PDT, Yusuke Suzuki 		mark.lam: review+
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Yusuke Suzuki 2019-10-18 17:35:17 PDT 
[JSC] DFG::CommonData modification by DFG reallyAdd should be guarded by CodeBlock's lock
Comment 1 Yusuke Suzuki 2019-10-18 17:37:42 PDT 
Created attachment 381354 [details]
Patch
Comment 2 Yusuke Suzuki 2019-10-18 17:39:15 PDT 
<rdar://problem/56404068>
Comment 3 Yusuke Suzuki 2019-10-18 17:41:35 PDT 
Created attachment 381356 [details]
Patch
Comment 4 Mark Lam 2019-10-18 21:10:28 PDT 
Comment on attachment 381356 [details]
Patch

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

r=me.

> Source/JavaScriptCore/dfg/DFGDesiredGlobalProperties.cpp:69
> +        watchpointSet.add(watchpoint);

Why not WTFMove(watchpoint) here too?

> JSTests/stress/dfg-really-add-locking.js:1
> +//@ runDefault("--collectContinuously=1", "--useGenerationalGC=0")

How long does this test take to run?  if it's a slow test, then consider adding "//@ slow!" here too.
Comment 5 Yusuke Suzuki 2019-10-18 23:30:23 PDT 
Comment on attachment 381356 [details]
Patch

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

>> Source/JavaScriptCore/dfg/DFGDesiredGlobalProperties.cpp:69
>> +        watchpointSet.add(watchpoint);
> 
> Why not WTFMove(watchpoint) here too?

Fixed.

>> JSTests/stress/dfg-really-add-locking.js:1
>> +//@ runDefault("--collectContinuously=1", "--useGenerationalGC=0")
> 
> How long does this test take to run?  if it's a slow test, then consider adding "//@ slow!" here too.

It takes not so much time, but to make more reliable, I need to add more iterations.
I'll add `for (var i = 0; i < 10; ++i)` while adding `slow!`. But even though, reliable reproduce is requiring much more repeated test runs.
Comment 6 Yusuke Suzuki 2019-10-18 23:31:34 PDT 
Committed r251321: <https://trac.webkit.org/changeset/251321>