[JSC] Potential GC fix for JSPropertyNameEnumerator
authorysuzuki@apple.com <ysuzuki@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 27 Jul 2019 04:55:11 +0000 (04:55 +0000)
committerysuzuki@apple.com <ysuzuki@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 27 Jul 2019 04:55:11 +0000 (04:55 +0000)
commitc90464d8390afc3d741cabeb7b10195e35d768c4
tree83d55d493d04f49c6830253d9616987addec5736
parenta1b51f8a3156ade619b29bed46d96756a97c82e0
[JSC] Potential GC fix for JSPropertyNameEnumerator
https://bugs.webkit.org/show_bug.cgi?id=200151

Reviewed by Mark Lam.

JSTests:

* stress/for-in-stress.js: Added.
(keys):

Source/JavaScriptCore:

We have been seeing some JSPropertyNameEnumerator::visitChildren crashes for a long time. The crash frequency itself is not high, but it has existed for a long time.
The crash happens when visiting m_propertyNames. It is also possible that this crash is caused by random corruption somewhere, but JSPropertyNameEnumerator
has some tricky (and potentially dangerous) implementations anyway.

1. JSPropertyNameEnumerator have Vector<WriteBarrier<JSString>> and it is extended in finishCreation with a lock.
   We should use Auxiliary memory for this use case. And we should set this memory in the constructor so that
   we do not extend it in finishCreation, and we do not need a lock.
2. JSPropertyNameEnumerator gets StructureID before allocating JSPropertyNameEnumerator. This is potentially dangerous because the conservative scan
   cannot find the Structure* since we could only have StructureID. Since allocation code happens after StructureID is retrieved, it is possible that
   the allocation causes GC and Structure* is collected.

In this patch, we align JSPropertyNameEnumerator implementation to the modern one to avoid using Vector<WriteBarrier<JSString>>. And we can make JSPropertyNameEnumerator
a non-destructible cell. Since JSCell's destructor is one of the cause of various issues, we should avoid it if we can.

No behavior change. This patch adds a test stressing JSPropertyNameEnumerator.

* dfg/DFGOperations.cpp:
* runtime/CommonSlowPaths.cpp:
(JSC::SLOW_PATH_DECL):
* runtime/JSPropertyNameEnumerator.cpp:
(JSC::JSPropertyNameEnumerator::create):
(JSC::JSPropertyNameEnumerator::JSPropertyNameEnumerator):
(JSC::JSPropertyNameEnumerator::finishCreation):
(JSC::JSPropertyNameEnumerator::visitChildren):
(JSC::JSPropertyNameEnumerator::destroy): Deleted.
* runtime/JSPropertyNameEnumerator.h:
* runtime/VM.cpp:
(JSC::VM::emptyPropertyNameEnumeratorSlow):
* runtime/VM.h:
(JSC::VM::emptyPropertyNameEnumerator):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@247888 268f45cc-cd09-0410-ab3c-d52691b4dbfc
JSTests/ChangeLog
JSTests/stress/for-in-stress.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGOperations.cpp
Source/JavaScriptCore/runtime/CommonSlowPaths.cpp
Source/JavaScriptCore/runtime/JSPropertyNameEnumerator.cpp
Source/JavaScriptCore/runtime/JSPropertyNameEnumerator.h
Source/JavaScriptCore/runtime/VM.cpp
Source/JavaScriptCore/runtime/VM.h