Bug 203177

Summary: [JSC] DFG::CommonData modification by DFG reallyAdd should be guarded by CodeBlock's lock
Product: WebKit Reporter: Yusuke Suzuki <ysuzuki>
Component: New BugsAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, keith_miller, mark.lam, msaboff, saam, tzagallo, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch mark.lam: review+

Yusuke Suzuki
Reported 2019-10-18 17:35:17 PDT
[JSC] DFG::CommonData modification by DFG reallyAdd should be guarded by CodeBlock's lock
Attachments
Patch (11.58 KB, patch)
2019-10-18 17:37 PDT, Yusuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (11.39 KB, patch)
2019-10-18 17:41 PDT, Yusuke Suzuki 		mark.lam: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Yusuke Suzuki
Comment 1 2019-10-18 17:37:42 PDT
Created attachment 381354 [details] Patch
Yusuke Suzuki
Comment 2 2019-10-18 17:39:15 PDT
<rdar://problem/56404068>
Yusuke Suzuki
Comment 3 2019-10-18 17:41:35 PDT
Created attachment 381356 [details] Patch
Mark Lam
Comment 4 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.
Yusuke Suzuki
Comment 5 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.
Yusuke Suzuki
Comment 6 2019-10-18 23:31:34 PDT
Committed r251321: <https://trac.webkit.org/changeset/251321>
Note You need to log in before you can comment on or make changes to this bug.