Array unshift/shift should not race against the AI in the compiler thread.
authormark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 18 Dec 2018 06:56:51 +0000 (06:56 +0000)
committermark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 18 Dec 2018 06:56:51 +0000 (06:56 +0000)
commit41c60b08fd42783cd7aa5989d8f50d77110b0607
tree2612654dc53909cb80fb59df78e4d311f10489a3
parentaeedd590e8e7a18f570443493ce9177ab3690b32
Array unshift/shift should not race against the AI in the compiler thread.
https://bugs.webkit.org/show_bug.cgi?id=192795
<rdar://problem/46724263>

Reviewed by Saam Barati.

JSTests:

* stress/array-unshift-should-not-race-against-compiler-thread.js: Added.

Source/JavaScriptCore:

The Array unshift and shift operations for ArrayStorage type arrays are protected
using the cellLock.  The AbstractInterpreter's foldGetByValOnConstantProperty()
function does grab the cellLock before reading a value from the array's ArrayStorage,
but does not get the array butterfly under the protection of the cellLock.

This is insufficient and racy.  For ArrayStorage type arrays, the fetching of the
butterfly also needs to be protected by the cellLock.  The unshift / shift
operations can move values around in the butterfly.  Hence, the fact that AI has
fetched a butterfly pointer (while ensuring no structure change) is insufficient
to guarantee that the values in the butterfly haven't shifted.

Having AI hold the cellLock the whole time (from before fetching the butterfly
till after reading the value from it) eliminates this race.  Note: we only need
to do this for ArrayStorage type arrays.

Note also that though AI is holding the cellLock in this case, we still need to
ensure that the array structure hasn't changed around the fetching of the butterfly.
This is because operations other than unshift and shift are guarded by this
protocol, and not the cellLock.

* dfg/DFGAbstractInterpreterInlines.h:
(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
* runtime/JSArray.cpp:
(JSC::JSArray::unshiftCountSlowCase):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@239325 268f45cc-cd09-0410-ab3c-d52691b4dbfc
JSTests/ChangeLog
JSTests/stress/array-unshift-should-not-race-against-compiler-thread.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
Source/JavaScriptCore/runtime/JSArray.cpp