Properly zero unused property storage offsets
https://bugs.webkit.org/show_bug.cgi?id=186692
Reviewed by Filip Pizlo.
JSTests:
* stress/butterfly-zero-unused-butterfly-properties.js: Added.
Source/JavaScriptCore:
Since the concurrent GC might see a property slot before the mutator has actually
stored the value there, we need to ensure that slot doesn't have garbage in it.
Right now when calling constructConvertedArrayStorageWithoutCopyingElements
or creating a RegExp matches array, we never cleared the unused
property storage. ObjectIntializationScope has also been upgraded
to look for our invariants around property storage. Additionally,
a new assertion has been added to check for JSValue() when adding
a new property.
We used to put undefined into deleted property offsets. To
make things simpler, this patch causes us to store JSValue() there
instead.
Lastly, this patch fixes an issue where we would initialize the
array storage of RegExpMatchesArray twice. First with 0 and
secondly with the actual result. Now we only zero memory between
vector length and public length.
* runtime/Butterfly.h:
(JSC::Butterfly::offsetOfVectorLength):
* runtime/ButterflyInlines.h:
(JSC::Butterfly::tryCreateUninitialized):
(JSC::Butterfly::createUninitialized):
(JSC::Butterfly::tryCreate):
(JSC::Butterfly::create):
(JSC::Butterfly::createOrGrowPropertyStorage):
(JSC::Butterfly::createOrGrowArrayRight):
(JSC::Butterfly::growArrayRight):
(JSC::Butterfly::resizeArray):
* runtime/JSArray.cpp:
(JSC::JSArray::tryCreateUninitializedRestricted):
(JSC::createArrayButterflyInDictionaryIndexingMode): Deleted.
* runtime/JSArray.h:
(JSC::tryCreateArrayButterfly):
* runtime/JSObject.cpp:
(JSC::JSObject::createArrayStorageButterfly):
(JSC::JSObject::constructConvertedArrayStorageWithoutCopyingElements):
(JSC::JSObject::deleteProperty):
(JSC::JSObject::shiftButterflyAfterFlattening):
* runtime/JSObject.h:
* runtime/JSObjectInlines.h:
(JSC::JSObject::prepareToPutDirectWithoutTransition):
* runtime/ObjectInitializationScope.cpp:
(JSC::ObjectInitializationScope::verifyPropertiesAreInitialized):
* runtime/ObjectInitializationScope.h:
(JSC::ObjectInitializationScope::release):
* runtime/RegExpMatchesArray.h:
(JSC::tryCreateUninitializedRegExpMatchesArray):
(JSC::createRegExpMatchesArray):
* runtime/Butterfly.h:
(JSC::Butterfly::offsetOfVectorLength):
* runtime/ButterflyInlines.h:
(JSC::Butterfly::tryCreateUninitialized):
(JSC::Butterfly::createUninitialized):
(JSC::Butterfly::tryCreate):
(JSC::Butterfly::create):
(JSC::Butterfly::createOrGrowPropertyStorage):
(JSC::Butterfly::createOrGrowArrayRight):
(JSC::Butterfly::growArrayRight):
(JSC::Butterfly::resizeArray):
* runtime/JSArray.cpp:
(JSC::JSArray::tryCreateUninitializedRestricted):
(JSC::createArrayButterflyInDictionaryIndexingMode): Deleted.
* runtime/JSArray.h:
(JSC::tryCreateArrayButterfly):
* runtime/JSObject.cpp:
(JSC::JSObject::createArrayStorageButterfly):
(JSC::JSObject::constructConvertedArrayStorageWithoutCopyingElements):
(JSC::JSObject::deleteProperty):
(JSC::JSObject::shiftButterflyAfterFlattening):
* runtime/JSObject.h:
* runtime/JSObjectInlines.h:
(JSC::JSObject::prepareToPutDirectWithoutTransition):
* runtime/ObjectInitializationScope.cpp:
(JSC::ObjectInitializationScope::verifyPropertiesAreInitialized):
* runtime/RegExpMatchesArray.cpp:
(JSC::createEmptyRegExpMatchesArray):
* runtime/RegExpMatchesArray.h:
(JSC::tryCreateUninitializedRegExpMatchesArray):
(JSC::createRegExpMatchesArray):
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@232951 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/JavaScriptCore/runtime/RegExpMatchesArray.h b/Source/JavaScriptCore/runtime/RegExpMatchesArray.h
index d1a1cbe..e957bf9 100644
--- a/Source/JavaScriptCore/runtime/RegExpMatchesArray.h
+++ b/Source/JavaScriptCore/runtime/RegExpMatchesArray.h
@@ -40,20 +40,20 @@
if (vectorLength > MAX_STORAGE_VECTOR_LENGTH)
return 0;
- JSGlobalObject* globalObject = structure->globalObject();
- bool createUninitialized = globalObject->isOriginalArrayStructure(structure);
- void* temp = vm.jsValueGigacageAuxiliarySpace.allocateNonVirtual(vm, Butterfly::totalSize(0, structure->outOfLineCapacity(), true, vectorLength * sizeof(EncodedJSValue)), deferralContext, AllocationFailureMode::ReturnNull);
- if (UNLIKELY(!temp))
+ const bool hasIndexingHeader = true;
+ Butterfly* butterfly = Butterfly::tryCreateUninitialized(vm, nullptr, 0, structure->outOfLineCapacity(), hasIndexingHeader, vectorLength * sizeof(EncodedJSValue), deferralContext);
+ if (UNLIKELY(!butterfly))
return nullptr;
- Butterfly* butterfly = Butterfly::fromBase(temp, 0, structure->outOfLineCapacity());
+
butterfly->setVectorLength(vectorLength);
butterfly->setPublicLength(initialLength);
- unsigned i = (createUninitialized ? initialLength : 0);
- for (; i < vectorLength; ++i)
+ for (unsigned i = initialLength; i < vectorLength; ++i)
butterfly->contiguous().atUnsafe(i).clear();
JSArray* result = JSArray::createWithButterfly(vm, deferralContext, structure, butterfly);
+
+ const bool createUninitialized = true;
scope.notifyAllocated(result, createUninitialized);
return result;
}
@@ -80,23 +80,29 @@
unsigned numSubpatterns = regExp->numSubpatterns();
bool hasNamedCaptures = regExp->hasNamedCaptures();
JSObject* groups = nullptr;
+ Structure* matchStructure = globalObject->regExpMatchesArrayStructure();
+ if (hasNamedCaptures) {
+ groups = JSFinalObject::create(vm, JSFinalObject::createStructure(vm, globalObject, globalObject->objectPrototype(), 0));
+ matchStructure = globalObject->regExpMatchesArrayWithGroupsStructure();
+ }
auto setProperties = [&] () {
array->putDirect(vm, RegExpMatchesArrayIndexPropertyOffset, jsNumber(result.start));
array->putDirect(vm, RegExpMatchesArrayInputPropertyOffset, input);
- if (hasNamedCaptures) {
- groups = JSFinalObject::create(vm, JSFinalObject::createStructure(vm, globalObject, globalObject->objectPrototype(), 0));
+ if (hasNamedCaptures)
array->putDirect(vm, RegExpMatchesArrayGroupsPropertyOffset, groups);
- }
+
+ ASSERT(!array->butterfly()->indexingHeader()->preCapacity(matchStructure));
+ auto capacity = matchStructure->outOfLineCapacity();
+ auto size = matchStructure->outOfLineSize();
+ memset(array->butterfly()->base(0, capacity), 0, (capacity - size) * sizeof(JSValue));
};
- GCDeferralContext deferralContext(vm.heap);
-
- Structure* matchStructure = hasNamedCaptures ? globalObject->regExpMatchesArrayWithGroupsStructure() : globalObject->regExpMatchesArrayStructure();
-
if (UNLIKELY(globalObject->isHavingABadTime())) {
+ GCDeferralContext deferralContext(vm.heap);
ObjectInitializationScope scope(vm);
array = JSArray::tryCreateUninitializedRestricted(scope, &deferralContext, matchStructure, numSubpatterns + 1);
+
// FIXME: we should probably throw an out of memory error here, but
// when making this change we should check that all clients of this
// function will correctly handle an exception being thrown from here.
@@ -115,21 +121,20 @@
else
value = jsUndefined();
array->initializeIndexWithoutBarrier(scope, i, value);
- if (groups) {
- String groupName = regExp->getCaptureGroupName(i);
- if (!groupName.isEmpty())
- groups->putDirect(vm, Identifier::fromString(&vm, groupName), value);
- }
}
} else {
+ GCDeferralContext deferralContext(vm.heap);
ObjectInitializationScope scope(vm);
array = tryCreateUninitializedRegExpMatchesArray(scope, &deferralContext, matchStructure, numSubpatterns + 1);
+
+ // FIXME: we should probably throw an out of memory error here, but
+ // when making this change we should check that all clients of this
+ // function will correctly handle an exception being thrown from here.
+ // https://bugs.webkit.org/show_bug.cgi?id=169786
RELEASE_ASSERT(array);
setProperties();
- // Now the object is safe to scan by GC.
-
array->initializeIndexWithoutBarrier(scope, 0, jsSubstringOfResolved(vm, &deferralContext, input, result.start, result.end - result.start), ArrayWithContiguous);
for (unsigned i = 1; i <= numSubpatterns; ++i) {
@@ -140,11 +145,18 @@
else
value = jsUndefined();
array->initializeIndexWithoutBarrier(scope, i, value, ArrayWithContiguous);
- if (groups) {
- String groupName = regExp->getCaptureGroupName(i);
- if (!groupName.isEmpty())
- groups->putDirect(vm, Identifier::fromString(&vm, groupName), value);
- }
+ }
+ }
+
+ // Now the object is safe to scan by GC.
+
+ // We initialize the groups object late as it could allocate, which with the current API could cause
+ // allocations.
+ if (groups) {
+ for (unsigned i = 1; i <= numSubpatterns; ++i) {
+ String groupName = regExp->getCaptureGroupName(i);
+ if (!groupName.isEmpty())
+ groups->putDirect(vm, Identifier::fromString(&vm, groupName), array->getIndexQuickly(i));
}
}
return array;
