Bug 185738 - AI for Atomics.load() is too conservative in always clobbering world
Summary: AI for Atomics.load() is too conservative in always clobbering world
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Saam Barati
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-05-17 11:49 PDT by Rick Waldron
Modified: 2018-05-30 09:21 PDT (History)
11 users (show)

See Also:


Attachments
wake-in-order-seg-fault.txt (39.89 KB, text/plain)
2018-05-17 11:49 PDT, Rick Waldron
no flags Details
patch (1.77 KB, patch)
2018-05-29 17:04 PDT, Saam Barati
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Rick Waldron 2018-05-17 11:49:29 PDT
Created attachment 340614 [details]
wake-in-order-seg-fault.txt

For the purpose of testing Atomics & SharedArrayBuffer, I've been working with a build of JavaScriptCore that I've patched to "#define ENABLE_SHARED_ARRAY_BUFFER 1", that can be found here: https://gist.github.com/rwaldron/89ed9a4bb7a459db8d54c8fe77ead4b1

While using this build, I've encountered a test in Test262 that consistently produces a "Segmentation fault: 11". 

I've attached a standalone copy of the test, as well as a complete seg fault dump. 

To run the test: 

1. Enable SharedArrayBuffers (either manually, or apply the patch I've provided in the gist above

    wget https://gist.githubusercontent.com/rwaldron/89ed9a4bb7a459db8d54c8fe77ead4b1/raw/0001-Enable-SharedArrayBuffer-for-Testing.patch
    git apply 0001-Enable-SharedArrayBuffer-for-Testing.patch

2. Build a JSC:

    Tools/Scripts/build-jsc --debug

3. Download test file: wake-in-order-standalone.js

   wget https://gist.githubusercontent.com/rwaldron/90f5ce7ceb318c1030942ca074a6daa8/raw/wake-in-order-standalone.js

4. Run the test file: 

   ./WebKitBuild/Debug/jsc wake-in-order-standalone.js


If this test runs successfully, it will output nothing at all. 



The key portion of the test that causes the seg fault is this code:


    while (Atomics.load(i32a, ${SPIN + i}) === 0)
        /* nothing */ ;


Which is found in the source string of the agent, lines 35-36. Changing that to: 


    while (Atomics.load(i32a, ${SPIN + i}) === 0) {
      $262.agent.sleep(1);
    }


Will prevent the seg fault, but the issue remains.
Comment 1 Alexey Proskuryakov 2018-05-17 13:03:44 PDT
DFG ASSERTION FAILED: AI-clobberize disagreement; AI says ClobberedStructures while clobberize says (Direct:[TypedArrayProperties], Super:[World, Heap])
./dfg/DFGCFAPhase.cpp(185) : void JSC::DFG::CFAPhase::performBlockCFA(JSC::DFG::BasicBlock *)
1   0x105e715e9 WTFCrash
2   0x105e733bb WTFCrashWithInfo(int, char const*, char const*, int)
3   0x1065a2515 JSC::DFG::CFAPhase::performBlockCFA(JSC::DFG::BasicBlock*)
4   0x1065a1d72 JSC::DFG::CFAPhase::performForwardCFA()
5   0x1065a17f7 JSC::DFG::CFAPhase::run()
6   0x1065a0fde bool JSC::DFG::runAndLog<JSC::DFG::CFAPhase>(JSC::DFG::CFAPhase&)
7   0x106544b7e bool JSC::DFG::runPhase<JSC::DFG::CFAPhase>(JSC::DFG::Graph&)
8   0x106544b45 JSC::DFG::performCFA(JSC::DFG::Graph&)
Comment 2 Leo Balter 2018-05-17 13:21:01 PDT
As a reference for myself, I'd like to add the path to the matching test file from Test262: test/built-ins/Atomics/wake/wake-in-order.js
Comment 3 Radar WebKit Bug Importer 2018-05-17 13:23:10 PDT
<rdar://problem/40342214>
Comment 4 Saam Barati 2018-05-29 17:04:57 PDT
Created attachment 341539 [details]
patch
Comment 5 Yusuke Suzuki 2018-05-30 08:52:10 PDT
Comment on attachment 341539 [details]
patch

r=me
Comment 6 WebKit Commit Bot 2018-05-30 09:21:03 PDT
Comment on attachment 341539 [details]
patch

Clearing flags on attachment: 341539

Committed r232294: <https://trac.webkit.org/changeset/232294>
Comment 7 WebKit Commit Bot 2018-05-30 09:21:05 PDT
All reviewed patches have been landed.  Closing bug.