Clients of JSArray::tryCreateUninitializedRestricted() should invoke the mutatorFence().
https://bugs.webkit.org/show_bug.cgi?id=203231
<rdar://problem/56486552>
Reviewed by Saam Barati.
Clients of JSArray::tryCreateUninitializedRestricted() creates a partially
initialized JSArray butterfly, with the contract that it (the client) will take
care of filling in all the missing indexed properties before setting the newly
created array loose in the world. We intentionally do not unconditionally write
barrier the newly created array but, instead, rely on an owner object (or GC root)
that it gets put into to scan it.
That said, we do need to ensure that all the stores are completed before this
array is put in an owner object (or GC root) which makes it scannable by the GC.
This ensures that the GC will not be scanning a partially initialized array
butterfly. To achieve this, we should invoke the mutatorFence after the clients
of JSArray::tryCreateUninitializedRestricted() finish initializing the array.
By design, all clients of tryCreateUninitializedRestricted() must instantiate an
ObjectInitializationScope RAII object. This patch makes use of the
ObjectInitializationScope destructor to invoke the mutatorFence.
Note: we technically only need to invoke the fence if we succeeded in allocating
the array. However, we just invoke the fence unconditionally because we expect
that in the common path, we will succeed in allocating the array. The release
build version of ObjectInitializationScope does not keep record of whether we
succeed in allocating the array anyway. To keep the behavior consistent, the
debug build version of ObjectInitializationScope will also unconditionally
invoke the fence even if we failed to allocate the array.
This patch also does the following:
1. Replaced the setting of the public length in arrayProtoPrivateFuncConcatMemcpy()
with an assertion. The public length was already set by
tryCreateUninitializedRestricted() earlier.
Ditto for JSArray::fastSlice().
2. Removed a redundant instance of ObjectInitializationScope in
createEmptyRegExpMatchesArray().
* runtime/ArrayPrototype.cpp:
(JSC::arrayProtoPrivateFuncConcatMemcpy):
* runtime/JSArray.cpp:
(JSC::JSArray::fastSlice):
* runtime/ObjectInitializationScope.cpp:
(JSC::ObjectInitializationScope::~ObjectInitializationScope):
* runtime/ObjectInitializationScope.h:
(JSC::ObjectInitializationScope::~ObjectInitializationScope):
* runtime/RegExpMatchesArray.cpp:
(JSC::createEmptyRegExpMatchesArray):
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@251456 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index 979634d..7381965 100644
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,5 +1,60 @@
2019-10-22 Mark Lam <mark.lam@apple.com>
+ Clients of JSArray::tryCreateUninitializedRestricted() should invoke the mutatorFence().
+ https://bugs.webkit.org/show_bug.cgi?id=203231
+ <rdar://problem/56486552>
+
+ Reviewed by Saam Barati.
+
+ Clients of JSArray::tryCreateUninitializedRestricted() creates a partially
+ initialized JSArray butterfly, with the contract that it (the client) will take
+ care of filling in all the missing indexed properties before setting the newly
+ created array loose in the world. We intentionally do not unconditionally write
+ barrier the newly created array but, instead, rely on an owner object (or GC root)
+ that it gets put into to scan it.
+
+ That said, we do need to ensure that all the stores are completed before this
+ array is put in an owner object (or GC root) which makes it scannable by the GC.
+ This ensures that the GC will not be scanning a partially initialized array
+ butterfly. To achieve this, we should invoke the mutatorFence after the clients
+ of JSArray::tryCreateUninitializedRestricted() finish initializing the array.
+
+ By design, all clients of tryCreateUninitializedRestricted() must instantiate an
+ ObjectInitializationScope RAII object. This patch makes use of the
+ ObjectInitializationScope destructor to invoke the mutatorFence.
+
+ Note: we technically only need to invoke the fence if we succeeded in allocating
+ the array. However, we just invoke the fence unconditionally because we expect
+ that in the common path, we will succeed in allocating the array. The release
+ build version of ObjectInitializationScope does not keep record of whether we
+ succeed in allocating the array anyway. To keep the behavior consistent, the
+ debug build version of ObjectInitializationScope will also unconditionally
+ invoke the fence even if we failed to allocate the array.
+
+ This patch also does the following:
+
+ 1. Replaced the setting of the public length in arrayProtoPrivateFuncConcatMemcpy()
+ with an assertion. The public length was already set by
+ tryCreateUninitializedRestricted() earlier.
+
+ Ditto for JSArray::fastSlice().
+
+ 2. Removed a redundant instance of ObjectInitializationScope in
+ createEmptyRegExpMatchesArray().
+
+ * runtime/ArrayPrototype.cpp:
+ (JSC::arrayProtoPrivateFuncConcatMemcpy):
+ * runtime/JSArray.cpp:
+ (JSC::JSArray::fastSlice):
+ * runtime/ObjectInitializationScope.cpp:
+ (JSC::ObjectInitializationScope::~ObjectInitializationScope):
+ * runtime/ObjectInitializationScope.h:
+ (JSC::ObjectInitializationScope::~ObjectInitializationScope):
+ * runtime/RegExpMatchesArray.cpp:
+ (JSC::createEmptyRegExpMatchesArray):
+
+2019-10-22 Mark Lam <mark.lam@apple.com>
+
Fix incorrect assertion in operationRegExpExecNonGlobalOrSticky().
https://bugs.webkit.org/show_bug.cgi?id=203230
<rdar://problem/56460749>
diff --git a/Source/JavaScriptCore/runtime/ArrayPrototype.cpp b/Source/JavaScriptCore/runtime/ArrayPrototype.cpp
index 4d88ffb..1d617d0 100644
--- a/Source/JavaScriptCore/runtime/ArrayPrototype.cpp
+++ b/Source/JavaScriptCore/runtime/ArrayPrototype.cpp
@@ -1581,7 +1581,7 @@
copyElements(buffer, firstArraySize, secondButterfly->contiguous().data(), secondArraySize, secondType);
}
- result->butterfly()->setPublicLength(resultSize);
+ ASSERT(result->butterfly()->publicLength() == resultSize);
return JSValue::encode(result);
}
diff --git a/Source/JavaScriptCore/runtime/JSArray.cpp b/Source/JavaScriptCore/runtime/JSArray.cpp
index ec71122..d087d05 100644
--- a/Source/JavaScriptCore/runtime/JSArray.cpp
+++ b/Source/JavaScriptCore/runtime/JSArray.cpp
@@ -792,8 +792,8 @@
memcpy(resultButterfly.contiguousDouble().data(), butterfly()->contiguousDouble().data() + startIndex, sizeof(JSValue) * count);
else
memcpy(resultButterfly.contiguous().data(), butterfly()->contiguous().data() + startIndex, sizeof(JSValue) * count);
- resultButterfly.setPublicLength(count);
+ ASSERT(resultButterfly.publicLength() == count);
return resultArray;
}
default:
diff --git a/Source/JavaScriptCore/runtime/ObjectInitializationScope.cpp b/Source/JavaScriptCore/runtime/ObjectInitializationScope.cpp
index e19c8a9..a44fbb9 100644
--- a/Source/JavaScriptCore/runtime/ObjectInitializationScope.cpp
+++ b/Source/JavaScriptCore/runtime/ObjectInitializationScope.cpp
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2017-2018 Apple Inc. All rights reserved.
+ * Copyright (C) 2017-2019 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -42,6 +42,7 @@
ObjectInitializationScope::~ObjectInitializationScope()
{
+ m_vm.heap.mutatorFence();
if (!m_object)
return;
verifyPropertiesAreInitialized(m_object);
diff --git a/Source/JavaScriptCore/runtime/ObjectInitializationScope.h b/Source/JavaScriptCore/runtime/ObjectInitializationScope.h
index ea62190..67be07e 100644
--- a/Source/JavaScriptCore/runtime/ObjectInitializationScope.h
+++ b/Source/JavaScriptCore/runtime/ObjectInitializationScope.h
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2017-2018 Apple Inc. All rights reserved.
+ * Copyright (C) 2017-2019 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -40,6 +40,10 @@
ALWAYS_INLINE ObjectInitializationScope(VM& vm)
: m_vm(vm)
{ }
+ ALWAYS_INLINE ~ObjectInitializationScope()
+ {
+ m_vm.heap.mutatorFence();
+ }
ALWAYS_INLINE VM& vm() const { return m_vm; }
ALWAYS_INLINE void notifyAllocated(JSObject*, bool) { }
diff --git a/Source/JavaScriptCore/runtime/RegExpMatchesArray.cpp b/Source/JavaScriptCore/runtime/RegExpMatchesArray.cpp
index dce1f37..c42be7f 100644
--- a/Source/JavaScriptCore/runtime/RegExpMatchesArray.cpp
+++ b/Source/JavaScriptCore/runtime/RegExpMatchesArray.cpp
@@ -54,7 +54,6 @@
array->initializeIndexWithoutBarrier(scope, i, jsUndefined());
}
} else {
- ObjectInitializationScope scope(vm);
array = tryCreateUninitializedRegExpMatchesArray(scope, &deferralContext, globalObject->regExpMatchesArrayStructure(), regExp->numSubpatterns() + 1);
RELEASE_ASSERT(array);
