From b7ecdfa767a359bcc8c660523644122d765e77ee Mon Sep 17 00:00:00 2001 From: Yusuke Suzuki Date: Tue, 11 Apr 2023 15:26:02 -0700 Subject: [PATCH] [JSC] Fix Object.assign's temporary structure transition https://bugs.webkit.org/show_bug.cgi?id=255304 rdar://107903084 Reviewed by Mark Lam. Fixes Object.assign's temporary structure transition. We should store oldStructure's StructureID, but we were storing oldStructure->structure()'s ID. Interestingly this does not affect on release build because, 1. After setting this and before setStructure, we have no GC invocation operations. 2. Inline property offset is static. It is not depending on Structure. So, every store just works as expected. So, it is just assertion hit ultimately. But it is not correct. This patch fixes it. Also, we need to visit BrandedStructures' finalizer too. This patch also fixes it. * Source/JavaScriptCore/heap/Heap.cpp: (JSC::Heap::finalizeUnconditionalFinalizers): * Source/JavaScriptCore/runtime/JSObject.cpp: (JSC::JSObject::putOwnDataPropertyBatching): Canonical link: https://commits.webkit.org/262843@main --- Source/JavaScriptCore/heap/Heap.cpp | 4 +++- Source/JavaScriptCore/runtime/JSObject.cpp | 10 +++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/Source/JavaScriptCore/heap/Heap.cpp b/Source/JavaScriptCore/heap/Heap.cpp index 1616151ac7d9..4679be6bea67 100644 --- a/Source/JavaScriptCore/heap/Heap.cpp +++ b/Source/JavaScriptCore/heap/Heap.cpp @@ -700,8 +700,10 @@ void Heap::finalizeUnconditionalFinalizers() [&] (auto& space) { this->finalizeMarkedUnconditionalFinalizers(space.set, collectionScope); }); - if (collectionScope == CollectionScope::Full) + if (collectionScope == CollectionScope::Full) { finalizeMarkedUnconditionalFinalizers(structureSpace, collectionScope); + finalizeMarkedUnconditionalFinalizers(brandedStructureSpace, collectionScope); + } finalizeMarkedUnconditionalFinalizers(structureRareDataSpace, collectionScope); finalizeMarkedUnconditionalFinalizers(unlinkedFunctionExecutableSpaceAndSet.set, collectionScope); if (m_weakSetSpace) diff --git a/Source/JavaScriptCore/runtime/JSObject.cpp b/Source/JavaScriptCore/runtime/JSObject.cpp index 6ade91b895c0..2b8c4c61e340 100644 --- a/Source/JavaScriptCore/runtime/JSObject.cpp +++ b/Source/JavaScriptCore/runtime/JSObject.cpp @@ -4092,7 +4092,6 @@ void JSObject::putOwnDataPropertyBatching(VM& vm, const RefPtrisDictionary() || (structure->transitionCountEstimate() + size) > Structure::s_maxTransitionLength || !structure->canPerformFastPropertyEnumeration() || (structure->transitionWatchpointSet().isBeingWatched() && structure->transitionWatchpointSet().isStillValid()))) { Vector offsets; offsets.reserveInitialCapacity(size); - Structure* originalStructure = structure; for (unsigned index = 0; index < size; ++index) { PropertyName propertyName(properties[index].get()); @@ -4135,10 +4134,11 @@ void JSObject::putOwnDataPropertyBatching(VM& vm, const RefPtroutOfLineCapacity() != structure->outOfLineCapacity()) { - ASSERT(structure != originalStructure); - newButterfly = allocateMoreOutOfLineStorage(vm, originalStructure->outOfLineCapacity(), structure->outOfLineCapacity()); - nukeStructureAndSetButterfly(vm, originalStructure->structureID(), newButterfly); + auto* oldStructure = this->structure(); + if (oldStructure->outOfLineCapacity() != structure->outOfLineCapacity()) { + ASSERT(structure != oldStructure); + newButterfly = allocateMoreOutOfLineStorage(vm, oldStructure->outOfLineCapacity(), structure->outOfLineCapacity()); + nukeStructureAndSetButterfly(vm, StructureID::encode(oldStructure), newButterfly); } for (unsigned index = 0; index < offsets.size(); ++index)