Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Intermittent removal of adoptedStyleSheet CSSStyleSheet instances whe…
…n assigning adoptedStyleSheet array

https://bugs.webkit.org/show_bug.cgi?id=254844
rdar://107768559

Reviewed by Mark Lam.

JSObservableArray is using ArrayClass, but this is wrong: this is not implementing what Array in DFG etc. requires.
As a result, DFG attempt to read length in the same way to normal array, and it just reads empty butterfly.

1. JSObservableArray must not say ArrayClass. ArrayClass is more strict form (like, ArrayType), and DerivedArray normally
   should not use it.
2. We also fix NPAPI's half-broken RuntimeArray's ArrayClass to NonArray.
3. We also change iteration protocol to consider this new scheme: we should only allow fast iteration for normal pure JSArray.

* JSTests/stress/spread-for-runtime-array.js: Added.
(shouldBe):
(test):
* Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::parseBlock):
* Source/JavaScriptCore/runtime/CommonSlowPaths.cpp:
(JSC::iteratorNextTryFastImpl):
* Source/JavaScriptCore/runtime/IteratorOperations.cpp:
(JSC::getIterationMode):
* Source/JavaScriptCore/runtime/JSGenericTypedArrayViewConstructorInlines.h:
(JSC::constructGenericTypedArrayViewWithArguments):
* Source/JavaScriptCore/runtime/JSGenericTypedArrayViewInlines.h:
(JSC::JSGenericTypedArrayView<Adaptor>::setFromArrayLike):
* Source/JavaScriptCore/runtime/JSGenericTypedArrayViewPrototypeFunctions.h:
(JSC::genericTypedArrayViewPrivateFuncFromFast):
* Source/JavaScriptCore/tools/JSDollarVM.cpp:
* Source/WebCore/bindings/js/JSObservableArray.h:
* Source/WebCore/bridge/runtime_array.h:

Canonical link: https://commits.webkit.org/266464@main
  • Loading branch information
Constellation committed Aug 1, 2023
1 parent 1989bf2 commit a347abe
Show file tree
Hide file tree
Showing 10 changed files with 46 additions and 10 deletions.
33 changes: 33 additions & 0 deletions JSTests/stress/spread-for-runtime-array.js
@@ -0,0 +1,33 @@
function shouldBe(actual, expected) {
if (actual !== expected)
throw new Error('bad value: ' + actual);
}

var list = [
[0, 1, 2, 3, 4, 5],
[],
["Hello", "World"],
];

for (var i = 0; i < 1e4; ++i) {
for (var array of list) {
$vm.ensureArrayStorage(array);
array.hey = 42;
var iterator = array[Symbol.iterator]();
while (!iterator.next().done);
}
}

var runtimeArray = $vm.createRuntimeArray(0, 1, 2, 3, 4, 5, 6);
runtimeArray[10] = "Hello";
shouldBe(runtimeArray[10], "Hello");

function test(runtimeArray)
{
var array = [...runtimeArray];
shouldBe(array.length, 7);
}
noInline(test);

for (var i = 0; i < 1e5; ++i)
test(runtimeArray);
2 changes: 1 addition & 1 deletion Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
Expand Up @@ -7872,7 +7872,7 @@ void ByteCodeParser::parseBlock(unsigned limit)
numberOfRemainingModes--;
if (!numberOfRemainingModes) {
addToGraph(CheckIsConstant, OpInfo(frozenSymbolIteratorFunction), symbolIterator);
addToGraph(CheckJSCast, OpInfo(JSArray::info()), get(bytecode.m_iterable));
addToGraph(Check, Edge(get(bytecode.m_iterable), ArrayUse));
} else {
BasicBlock* fastArrayBlock = allocateUntargetableBlock();
genericBlock = allocateUntargetableBlock();
Expand Down
2 changes: 1 addition & 1 deletion Source/JavaScriptCore/runtime/CommonSlowPaths.cpp
Expand Up @@ -858,7 +858,7 @@ ALWAYS_INLINE UGPRPair iteratorNextTryFastImpl(VM& vm, JSGlobalObject* globalObj
JSObject* iterator = jsCast<JSObject*>(GET(bytecode.m_iterator).jsValue());;
JSCell* iterable = GET(bytecode.m_iterable).jsValue().asCell();
if (auto arrayIterator = jsDynamicCast<JSArrayIterator*>(iterator)) {
if (auto array = jsDynamicCast<JSArray*>(iterable)) {
if (auto array = jsDynamicCast<JSArray*>(iterable); array && isJSArray(array)) {
metadata.m_iterableProfile.observeStructureID(array->structureID());

metadata.m_iterationMetadata.seenModes = metadata.m_iterationMetadata.seenModes | IterationMode::FastArray;
Expand Down
4 changes: 2 additions & 2 deletions Source/JavaScriptCore/runtime/IteratorOperations.cpp
Expand Up @@ -238,7 +238,7 @@ IterationRecord iteratorForIterable(JSGlobalObject* globalObject, JSValue iterab

IterationMode getIterationMode(VM&, JSGlobalObject* globalObject, JSValue iterable, JSValue symbolIterator)
{
if (!iterable.inherits<JSArray>())
if (!isJSArray(iterable))
return IterationMode::Generic;

if (!globalObject->arrayIteratorProtocolWatchpointSet().isStillValid())
Expand All @@ -259,7 +259,7 @@ IterationMode getIterationMode(VM&, JSGlobalObject* globalObject, JSValue iterab

IterationMode getIterationMode(VM&, JSGlobalObject* globalObject, JSValue iterable)
{
if (!iterable.inherits<JSArray>())
if (!isJSArray(iterable))
return IterationMode::Generic;

JSArray* array = jsCast<JSArray*>(iterable);
Expand Down
Expand Up @@ -184,7 +184,7 @@ inline JSObject* constructGenericTypedArrayViewWithArguments(JSGlobalObject* glo

// This getPropertySlot operation should not be observed by the Proxy.
// So we use VMInquiry. And purge the opaque object cases (proxy and namespace object) by isTaintedByOpaqueObject() guard.
if (JSArray* array = jsDynamicCast<JSArray*>(object); LIKELY(array && array->isIteratorProtocolFastAndNonObservable()))
if (JSArray* array = jsDynamicCast<JSArray*>(object); LIKELY(array && isJSArray(array) && array->isIteratorProtocolFastAndNonObservable()))
length = array->length();
else {
PropertySlot lengthSlot(object, PropertySlot::InternalMethodType::VMInquiry, &vm);
Expand Down
Expand Up @@ -412,7 +412,7 @@ bool JSGenericTypedArrayView<Adaptor>::setFromArrayLike(JSGlobalObject* globalOb
size_t safeLength = objectOffset <= safeUnadjustedLength ? safeUnadjustedLength - objectOffset : 0;

if constexpr (TypedArrayStorageType != TypeBigInt64 || TypedArrayStorageType != TypeBigUint64) {
if (JSArray* array = jsDynamicCast<JSArray*>(object); LIKELY(array)) {
if (JSArray* array = jsDynamicCast<JSArray*>(object); LIKELY(array && isJSArray(array))) {
if (safeLength == length && (safeLength + objectOffset) >= array->length() && array->isIteratorProtocolFastAndNonObservable()) {
IndexingType indexingType = array->indexingType() & IndexingShapeMask;
if (indexingType == Int32Shape) {
Expand Down Expand Up @@ -460,7 +460,7 @@ bool JSGenericTypedArrayView<Adaptor>::setFromArrayLike(JSGlobalObject* globalOb
return false;
}

if (JSArray* array = jsDynamicCast<JSArray*>(sourceValue); LIKELY(array))
if (JSArray* array = jsDynamicCast<JSArray*>(sourceValue); LIKELY(array && isJSArray(array)))
RELEASE_AND_RETURN(scope, setFromArrayLike(globalObject, offset, array, 0, array->length()));

size_t targetLength = this->length();
Expand Down
Expand Up @@ -764,6 +764,9 @@ ALWAYS_INLINE EncodedJSValue genericTypedArrayViewPrivateFuncFromFast(VM& vm, JS
if (!array)
return JSValue::encode(jsUndefined());

if (!isJSArray(array))
return JSValue::encode(jsUndefined());

if (!array->isIteratorProtocolFastAndNonObservable())
return JSValue::encode(jsUndefined());

Expand Down
2 changes: 1 addition & 1 deletion Source/JavaScriptCore/tools/JSDollarVM.cpp
Expand Up @@ -674,7 +674,7 @@ IGNORE_WARNINGS_END
static Structure* createStructure(VM& vm, JSGlobalObject* globalObject, JSValue prototype)
{
DollarVMAssertScope assertScope;
return Structure::create(vm, globalObject, prototype, TypeInfo(DerivedArrayType, StructureFlags), info(), ArrayClass);
return Structure::create(vm, globalObject, prototype, TypeInfo(DerivedArrayType, StructureFlags), info(), NonArray);
}

protected:
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/bindings/js/JSObservableArray.h
Expand Up @@ -88,7 +88,7 @@ class JSObservableArray final : public JSArray {

static Structure* createStructure(VM& vm, JSGlobalObject* globalObject, JSValue prototype)
{
return Structure::create(vm, globalObject, prototype, TypeInfo(DerivedArrayType, StructureFlags), info(), ArrayClass);
return Structure::create(vm, globalObject, prototype, TypeInfo(DerivedArrayType, StructureFlags), info(), NonArray);
}

private:
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/bridge/runtime_array.h
Expand Up @@ -81,7 +81,7 @@ class RuntimeArray final : public JSArray {

static Structure* createStructure(VM& vm, JSGlobalObject* globalObject, JSValue prototype)
{
return Structure::create(vm, globalObject, prototype, TypeInfo(DerivedArrayType, StructureFlags), info(), ArrayClass);
return Structure::create(vm, globalObject, prototype, TypeInfo(DerivedArrayType, StructureFlags), info(), NonArray);
}

private:
Expand Down

1 comment on commit a347abe

@ilhan007
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi colleagues, @Constellation, my team is also interested in getting this fix.
Do you already know when will it be released, in which version?

Please sign in to comment.